From 7faef5427faf6f6b16e7e8a6388b60ea09582d58 Mon Sep 17 00:00:00 2001 From: Craig Broady <79261988+cb-cs@users.noreply.github.com> Date: Thu, 8 Aug 2024 12:12:00 +0100 Subject: [PATCH] feat(cb2-13369): Amend centralDocs object validation to be optional (#413) * feat(cb2-13369): updated logic for central docs to be optional * feat(cb2-13369): corrected unit test * feat(cb2-13369): updated tests following changes to test type id list * feat(cb2-13369): linting * feat(cb2-13369): added more tests for better coverage of scenarios * feat(cb2-13369): removed fail logic as per ticket change * feat(cb2-13369): removed unused enum * feat(cb2-13369): removed test status submitted check as per po request --- package-lock.json | 8 +- package.json | 2 +- src/assets/Enums.ts | 1 + src/utils/validationUtil.ts | 33 ++-- tests/integration/postTestResults.intTest.ts | 16 -- tests/resources/test-results-post.json | 165 ++++++++----------- tests/resources/test-results.json | 7 +- tests/unit/insertTestResult.unitTest.ts | 105 ++++++++---- 8 files changed, 162 insertions(+), 175 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6910acc0d..bd9a88c5c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,7 @@ "@aws-sdk/client-lambda": "^3.552.0", "@aws-sdk/lib-dynamodb": "^3.552.0", "@aws-sdk/smithy-client": "^3.374.0", - "@dvsa/cvs-microservice-common": "1.2.0", + "@dvsa/cvs-microservice-common": "1.2.1", "@dvsa/cvs-type-definitions": "6.0.0", "@smithy/smithy-client": "^2.5.1", "@smithy/util-utf8": "^2.3.0", @@ -3515,9 +3515,9 @@ } }, "node_modules/@dvsa/cvs-microservice-common": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/@dvsa/cvs-microservice-common/-/cvs-microservice-common-1.2.0.tgz", - "integrity": "sha512-OqY5LkH9Is0Gv8DGkWfHiTHUnXCprF+d3VyjPZxjsnrKihPcJpzdo9B7eybYL3rxSG4STnw8iJb0dIqfNqqShQ==", + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/@dvsa/cvs-microservice-common/-/cvs-microservice-common-1.2.1.tgz", + "integrity": "sha512-/1uAhshsNKik/PLQ6VrDqnh1x2jOWAlnFP90o2a/CdVgv74YtWwUqYutFiclUkTpE0sefasVAhpqOIyRob6yDg==", "dependencies": { "class-transformer": "^0.5.1", "dayjs": "^1.11.11" diff --git a/package.json b/package.json index 986e55e76..0eea739bc 100644 --- a/package.json +++ b/package.json @@ -92,7 +92,7 @@ "joi": "^17.12.1", "js-yaml": "^3.14.1", "lodash": "^4.17.21", - "@dvsa/cvs-microservice-common": "1.2.0", + "@dvsa/cvs-microservice-common": "1.2.1", "moment": "^2.29.4", "moment-timezone": "^0.5.40", "node-yaml": "^4.0.1", diff --git a/src/assets/Enums.ts b/src/assets/Enums.ts index e4954864b..790b58c9c 100644 --- a/src/assets/Enums.ts +++ b/src/assets/Enums.ts @@ -63,6 +63,7 @@ export enum MESSAGES { ID_ALREADY_EXISTS = 'Test Result id already exists', CONDITIONAL_REQUEST_FAILED = 'The conditional request failed', REASON_FOR_ABANDONING_NOT_PRESENT = 'Reason for Abandoning not present on all abandoned tests', + CENTRAL_DOCS_NOT_AVAILABLE_FOR_TEST_TYPE = 'Central documents can not be issued for test type', } export enum VEHICLE_TYPE { diff --git a/src/utils/validationUtil.ts b/src/utils/validationUtil.ts index 664dbd170..6bbfd5cee 100644 --- a/src/utils/validationUtil.ts +++ b/src/utils/validationUtil.ts @@ -581,31 +581,24 @@ export class ValidationUtil { */ public static validateCentralDocs(testTypes: TestType[]): void { testTypes.forEach((testType) => { - if ( - TestTypeHelper.validateTestTypeIdInList( - CENTRAL_DOCS_TEST, - testType.testTypeId, - ) && - !testType?.centralDocs - ) { - throw new models.HTTPError( - 400, - `Central docs required for test type ${testType.testTypeId}`, - ); + // if centralDocs is not present, then no object to validate immediately return true + if (!testType.centralDocs) { + return true; } - if ( - !TestTypeHelper.validateTestTypeIdInList( - CENTRAL_DOCS_TEST, - testType.testTypeId, - ) && - testType?.centralDocs - ) { + + // if centralDocs is present, does the test type id exist in the list of central docs test types + const validTestTypeId = TestTypeHelper.validateTestTypeIdInList( + CENTRAL_DOCS_TEST, + testType.testTypeId, + ); + + // if the test type is not in the list of central docs test types, throw an error + if (!validTestTypeId) { throw new models.HTTPError( 400, - `Central documents can not be issued for test type ${testType.testTypeId}`, + `${enums.MESSAGES.CENTRAL_DOCS_NOT_AVAILABLE_FOR_TEST_TYPE} ${testType.testTypeId}`, ); } - return true; }); } } diff --git a/tests/integration/postTestResults.intTest.ts b/tests/integration/postTestResults.intTest.ts index 011436d3d..393efcd98 100644 --- a/tests/integration/postTestResults.intTest.ts +++ b/tests/integration/postTestResults.intTest.ts @@ -1,5 +1,4 @@ import supertest from 'supertest'; -import { CENTRAL_DOCS_TEST } from '@dvsa/cvs-microservice-common/classes/testTypes/Constants'; import { ITestResultPayload } from '../../src/models'; import testResultsPostMock from '../resources/test-results-post.json'; @@ -7,21 +6,6 @@ const url = 'http://localhost:3006/'; const request = supertest(url); describe('postTestResults', () => { - context('when submitting a test result with central docs', () => { - it('should return 400 when central docs are required but missing', async () => { - const testResult = - testResultsPostMock[15] as unknown as ITestResultPayload; - - testResult.testTypes[0].testTypeId = CENTRAL_DOCS_TEST.IDS[3]; - delete testResult.testTypes[0].centralDocs; - - const res = await request.post('test-results').send(testResult); - - expect(res.status).toBe(400); - expect(res.body).toBe('Central docs required for test type 47'); - }); - }); - context('when submitting an invalid test result', () => { it('should return 400 for missing required fields', async () => { const testResult = diff --git a/tests/resources/test-results-post.json b/tests/resources/test-results-post.json index ed3c91826..8f584e069 100644 --- a/tests/resources/test-results-post.json +++ b/tests/resources/test-results-post.json @@ -525,11 +525,6 @@ "certificateNumber": "12512ds", "testTypeName": "Annual test", "additionalNotesRecorded": "VEHICLE FRONT REGISTRATION PLATE MISSING", - "centralDocs": { - "issueRequired": false, - "notes": "notes", - "reasonsForIssue": ["issue reason"] - }, "customDefects": [ { "referenceNumber": "abcd", @@ -720,11 +715,6 @@ "code": "m", "description": "modification or change of engine" }, - "centralDocs": { - "issueRequired": false, - "notes": "notes", - "reasonsForIssue": ["issue reason"] - }, "emissionStandard": "0.03 g/kWh Euro IV PM", "fuelType": "gas-cng", "smokeTestKLimitApplied": "123", @@ -818,11 +808,6 @@ "reasonForAbandoning": null, "additionalCommentsForAbandon": null, "additionalNotesRecorded": "Test notes", - "centralDocs": { - "issueRequired": false, - "notes": "notes", - "reasonsForIssue": ["issue reason"] - }, "customDefects": [ { "referenceNumber": "abcd", @@ -896,11 +881,6 @@ "defectNotes": null } ], - "centralDocs": { - "issueRequired": false, - "notes": "notes", - "reasonsForIssue": ["issue reason"] - }, "defects": [] } ] @@ -967,11 +947,6 @@ "defectNotes": null } ], - "centralDocs": { - "issueRequired": false, - "notes": "notes", - "reasonsForIssue": ["issue reason"] - }, "defects": [] } ] @@ -1037,11 +1012,6 @@ "defectNotes": null } ], - "centralDocs": { - "issueRequired": false, - "notes": "notes", - "reasonsForIssue": ["issue reason"] - }, "defects": [] } ], @@ -1085,12 +1055,7 @@ } ], "defects": [], - "requiredStandards": [], - "centralDocs": { - "issueRequired": false, - "notes": "notes", - "reasonsForIssue": ["issue reason"] - } + "requiredStandards": [] } ], "testStationName": "Abshire-Kub", @@ -1191,42 +1156,62 @@ "bodyType": { "code": "test code", "description": "test description" } }, { - "vin": "YV3S2G5265A101407", - "vrm": "PO54ACV", - "countryOfRegistration": "gbz", - "euVehicleCategory": "m2", - "odometerReading": 4000, - "odometerReadingUnits": "kilometres", - "preparerName": "", - "preparerId": "", - "make": "DARWIN/EAST LANCS", - "model": "VYKING MYLLENIUM", - "bodyType": { - "code": "d", - "description": "double decker" - }, - "contingencyTestNumber": "1534432", - "testStartTimestamp": "2024-07-18T10:00:00.000", - "testEndTimestamp": "2024-07-18T12:00:00.000", + "testerStaffId": "1", + "systemNumber": "1218", + "testResultId": "195", + "testStartTimestamp": "2019-01-14T10:36:33.987Z", + "testEndTimestamp": "2019-01-14T10:36:33.987Z", + "testStatus": "submitted", + "numberOfWheelsDriven": 3, "testTypes": [ { - "testResult": "pass", - "testTypeName": "COIF with annual test", - "reasonForAbandoning": null, - "additionalCommentsForAbandon": null, - "certificateNumber": "", - "secondaryCertificateNumber": "34523453452345", - "testTypeStartTimestamp": "2024-07-18T10:00:00.000", - "testTypeEndTimestamp": "2024-07-18T12:00:00.000", "prohibitionIssued": false, - "seatbeltInstallationCheckDate": true, - "numberOfSeatbeltsFitted": 4, - "lastSeatbeltInstallationCheckDate": "2024-07-18T00:00:00.000", - "additionalNotesRecorded": "", - "defects": [], - "testTypeId": "142", - "name": "With annual test", - "customDefects": [], + "additionalCommentsForAbandon": "none", + "testTypeEndTimestamp": "2019-01-14T10:36:33.987Z", + "secondaryCertificateNumber": "1234", + "reasonForAbandoning": "none", + "testTypeId": "95", + "testTypeStartTimestamp": "2019-01-14T10:36:33.987Z", + "certificateNumber": "W01A00209", + "testTypeName": "First test", + "additionalNotesRecorded": "VEHICLE FRONT REGISTRATION PLATE MISSING", + "customDefects": [ + { + "referenceNumber": "abcd", + "defectName": "Some custom defect", + "defectNotes": "some defect noe" + } + ], + "defects": [ + { + "prohibitionIssued": false, + "deficiencyCategory": "major", + "deficiencyText": "missing.", + "prs": false, + "additionalInformation": { + "location": { + "axleNumber": null, + "horizontal": null, + "vertical": null, + "longitudinal": "front", + "rowNumber": null, + "lateral": null, + "seatNumber": null + }, + "notes": "None" + }, + "itemNumber": 1, + "deficiencyRef": "1.1.a", + "stdForProhibition": false, + "deficiencySubId": null, + "imDescription": "Registration Plate", + "deficiencyId": "a", + "itemDescription": "A registration plate:", + "imNumber": 1 + } + ], + "name": "First test", + "testResult": "pass", "centralDocs": { "issueRequired": false, "notes": "notes", @@ -1234,35 +1219,25 @@ } } ], - "testStationName": "Swansea Test HQ", - "testStationPNumber": "14-7160003", - "testStationType": "hq", - "testerStaffId": "8bd2avcf-c31e-4669-acb7-54bc3878e1b0", - "testerName": "CVS Dev", - "testerEmailAddress": "CVS.Dev@dvsagov.onmicrosoft.com", - "reasonForCreation": "TEST DATA", - "testResultId": "4ea8a6a9-b670-4960-9dfc-6a2242791c68", - "vehicleType": "psv", - "testStatus": "submitted", - "reasonForCancellation": "", - "systemNumber": "3128310", "vehicleClass": { - "code": "l", - "description": "large psv(ie: greater than 23 seats)" + "description": "motorbikes over 200cc or with a sidecar", + "code": "2" }, + "vin": "P012301230123", + "testStationName": "Rowe, Wunsch and Wisoky", "noOfAxles": 2, - "numberOfWheelsDriven": 2, - "regnDate": "2004-08-23", - "firstUseDate": null, - "createdByName": "Dobbie1", - "createdById": "3341037-bb122-4c54-9gfa-a2413asdfdsaf45", - "lastUpdatedAt": "2024-07-18T14:03:23.155Z", - "lastUpdatedByName": "SA_Daniel Searle1", - "lastUpdatedById": "3341037-bb122-4c54-9gfa-a2413asdfdsaf45", - "numberOfSeats": 70, + "vehicleType": "trl", + "countryOfRegistration": "united kingdom", + "preparerId": "ak4434", + "preparerName": "Durrell Vehicles Limited", "vehicleConfiguration": "rigid", - "typeOfTest": "contingency", - "source": "vtm", - "vehicleSize": "large" + "testStationType": "gvts", + "reasonForCancellation": "none", + "testerName": "Dorel", + "testStationPNumber": "87-1369569", + "testerEmailAddress": "dorel.popescu@dvsagov.uk", + "euVehicleCategory": "o1", + "trailerId": "abcd", + "firstUseDate": "2018-11-11" } ] diff --git a/tests/resources/test-results.json b/tests/resources/test-results.json index bc91efd6d..c45349307 100644 --- a/tests/resources/test-results.json +++ b/tests/resources/test-results.json @@ -1939,12 +1939,7 @@ "testTypeEndTimestamp": "2023-01-14T10:36:33.987Z", "testTypeId": "95", "testTypeName": "First test", - "testTypeStartTimestamp": "2023-01-14T10:36:33.987Z", - "centralDocs": { - "issueRequired": false, - "notes": "notes", - "reasonsForIssue": ["issue reason"] - } + "testTypeStartTimestamp": "2023-01-14T10:36:33.987Z" } ], "trailerId": "C234567", diff --git a/tests/unit/insertTestResult.unitTest.ts b/tests/unit/insertTestResult.unitTest.ts index 5762a7a2e..9ae741922 100644 --- a/tests/unit/insertTestResult.unitTest.ts +++ b/tests/unit/insertTestResult.unitTest.ts @@ -3239,7 +3239,7 @@ describe('insertTestResult', () => { expect(validationResult).toBe(true); }); - it('should create the record successfully when notes and reason for issue are present', () => { + it('should create the record successfully when notes and reasons for issue are present', () => { testResult.testTypes[0].centralDocs = { issueRequired: true, notes: 'notes', @@ -3250,20 +3250,21 @@ describe('insertTestResult', () => { expect(validationResult).toBe(true); }); - it('should throw a validation error when reason for issue is not present', () => { + it('should create the record successfully when all present and test result fail', () => { testResult.testTypes[0].centralDocs = { - issueRequired: true, - reasonsForIssue: ['reason'], + issueRequired: true, + notes: 'notes', + reasonsForIssue: ['reason'], }; + testResult.testTypes[0].testResult = 'fail'; const validationResult = - ValidationUtil.validateInsertTestResultPayload(testResult); + ValidationUtil.validateInsertTestResultPayload(testResult); expect(validationResult).toBe(true); }); - it('should throw a validation error when issue required is missing', () => { + it('should throw a validation error when reasons for issue is missing', () => { testResult.testTypes[0].centralDocs = { issueRequired: true, - notes: 'notes', } as any; expect(() => @@ -3280,16 +3281,64 @@ describe('insertTestResult', () => { ); } }); + + it('should throw a validation error when issue required is missing', () => { + testResult.testTypes[0].centralDocs = { + notes: 'notes', + reasonsForIssue: ['reason'], + } as any; + + expect(() => + ValidationUtil.validateInsertTestResultPayload(testResult), + ).toThrow(HTTPError); + + try { + ValidationUtil.validateInsertTestResultPayload(testResult); + } catch (err) { + const error = err as HTTPError; + expect(error.statusCode).toBe(400); + expect(error.body.errors[0]).toBe( + '"testTypes[0].centralDocs.issueRequired" is required', + ); + } + }); }); describe('when submitting a valid test without central docs present', () => { - it('should create the record successfully', () => { + it('should create the record successfully for a test type id not in the list and status of pass or prs', () => { testResult.testTypes[0].testTypeId = '1'; delete testResult.testTypes[0].centralDocs; const validationResult = ValidationUtil.validateInsertTestResultPayload(testResult); expect(validationResult).toBe(true); }); + + it('should create the record successfully for a test type id not in the list and status of fail', () => { + testResult.testTypes[0].testTypeId = '1'; + testResult.testTypes[0].testResult = 'fail'; + delete testResult.testTypes[0].centralDocs; + const validationResult = + ValidationUtil.validateInsertTestResultPayload(testResult); + expect(validationResult).toBe(true); + }); + + it('should create the record successfully for a test type id in the list with status of pass or prs', () => { + testResult.testTypes[0].testTypeId = '41'; + testResult.testTypes[0].testResult = 'prs'; + delete testResult.testTypes[0].centralDocs; + const validationResult = + ValidationUtil.validateInsertTestResultPayload(testResult); + expect(validationResult).toBe(true); + }); + + it('should create the record successfully for a test type id in the list with status of fail', () => { + testResult.testTypes[0].testTypeId = '41'; + testResult.testTypes[0].testResult = 'fail'; + delete testResult.testTypes[0].centralDocs; + const validationResult = + ValidationUtil.validateInsertTestResultPayload(testResult); + expect(validationResult).toBe(true); + }); }); }); @@ -3341,29 +3390,9 @@ describe('insertTestResult', () => { ).not.toThrow(); }); - it('should throw for invalid central docs', () => { - const testTypes = [ - createTestType(CENTRAL_DOCS_TEST.IDS[0], undefined), - createTestType('non-central-doc-id'), - ]; - expect(() => ValidationUtil.validateCentralDocs(testTypes)).toThrow( - HTTPError, - ); - try { - ValidationUtil.validateCentralDocs(testTypes); - } catch (error) { - console.log(); - expect(error).toBeInstanceOf(HTTPError); - expect(error.statusCode).toBe(400); - expect(error.body).toBe( - `Central docs required for test type ${CENTRAL_DOCS_TEST.IDS[0]}`, - ); - } - }); - it('should throw for invalid test type with central docs', () => { const testTypes = [ - createTestType('1', { + createTestType('1', { issueRequired: true, notes: 'notes', reasonsForIssue: ['reason'], @@ -3380,16 +3409,26 @@ describe('insertTestResult', () => { expect(error).toBeInstanceOf(HTTPError); expect(error.statusCode).toBe(400); expect(error.body).toBe( - "Central documents can not be issued for test type 1", + 'Central documents can not be issued for test type 1', ); } }); it('should throw for mixed valid and invalid types', () => { + const testResultFail = { + ...testResultsPostMock[15], + } as ITestResultPayload; + testResultFail.testTypes[0].centralDocs = { + issueRequired: true, + reasonsForIssue: [], + }; const testTypes = [ createTestType(CENTRAL_DOCS_TEST.IDS[0], { issueRequired: true }), - createTestType(CENTRAL_DOCS_TEST.IDS[1], undefined), - createTestType('non-central-doc-id'), + createTestType('1', { + issueRequired: true, + notes: 'notes', + reasonsForIssue: ['reason'], + }), ]; expect(() => ValidationUtil.validateCentralDocs(testTypes)).toThrow( HTTPError, @@ -3400,7 +3439,7 @@ describe('insertTestResult', () => { expect(error).toBeInstanceOf(HTTPError); expect(error.statusCode).toBe(400); expect(error.body).toBe( - `Central docs required for test type ${CENTRAL_DOCS_TEST.IDS[1]}`, + `Central documents can not be issued for test type 1`, ); } });