From b4d5e79162f6fb3fbbe6ee3b9e33f60d6c402334 Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Wed, 29 May 2024 16:23:11 -0700 Subject: [PATCH 1/3] fix: FORMS-1113 handle bad json in export (#1373) --- app/src/docs/v1.api-spec.yaml | 10 +++--- app/src/forms/form/exportService.js | 33 +++++++++++++++---- .../unit/forms/form/exportService.spec.js | 9 +++++ 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/app/src/docs/v1.api-spec.yaml b/app/src/docs/v1.api-spec.yaml index ce9873579..d20c29a9e 100755 --- a/app/src/docs/v1.api-spec.yaml +++ b/app/src/docs/v1.api-spec.yaml @@ -615,28 +615,28 @@ paths: name: preference schema: type: object - example: '{ minDate=2020-12-17T08:00:00Z, maxDate=2020-12-17T08:00:00Z, updatedMinDate=2020-12-17T08:00:00Z, updatedMaxDate=2020-12-17T08:00:00Z }' + example: '{minDate=2020-12-17T08:00:00Z,maxDate=2020-12-18T08:00:00Z,updatedMinDate=2020-12-17T08:00:00Z,updatedMaxDate=2020-12-18T08:00:00Z}' description: form submissions export preferences - in: query name: deleted schema: type: boolean - description: (optional) This optional parameter should be set to true if deleted records (submissions) need to be fetched + description: Set to true to return deleted submissions example: false - in: query name: drafts schema: type: boolean - description: (optional) This optional parameter should be set to true if draft records (submissions) need to be fetched + description: Set to true to return draft submissions example: false - in: query name: columns schema: type: array - description: (optional) List of form level columns (Only Allowed draft, deleted, updatedAt columns) to be include. Other then allowed columns will be ignored + description: List of form level columns (Only Allowed `deleted`, `draft`, `updatedAt`) to be included, others will be ignored example: - - draft, - deleted, + - draft, - updatedAt - in: query name: status diff --git a/app/src/forms/form/exportService.js b/app/src/forms/form/exportService.js index 333ee557b..7dbaf81ae 100644 --- a/app/src/forms/form/exportService.js +++ b/app/src/forms/form/exportService.js @@ -192,14 +192,35 @@ const service = { }); }, - _getSubmissions: async (form, params, version) => { - //let preference = params.preference ? JSON.parse(params.preference) : undefined; - let preference; - if (params.preference && _.isString(params.preference)) { - preference = JSON.parse(params.preference); + /** + * Parse the preferences that come in on the request. + * + * @param {any} preferences the preferences - probably a string or undefined + * but unsure exactly what this could be. + * @returns {any} the preferences. If a string was given as an argument then + * it's treated as JSON and an object is returned. + */ + _parsePreferences: (preferences) => { + let parsedPreferences; + if (preferences && _.isString(preferences)) { + try { + parsedPreferences = JSON.parse(preferences); + } catch (error) { + throw new Problem(400, { + detail: 'Bad preferences: ' + error.message, + }); + } } else { - preference = params.preference; + // What is this? How is it not a string? Is this just handling undefined? + parsedPreferences = preferences; } + + return parsedPreferences; + }, + + _getSubmissions: async (form, params, version) => { + const preference = service._parsePreferences(params.preference); + // let submissionData; // params for this export include minDate and maxDate (full timestamp dates). return SubmissionData.query() diff --git a/app/tests/unit/forms/form/exportService.spec.js b/app/tests/unit/forms/form/exportService.spec.js index 5b9842993..b99fe697d 100644 --- a/app/tests/unit/forms/form/exportService.spec.js +++ b/app/tests/unit/forms/form/exportService.spec.js @@ -36,6 +36,15 @@ describe('export', () => { }; exportService._getForm = jest.fn().mockReturnValue(form); + describe('400 response when', () => { + test('invalid preferences json', async () => { + const params = { preference: '{' }; + exportService._readLatestFormSchema = jest.fn().mockReturnValueOnce(); + + await expect(exportService.export(formId, params, currentUser)).rejects.toThrow('400'); + }); + }); + describe('type 1', () => { const params = { emailExport: false, From c582213eba618c951ba4d87cd4eb73c2592b6238 Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Thu, 30 May 2024 10:25:38 -0700 Subject: [PATCH 2/3] fix: FORMS-1038 handle sort by column not existing (#1374) --- app/src/forms/form/service.js | 24 ++++++++++++----------- app/tests/unit/forms/form/service.spec.js | 12 ++++++++++++ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/app/src/forms/form/service.js b/app/src/forms/form/service.js index 5033711ab..b65f3316d 100644 --- a/app/src/forms/form/service.js +++ b/app/src/forms/form/service.js @@ -365,8 +365,8 @@ const service = { const selection = ['confirmationId', 'createdAt', 'formId', 'formSubmissionStatusCode', 'submissionId', 'deleted', 'createdBy', 'formVersionId']; + let fields = []; if (params.fields && params.fields.length) { - let fields = []; if (typeof params.fields !== 'string' && params.fields.includes('updatedAt')) { selection.push('updatedAt'); } @@ -383,19 +383,21 @@ const service = { // columns. Also remove empty values to handle the case of trailing commas // and other malformed data too. fields = fields.filter((f) => f !== 'updatedAt' && f !== 'updatedBy' && f.trim() !== ''); + } - fields.push('lateEntry'); - query.select( - selection, - fields.map((f) => ref(`submission:data.${f}`).as(f.split('.').slice(-1))) - ); - } else { - query.select( - selection, - ['lateEntry'].map((f) => ref(`submission:data.${f}`).as(f.split('.').slice(-1))) - ); + fields.push('lateEntry'); + + if (params.sortBy?.column && !selection.includes(params.sortBy.column) && !fields.includes(params.sortBy.column)) { + throw new Problem(400, { + details: `orderBy column '${params.sortBy.column}' not in selected columns`, + }); } + query.select( + selection, + fields.map((f) => ref(`submission:data.${f}`).as(f.split('.').slice(-1))) + ); + if (params.paginationEnabled) { return await service.processPaginationData(query, parseInt(params.page), parseInt(params.itemsPerPage), params.totalSubmissions, params.search, params.searchEnabled); } diff --git a/app/tests/unit/forms/form/service.spec.js b/app/tests/unit/forms/form/service.spec.js index eccf00d24..4da002766 100644 --- a/app/tests/unit/forms/form/service.spec.js +++ b/app/tests/unit/forms/form/service.spec.js @@ -447,6 +447,18 @@ describe('_findFileIds', () => { }); describe('listFormSubmissions', () => { + describe('400 response when', () => { + test('sort by column not in select', async () => { + await expect( + service.listFormSubmissions(formId, { + sortBy: { + column: 'x', + }, + }) + ).rejects.toThrow('400'); + }); + }); + it('should not error if fields has a trailing commma', async () => { await service.listFormSubmissions(formId, { fields: 'x,' }); From aa0249ed68f3b5810182ec8e0cbe9de2b25757ba Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Thu, 30 May 2024 14:30:43 -0700 Subject: [PATCH 3/3] fix: FORMS-1284 update cronjob curl image (#1375) --- openshift/app.cronjob.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openshift/app.cronjob.yaml b/openshift/app.cronjob.yaml index 4bca4ba49..3da8ba744 100644 --- a/openshift/app.cronjob.yaml +++ b/openshift/app.cronjob.yaml @@ -54,7 +54,7 @@ objects: spec: containers: - name: my-container - image: docker.io/curlimages/curl:8.1.0 + image: docker.io/curlimages/curl:8.8.0 env: - name: APITOKEN valueFrom: