From 7a066b85cba307c2dc0bd732b1984feaca8da104 Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Thu, 27 Apr 2023 15:41:39 -0700 Subject: [PATCH 01/14] display list of missing sheets --- api/src/json-schema/validation-schema.ts | 3 ++ api/src/services/summary-service.test.ts | 8 +-- api/src/services/summary-service.ts | 4 +- api/src/services/validation-service.test.ts | 58 ++++++++++----------- api/src/services/validation-service.ts | 14 +++-- 5 files changed, 44 insertions(+), 43 deletions(-) diff --git a/api/src/json-schema/validation-schema.ts b/api/src/json-schema/validation-schema.ts index 708bdcdcdb..2c5d52215e 100644 --- a/api/src/json-schema/validation-schema.ts +++ b/api/src/json-schema/validation-schema.ts @@ -118,6 +118,7 @@ export const submissionValidationSchema = { title: 'File/Sheet Validation', description: 'The validators that can be applied against a file/sheet within a submission file.', anyOf: [ + //below: column header validator { $ref: '#/$defs/file_required_columns_validator' }, @@ -130,6 +131,7 @@ export const submissionValidationSchema = { { $ref: '#/$defs/file_valid_columns_validator' }, + //below: column cell validator { $ref: '#/$defs/file_column_unique_validator' } @@ -334,6 +336,7 @@ export const submissionValidationSchema = { }, additionalProperties: false }, + //for the cell to have a certain rule, pairs with format/range, etc column_required_validator: { description: 'Validates that this column value is not empty', type: 'object', diff --git a/api/src/services/summary-service.test.ts b/api/src/services/summary-service.test.ts index e2794a05fb..7c4e1a48be 100644 --- a/api/src/services/summary-service.test.ts +++ b/api/src/services/summary-service.test.ts @@ -490,7 +490,7 @@ describe('SummaryService', () => { ]); const getRules = sinon.stub(service, 'getValidationRules').resolves(''); - const validate = sinon.stub(service, 'validateXLSX').resolves({}); + const validate = sinon.stub(service, 'validateXLSX_summary').resolves({}); const persistResults = sinon.stub(service, 'persistSummaryValidationResults').resolves(); const logFoundValidation = sinon.stub(SummaryRepository.prototype, 'insertSummarySubmissionMessage').resolves(); @@ -520,7 +520,7 @@ describe('SummaryService', () => { .stub(service, 'getSummaryTemplateSpeciesRecords') .resolves([makeMockTemplateSpeciesRecord(1)]); const getRules = sinon.stub(service, 'getValidationRules').resolves(''); - const validate = sinon.stub(service, 'validateXLSX').resolves({}); + const validate = sinon.stub(service, 'validateXLSX_summary').resolves({}); const persistResults = sinon.stub(service, 'persistSummaryValidationResults').resolves(); await service.summaryTemplateValidation(xlsxCsv, 1); @@ -541,7 +541,7 @@ describe('SummaryService', () => { const getValidation = sinon.stub(service, 'getSummaryTemplateSpeciesRecords').resolves(templateSpeciesRecords); const getRules = sinon.stub(service, 'getValidationRules').resolves(''); - const validate = sinon.stub(service, 'validateXLSX').resolves({}); + const validate = sinon.stub(service, 'validateXLSX_summary').resolves({}); const persistResults = sinon.stub(service, 'persistSummaryValidationResults').resolves(); await service.summaryTemplateValidation(xlsxCsv, 1); @@ -776,7 +776,7 @@ describe('SummaryService', () => { const service = mockService(); const xlsx = new XLSXCSV(buildFile('test file', {})); const parser = new ValidationSchemaParser({}); - const response = await service.validateXLSX(xlsx, parser); + const response = await service.validateXLSX_summary(xlsx, parser); expect(response.media_state.isValid).to.be.true; expect(response.media_state.fileErrors).is.empty; diff --git a/api/src/services/summary-service.ts b/api/src/services/summary-service.ts index a930580039..46591b7c36 100644 --- a/api/src/services/summary-service.ts +++ b/api/src/services/summary-service.ts @@ -211,7 +211,7 @@ export class SummaryService extends DBService { } const schemaParser = this.getValidationRules(validationSchema); - const csvState = this.validateXLSX(xlsx, schemaParser); + const csvState = this.validateXLSX_summary(xlsx, schemaParser); await this.persistSummaryValidationResults(csvState.csv_state, csvState.media_state); } catch (error) { if (error instanceof SubmissionError) { @@ -300,7 +300,7 @@ export class SummaryService extends DBService { * @param {ValidationSchemaParser} parser * @returns {ICsvMediaState} */ - validateXLSX(file: XLSXCSV, parser: ValidationSchemaParser): ICsvMediaState { + validateXLSX_summary(file: XLSXCSV, parser: ValidationSchemaParser): ICsvMediaState { defaultLog.debug({ label: 'validateXLSX' }); // Run media validations diff --git a/api/src/services/validation-service.test.ts b/api/src/services/validation-service.test.ts index bfa960d6c8..46dd29cc09 100644 --- a/api/src/services/validation-service.test.ts +++ b/api/src/services/validation-service.test.ts @@ -157,11 +157,11 @@ describe('ValidationService', () => { const getValidation = sinon.stub(ValidationService.prototype, 'getValidationSchema').resolves(''); const getRules = sinon.stub(ValidationService.prototype, 'getValidationRules').resolves(''); - const validate = sinon.stub(ValidationService.prototype, 'validateXLSX').resolves({}); + const validate = sinon.stub(ValidationService.prototype, 'validateXLSX_occurrence').resolves({}); const persistResults = sinon.stub(ValidationService.prototype, 'persistValidationResults').resolves(true); const service = mockService(); - await service.templateValidation(xlsxCsv, 1); + await service.occurrenceTemplateValidation(xlsxCsv, 1); expect(getValidation).to.be.calledOnce; expect(getRules).to.be.calledOnce; @@ -176,13 +176,13 @@ describe('ValidationService', () => { sinon.stub(ValidationService.prototype, 'getValidationSchema').throws(new SubmissionError({})); sinon.stub(ValidationService.prototype, 'getValidationRules').resolves({}); - sinon.stub(ValidationService.prototype, 'validateXLSX').resolves({}); + sinon.stub(ValidationService.prototype, 'validateXLSX_occurrence').resolves({}); sinon.stub(ValidationService.prototype, 'persistValidationResults').resolves(true); try { const dbConnection = getMockDBConnection(); const service = new ValidationService(dbConnection); - await service.templateValidation(xlsxCsv, 1); + await service.occurrenceTemplateValidation(xlsxCsv, 1); expect.fail(); } catch (error) { expect(error).to.be.instanceOf(SubmissionError); @@ -505,7 +505,7 @@ describe('ValidationService', () => { xlsx: new XLSXCSV(buildFile('test file', {})) }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); - const validation = sinon.stub(service, 'templateValidation').resolves(); + const validation = sinon.stub(service, 'occurrenceTemplateValidation').resolves(); const submissionStatus = sinon.stub(service.submissionRepository, 'insertSubmissionStatus').resolves(); await service.validateFile(1, 1); @@ -522,7 +522,7 @@ describe('ValidationService', () => { }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); const validation = sinon - .stub(service, 'templateValidation') + .stub(service, 'occurrenceTemplateValidation') .throws(SubmissionErrorFromMessageType(SUBMISSION_MESSAGE_TYPE.MISSING_VALIDATION_SCHEMA)); const submissionStatus = sinon.stub(service.submissionRepository, 'insertSubmissionStatus').resolves(); const insertError = sinon.stub(service.errorService, 'insertSubmissionError').resolves(); @@ -545,7 +545,7 @@ describe('ValidationService', () => { xlsx: new XLSXCSV(buildFile('test file', {})) }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); - const validation = sinon.stub(service, 'templateValidation').throws(new Error()); + const validation = sinon.stub(service, 'occurrenceTemplateValidation').throws(new Error()); const submissionStatus = sinon.stub(service.submissionRepository, 'insertSubmissionStatus').resolves(); const insertError = sinon.stub(service.errorService, 'insertSubmissionError').resolves(); @@ -707,7 +707,7 @@ describe('ValidationService', () => { }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); - const validate = sinon.stub(service, 'templateValidation').resolves(); + const validate = sinon.stub(service, 'occurrenceTemplateValidation').resolves(); const transform = sinon .stub(service, 'templateTransformation') .throws(SubmissionErrorFromMessageType(SUBMISSION_MESSAGE_TYPE.FAILED_TRANSFORM_XLSX)); @@ -734,7 +734,7 @@ describe('ValidationService', () => { }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); - const validate = sinon.stub(service, 'templateValidation').resolves(); + const validate = sinon.stub(service, 'occurrenceTemplateValidation').resolves(); const transform = sinon.stub(service, 'templateTransformation').throws(); const insertError = sinon.stub(service.errorService, 'insertSubmissionError').resolves(); sinon.stub(service, 'scrapeDwCAndUploadOccurrences').resolves(); @@ -759,7 +759,7 @@ describe('ValidationService', () => { }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); - const validate = sinon.stub(service, 'templateValidation').resolves(); + const validate = sinon.stub(service, 'occurrenceTemplateValidation').resolves(); const status = sinon.stub(service.submissionRepository, 'insertSubmissionStatus').resolves(); const transform = sinon.stub(service, 'templateTransformation').resolves(); const decorate = sinon.stub(service.dwCService, 'decorateDwCJSON').resolves(); @@ -1026,31 +1026,31 @@ describe('ValidationService', () => { const service = mockService(); const xlsx = new XLSXCSV(buildFile('test file', {})); const parser = new ValidationSchemaParser({}); - const response = await service.validateXLSX(xlsx, parser); + const response = await service.validateXLSX_occurrence(xlsx, parser); expect(response.media_state.isValid).to.be.true; expect(response.media_state.fileErrors).is.empty; }); - it('should throw Media is invalid error', async () => { - const service = mockService(); - const mockMediaState = { - fileName: 'test file', - isValid: false - } as IMediaState; - const xlsx = new XLSXCSV(buildFile('test file', {})); - const parser = new ValidationSchemaParser({}); + // it('should throw Media is invalid error', async () => { + // const service = mockService(); + // const mockMediaState = { + // fileName: 'test file', + // isValid: false + // } as IMediaState; + // const xlsx = new XLSXCSV(buildFile('test file', {})); + // const parser = new ValidationSchemaParser({}); - sinon.stub(XLSXCSV.prototype, 'getMediaState').returns(mockMediaState); + // sinon.stub(XLSXCSV.prototype, 'getMediaState').returns(mockMediaState); - try { - await service.validateXLSX(xlsx, parser); - expect.fail(); - } catch (error) { - expect(error).to.be.instanceOf(SubmissionError); - expect((error as SubmissionError).submissionMessages[0].type).to.be.eql(SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA); - } - }); + // try { + // await service.validateXLSX_occurrence(xlsx, parser); + // expect.fail(); + // } catch (error) { + // expect(error).to.be.instanceOf(SubmissionError); + // expect((error as SubmissionError).submissionMessages[0].type).to.be.eql(SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA); + // } + // }); it('should return valid state object with content errors', async () => { const service = mockService(); @@ -1086,7 +1086,7 @@ describe('ValidationService', () => { sinon.stub(DWCArchive.prototype, 'validateContent'); sinon.stub(XLSXCSV.prototype, 'getContentState').returns([mockState]); - const response = await service.validateXLSX(xlsx, parser); + const response = await service.validateXLSX_occurrence(xlsx, parser); expect(response.csv_state).is.not.empty; expect(response.csv_state[0].headerErrors).is.not.empty; expect(response.csv_state[0].rowErrors).is.not.empty; diff --git a/api/src/services/validation-service.ts b/api/src/services/validation-service.ts index 59217df641..92d4f6800c 100644 --- a/api/src/services/validation-service.ts +++ b/api/src/services/validation-service.ts @@ -74,7 +74,7 @@ export class ValidationService extends DBService { defaultLog.debug({ label: 'validateFile', submissionId, surveyId }); try { const submissionPrep = await this.templatePreparation(submissionId); - await this.templateValidation(submissionPrep.xlsx, surveyId); + await this.occurrenceTemplateValidation(submissionPrep.xlsx, surveyId); // insert template validated status await this.submissionRepository.insertSubmissionStatus(submissionId, SUBMISSION_STATUS_TYPE.TEMPLATE_VALIDATED); @@ -158,7 +158,7 @@ export class ValidationService extends DBService { const submissionPrep = await this.templatePreparation(submissionId); // Run template validations - await this.templateValidation(submissionPrep.xlsx, surveyId); + await this.occurrenceTemplateValidation(submissionPrep.xlsx, surveyId); // Insert validation complete status await this.submissionRepository.insertSubmissionStatus(submissionId, SUBMISSION_STATUS_TYPE.TEMPLATE_VALIDATED); @@ -264,12 +264,12 @@ export class ValidationService extends DBService { } } - async templateValidation(xlsx: XLSXCSV, surveyId: number) { + async occurrenceTemplateValidation(xlsx: XLSXCSV, surveyId: number) { defaultLog.debug({ label: 'templateValidation' }); try { const schema = await this.getValidationSchema(xlsx, surveyId); const schemaParser = this.getValidationRules(schema); - const csvState = this.validateXLSX(xlsx, schemaParser); + const csvState = this.validateXLSX_occurrence(xlsx, schemaParser); await this.persistValidationResults(csvState.csv_state, csvState.media_state); } catch (error) { if (error instanceof SubmissionError) { @@ -304,6 +304,7 @@ export class ValidationService extends DBService { // not sure how to trigger these through testing if (!(parsedMedia instanceof MediaFile)) { + console.log('!parsedMedia instanceof MediaFile'); throw SubmissionErrorFromMessageType(SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA); } @@ -359,14 +360,11 @@ export class ValidationService extends DBService { return validationSchemaParser; } - validateXLSX(file: XLSXCSV, parser: ValidationSchemaParser) { + validateXLSX_occurrence(file: XLSXCSV, parser: ValidationSchemaParser) { // Run media validations file.validateMedia(parser); const media_state = file.getMediaState(); - if (!media_state.isValid) { - throw SubmissionErrorFromMessageType(SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA); - } // Run CSV content validations file.validateContent(parser); From 450b125e84694720cb7a5a4d96ca9906b9e3ac4e Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Thu, 27 Apr 2023 16:22:35 -0700 Subject: [PATCH 02/14] test --- api/src/services/summary-service.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/services/summary-service.test.ts b/api/src/services/summary-service.test.ts index 7c4e1a48be..626fd4fea0 100644 --- a/api/src/services/summary-service.test.ts +++ b/api/src/services/summary-service.test.ts @@ -601,7 +601,7 @@ describe('SummaryService', () => { } }); - it('should throw INVALID_MEDIA error if validateXLSX fails with invalid media', async () => { + it('should1 throw INVALID_MEDIA error if validateXLSX fails with invalid media', async () => { const service = mockService(); const file = new MediaFile('test.txt', 'text/plain', Buffer.of(0)); const xlsxCsv = new XLSXCSV(file); From ad014599fcd0eafb1b37466d3263e1726fbdf043 Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Thu, 27 Apr 2023 16:59:15 -0700 Subject: [PATCH 03/14] remove logs --- api/src/services/validation-service.test.ts | 20 -------------------- api/src/services/validation-service.ts | 1 - 2 files changed, 21 deletions(-) diff --git a/api/src/services/validation-service.test.ts b/api/src/services/validation-service.test.ts index 46dd29cc09..ebd54227cd 100644 --- a/api/src/services/validation-service.test.ts +++ b/api/src/services/validation-service.test.ts @@ -1032,26 +1032,6 @@ describe('ValidationService', () => { expect(response.media_state.fileErrors).is.empty; }); - // it('should throw Media is invalid error', async () => { - // const service = mockService(); - // const mockMediaState = { - // fileName: 'test file', - // isValid: false - // } as IMediaState; - // const xlsx = new XLSXCSV(buildFile('test file', {})); - // const parser = new ValidationSchemaParser({}); - - // sinon.stub(XLSXCSV.prototype, 'getMediaState').returns(mockMediaState); - - // try { - // await service.validateXLSX_occurrence(xlsx, parser); - // expect.fail(); - // } catch (error) { - // expect(error).to.be.instanceOf(SubmissionError); - // expect((error as SubmissionError).submissionMessages[0].type).to.be.eql(SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA); - // } - // }); - it('should return valid state object with content errors', async () => { const service = mockService(); const mockState = { diff --git a/api/src/services/validation-service.ts b/api/src/services/validation-service.ts index 92d4f6800c..d3bf87d5e4 100644 --- a/api/src/services/validation-service.ts +++ b/api/src/services/validation-service.ts @@ -304,7 +304,6 @@ export class ValidationService extends DBService { // not sure how to trigger these through testing if (!(parsedMedia instanceof MediaFile)) { - console.log('!parsedMedia instanceof MediaFile'); throw SubmissionErrorFromMessageType(SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA); } From 65710ae45106e1c97e777c811bbf25bea2270b3b Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Mon, 1 May 2023 11:40:29 -0700 Subject: [PATCH 04/14] addressed feedback and fixed a bug --- api/src/json-schema/validation-schema.ts | 30 ++++++++++--------- .../{surveyId}/observation/submission/get.ts | 1 + api/src/services/validation-service.test.ts | 24 +++++++-------- api/src/services/validation-service.ts | 28 ++++++++++------- 4 files changed, 46 insertions(+), 37 deletions(-) diff --git a/api/src/json-schema/validation-schema.ts b/api/src/json-schema/validation-schema.ts index 2c5d52215e..50ec0a77ff 100644 --- a/api/src/json-schema/validation-schema.ts +++ b/api/src/json-schema/validation-schema.ts @@ -118,22 +118,25 @@ export const submissionValidationSchema = { title: 'File/Sheet Validation', description: 'The validators that can be applied against a file/sheet within a submission file.', anyOf: [ - //below: column header validator { - $ref: '#/$defs/file_required_columns_validator' + $ref: '#/$defs/file_required_columns_validator', + description: 'column header validator' }, { - $ref: '#/$defs/file_recommended_columns_validator' + $ref: '#/$defs/file_recommended_columns_validator', + description: 'column header validator' }, { - $ref: '#/$defs/file_duplicate_columns_validator' + $ref: '#/$defs/file_duplicate_columns_validator', + description: 'column header validator' }, { - $ref: '#/$defs/file_valid_columns_validator' + $ref: '#/$defs/file_valid_columns_validator', + description: 'column header validator ' }, - //below: column cell validator { - $ref: '#/$defs/file_column_unique_validator' + $ref: '#/$defs/file_column_unique_validator', + description: 'column cell validator ' } ] }, @@ -241,7 +244,7 @@ export const submissionValidationSchema = { additionalProperties: false }, file_required_columns_validator: { - description: 'Validates that this file/sheet contains required columns', + description: 'Column header validator that checks whether the file/sheet contains required columns', type: 'object', properties: { file_required_columns_validator: { @@ -267,7 +270,7 @@ export const submissionValidationSchema = { additionalProperties: false }, file_recommended_columns_validator: { - description: 'Validates that this file/sheet contains recommended columns', + description: 'Column header validator that checks whether the file/sheet contains recommended columns', type: 'object', properties: { file_recommended_columns_validator: { @@ -293,7 +296,7 @@ export const submissionValidationSchema = { additionalProperties: false }, file_duplicate_columns_validator: { - description: 'Validates that this file/sheet contains recommended columns', + description: 'Column header validator that checks whether this file/sheet contains recommended columns', type: 'object', properties: { file_duplicate_columns_validator: { @@ -312,7 +315,7 @@ export const submissionValidationSchema = { additionalProperties: false }, file_valid_columns_validator: { - description: 'Validates that this file/sheet contains only valid (known) columns', + description: 'Column header validator that checks whether this file/sheet contains only valid (known) columns', type: 'object', properties: { file_valid_columns_validator: { @@ -336,9 +339,8 @@ export const submissionValidationSchema = { }, additionalProperties: false }, - //for the cell to have a certain rule, pairs with format/range, etc column_required_validator: { - description: 'Validates that this column value is not empty', + description: 'Column cell validator that checks that this column value is not empty.', type: 'object', properties: { column_required_validator: { @@ -455,7 +457,7 @@ export const submissionValidationSchema = { additionalProperties: false }, file_column_unique_validator: { - description: 'Validates that the column(s) are unique', + description: 'Column cell validator the checks that the column(s) are unique', type: 'object', properties: { file_column_unique_validator: { diff --git a/api/src/paths/project/{projectId}/survey/{surveyId}/observation/submission/get.ts b/api/src/paths/project/{projectId}/survey/{surveyId}/observation/submission/get.ts index af65666175..9d4f75af57 100644 --- a/api/src/paths/project/{projectId}/survey/{surveyId}/observation/submission/get.ts +++ b/api/src/paths/project/{projectId}/survey/{surveyId}/observation/submission/get.ts @@ -242,6 +242,7 @@ export function getOccurrenceSubmission(): RequestHandler { SUBMISSION_STATUS_TYPE.SYSTEM_ERROR, SUBMISSION_STATUS_TYPE.FAILED_OCCURRENCE_PREPARATION, SUBMISSION_STATUS_TYPE.FAILED_VALIDATION, + SUBMISSION_STATUS_TYPE.INVALID_MEDIA, SUBMISSION_STATUS_TYPE.FAILED_TRANSFORMED, SUBMISSION_STATUS_TYPE.FAILED_PROCESSING_OCCURRENCE_DATA, SUBMISSION_STATUS_TYPE['AWAITING CURRATION'], diff --git a/api/src/services/validation-service.test.ts b/api/src/services/validation-service.test.ts index ebd54227cd..615a178ec3 100644 --- a/api/src/services/validation-service.test.ts +++ b/api/src/services/validation-service.test.ts @@ -157,11 +157,11 @@ describe('ValidationService', () => { const getValidation = sinon.stub(ValidationService.prototype, 'getValidationSchema').resolves(''); const getRules = sinon.stub(ValidationService.prototype, 'getValidationRules').resolves(''); - const validate = sinon.stub(ValidationService.prototype, 'validateXLSX_occurrence').resolves({}); + const validate = sinon.stub(ValidationService.prototype, 'validateXLSX').resolves({}); const persistResults = sinon.stub(ValidationService.prototype, 'persistValidationResults').resolves(true); const service = mockService(); - await service.occurrenceTemplateValidation(xlsxCsv, 1); + await service.templateValidation(xlsxCsv, 1); expect(getValidation).to.be.calledOnce; expect(getRules).to.be.calledOnce; @@ -176,13 +176,13 @@ describe('ValidationService', () => { sinon.stub(ValidationService.prototype, 'getValidationSchema').throws(new SubmissionError({})); sinon.stub(ValidationService.prototype, 'getValidationRules').resolves({}); - sinon.stub(ValidationService.prototype, 'validateXLSX_occurrence').resolves({}); + sinon.stub(ValidationService.prototype, 'validateXLSX').resolves({}); sinon.stub(ValidationService.prototype, 'persistValidationResults').resolves(true); try { const dbConnection = getMockDBConnection(); const service = new ValidationService(dbConnection); - await service.occurrenceTemplateValidation(xlsxCsv, 1); + await service.templateValidation(xlsxCsv, 1); expect.fail(); } catch (error) { expect(error).to.be.instanceOf(SubmissionError); @@ -505,7 +505,7 @@ describe('ValidationService', () => { xlsx: new XLSXCSV(buildFile('test file', {})) }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); - const validation = sinon.stub(service, 'occurrenceTemplateValidation').resolves(); + const validation = sinon.stub(service, 'templateValidation').resolves(); const submissionStatus = sinon.stub(service.submissionRepository, 'insertSubmissionStatus').resolves(); await service.validateFile(1, 1); @@ -522,7 +522,7 @@ describe('ValidationService', () => { }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); const validation = sinon - .stub(service, 'occurrenceTemplateValidation') + .stub(service, 'templateValidation') .throws(SubmissionErrorFromMessageType(SUBMISSION_MESSAGE_TYPE.MISSING_VALIDATION_SCHEMA)); const submissionStatus = sinon.stub(service.submissionRepository, 'insertSubmissionStatus').resolves(); const insertError = sinon.stub(service.errorService, 'insertSubmissionError').resolves(); @@ -545,7 +545,7 @@ describe('ValidationService', () => { xlsx: new XLSXCSV(buildFile('test file', {})) }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); - const validation = sinon.stub(service, 'occurrenceTemplateValidation').throws(new Error()); + const validation = sinon.stub(service, 'templateValidation').throws(new Error()); const submissionStatus = sinon.stub(service.submissionRepository, 'insertSubmissionStatus').resolves(); const insertError = sinon.stub(service.errorService, 'insertSubmissionError').resolves(); @@ -707,7 +707,7 @@ describe('ValidationService', () => { }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); - const validate = sinon.stub(service, 'occurrenceTemplateValidation').resolves(); + const validate = sinon.stub(service, 'templateValidation').resolves(); const transform = sinon .stub(service, 'templateTransformation') .throws(SubmissionErrorFromMessageType(SUBMISSION_MESSAGE_TYPE.FAILED_TRANSFORM_XLSX)); @@ -734,7 +734,7 @@ describe('ValidationService', () => { }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); - const validate = sinon.stub(service, 'occurrenceTemplateValidation').resolves(); + const validate = sinon.stub(service, 'templateValidation').resolves(); const transform = sinon.stub(service, 'templateTransformation').throws(); const insertError = sinon.stub(service.errorService, 'insertSubmissionError').resolves(); sinon.stub(service, 'scrapeDwCAndUploadOccurrences').resolves(); @@ -759,7 +759,7 @@ describe('ValidationService', () => { }; const prep = sinon.stub(service, 'templatePreparation').resolves(mockPrep); - const validate = sinon.stub(service, 'occurrenceTemplateValidation').resolves(); + const validate = sinon.stub(service, 'templateValidation').resolves(); const status = sinon.stub(service.submissionRepository, 'insertSubmissionStatus').resolves(); const transform = sinon.stub(service, 'templateTransformation').resolves(); const decorate = sinon.stub(service.dwCService, 'decorateDwCJSON').resolves(); @@ -1026,7 +1026,7 @@ describe('ValidationService', () => { const service = mockService(); const xlsx = new XLSXCSV(buildFile('test file', {})); const parser = new ValidationSchemaParser({}); - const response = await service.validateXLSX_occurrence(xlsx, parser); + const response = await service.validateXLSX(xlsx, parser); expect(response.media_state.isValid).to.be.true; expect(response.media_state.fileErrors).is.empty; @@ -1066,7 +1066,7 @@ describe('ValidationService', () => { sinon.stub(DWCArchive.prototype, 'validateContent'); sinon.stub(XLSXCSV.prototype, 'getContentState').returns([mockState]); - const response = await service.validateXLSX_occurrence(xlsx, parser); + const response = await service.validateXLSX(xlsx, parser); expect(response.csv_state).is.not.empty; expect(response.csv_state[0].headerErrors).is.not.empty; expect(response.csv_state[0].rowErrors).is.not.empty; diff --git a/api/src/services/validation-service.ts b/api/src/services/validation-service.ts index d3bf87d5e4..d1c3dd1485 100644 --- a/api/src/services/validation-service.ts +++ b/api/src/services/validation-service.ts @@ -74,7 +74,7 @@ export class ValidationService extends DBService { defaultLog.debug({ label: 'validateFile', submissionId, surveyId }); try { const submissionPrep = await this.templatePreparation(submissionId); - await this.occurrenceTemplateValidation(submissionPrep.xlsx, surveyId); + await this.templateValidation(submissionPrep.xlsx, surveyId); // insert template validated status await this.submissionRepository.insertSubmissionStatus(submissionId, SUBMISSION_STATUS_TYPE.TEMPLATE_VALIDATED); @@ -158,7 +158,7 @@ export class ValidationService extends DBService { const submissionPrep = await this.templatePreparation(submissionId); // Run template validations - await this.occurrenceTemplateValidation(submissionPrep.xlsx, surveyId); + await this.templateValidation(submissionPrep.xlsx, surveyId); // Insert validation complete status await this.submissionRepository.insertSubmissionStatus(submissionId, SUBMISSION_STATUS_TYPE.TEMPLATE_VALIDATED); @@ -264,12 +264,12 @@ export class ValidationService extends DBService { } } - async occurrenceTemplateValidation(xlsx: XLSXCSV, surveyId: number) { + async templateValidation(xlsx: XLSXCSV, surveyId: number) { defaultLog.debug({ label: 'templateValidation' }); try { const schema = await this.getValidationSchema(xlsx, surveyId); const schemaParser = this.getValidationRules(schema); - const csvState = this.validateXLSX_occurrence(xlsx, schemaParser); + const csvState = this.validateXLSX(xlsx, schemaParser); await this.persistValidationResults(csvState.csv_state, csvState.media_state); } catch (error) { if (error instanceof SubmissionError) { @@ -359,12 +359,16 @@ export class ValidationService extends DBService { return validationSchemaParser; } - validateXLSX_occurrence(file: XLSXCSV, parser: ValidationSchemaParser) { + validateXLSX(file: XLSXCSV, parser: ValidationSchemaParser) { // Run media validations file.validateMedia(parser); const media_state = file.getMediaState(); + if (!media_state.isValid) { + return { csv_state: ([] as unknown) as ICsvState[], media_state }; + } + // Run CSV content validations file.validateContent(parser); const csv_state = file.getContentState(); @@ -398,7 +402,9 @@ export class ValidationService extends DBService { const errors: MessageError[] = []; mediaState.fileErrors?.forEach((fileError) => { - errors.push(new MessageError(SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA, `${fileError}`, 'Miscellaneous')); + errors.push( + new MessageError(SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA, `${fileError}`, SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA) + ); }); csvState?.forEach((csvStateItem) => { @@ -431,13 +437,13 @@ export class ValidationService extends DBService { ) ); }); - - if (!mediaState.isValid || csvState?.some((item) => !item.isValid)) { - // At least 1 error exists, skip remaining steps - parseError = true; - } }); + if (mediaState.fileErrors?.length || csvState?.some((item) => !item.isValid)) { + // At least 1 error exists, skip remaining steps + parseError = true; + } + if (parseError) { throw new SubmissionError({ messages: errors }); } From fab506bbfefedb37c5704806a3e708fef07a2db9 Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Mon, 1 May 2023 12:00:42 -0700 Subject: [PATCH 05/14] fixed tests --- api/src/services/summary-service.test.ts | 8 ++++---- api/src/services/summary-service.ts | 4 ++-- api/src/services/validation-service.test.ts | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/api/src/services/summary-service.test.ts b/api/src/services/summary-service.test.ts index 626fd4fea0..a4203cf3f0 100644 --- a/api/src/services/summary-service.test.ts +++ b/api/src/services/summary-service.test.ts @@ -490,7 +490,7 @@ describe('SummaryService', () => { ]); const getRules = sinon.stub(service, 'getValidationRules').resolves(''); - const validate = sinon.stub(service, 'validateXLSX_summary').resolves({}); + const validate = sinon.stub(service, 'validateXLSX').resolves({}); const persistResults = sinon.stub(service, 'persistSummaryValidationResults').resolves(); const logFoundValidation = sinon.stub(SummaryRepository.prototype, 'insertSummarySubmissionMessage').resolves(); @@ -520,7 +520,7 @@ describe('SummaryService', () => { .stub(service, 'getSummaryTemplateSpeciesRecords') .resolves([makeMockTemplateSpeciesRecord(1)]); const getRules = sinon.stub(service, 'getValidationRules').resolves(''); - const validate = sinon.stub(service, 'validateXLSX_summary').resolves({}); + const validate = sinon.stub(service, 'validateXLSX').resolves({}); const persistResults = sinon.stub(service, 'persistSummaryValidationResults').resolves(); await service.summaryTemplateValidation(xlsxCsv, 1); @@ -541,7 +541,7 @@ describe('SummaryService', () => { const getValidation = sinon.stub(service, 'getSummaryTemplateSpeciesRecords').resolves(templateSpeciesRecords); const getRules = sinon.stub(service, 'getValidationRules').resolves(''); - const validate = sinon.stub(service, 'validateXLSX_summary').resolves({}); + const validate = sinon.stub(service, 'validateXLSX').resolves({}); const persistResults = sinon.stub(service, 'persistSummaryValidationResults').resolves(); await service.summaryTemplateValidation(xlsxCsv, 1); @@ -776,7 +776,7 @@ describe('SummaryService', () => { const service = mockService(); const xlsx = new XLSXCSV(buildFile('test file', {})); const parser = new ValidationSchemaParser({}); - const response = await service.validateXLSX_summary(xlsx, parser); + const response = await service.validateXLSX(xlsx, parser); expect(response.media_state.isValid).to.be.true; expect(response.media_state.fileErrors).is.empty; diff --git a/api/src/services/summary-service.ts b/api/src/services/summary-service.ts index 46591b7c36..a930580039 100644 --- a/api/src/services/summary-service.ts +++ b/api/src/services/summary-service.ts @@ -211,7 +211,7 @@ export class SummaryService extends DBService { } const schemaParser = this.getValidationRules(validationSchema); - const csvState = this.validateXLSX_summary(xlsx, schemaParser); + const csvState = this.validateXLSX(xlsx, schemaParser); await this.persistSummaryValidationResults(csvState.csv_state, csvState.media_state); } catch (error) { if (error instanceof SubmissionError) { @@ -300,7 +300,7 @@ export class SummaryService extends DBService { * @param {ValidationSchemaParser} parser * @returns {ICsvMediaState} */ - validateXLSX_summary(file: XLSXCSV, parser: ValidationSchemaParser): ICsvMediaState { + validateXLSX(file: XLSXCSV, parser: ValidationSchemaParser): ICsvMediaState { defaultLog.debug({ label: 'validateXLSX' }); // Run media validations diff --git a/api/src/services/validation-service.test.ts b/api/src/services/validation-service.test.ts index 615a178ec3..87a22a5c4f 100644 --- a/api/src/services/validation-service.test.ts +++ b/api/src/services/validation-service.test.ts @@ -1032,6 +1032,24 @@ describe('ValidationService', () => { expect(response.media_state.fileErrors).is.empty; }); + it('should return early if media_state is invalid', async () => { + const service = mockService(); + const mockMediaState = { + fileName: 'test file', + isValid: false + } as IMediaState; + const xlsx = new XLSXCSV(buildFile('test file', {})); + const parser = new ValidationSchemaParser({}); + + sinon.stub(XLSXCSV.prototype, 'getMediaState').returns(mockMediaState); + + const result = await service.validateXLSX(xlsx, parser); + + expect(result.csv_state).to.be.eql([]); + expect(result.media_state.fileName).to.be.eql(mockMediaState.fileName); + expect(result.media_state.isValid).to.be.eql(false); + }); + it('should return valid state object with content errors', async () => { const service = mockService(); const mockState = { From 6b442232c2c8b18ebb18e5d95fd5dddeb218069f Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Mon, 1 May 2023 12:05:42 -0700 Subject: [PATCH 06/14] tweak --- api/src/services/summary-service.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/services/summary-service.test.ts b/api/src/services/summary-service.test.ts index a4203cf3f0..e2794a05fb 100644 --- a/api/src/services/summary-service.test.ts +++ b/api/src/services/summary-service.test.ts @@ -601,7 +601,7 @@ describe('SummaryService', () => { } }); - it('should1 throw INVALID_MEDIA error if validateXLSX fails with invalid media', async () => { + it('should throw INVALID_MEDIA error if validateXLSX fails with invalid media', async () => { const service = mockService(); const file = new MediaFile('test.txt', 'text/plain', Buffer.of(0)); const xlsxCsv = new XLSXCSV(file); From faaa6d831dbb73b1b5ab400cc0e20b833b9ce57c Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Tue, 2 May 2023 10:49:27 -0700 Subject: [PATCH 07/14] apply fix to summary result validateXLSX --- api/src/services/summary-service.test.ts | 38 ++++++++---------------- api/src/services/summary-service.ts | 20 ++++++++----- api/src/services/validation-service.ts | 3 +- 3 files changed, 26 insertions(+), 35 deletions(-) diff --git a/api/src/services/summary-service.test.ts b/api/src/services/summary-service.test.ts index e2794a05fb..dbd2d8ec3f 100644 --- a/api/src/services/summary-service.test.ts +++ b/api/src/services/summary-service.test.ts @@ -601,36 +601,22 @@ describe('SummaryService', () => { } }); - it('should throw INVALID_MEDIA error if validateXLSX fails with invalid media', async () => { + it('should return early if media_state is invalid', async () => { const service = mockService(); - const file = new MediaFile('test.txt', 'text/plain', Buffer.of(0)); - const xlsxCsv = new XLSXCSV(file); - const validation = 'test-template-validation-schema'; - const mockSchemaParser = { validationSchema: validation }; + const mockMediaState = { + fileName: 'test file', + isValid: false + } as IMediaState; + const xlsx = new XLSXCSV(buildFile('test file', {})); + const parser = new ValidationSchemaParser({}); - sinon.stub(XLSXCSV.prototype, 'validateMedia'); - sinon.stub(XLSXCSV.prototype, 'getMediaState').returns({ - isValid: false, - fileName: 'test filename' - }); + sinon.stub(XLSXCSV.prototype, 'getMediaState').returns(mockMediaState); - const getValidation = sinon.stub(service, 'getValidationRules').resolves(mockSchemaParser); - sinon.stub(FileUtils, 'getFileFromS3').resolves('file from s3' as any); - sinon - .stub(service, 'getSummaryTemplateSpeciesRecords') - .resolves([{ ...makeMockTemplateSpeciesRecord(1), validation }]); + const result = await service.validateXLSX(xlsx, parser); - try { - await service.summaryTemplateValidation(xlsxCsv, 1); - expect.fail(); - } catch (error) { - expect(getValidation).to.be.calledWith('test-template-validation-schema'); - expect(error).to.be.instanceOf(SummarySubmissionError); - if (error instanceof SummarySubmissionError) { - expect(error.summarySubmissionMessages.length).to.equal(1); - expect(error.summarySubmissionMessages[0].type).to.equal(SUMMARY_SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA); - } - } + expect(result.csv_state).to.be.eql([]); + expect(result.media_state.fileName).to.be.eql(mockMediaState.fileName); + expect(result.media_state.isValid).to.be.eql(false); }); }); diff --git a/api/src/services/summary-service.ts b/api/src/services/summary-service.ts index a930580039..121e06f478 100644 --- a/api/src/services/summary-service.ts +++ b/api/src/services/summary-service.ts @@ -308,7 +308,7 @@ export class SummaryService extends DBService { const media_state = file.getMediaState(); if (!media_state.isValid) { - throw SummarySubmissionErrorFromMessageType(SUMMARY_SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA); + return { csv_state: ([] as unknown) as ICsvState[], media_state }; } // Run CSV content validations @@ -331,7 +331,13 @@ export class SummaryService extends DBService { const errors: MessageError[] = []; mediaState.fileErrors?.forEach((fileError) => { - errors.push(new MessageError(SUMMARY_SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA, `${fileError}`, 'Miscellaneous')); + errors.push( + new MessageError( + SUMMARY_SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA, + `${fileError}`, + SUMMARY_SUBMISSION_MESSAGE_TYPE.INVALID_MEDIA + ) + ); }); csvState?.forEach((csvStateItem) => { @@ -364,13 +370,13 @@ export class SummaryService extends DBService { ) ); }); - - if (!mediaState.isValid || csvState?.some((item) => !item.isValid)) { - // At least 1 error exists, skip remaining steps - parseError = true; - } }); + if (mediaState.fileErrors?.length || csvState?.some((item) => !item.isValid)) { + // At least 1 error exists, skip remaining steps + parseError = true; + } + if (parseError) { throw new SummarySubmissionError({ messages: errors }); } diff --git a/api/src/services/validation-service.ts b/api/src/services/validation-service.ts index d1c3dd1485..b30d88879e 100644 --- a/api/src/services/validation-service.ts +++ b/api/src/services/validation-service.ts @@ -360,7 +360,7 @@ export class ValidationService extends DBService { } validateXLSX(file: XLSXCSV, parser: ValidationSchemaParser) { - // Run media validations + defaultLog.debug({ label: 'validateXLSX' }); file.validateMedia(parser); const media_state = file.getMediaState(); @@ -369,7 +369,6 @@ export class ValidationService extends DBService { return { csv_state: ([] as unknown) as ICsvState[], media_state }; } - // Run CSV content validations file.validateContent(parser); const csv_state = file.getContentState(); From 49775a54a021593a5bb33bf30a310193d031e34b Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Tue, 2 May 2023 10:57:28 -0700 Subject: [PATCH 08/14] fixed bug found by sonar cloud --- api/src/services/summary-service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/services/summary-service.ts b/api/src/services/summary-service.ts index 121e06f478..d9fdc7e698 100644 --- a/api/src/services/summary-service.ts +++ b/api/src/services/summary-service.ts @@ -203,7 +203,7 @@ export class SummaryService extends DBService { if (summarySubmissionId) { const { summary_template_species_id } = templateRecord; const count = summaryTemplateSpeciesRecords.length; - this.summaryRepository.insertSummarySubmissionMessage( + await this.summaryRepository.insertSummarySubmissionMessage( summarySubmissionId, SUMMARY_SUBMISSION_MESSAGE_TYPE.FOUND_VALIDATION, `Found validation having summary template species ID '${summary_template_species_id}' among ${count} record(s).` From 16223f3e3b78e9c311aca5f5c1c5ef8a73dc5c3c Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Tue, 2 May 2023 11:01:15 -0700 Subject: [PATCH 09/14] more sonar cloud feedback --- .../{projectId}/survey/{surveyId}/observation/submission/get.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/paths/project/{projectId}/survey/{surveyId}/observation/submission/get.ts b/api/src/paths/project/{projectId}/survey/{surveyId}/observation/submission/get.ts index 9d4f75af57..747ef9d161 100644 --- a/api/src/paths/project/{projectId}/survey/{surveyId}/observation/submission/get.ts +++ b/api/src/paths/project/{projectId}/survey/{surveyId}/observation/submission/get.ts @@ -272,7 +272,7 @@ export function getOccurrenceSubmission(): RequestHandler { surveyObservationData: { occurrence_submission_id: occurrenceSubmission.occurrence_submission_id, inputFileName: occurrenceSubmission.input_file_name, - status: occurrenceSubmission.submission_status_type_name || null, + status: occurrenceSubmission.submission_status_type_name ?? null, isValidating: !isDoneValidating, messageTypes }, From 2b99726eea40dd84869777273c99ec12fee80eb7 Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Tue, 2 May 2023 11:14:10 -0700 Subject: [PATCH 10/14] removed unused test artifact --- app/src/features/projects/create/CreateProjectPage.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/features/projects/create/CreateProjectPage.test.tsx b/app/src/features/projects/create/CreateProjectPage.test.tsx index 76c816c2f3..e284620b2e 100644 --- a/app/src/features/projects/create/CreateProjectPage.test.tsx +++ b/app/src/features/projects/create/CreateProjectPage.test.tsx @@ -402,7 +402,7 @@ describe('CreateProjectPage', () => { date: '2021-01-20' }); - const { getByText, getByTestId } = renderContainer(); + const { getByTestId } = renderContainer(); const saveDraftButton = await getByTestId('save-draft-button'); From f3342577b2b09fc12ef56aa29a6902e49daab98a Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Wed, 3 May 2023 17:57:13 -0700 Subject: [PATCH 11/14] adding the row number to the primary key --- api/src/services/validation-service.ts | 2 +- .../utils/media/csv/validation/csv-row-validator.ts | 6 ++++-- .../media/xlsx/transformation/xlsx-transform.ts | 12 +++++++----- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/api/src/services/validation-service.ts b/api/src/services/validation-service.ts index b30d88879e..5460127e04 100644 --- a/api/src/services/validation-service.ts +++ b/api/src/services/validation-service.ts @@ -438,7 +438,7 @@ export class ValidationService extends DBService { }); }); - if (mediaState.fileErrors?.length || csvState?.some((item) => !item.isValid)) { + if (!mediaState.isValid || csvState?.some((item) => !item.isValid)) { // At least 1 error exists, skip remaining steps parseError = true; } diff --git a/api/src/utils/media/csv/validation/csv-row-validator.ts b/api/src/utils/media/csv/validation/csv-row-validator.ts index d9347422c8..cf69bbc630 100644 --- a/api/src/utils/media/csv/validation/csv-row-validator.ts +++ b/api/src/utils/media/csv/validation/csv-row-validator.ts @@ -387,9 +387,11 @@ export const getUniqueColumnsValidator = (config?: FileColumnUniqueValidatorConf csvWorksheet.csvValidation.addRowErrors([ { errorCode: SUBMISSION_MESSAGE_TYPE.NON_UNIQUE_KEY, - message: `Duplicate key(s): ${key} found in column(s): ${config.file_column_unique_validator.column_names.join( + message: `Row ${ + rowIndex + 2 + } has duplicate values (${key}) to another row. The combination of values in columns: ${config.file_column_unique_validator.column_names.join( ', ' - )}. Keys must be unique for proper template transformation`, + )} must be unique across rows. Details: `, col: key, row: rowIndex + 2 } diff --git a/api/src/utils/media/xlsx/transformation/xlsx-transform.ts b/api/src/utils/media/xlsx/transformation/xlsx-transform.ts index 451eae2682..9747e492fd 100644 --- a/api/src/utils/media/xlsx/transformation/xlsx-transform.ts +++ b/api/src/utils/media/xlsx/transformation/xlsx-transform.ts @@ -135,17 +135,17 @@ export class XLSXTransform { const worksheetJSONWithKey: RowObject[] = []; for (let i = 0; i < numberOfRows; i++) { - const primaryKey = this._getKeyForRowObject(worksheetJSON[i], templateMetaSchema.primaryKey); + const primaryKey = this._getKeyForRowObject(worksheetJSON[i], templateMetaSchema.primaryKey, i); if (!primaryKey) { continue; } - const parentKey = this._getKeyForRowObject(worksheetJSON[i], templateMetaSchema.parentKey); + const parentKey = this._getKeyForRowObject(worksheetJSON[i], templateMetaSchema.parentKey, i); const childKeys = templateMetaSchema.foreignKeys .map((foreignKeys: { sheetName: TemplateColumnName; primaryKey: string[] }) => { - return this._getKeyForRowObject(worksheetJSON[i], foreignKeys.primaryKey); + return this._getKeyForRowObject(worksheetJSON[i], foreignKeys.primaryKey, i); }) .filter((item): item is string => !!item); @@ -164,7 +164,7 @@ export class XLSXTransform { return worksheetJSONWithKey; } - _getKeyForRowObject(RowObject: Record, keyColumnNames: string[]): string { + _getKeyForRowObject(RowObject: Record, keyColumnNames: string[], rowNumber: number): string { if (!keyColumnNames.length) { return ''; } @@ -178,7 +178,9 @@ export class XLSXTransform { return RowObject[columnName]; }) .filter((value) => !isNaN || value) - .join(':'); + .join(':') + .concat(':') + .concat(rowNumber.toString()); return primaryKey; } From a3bbfec8cb570342987120422485e9ad9ca77640 Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Thu, 4 May 2023 09:34:47 -0700 Subject: [PATCH 12/14] addressed sonar cloud feedback --- .../media/csv/validation/csv-row-validator.ts | 2 +- .../media/xlsx/transformation/xlsx-transform.ts | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/api/src/utils/media/csv/validation/csv-row-validator.ts b/api/src/utils/media/csv/validation/csv-row-validator.ts index cf69bbc630..07d895df0e 100644 --- a/api/src/utils/media/csv/validation/csv-row-validator.ts +++ b/api/src/utils/media/csv/validation/csv-row-validator.ts @@ -324,7 +324,7 @@ export const getValidFormatFieldsValidator = (config?: ColumnFormatValidatorConf return csvWorksheet; } - const regexFlags = config.column_format_validator.reg_exp_flags || ''; + const regexFlags = config.column_format_validator.reg_exp_flags ?? ''; const regex = new RegExp(config.column_format_validator.reg_exp, regexFlags); diff --git a/api/src/utils/media/xlsx/transformation/xlsx-transform.ts b/api/src/utils/media/xlsx/transformation/xlsx-transform.ts index 9747e492fd..40c078e370 100644 --- a/api/src/utils/media/xlsx/transformation/xlsx-transform.ts +++ b/api/src/utils/media/xlsx/transformation/xlsx-transform.ts @@ -45,7 +45,7 @@ export type RowObject = { _data: { [key: string]: NonObjectPrimitive }; _name: string; _key: string; - _parentKey: string | ''; + _parentKey: string; _type: 'root' | 'leaf' | ''; _row: number; _childKeys: string[]; @@ -388,7 +388,7 @@ export class XLSXTransform { const fields = mapSchemaItem.fields; - if (fields && fields.length) { + if (fields?.length) { // For each item in the `fields` array for (const fieldsItem of fields) { // The final computed cell value for this particular schema field element @@ -423,7 +423,7 @@ export class XLSXTransform { let pathValue = ''; if (Array.isArray(pathValues)) { // cell value is the concatenation of multiple values - pathValue = (pathValues.length && pathValues.flat(Infinity).join(columnValueItem.join || ':')) || ''; + pathValue = (pathValues.length && pathValues.flat(Infinity).join(columnValueItem.join ?? ':')) || ''; } else { // cell value is a single value pathValue = pathValues || ''; @@ -450,20 +450,20 @@ export class XLSXTransform { if (Array.isArray(postfixPathValues)) { // postfix value is the concatenation of multiple values postfixValue = - (postfixPathValues.length && postfixPathValues.join(columnValueItem.join || ':')) || ''; + (postfixPathValues.length && postfixPathValues.join(columnValueItem.join ?? ':')) || ''; } else { // postfix value is a single value postfixValue = postfixPathValues || ''; } } - cellValue = `${cellValue}${columnValueItem.join || ':'}${postfixValue}`; + cellValue = `${cellValue}${columnValueItem.join ?? ':'}${postfixValue}`; } } // Check for `add` additions at the field level const columnValueItemAdd = columnValueItem.add; - if (columnValueItemAdd && columnValueItemAdd.length) { + if (columnValueItemAdd?.length) { for (const columnValueItemAddItem of columnValueItemAdd) { mapSchema.push(columnValueItemAddItem); } @@ -485,7 +485,7 @@ export class XLSXTransform { // Check for additions at the sheet level const sheetAdds = mapSchemaItem.add; - if (sheetAdds && sheetAdds.length) { + if (sheetAdds?.length) { for (const sheetAddsItem of sheetAdds) { mapSchema.push(sheetAddsItem); } @@ -527,7 +527,7 @@ export class XLSXTransform { _processIfNotEmptyCondition(check: IfNotEmptyCheck, rowObjects: RowObject[]): boolean { const pathValues = this._processPaths([check.ifNotEmpty], rowObjects); - if (!pathValues || !pathValues.length) { + if (!pathValues?.length) { // condition failed return false; } From 6c8329920e0ff2566ff48a0151f42f19a7726b9b Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Mon, 8 May 2023 09:47:05 -0700 Subject: [PATCH 13/14] fixes --- api/src/services/summary-service.ts | 2 +- api/src/services/validation-service.ts | 11 +++++------ .../media/xlsx/transformation/xlsx-transform.ts | 12 +++++------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/api/src/services/summary-service.ts b/api/src/services/summary-service.ts index d9fdc7e698..bed92ff4e1 100644 --- a/api/src/services/summary-service.ts +++ b/api/src/services/summary-service.ts @@ -308,7 +308,7 @@ export class SummaryService extends DBService { const media_state = file.getMediaState(); if (!media_state.isValid) { - return { csv_state: ([] as unknown) as ICsvState[], media_state }; + return { csv_state: [], media_state }; } // Run CSV content validations diff --git a/api/src/services/validation-service.ts b/api/src/services/validation-service.ts index 5460127e04..9261e90a3d 100644 --- a/api/src/services/validation-service.ts +++ b/api/src/services/validation-service.ts @@ -366,7 +366,10 @@ export class ValidationService extends DBService { const media_state = file.getMediaState(); if (!media_state.isValid) { - return { csv_state: ([] as unknown) as ICsvState[], media_state }; + return { + csv_state: [], + media_state + }; } file.validateContent(parser); @@ -397,7 +400,7 @@ export class ValidationService extends DBService { async persistValidationResults(csvState: ICsvState[], mediaState: IMediaState): Promise { defaultLog.debug({ label: 'persistValidationResults', message: 'validationResults' }); - let parseError = false; + const parseError = false; const errors: MessageError[] = []; mediaState.fileErrors?.forEach((fileError) => { @@ -440,10 +443,6 @@ export class ValidationService extends DBService { if (!mediaState.isValid || csvState?.some((item) => !item.isValid)) { // At least 1 error exists, skip remaining steps - parseError = true; - } - - if (parseError) { throw new SubmissionError({ messages: errors }); } diff --git a/api/src/utils/media/xlsx/transformation/xlsx-transform.ts b/api/src/utils/media/xlsx/transformation/xlsx-transform.ts index 40c078e370..6d8d48766c 100644 --- a/api/src/utils/media/xlsx/transformation/xlsx-transform.ts +++ b/api/src/utils/media/xlsx/transformation/xlsx-transform.ts @@ -135,17 +135,17 @@ export class XLSXTransform { const worksheetJSONWithKey: RowObject[] = []; for (let i = 0; i < numberOfRows; i++) { - const primaryKey = this._getKeyForRowObject(worksheetJSON[i], templateMetaSchema.primaryKey, i); + const primaryKey = this._getKeyForRowObject(worksheetJSON[i], templateMetaSchema.primaryKey); if (!primaryKey) { continue; } - const parentKey = this._getKeyForRowObject(worksheetJSON[i], templateMetaSchema.parentKey, i); + const parentKey = this._getKeyForRowObject(worksheetJSON[i], templateMetaSchema.parentKey); const childKeys = templateMetaSchema.foreignKeys .map((foreignKeys: { sheetName: TemplateColumnName; primaryKey: string[] }) => { - return this._getKeyForRowObject(worksheetJSON[i], foreignKeys.primaryKey, i); + return this._getKeyForRowObject(worksheetJSON[i], foreignKeys.primaryKey); }) .filter((item): item is string => !!item); @@ -164,7 +164,7 @@ export class XLSXTransform { return worksheetJSONWithKey; } - _getKeyForRowObject(RowObject: Record, keyColumnNames: string[], rowNumber: number): string { + _getKeyForRowObject(RowObject: Record, keyColumnNames: string[]): string { if (!keyColumnNames.length) { return ''; } @@ -178,9 +178,7 @@ export class XLSXTransform { return RowObject[columnName]; }) .filter((value) => !isNaN || value) - .join(':') - .concat(':') - .concat(rowNumber.toString()); + .join(':'); return primaryKey; } From f44ce8430587f3f2d6800b00c4f33789aaa75b38 Mon Sep 17 00:00:00 2001 From: Anissa Agahchen Date: Mon, 8 May 2023 15:29:59 -0700 Subject: [PATCH 14/14] remove parse error --- api/src/services/summary-service.ts | 7 +------ api/src/services/validation-service.test.ts | 20 ++++++++++++++------ api/src/services/validation-service.ts | 5 +---- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/api/src/services/summary-service.ts b/api/src/services/summary-service.ts index bed92ff4e1..46d1a00146 100644 --- a/api/src/services/summary-service.ts +++ b/api/src/services/summary-service.ts @@ -327,7 +327,6 @@ export class SummaryService extends DBService { async persistSummaryValidationResults(csvState: ICsvState[], mediaState: IMediaState): Promise { defaultLog.debug({ label: 'persistSummaryValidationResults', message: 'validationResults' }); - let parseError = false; const errors: MessageError[] = []; mediaState.fileErrors?.forEach((fileError) => { @@ -372,12 +371,8 @@ export class SummaryService extends DBService { }); }); - if (mediaState.fileErrors?.length || csvState?.some((item) => !item.isValid)) { + if (!mediaState.isValid || csvState?.some((item) => !item.isValid)) { // At least 1 error exists, skip remaining steps - parseError = true; - } - - if (parseError) { throw new SummarySubmissionError({ messages: errors }); } } diff --git a/api/src/services/validation-service.test.ts b/api/src/services/validation-service.test.ts index 87a22a5c4f..f9a2ad137f 100644 --- a/api/src/services/validation-service.test.ts +++ b/api/src/services/validation-service.test.ts @@ -158,7 +158,7 @@ describe('ValidationService', () => { const getValidation = sinon.stub(ValidationService.prototype, 'getValidationSchema').resolves(''); const getRules = sinon.stub(ValidationService.prototype, 'getValidationRules').resolves(''); const validate = sinon.stub(ValidationService.prototype, 'validateXLSX').resolves({}); - const persistResults = sinon.stub(ValidationService.prototype, 'persistValidationResults').resolves(true); + const persistResults = sinon.stub(ValidationService.prototype, 'persistValidationResults').resolves(); const service = mockService(); await service.templateValidation(xlsxCsv, 1); @@ -177,7 +177,7 @@ describe('ValidationService', () => { sinon.stub(ValidationService.prototype, 'getValidationSchema').throws(new SubmissionError({})); sinon.stub(ValidationService.prototype, 'getValidationRules').resolves({}); sinon.stub(ValidationService.prototype, 'validateXLSX').resolves({}); - sinon.stub(ValidationService.prototype, 'persistValidationResults').resolves(true); + sinon.stub(ValidationService.prototype, 'persistValidationResults').resolves(); try { const dbConnection = getMockDBConnection(); @@ -389,16 +389,24 @@ describe('ValidationService', () => { } }); - it('should return false if no errors are present', async () => { + it('should pass if no errors are thrown', async () => { const service = mockService(); - const csvState: ICsvState[] = []; + const csvState: ICsvState[] = [ + { + fileName: '', + isValid: true, + keyErrors: [], + headerErrors: [], + rowErrors: [] + } + ]; const mediaState: IMediaState = { fileName: 'Test.xlsx', isValid: true }; + const response = await service.persistValidationResults(csvState, mediaState); - // no errors found, data is valid - expect(response).to.be.false; + expect(response).to.be.undefined; }); }); diff --git a/api/src/services/validation-service.ts b/api/src/services/validation-service.ts index 9261e90a3d..d4032e48a1 100644 --- a/api/src/services/validation-service.ts +++ b/api/src/services/validation-service.ts @@ -397,10 +397,9 @@ export class ValidationService extends DBService { return normalized; } - async persistValidationResults(csvState: ICsvState[], mediaState: IMediaState): Promise { + async persistValidationResults(csvState: ICsvState[], mediaState: IMediaState): Promise { defaultLog.debug({ label: 'persistValidationResults', message: 'validationResults' }); - const parseError = false; const errors: MessageError[] = []; mediaState.fileErrors?.forEach((fileError) => { @@ -445,8 +444,6 @@ export class ValidationService extends DBService { // At least 1 error exists, skip remaining steps throw new SubmissionError({ messages: errors }); } - - return parseError; } async getTransformationSchema(file: XLSXCSV, surveyId: number): Promise {