From 77e526b32ff7cb1db5b67be17d2516e0aace91f6 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Thu, 31 Oct 2024 13:26:37 +0000 Subject: [PATCH 01/33] (WIP) adding logic to handle transferStaffRecord --- .../models/BulkImport/csv/crossValidate.js | 14 ++++++ backend/server/models/classes/worker.js | 23 +++++++++ .../bulkUpload/data/workerHeaders.js | 48 +++++++++++++++++++ .../bulkUpload/validate/headers/worker.js | 14 +++++- .../establishments/bulkUpload/whichFile.js | 7 ++- .../bulkUpload/classes/workerCSVValidator.js | 30 ++++++++++++ lambdas/bulkUpload/serverless.yml | 2 +- 7 files changed, 134 insertions(+), 4 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 54d3ead0b4..d86d71b234 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -9,9 +9,23 @@ const crossValidate = async (csvWorkerSchemaErrors, myEstablishments, JSONWorker const isCqcRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); + await crossValidateTransferStaffRecord(myEstablishments, JSONWorker); + _crossValidateMainJobRole(csvWorkerSchemaErrors, isCqcRegulated, JSONWorker); }; +const crossValidateTransferStaffRecord = async (myEstablishments, JSONWorker) => { + if (!JSONWorker.transferStaffRecord) { + return; + } + console.log(JSONWorker.transferStaffRecord, '<--- new workerplace'); + console.log(JSONWorker.localId, '<--- previous workplace'); + console.log(JSONWorker.uniqueWorkerId, '<--- uniqueWorkerId'); + + // we can do something here to validate that this worker exists in the old workplace, + // and that the uniqueworkerid isn't duplicated in new workplace +}; + const _crossValidateMainJobRole = (csvWorkerSchemaErrors, isCqcRegulated, JSONWorker) => { if (!isCqcRegulated && JSONWorker.mainJobRoleId === 4) { csvWorkerSchemaErrors.unshift({ diff --git a/backend/server/models/classes/worker.js b/backend/server/models/classes/worker.js index ac2b9f167b..3bcf38379c 100644 --- a/backend/server/models/classes/worker.js +++ b/backend/server/models/classes/worker.js @@ -75,6 +75,8 @@ class Worker extends EntityValidator { // bulk upload status - this is never stored in database this._status = bulkUploadStatus; + + this._transferStaffRecord = null; } // returns true if valid establishment id @@ -193,6 +195,10 @@ class Worker extends EntityValidator { return this._status; } + get transferStaffRecord() { + return this._transferStaffRecord; + } + get contract() { return this._properties.get('Contract') ? this._properties.get('Contract').property : null; } @@ -355,6 +361,12 @@ class Worker extends EntityValidator { this._status = document.status; } + if (document.transferStaffRecord) { + console.log('============= detect transferStaffRecord ========== '); + console.log(document.transferStaffRecord, 'the new workplace this worker should move to'); + this._transferStaffRecord = document.transferStaffRecord; + } + // Consequential updates when one value means another should be empty or null // If their job isn't a registered nurse, remove their specialism and category @@ -599,6 +611,13 @@ class Worker extends EntityValidator { return; } + if (bulkUploaded && this._status === 'UPDATE' && this.transferStaffRecord) { + console.log('calling save on a worker which have transferStaffRecord filled'); + console.log(this.transferStaffRecord, '<--- new workplace that this worker should move to'); + // either we add a UPDATE workplaceFk call to externalTransaction here, + // or we return early and handle the workplaceFk update elsewhere + } + if (!this.uid) { this._log(Worker.LOG_ERROR, 'Not able to save an unknown uid'); throw new WorkerExceptions.WorkerSaveException( @@ -1352,6 +1371,10 @@ class Worker extends EntityValidator { myDefaultJSON.status = this._status; } + if (this._transferStaffRecord != null) { + myDefaultJSON.transferStaffRecord = this._transferStaffRecord; + } + // TODO: JSON schema validation if (showHistory && !showPropertyHistoryOnly) { return { diff --git a/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js b/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js index c016b83b4c..f2741a6845 100644 --- a/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js +++ b/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js @@ -45,9 +45,57 @@ const workerHeaders = [ 'QUALACH03NOTES', ]; +const workerHeadersWithTransferStaff = [ + 'LOCALESTID', + 'UNIQUEWORKERID', + 'TRANSFERSTAFFRECORD', + 'STATUS', + 'DISPLAYID', + 'NINUMBER', + 'POSTCODE', + 'DOB', + 'GENDER', + 'ETHNICITY', + 'NATIONALITY', + 'BRITISHCITIZENSHIP', + 'COUNTRYOFBIRTH', + 'YEAROFENTRY', + 'DISABLED', + 'CARECERT', + 'L2CARECERT', + 'RECSOURCE', + 'HANDCVISA', + 'INOUTUK', + 'STARTDATE', + 'STARTINSECT', + 'APPRENTICE', + 'EMPLSTATUS', + 'ZEROHRCONT', + 'DAYSSICK', + 'SALARYINT', + 'SALARY', + 'HOURLYRATE', + 'MAINJOBROLE', + 'MAINJRDESC', + 'CONTHOURS', + 'AVGHOURS', + 'NMCREG', + 'NURSESPEC', + 'AMHP', + 'SCQUAL', + 'NONSCQUAL', + 'QUALACH01', + 'QUALACH01NOTES', + 'QUALACH02', + 'QUALACH02NOTES', + 'QUALACH03', + 'QUALACH03NOTES', +]; + const workerHeadersWithoutCHGUNIQUEWRKIDasArray = workerHeaders.filter((header) => header !== 'CHGUNIQUEWRKID'); exports.workerHeadersWithCHGUNIQUEWRKID = workerHeaders.join(','); exports.workerHeadersWithoutCHGUNIQUEWRKID = workerHeaders.filter((header) => header !== 'CHGUNIQUEWRKID').join(','); +exports.workerHeadersWithTransferStaff = workerHeadersWithTransferStaff; exports.getWorkerColumnIndex = (columnName) => workerHeadersWithoutCHGUNIQUEWRKIDasArray.findIndex((header) => header === columnName); diff --git a/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js b/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js index ab5fe3eb92..db3a902305 100644 --- a/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js +++ b/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js @@ -1,13 +1,23 @@ -const { workerHeadersWithCHGUNIQUEWRKID, workerHeadersWithoutCHGUNIQUEWRKID } = require('../../data/workerHeaders'); +const { + workerHeadersWithCHGUNIQUEWRKID, + workerHeadersWithoutCHGUNIQUEWRKID, + workerHeadersWithTransferStaff, +} = require('../../data/workerHeaders'); const validateWorkerHeaders = (headers) => { const matchesWithChgUnique = headers.startsWith(workerHeadersWithCHGUNIQUEWRKID); const matchesWithoutChgUnique = headers.startsWith(workerHeadersWithoutCHGUNIQUEWRKID); + const matchesWithTransferStaff = headers.startsWith(workerHeadersWithTransferStaff); - if (!matchesWithChgUnique && !matchesWithoutChgUnique) { + if (!matchesWithChgUnique && !matchesWithoutChgUnique && !matchesWithTransferStaff) { return false; } + if (matchesWithTransferStaff) { + // temp shortcut + return true; + } + const additionalQualsHeaders = matchesWithChgUnique ? headers.slice(workerHeadersWithCHGUNIQUEWRKID.length) : headers.slice(workerHeadersWithoutCHGUNIQUEWRKID.length); diff --git a/backend/server/routes/establishments/bulkUpload/whichFile.js b/backend/server/routes/establishments/bulkUpload/whichFile.js index 2f27dd205b..5e29c11777 100644 --- a/backend/server/routes/establishments/bulkUpload/whichFile.js +++ b/backend/server/routes/establishments/bulkUpload/whichFile.js @@ -4,8 +4,13 @@ const isWorkerFile = (fileAsString) => { //TODO investiagte const contentRegex1 = /LOCALESTID,UNIQUEWORKERID,CHGUNIQUEWRKID,STATUS,DI/; const contentRegex2 = /LOCALESTID,UNIQUEWORKERID,STATUS,DISPLAYID,/; + const contentRegex3 = /LOCALESTID,UNIQUEWORKERID,TRANSFERSTAFFRECORD,STATUS,DISPLAYID,/; - return contentRegex1.test(fileAsString.substring(0, 50)) || contentRegex2.test(fileAsString.substring(0, 50)); + return ( + contentRegex1.test(fileAsString.substring(0, 50)) || + contentRegex2.test(fileAsString.substring(0, 50)) || + contentRegex3.test(fileAsString) + ); }; const isTrainingFile = (fileAsString) => { diff --git a/lambdas/bulkUpload/classes/workerCSVValidator.js b/lambdas/bulkUpload/classes/workerCSVValidator.js index 2ed2063714..814206bdd1 100644 --- a/lambdas/bulkUpload/classes/workerCSVValidator.js +++ b/lambdas/bulkUpload/classes/workerCSVValidator.js @@ -20,6 +20,7 @@ class WorkerCsvValidator { this._status = null; this._key = null; this._establishmentKey = null; + this._transferStaffRecord = null; this._NINumber = null; this._postCode = null; @@ -403,6 +404,10 @@ class WorkerCsvValidator { return this._changeUniqueWorkerId; } + get transferStaffRecord() { + return this._transferStaffRecord; + } + get contractType() { return this._contractType; } @@ -771,6 +776,28 @@ class WorkerCsvValidator { } } + _validateTransferStaffRecord() { + const newWorkplaceLocalId = this._currentLine.TRANSFERSTAFFRECORD; + + const workerExists = !!this._currentWorker; + if (newWorkplaceLocalId && !workerExists) { + this._validationErrors.push({ + name: this._currentLine.LOCALESTID, + worker: this._currentLine.UNIQUEWORKERID, + lineNumber: this._lineNumber, + errCode: WorkerCsvValidator.TRANSFERSTAFFRECORD_ERROR, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: `TRANSFERSTAFFRECORD is provided but cannot find the worker in the old workplace`, + source: myStatus, + column: 'TRANSFERSTAFFRECORD', + }); + return false; + } + + this._transferStaffRecord = newWorkplaceLocalId; + return true; + } + _validateDisplayId() { const myDisplayId = this._currentLine.DISPLAYID; const MAX_LENGTH = 50; // lowering to 50 because this is restricted in ASC WDS @@ -2861,6 +2888,7 @@ class WorkerCsvValidator { status = !this._validateLocalId() ? false : status; status = !this._validateUniqueWorkerId() ? false : status; status = !this._validateChangeUniqueWorkerId() ? false : status; + status = !this._validateTransferStaffRecord() ? false : status; status = !this._validateDisplayId() ? false : status; status = !this._validateStatus() ? false : status; status = !this._validateDaysSickChanged() ? false : status; @@ -2946,6 +2974,7 @@ class WorkerCsvValidator { status: this._status, uniqueWorkerId: this._uniqueWorkerId, changeUniqueWorker: this._changeUniqueWorkerId ? this._changeUniqueWorkerId : undefined, + transferStaffRecord: this._transferStaffRecord ? this._transferStaffRecord : undefined, displayId: this._displayId, niNumber: this._NINumber ? this._NINumber : undefined, postcode: this._postCode ? this._postCode : undefined, @@ -3021,6 +3050,7 @@ class WorkerCsvValidator { // the minimum to create a new worker localIdentifier: this._uniqueWorkerId, status: this._status, + transferStaffRecord: this._transferStaffRecord ? this._transferStaffRecord : undefined, nameOrId: this._displayId, contract: this._contractType, mainJob: { diff --git a/lambdas/bulkUpload/serverless.yml b/lambdas/bulkUpload/serverless.yml index 0228509e23..40991a2d75 100644 --- a/lambdas/bulkUpload/serverless.yml +++ b/lambdas/bulkUpload/serverless.yml @@ -22,7 +22,7 @@ frameworkVersion: '2' provider: name: aws - runtime: nodejs12.x + runtime: nodejs18.x lambdaHashingVersion: 20201221 stage: dev region: eu-west-2 From c5e47308aa2aef96e2096a3bd69591eb8f2a4914 Mon Sep 17 00:00:00 2001 From: Duncan Carter Date: Thu, 31 Oct 2024 15:52:30 +0000 Subject: [PATCH 02/33] Add update of establishment ID to worker with transferstaffrecord --- backend/server/models/classes/worker.js | 27 ++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/backend/server/models/classes/worker.js b/backend/server/models/classes/worker.js index 3bcf38379c..ffb06441de 100644 --- a/backend/server/models/classes/worker.js +++ b/backend/server/models/classes/worker.js @@ -11,6 +11,7 @@ const { v4: uuidv4 } = require('uuid'); uuidv4(); // database models +const { Op } = require('sequelize'); const models = require('../index'); const EntityValidator = require('./validations/entityValidator').EntityValidator; @@ -611,13 +612,6 @@ class Worker extends EntityValidator { return; } - if (bulkUploaded && this._status === 'UPDATE' && this.transferStaffRecord) { - console.log('calling save on a worker which have transferStaffRecord filled'); - console.log(this.transferStaffRecord, '<--- new workplace that this worker should move to'); - // either we add a UPDATE workplaceFk call to externalTransaction here, - // or we return early and handle the workplaceFk update elsewhere - } - if (!this.uid) { this._log(Worker.LOG_ERROR, 'Not able to save an unknown uid'); throw new WorkerExceptions.WorkerSaveException( @@ -766,6 +760,25 @@ class Worker extends EntityValidator { updatedBy: savedBy.toLowerCase(), }; + if (bulkUploaded && this._status === 'UPDATE' && this.transferStaffRecord) { + const establishmentFk = await models.establishment.findOne({ + attributes: ['id'], + where: { + LocalIdentifierValue: this.transferStaffRecord, + [Op.or]: [ + { + id: this.establishmentId, + }, + { + parentId: this.establishmentId, + }, + ], + }, + }); + + updateDocument.establishmentFk = establishmentFk.id; + } + if (this._changeLocalIdentifer) { // during bulk upload only, if the change local identifier value is set, then when saving this worker, update it's local identifier; updateDocument.LocalIdentifierValue = this._changeLocalIdentifer; From ef5d2d1cd553169f1a09a60e5d03215f96bcde4f Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Thu, 31 Oct 2024 17:12:47 +0000 Subject: [PATCH 03/33] validate transferStaffRecord and add newWorkplaceId field to worker in crossValidate --- .../models/BulkImport/csv/crossValidate.js | 40 ++++++++++++++----- backend/server/models/classes/worker.js | 32 ++++++--------- .../validate/validateBulkUploadFiles.js | 4 +- 3 files changed, 45 insertions(+), 31 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index d86d71b234..11c07e6897 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -9,21 +9,40 @@ const crossValidate = async (csvWorkerSchemaErrors, myEstablishments, JSONWorker const isCqcRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); - await crossValidateTransferStaffRecord(myEstablishments, JSONWorker); - _crossValidateMainJobRole(csvWorkerSchemaErrors, isCqcRegulated, JSONWorker); }; -const crossValidateTransferStaffRecord = async (myEstablishments, JSONWorker) => { - if (!JSONWorker.transferStaffRecord) { - return; +const crossValidateTransferStaffRecord = async (csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments) => { + const relatedEstablishmentIds = myEstablishments.map((establishment) => establishment.id); + + for (const establishment of Object.values(myAPIEstablishments)) { + if (!establishment._workerEntities) { + continue; + } + + for (const workerEntity of Object.values(establishment._workerEntities)) { + if (workerEntity.transferStaffRecord && workerEntity.status === 'UPDATE') { + const newWorkplaceLocalRef = workerEntity.transferStaffRecord; + const newWorkplaceId = await getNewWorkplaceId(newWorkplaceLocalRef, relatedEstablishmentIds); + // TODO: add error to csvWorkerSchemaErrors if newWorkplaceId is null; + workerEntity._newWorkplaceId = newWorkplaceId; + } + } + } +}; + +const getNewWorkplaceId = async (newWorkplaceLocalRef, relatedEstablishmentIds) => { + const newWorkplaceFound = await models.establishment.findOne({ + where: { + LocalIdentifierValue: newWorkplaceLocalRef, + id: relatedEstablishmentIds, + }, + }); + if (newWorkplaceFound) { + return newWorkplaceFound.id; } - console.log(JSONWorker.transferStaffRecord, '<--- new workerplace'); - console.log(JSONWorker.localId, '<--- previous workplace'); - console.log(JSONWorker.uniqueWorkerId, '<--- uniqueWorkerId'); - // we can do something here to validate that this worker exists in the old workplace, - // and that the uniqueworkerid isn't duplicated in new workplace + return null; }; const _crossValidateMainJobRole = (csvWorkerSchemaErrors, isCqcRegulated, JSONWorker) => { @@ -71,5 +90,6 @@ const workerNotChanged = (JSONWorker) => !['NEW', 'UPDATE'].includes(JSONWorker. module.exports = { crossValidate, _crossValidateMainJobRole, + crossValidateTransferStaffRecord, _isCQCRegulated, }; diff --git a/backend/server/models/classes/worker.js b/backend/server/models/classes/worker.js index ffb06441de..07f3484463 100644 --- a/backend/server/models/classes/worker.js +++ b/backend/server/models/classes/worker.js @@ -363,11 +363,14 @@ class Worker extends EntityValidator { } if (document.transferStaffRecord) { - console.log('============= detect transferStaffRecord ========== '); - console.log(document.transferStaffRecord, 'the new workplace this worker should move to'); this._transferStaffRecord = document.transferStaffRecord; } + if (document._newWorkplaceId) { + console.log('_newWorkplaceId received on worker.load()'); + this._newWorkplaceId = document._newWorkplaceId; + } + // Consequential updates when one value means another should be empty or null // If their job isn't a registered nurse, remove their specialism and category @@ -760,23 +763,8 @@ class Worker extends EntityValidator { updatedBy: savedBy.toLowerCase(), }; - if (bulkUploaded && this._status === 'UPDATE' && this.transferStaffRecord) { - const establishmentFk = await models.establishment.findOne({ - attributes: ['id'], - where: { - LocalIdentifierValue: this.transferStaffRecord, - [Op.or]: [ - { - id: this.establishmentId, - }, - { - parentId: this.establishmentId, - }, - ], - }, - }); - - updateDocument.establishmentFk = establishmentFk.id; + if (bulkUploaded && this._status === 'UPDATE' && this.transferStaffRecord && this._newWorkplaceId) { + updateDocument.establishmentFk = this._newWorkplaceId; } if (this._changeLocalIdentifer) { @@ -1384,10 +1372,14 @@ class Worker extends EntityValidator { myDefaultJSON.status = this._status; } - if (this._transferStaffRecord != null) { + if (this._transferStaffRecord !== null) { myDefaultJSON.transferStaffRecord = this._transferStaffRecord; } + if (this._newWorkplaceId !== null) { + myDefaultJSON._newWorkplaceId = this._newWorkplaceId; + } + // TODO: JSON schema validation if (showHistory && !showPropertyHistoryOnly) { return { diff --git a/backend/server/routes/establishments/bulkUpload/validate/validateBulkUploadFiles.js b/backend/server/routes/establishments/bulkUpload/validate/validateBulkUploadFiles.js index 520defe7aa..f2c7b25b13 100644 --- a/backend/server/routes/establishments/bulkUpload/validate/validateBulkUploadFiles.js +++ b/backend/server/routes/establishments/bulkUpload/validate/validateBulkUploadFiles.js @@ -15,7 +15,7 @@ const WorkplaceCsvValidator = require('../../../../models/BulkImport/csv/workpla const { validateWorkers } = require('./workers/validateWorkers'); const { validateWorkplace } = require('./workplace/validateWorkplace'); const { validateTraining } = require('./training/validateTraining'); -const { crossValidate } = require('../../../../models/BulkImport/csv/crossValidate'); +const { crossValidate, crossValidateTransferStaffRecord } = require('../../../../models/BulkImport/csv/crossValidate'); // if commit is false, then the results of validation are not uploaded to S3 const validateBulkUploadFiles = async (req, files) => { @@ -131,6 +131,8 @@ const validateBulkUploadFiles = async (req, files) => { }), ); + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments); + // Prepare validation results // prepare entities ready for upload/return From 323152b114d954b90eb96b2c643f4be0e4e6f12a Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Fri, 1 Nov 2024 14:47:20 +0000 Subject: [PATCH 04/33] add validation for transferStaffRecord --- .../models/BulkImport/csv/crossValidate.js | 112 ++++++++++++++++-- .../validate/validateBulkUploadFiles.js | 2 +- .../bulkUpload/classes/workerCSVValidator.js | 9 +- 3 files changed, 106 insertions(+), 17 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 11c07e6897..a8cd43bbc9 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -12,26 +12,59 @@ const crossValidate = async (csvWorkerSchemaErrors, myEstablishments, JSONWorker _crossValidateMainJobRole(csvWorkerSchemaErrors, isCqcRegulated, JSONWorker); }; -const crossValidateTransferStaffRecord = async (csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments) => { +const crossValidateTransferStaffRecord = async ( + csvWorkerSchemaErrors, + myAPIEstablishments, + myEstablishments, + myJSONWorkers, +) => { const relatedEstablishmentIds = myEstablishments.map((establishment) => establishment.id); - for (const establishment of Object.values(myAPIEstablishments)) { - if (!establishment._workerEntities) { + for (const JSONworker of myJSONWorkers) { + if (JSONworker.status !== 'UPDATE' || !JSONworker.transferStaffRecord) { continue; } - for (const workerEntity of Object.values(establishment._workerEntities)) { - if (workerEntity.transferStaffRecord && workerEntity.status === 'UPDATE') { - const newWorkplaceLocalRef = workerEntity.transferStaffRecord; - const newWorkplaceId = await getNewWorkplaceId(newWorkplaceLocalRef, relatedEstablishmentIds); - // TODO: add error to csvWorkerSchemaErrors if newWorkplaceId is null; - workerEntity._newWorkplaceId = newWorkplaceId; - } - } + await _crossValidateTransferStaffRecordForWorker( + csvWorkerSchemaErrors, + relatedEstablishmentIds, + myAPIEstablishments, + JSONworker, + ); } }; -const getNewWorkplaceId = async (newWorkplaceLocalRef, relatedEstablishmentIds) => { +const _crossValidateTransferStaffRecordForWorker = async ( + csvWorkerSchemaErrors, + relatedEstablishmentIds, + myAPIEstablishments, + JSONworker, +) => { + const oldWorkplaceLocalRef = JSONworker.localId; + const newWorkplaceLocalRef = JSONworker.transferStaffRecord; + const newWorkplaceId = await _getNewWorkplaceId(newWorkplaceLocalRef, relatedEstablishmentIds); + + if (newWorkplaceId === null) { + _addErrorForNewWorkplaceNotFound(csvWorkerSchemaErrors, JSONworker); + return; + } + + const sameLocalIdExistInNewWorkplace = await _checkDuplicateLocalIdInNewWorkplace( + newWorkplaceId, + JSONworker.uniqueWorkerId, + ); + + if (sameLocalIdExistInNewWorkplace) { + _addErrorForSameLocalIdExistInNewWorkplace(csvWorkerSchemaErrors, JSONworker); + return; + } + + const keyInApiEstablishments = JSONworker.uniqueWorkerId.replace(/\s/g, ''); + const workerEntity = myAPIEstablishments[oldWorkplaceLocalRef]._workerEntities[keyInApiEstablishments]; + workerEntity._newWorkplaceId = newWorkplaceId; +}; + +const _getNewWorkplaceId = async (newWorkplaceLocalRef, relatedEstablishmentIds) => { const newWorkplaceFound = await models.establishment.findOne({ where: { LocalIdentifierValue: newWorkplaceLocalRef, @@ -45,6 +78,61 @@ const getNewWorkplaceId = async (newWorkplaceLocalRef, relatedEstablishmentIds) return null; }; +const _checkDuplicateLocalIdInNewWorkplace = async (newWorkplaceId, uniqueWorkerId) => { + const uniqueWorkerIdHasWhitespace = /\s/g.test(uniqueWorkerId); + + if (uniqueWorkerIdHasWhitespace) { + // Handle special cases when uniqueWorkerId includes whitespace. + // As the legacy code does a /\s/g replacement in same places, we need to consider the case of local id collision with whitespaces stripped. + const allWorkersInNewWorkplace = await models.worker.findAll({ + attributes: ['LocalIdentifierValue'], + where: { + establishmentFk: newWorkplaceId, + }, + raw: true, + }); + const allRefsWithoutWhitespaces = allWorkersInNewWorkplace + .filter((worker) => worker?.LocalIdentifierValue) + .map((worker) => worker.LocalIdentifierValue.replace(/\s/g, '')); + + return allRefsWithoutWhitespaces.includes(uniqueWorkerId.replace(/\s/g, '')); + } + + const sameLocalIdFound = await models.worker.findOne({ + where: { + LocalIdentifierValue: uniqueWorkerId, + establishmentFk: newWorkplaceId, + }, + }); + return !!sameLocalIdFound; +}; + +const _addErrorForNewWorkplaceNotFound = (csvWorkerSchemaErrors, JSONWorker) => { + csvWorkerSchemaErrors.unshift({ + worker: JSONWorker.uniqueWorkerId, + name: JSONWorker.localId, + lineNumber: JSONWorker.lineNumber, + errCode: 99998, + errType: 'TRANSFERSTAFFRECORD_ERROR', + source: JSONWorker.transferStaffRecord, + column: 'TRANSFERSTAFFRECORD', + error: 'Cannot find an existing workplace with the localId provided in TRANSFERSTAFFRECORD', + }); +}; + +const _addErrorForSameLocalIdExistInNewWorkplace = (csvWorkerSchemaErrors, JSONWorker) => { + csvWorkerSchemaErrors.unshift({ + worker: JSONWorker.uniqueWorkerId, + name: JSONWorker.localId, + lineNumber: JSONWorker.lineNumber, + errCode: 99999, + errType: 'TRANSFERSTAFFRECORD_ERROR', + source: JSONWorker.uniqueWorkerId, + column: 'TRANSFERSTAFFRECORD', + error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD.', + }); +}; + const _crossValidateMainJobRole = (csvWorkerSchemaErrors, isCqcRegulated, JSONWorker) => { if (!isCqcRegulated && JSONWorker.mainJobRoleId === 4) { csvWorkerSchemaErrors.unshift({ diff --git a/backend/server/routes/establishments/bulkUpload/validate/validateBulkUploadFiles.js b/backend/server/routes/establishments/bulkUpload/validate/validateBulkUploadFiles.js index f2c7b25b13..ed3468096b 100644 --- a/backend/server/routes/establishments/bulkUpload/validate/validateBulkUploadFiles.js +++ b/backend/server/routes/establishments/bulkUpload/validate/validateBulkUploadFiles.js @@ -131,7 +131,7 @@ const validateBulkUploadFiles = async (req, files) => { }), ); - await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments); + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, myJSONWorkers); // Prepare validation results diff --git a/lambdas/bulkUpload/classes/workerCSVValidator.js b/lambdas/bulkUpload/classes/workerCSVValidator.js index 814206bdd1..36e6cb4e48 100644 --- a/lambdas/bulkUpload/classes/workerCSVValidator.js +++ b/lambdas/bulkUpload/classes/workerCSVValidator.js @@ -785,11 +785,12 @@ class WorkerCsvValidator { name: this._currentLine.LOCALESTID, worker: this._currentLine.UNIQUEWORKERID, lineNumber: this._lineNumber, - errCode: WorkerCsvValidator.TRANSFERSTAFFRECORD_ERROR, - errType: 'TRANSFERSTAFFRECORD_ERROR', + // for spike purpose, just use an existing error type + errCode: WorkerCsvValidator.DISPLAY_ID_ERROR, + errType: 'DISPLAY_ID_ERROR', error: `TRANSFERSTAFFRECORD is provided but cannot find the worker in the old workplace`, - source: myStatus, - column: 'TRANSFERSTAFFRECORD', + source: this._currentLine.LOCALESTID, + column: 'LOCALESTID', }); return false; } From 1c93deec813b57bc1a772ad808ee3d8c2330da37 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Tue, 5 Nov 2024 14:45:09 +0000 Subject: [PATCH 05/33] amend column headers validator --- .../bulkUpload/data/workerHeaders.js | 65 ++++--------------- .../establishments/bulkUpload/whichFile.js | 16 ++--- .../bulkUpload/whichFile.spec.js | 9 ++- 3 files changed, 27 insertions(+), 63 deletions(-) diff --git a/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js b/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js index f2741a6845..d4a3c49739 100644 --- a/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js +++ b/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js @@ -1,7 +1,6 @@ const workerHeaders = [ 'LOCALESTID', 'UNIQUEWORKERID', - 'CHGUNIQUEWRKID', 'STATUS', 'DISPLAYID', 'NINUMBER', @@ -45,57 +44,19 @@ const workerHeaders = [ 'QUALACH03NOTES', ]; -const workerHeadersWithTransferStaff = [ - 'LOCALESTID', - 'UNIQUEWORKERID', - 'TRANSFERSTAFFRECORD', - 'STATUS', - 'DISPLAYID', - 'NINUMBER', - 'POSTCODE', - 'DOB', - 'GENDER', - 'ETHNICITY', - 'NATIONALITY', - 'BRITISHCITIZENSHIP', - 'COUNTRYOFBIRTH', - 'YEAROFENTRY', - 'DISABLED', - 'CARECERT', - 'L2CARECERT', - 'RECSOURCE', - 'HANDCVISA', - 'INOUTUK', - 'STARTDATE', - 'STARTINSECT', - 'APPRENTICE', - 'EMPLSTATUS', - 'ZEROHRCONT', - 'DAYSSICK', - 'SALARYINT', - 'SALARY', - 'HOURLYRATE', - 'MAINJOBROLE', - 'MAINJRDESC', - 'CONTHOURS', - 'AVGHOURS', - 'NMCREG', - 'NURSESPEC', - 'AMHP', - 'SCQUAL', - 'NONSCQUAL', - 'QUALACH01', - 'QUALACH01NOTES', - 'QUALACH02', - 'QUALACH02NOTES', - 'QUALACH03', - 'QUALACH03NOTES', +const workerHeadersWithChangeUniqueWorkerIdAsArray = [ + ...workerHeaders.slice(0, 2), + 'CHGUNIQUEWRKID', + workerHeaders.slice(2), ]; -const workerHeadersWithoutCHGUNIQUEWRKIDasArray = workerHeaders.filter((header) => header !== 'CHGUNIQUEWRKID'); +const workerHeadersWithTransferStaffRecordAsArray = [ + ...workerHeaders.slice(0, 2), + 'TRANSFERSTAFFRECORD', + workerHeaders.slice(2), +]; -exports.workerHeadersWithCHGUNIQUEWRKID = workerHeaders.join(','); -exports.workerHeadersWithoutCHGUNIQUEWRKID = workerHeaders.filter((header) => header !== 'CHGUNIQUEWRKID').join(','); -exports.workerHeadersWithTransferStaff = workerHeadersWithTransferStaff; -exports.getWorkerColumnIndex = (columnName) => - workerHeadersWithoutCHGUNIQUEWRKIDasArray.findIndex((header) => header === columnName); +exports.workerHeadersWithCHGUNIQUEWRKID = workerHeadersWithChangeUniqueWorkerIdAsArray.join(','); +exports.workerHeadersWithTransferStaff = workerHeadersWithTransferStaffRecordAsArray.join(''); +exports.workerHeadersWithoutCHGUNIQUEWRKID = workerHeaders.join(','); +exports.getWorkerColumnIndex = (columnName) => workerHeaders.findIndex((header) => header === columnName); diff --git a/backend/server/routes/establishments/bulkUpload/whichFile.js b/backend/server/routes/establishments/bulkUpload/whichFile.js index 5e29c11777..aac8f697a2 100644 --- a/backend/server/routes/establishments/bulkUpload/whichFile.js +++ b/backend/server/routes/establishments/bulkUpload/whichFile.js @@ -1,16 +1,14 @@ const WorkplaceCSVValidator = require('../../../models/BulkImport/csv/workplaceCSVValidator').WorkplaceCSVValidator; const isWorkerFile = (fileAsString) => { - //TODO investiagte - const contentRegex1 = /LOCALESTID,UNIQUEWORKERID,CHGUNIQUEWRKID,STATUS,DI/; - const contentRegex2 = /LOCALESTID,UNIQUEWORKERID,STATUS,DISPLAYID,/; - const contentRegex3 = /LOCALESTID,UNIQUEWORKERID,TRANSFERSTAFFRECORD,STATUS,DISPLAYID,/; + const headersRegexBaseCase = /LOCALESTID,UNIQUEWORKERID,STATUS,DISPLAYID,/; + const headersRegexChangeUniqueWorkerId = /LOCALESTID,UNIQUEWORKERID,CHGUNIQUEWRKID,STATUS,DISPLAYID,/; + const headersRegexTransferStaffRecord = /LOCALESTID,UNIQUEWORKERID,TRANSFERSTAFFRECORD,STATUS,DISPLAYID,/; + const regexToCheckHeaders = [headersRegexBaseCase, headersRegexChangeUniqueWorkerId, headersRegexTransferStaffRecord]; - return ( - contentRegex1.test(fileAsString.substring(0, 50)) || - contentRegex2.test(fileAsString.substring(0, 50)) || - contentRegex3.test(fileAsString) - ); + const headerRow = fileAsString.split('\n')[0]; + + return regexToCheckHeaders.some((regex) => regex.test(headerRow)); }; const isTrainingFile = (fileAsString) => { diff --git a/backend/server/test/unit/routes/establishments/bulkUpload/whichFile.spec.js b/backend/server/test/unit/routes/establishments/bulkUpload/whichFile.spec.js index 15ae872563..8e7d3e13d1 100644 --- a/backend/server/test/unit/routes/establishments/bulkUpload/whichFile.spec.js +++ b/backend/server/test/unit/routes/establishments/bulkUpload/whichFile.spec.js @@ -8,7 +8,7 @@ const expect = require('chai').expect; describe('whichFile', () => { describe('isWorkerFile()', () => { it('return true when headings match with CHGUNIQUEWRKID', async () => { - const header = 'LOCALESTID,UNIQUEWORKERID,CHGUNIQUEWRKID,STATUS,DI'; + const header = 'LOCALESTID,UNIQUEWORKERID,CHGUNIQUEWRKID,STATUS,DISPLAYID,NINUMBER'; expect(isWorkerFile(header)).to.deep.equal(true); }); @@ -17,6 +17,11 @@ describe('whichFile', () => { expect(isWorkerFile(header)).to.deep.equal(true); }); + it('return true when headings match with TRANSFERSTAFFRECORD', async () => { + const header = 'LOCALESTID,UNIQUEWORKERID,TRANSFERSTAFFRECORD,STATUS,DISPLAYID,NINUMBER'; + expect(isWorkerFile(header)).to.deep.equal(true); + }); + it("return false when headings don't match", async () => { const header = 'NOTATALLWHATWEEXPECT,HOWCOULDYOUUPLOADTHISFILE,'; expect(isWorkerFile(header)).to.deep.equal(false); @@ -49,7 +54,7 @@ describe('whichFile', () => { }); it('should return the correct file type for workers if CHGUNIQUEWRKID column present', () => { - const fileType = getFileType('LOCALESTID,UNIQUEWORKERID,CHGUNIQUEWRKID,STATUS,DI'); + const fileType = getFileType('LOCALESTID,UNIQUEWORKERID,CHGUNIQUEWRKID,STATUS,DISPLAYID,NINUMBER'); expect(fileType).to.deep.equal('Worker'); }); From a14c8da270437b2b26ea060a6b56dc62aae8ab99 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Tue, 5 Nov 2024 15:00:15 +0000 Subject: [PATCH 06/33] --amend --- .../models/BulkImport/csv/crossValidate.js | 6 +++-- .../bulkUpload/data/workerHeaders.js | 6 ++--- .../validate/headers/worker.spec.js | 25 +++++++++++++------ 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index a8cd43bbc9..792ab004ce 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -2,6 +2,8 @@ const models = require('../../../models'); const MAIN_JOB_ROLE_ERROR = () => 1280; +const TRANSFERSTAFFRECORD_ERROR = () => 1400; + const crossValidate = async (csvWorkerSchemaErrors, myEstablishments, JSONWorker) => { if (workerNotChanged(JSONWorker)) { return false; @@ -112,7 +114,7 @@ const _addErrorForNewWorkplaceNotFound = (csvWorkerSchemaErrors, JSONWorker) => worker: JSONWorker.uniqueWorkerId, name: JSONWorker.localId, lineNumber: JSONWorker.lineNumber, - errCode: 99998, + errCode: TRANSFERSTAFFRECORD_ERROR(), errType: 'TRANSFERSTAFFRECORD_ERROR', source: JSONWorker.transferStaffRecord, column: 'TRANSFERSTAFFRECORD', @@ -125,7 +127,7 @@ const _addErrorForSameLocalIdExistInNewWorkplace = (csvWorkerSchemaErrors, JSONW worker: JSONWorker.uniqueWorkerId, name: JSONWorker.localId, lineNumber: JSONWorker.lineNumber, - errCode: 99999, + errCode: TRANSFERSTAFFRECORD_ERROR(), errType: 'TRANSFERSTAFFRECORD_ERROR', source: JSONWorker.uniqueWorkerId, column: 'TRANSFERSTAFFRECORD', diff --git a/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js b/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js index d4a3c49739..6b630b1843 100644 --- a/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js +++ b/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js @@ -47,16 +47,16 @@ const workerHeaders = [ const workerHeadersWithChangeUniqueWorkerIdAsArray = [ ...workerHeaders.slice(0, 2), 'CHGUNIQUEWRKID', - workerHeaders.slice(2), + ...workerHeaders.slice(2), ]; const workerHeadersWithTransferStaffRecordAsArray = [ ...workerHeaders.slice(0, 2), 'TRANSFERSTAFFRECORD', - workerHeaders.slice(2), + ...workerHeaders.slice(2), ]; exports.workerHeadersWithCHGUNIQUEWRKID = workerHeadersWithChangeUniqueWorkerIdAsArray.join(','); -exports.workerHeadersWithTransferStaff = workerHeadersWithTransferStaffRecordAsArray.join(''); +exports.workerHeadersWithTransferStaff = workerHeadersWithTransferStaffRecordAsArray.join(','); exports.workerHeadersWithoutCHGUNIQUEWRKID = workerHeaders.join(','); exports.getWorkerColumnIndex = (columnName) => workerHeaders.findIndex((header) => header === columnName); diff --git a/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js b/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js index 9e52900a63..70dc0bcc91 100644 --- a/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js +++ b/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js @@ -12,6 +12,11 @@ const workerHeadersWithCHGUNIQUEWRKID = 'NMCREG,NURSESPEC,AMHP,SCQUAL,NONSCQUAL,QUALACH01,QUALACH01NOTES,' + 'QUALACH02,QUALACH02NOTES,QUALACH03,QUALACH03NOTES'; +const workerHeadersWithTRANSFERSTAFFRECORD = workerHeadersWithCHGUNIQUEWRKID.replace( + 'CHGUNIQUEWRKID', + 'TRANSFERSTAFFRECORD', +); + const workerHeadersWithoutCHGUNIQUEWRKID = 'LOCALESTID,UNIQUEWORKERID,STATUS,DISPLAYID,NINUMBER,' + 'POSTCODE,DOB,GENDER,ETHNICITY,NATIONALITY,BRITISHCITIZENSHIP,COUNTRYOFBIRTH,YEAROFENTRY,' + @@ -20,27 +25,31 @@ const workerHeadersWithoutCHGUNIQUEWRKID = 'NMCREG,NURSESPEC,AMHP,SCQUAL,NONSCQUAL,QUALACH01,QUALACH01NOTES,' + 'QUALACH02,QUALACH02NOTES,QUALACH03,QUALACH03NOTES'; -describe('server/routes/establishments/bulkUpload/validate/headers/worker', () => { +describe.only('server/routes/establishments/bulkUpload/validate/headers/worker', () => { describe('validateWorkerHeaders()', () => { it('should return true when headings match with CHGUNIQUEWRKID', async () => { - expect(await validateWorkerHeaders(workerHeadersWithCHGUNIQUEWRKID)).to.deep.equal(true); + expect(validateWorkerHeaders(workerHeadersWithCHGUNIQUEWRKID)).to.deep.equal(true); + }); + + it('should return true when headings match with TRANSFERSTAFFRECORD', async () => { + expect(validateWorkerHeaders(workerHeadersWithTRANSFERSTAFFRECORD)).to.deep.equal(true); }); it('should return true when headings match without CHGUNIQUEWRKID', async () => { - expect(await validateWorkerHeaders(workerHeadersWithoutCHGUNIQUEWRKID)).to.deep.equal(true); + expect(validateWorkerHeaders(workerHeadersWithoutCHGUNIQUEWRKID)).to.deep.equal(true); }); it('should return false when header (NATIONALITY) missing', async () => { const invalidHeaders = workerHeadersWithoutCHGUNIQUEWRKID.replace('NATIONALITY,', ''); - expect(await validateWorkerHeaders(invalidHeaders)).to.deep.equal(false); + expect(validateWorkerHeaders(invalidHeaders)).to.deep.equal(false); }); describe('Extra qualifications', () => { it('should return true when headings match with headers for one extra QUAL', async () => { const headersWithExtraQuals = workerHeadersWithoutCHGUNIQUEWRKID.concat(',QUALACH04,QUALACH04NOTES'); - expect(await validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(true); + expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(true); }); it('should return true when headings match with headers for two extra QUALs', async () => { @@ -48,7 +57,7 @@ describe('server/routes/establishments/bulkUpload/validate/headers/worker', () = ',QUALACH04,QUALACH04NOTES,QUALACH05,QUALACH05NOTES', ); - expect(await validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(true); + expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(true); }); it('should return false when invalid extra QUALs headers (wrong qual number)', async () => { @@ -56,7 +65,7 @@ describe('server/routes/establishments/bulkUpload/validate/headers/worker', () = ',QUALACH04,QUALACH04NOTES,QUALACH04,QUALACH05NOTES', ); - expect(await validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(false); + expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(false); }); it('should return false when invalid extra QUALs headers (wrong qualNotes number)', async () => { @@ -64,7 +73,7 @@ describe('server/routes/establishments/bulkUpload/validate/headers/worker', () = ',QUALACH04,QUALACH04NOTES,QUALACH05,QUALACH03NOTES', ); - expect(await validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(false); + expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(false); }); }); }); From 98e906d8bf36a59bb6f66739e8ff2267e5f1038f Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Tue, 5 Nov 2024 16:37:38 +0000 Subject: [PATCH 07/33] add unit test for validateTransferStaffRecord at workerCSVValidator --- .../bulkUpload/classes/workerCSVValidator.js | 9 ++-- .../unit/classes/workerCSVValidator.spec.js | 41 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/lambdas/bulkUpload/classes/workerCSVValidator.js b/lambdas/bulkUpload/classes/workerCSVValidator.js index 36e6cb4e48..4730bc623d 100644 --- a/lambdas/bulkUpload/classes/workerCSVValidator.js +++ b/lambdas/bulkUpload/classes/workerCSVValidator.js @@ -197,6 +197,10 @@ class WorkerCsvValidator { return 1380; } + static get TRANSFERSTAFFRECORD_ERROR() { + return 1400; + } + static get UNIQUE_WORKER_ID_WARNING() { return 3020; } @@ -785,9 +789,8 @@ class WorkerCsvValidator { name: this._currentLine.LOCALESTID, worker: this._currentLine.UNIQUEWORKERID, lineNumber: this._lineNumber, - // for spike purpose, just use an existing error type - errCode: WorkerCsvValidator.DISPLAY_ID_ERROR, - errType: 'DISPLAY_ID_ERROR', + errCode: WorkerCsvValidator.TRANSFERSTAFFRECORD_ERROR, + errType: 'TRANSFERSTAFFRECORD_ERROR', error: `TRANSFERSTAFFRECORD is provided but cannot find the worker in the old workplace`, source: this._currentLine.LOCALESTID, column: 'LOCALESTID', diff --git a/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js b/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js index 5909071af8..f7836904e5 100644 --- a/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js +++ b/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js @@ -1429,5 +1429,46 @@ describe('/lambdas/bulkUpload/classes/workerCSVValidator', async () => { }); }); }); + + describe.only('_validateTransferStaffRecord', () => { + const worker = buildWorkerCsv({ + overrides: { + STATUS: 'UPDATE', + }, + }); + worker.TRANSFERSTAFFRECORD = 'workplace B'; + + it('should emit an error when TRANSFERSTAFFRECORD is provided but the worker does not exist', async () => { + const validator = new WorkerCsvValidator(worker, 2, null, mappings); + + validator.validate(); + validator.transform(); + + const expectedError = { + lineNumber: 2, + errCode: WorkerCsvValidator.TRANSFERSTAFFRECORD_ERROR, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: 'TRANSFERSTAFFRECORD is provided but cannot find the worker in the old workplace', + source: worker.LOCALESTID, + column: 'LOCALESTID', + name: 'MARMA', + worker: '3', + }; + + expect(validator._validationErrors).to.deep.include(expectedError); + expect(validator._transferStaffRecord).to.equal(null); + }); + + it('should set the _transferStaffRecord field if validation passed', async () => { + const mockExistingWorker = { nameOrId: 'mock worker name', id: 100, uid: 'mock-uid' }; + const validator = new WorkerCsvValidator(worker, 2, mockExistingWorker, mappings); + + validator.validate(); + validator.transform(); + + expect(validator._validationErrors).to.deep.equal([]); + expect(validator._transferStaffRecord).to.equal('workplace B'); + }); + }); }); }); From e6746d7ab7ba7799524c632f720eeaac30df3d34 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Wed, 6 Nov 2024 12:48:32 +0000 Subject: [PATCH 08/33] add more tests for cross validation --- .../models/BulkImport/csv/crossValidate.js | 170 ++++++++++++------ backend/server/models/classes/worker.js | 11 +- .../Bulkimport/csv/crossValidate.spec.js | 145 ++++++++++++++- .../validate/headers/worker.spec.js | 2 +- .../unit/classes/workerCSVValidator.spec.js | 2 +- 5 files changed, 264 insertions(+), 66 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 792ab004ce..b77fe6a754 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -14,6 +14,48 @@ const crossValidate = async (csvWorkerSchemaErrors, myEstablishments, JSONWorker _crossValidateMainJobRole(csvWorkerSchemaErrors, isCqcRegulated, JSONWorker); }; +const _crossValidateMainJobRole = (csvWorkerSchemaErrors, isCqcRegulated, JSONWorker) => { + if (!isCqcRegulated && JSONWorker.mainJobRoleId === 4) { + csvWorkerSchemaErrors.unshift({ + worker: JSONWorker.uniqueWorkerId, + name: JSONWorker.localId, + lineNumber: JSONWorker.lineNumber, + errCode: MAIN_JOB_ROLE_ERROR(), + errType: 'MAIN_JOB_ROLE_ERROR', + source: JSONWorker.mainJobRoleId, + column: 'MAINJOBROLE', + error: + 'Workers MAINJOBROLE is Registered Manager but you are not providing a CQC regulated service. Please change to another Job Role', + }); + } +}; + +const _isCQCRegulated = async (myEstablishments, JSONWorker) => { + const workerEstablishment = myEstablishments.find( + (establishment) => JSONWorker.establishmentKey === establishment.key, + ); + + if (workerEstablishment) { + switch (workerEstablishment.status) { + case 'NEW': + case 'UPDATE': + return workerEstablishment.regType === 2; + case 'UNCHECKED': + case 'NOCHANGE': + return await _checkEstablishmentRegulatedInDatabase(workerEstablishment.id); + case 'DELETE': + break; + } + } +}; + +const _checkEstablishmentRegulatedInDatabase = async (establishmentId) => { + const establishment = await models.establishment.findbyId(establishmentId); + return establishment.isRegulated; +}; + +const workerNotChanged = (JSONWorker) => !['NEW', 'UPDATE'].includes(JSONWorker.status); + const crossValidateTransferStaffRecord = async ( csvWorkerSchemaErrors, myAPIEstablishments, @@ -22,48 +64,49 @@ const crossValidateTransferStaffRecord = async ( ) => { const relatedEstablishmentIds = myEstablishments.map((establishment) => establishment.id); - for (const JSONworker of myJSONWorkers) { - if (JSONworker.status !== 'UPDATE' || !JSONworker.transferStaffRecord) { - continue; - } + const allMovingWorkers = myJSONWorkers.filter(isMovingToNewWorkplace); - await _crossValidateTransferStaffRecordForWorker( + _crossValidateWorkersWithSameRefMovingToSameWorkplace(csvWorkerSchemaErrors, allMovingWorkers); + + for (const JSONWorker of allMovingWorkers) { + const newWorkplaceId = await _validateTransferIsPossible( csvWorkerSchemaErrors, relatedEstablishmentIds, - myAPIEstablishments, - JSONworker, + JSONWorker, ); + + if (newWorkplaceId) { + _addNewWorkplaceIdToWorkerEntity(myAPIEstablishments, JSONWorker, newWorkplaceId); + } } }; -const _crossValidateTransferStaffRecordForWorker = async ( - csvWorkerSchemaErrors, - relatedEstablishmentIds, - myAPIEstablishments, - JSONworker, -) => { - const oldWorkplaceLocalRef = JSONworker.localId; - const newWorkplaceLocalRef = JSONworker.transferStaffRecord; +const isMovingToNewWorkplace = (JSONWorker) => { + return JSONWorker.status === 'UPDATE' && JSONWorker.transferStaffRecord; +}; + +const _validateTransferIsPossible = async (csvWorkerSchemaErrors, relatedEstablishmentIds, JSONWorker) => { + const newWorkplaceLocalRef = JSONWorker.transferStaffRecord; const newWorkplaceId = await _getNewWorkplaceId(newWorkplaceLocalRef, relatedEstablishmentIds); if (newWorkplaceId === null) { - _addErrorForNewWorkplaceNotFound(csvWorkerSchemaErrors, JSONworker); + _addErrorForNewWorkplaceNotFound(csvWorkerSchemaErrors, JSONWorker); return; } const sameLocalIdExistInNewWorkplace = await _checkDuplicateLocalIdInNewWorkplace( newWorkplaceId, - JSONworker.uniqueWorkerId, + JSONWorker.uniqueWorkerId, ); if (sameLocalIdExistInNewWorkplace) { - _addErrorForSameLocalIdExistInNewWorkplace(csvWorkerSchemaErrors, JSONworker); + _addErrorForSameLocalIdExistInNewWorkplace(csvWorkerSchemaErrors, JSONWorker); return; } - const keyInApiEstablishments = JSONworker.uniqueWorkerId.replace(/\s/g, ''); - const workerEntity = myAPIEstablishments[oldWorkplaceLocalRef]._workerEntities[keyInApiEstablishments]; - workerEntity._newWorkplaceId = newWorkplaceId; + if (_workerPassedAllValidations(csvWorkerSchemaErrors, JSONWorker)) { + return newWorkplaceId; + } }; const _getNewWorkplaceId = async (newWorkplaceLocalRef, relatedEstablishmentIds) => { @@ -109,12 +152,29 @@ const _checkDuplicateLocalIdInNewWorkplace = async (newWorkplaceId, uniqueWorker return !!sameLocalIdFound; }; +const _workerPassedAllValidations = (csvWorkerSchemaErrors, JSONWorker) => { + return ( + csvWorkerSchemaErrors.find( + (error) => error?.lineNumber === JSONWorker.lineNumber && error?.errType === 'TRANSFERSTAFFRECORD_ERROR', + ) === undefined + ); +}; + +const _addNewWorkplaceIdToWorkerEntity = (myAPIEstablishments, JSONWorker, newWorkplaceId) => { + const oldWorkplaceKey = JSONWorker.localId.replace(/\s/g, ''); + const workerEntityKey = JSONWorker.uniqueWorkerId.replace(/\s/g, ''); + + const workerEntity = myAPIEstablishments[oldWorkplaceKey].theWorker(workerEntityKey); + + workerEntity._newWorkplaceId = newWorkplaceId; +}; + const _addErrorForNewWorkplaceNotFound = (csvWorkerSchemaErrors, JSONWorker) => { csvWorkerSchemaErrors.unshift({ worker: JSONWorker.uniqueWorkerId, name: JSONWorker.localId, lineNumber: JSONWorker.lineNumber, - errCode: TRANSFERSTAFFRECORD_ERROR(), + errCode: TRANSFERSTAFFRECORD_ERROR() + 1, errType: 'TRANSFERSTAFFRECORD_ERROR', source: JSONWorker.transferStaffRecord, column: 'TRANSFERSTAFFRECORD', @@ -127,56 +187,50 @@ const _addErrorForSameLocalIdExistInNewWorkplace = (csvWorkerSchemaErrors, JSONW worker: JSONWorker.uniqueWorkerId, name: JSONWorker.localId, lineNumber: JSONWorker.lineNumber, - errCode: TRANSFERSTAFFRECORD_ERROR(), + errCode: TRANSFERSTAFFRECORD_ERROR() + 2, errType: 'TRANSFERSTAFFRECORD_ERROR', source: JSONWorker.uniqueWorkerId, - column: 'TRANSFERSTAFFRECORD', + column: 'UNIQUEWORKERID', error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD.', }); }; -const _crossValidateMainJobRole = (csvWorkerSchemaErrors, isCqcRegulated, JSONWorker) => { - if (!isCqcRegulated && JSONWorker.mainJobRoleId === 4) { - csvWorkerSchemaErrors.unshift({ - worker: JSONWorker.uniqueWorkerId, - name: JSONWorker.localId, - lineNumber: JSONWorker.lineNumber, - errCode: MAIN_JOB_ROLE_ERROR(), - errType: 'MAIN_JOB_ROLE_ERROR', - source: JSONWorker.mainJobRoleId, - column: 'MAINJOBROLE', - error: - 'Workers MAINJOBROLE is Registered Manager but you are not providing a CQC regulated service. Please change to another Job Role', - }); - } -}; +const _crossValidateWorkersWithSameRefMovingToSameWorkplace = (csvWorkerSchemaErrors, allMovingWorkers) => { + const destinations = {}; -const _isCQCRegulated = async (myEstablishments, JSONWorker) => { - const workerEstablishment = myEstablishments.find( - (establishment) => JSONWorker.establishmentKey === establishment.key, - ); + for (const JSONWorker of allMovingWorkers) { + const newWorkplace = JSONWorker.transferStaffRecord; + const workerRef = JSONWorker.uniqueWorkerId.replace(/\s/g, ''); - if (workerEstablishment) { - switch (workerEstablishment.status) { - case 'NEW': - case 'UPDATE': - return workerEstablishment.regType === 2; - case 'UNCHECKED': - case 'NOCHANGE': - return await _checkEstablishmentRegulatedInDatabase(workerEstablishment.id); - case 'DELETE': - break; + if (!destinations[newWorkplace]) { + destinations[newWorkplace] = new Set([workerRef]); + continue; + } + + if (!destinations[newWorkplace].has(workerRef)) { + destinations[newWorkplace].add(workerRef); + continue; } + + // if arrive at here, there is already another worker with that workerRef moving to the same new workplace + _addErrorForWorkersWithSameRefsMovingToSameWorkplace(csvWorkerSchemaErrors, JSONWorker); } }; -const _checkEstablishmentRegulatedInDatabase = async (establishmentId) => { - const establishment = await models.establishment.findbyId(establishmentId); - return establishment.isRegulated; +const _addErrorForWorkersWithSameRefsMovingToSameWorkplace = (csvWorkerSchemaErrors, JSONWorker) => { + csvWorkerSchemaErrors.unshift({ + worker: JSONWorker.uniqueWorkerId, + name: JSONWorker.localId, + lineNumber: JSONWorker.lineNumber, + errCode: TRANSFERSTAFFRECORD_ERROR() + 3, + errType: 'TRANSFERSTAFFRECORD_ERROR', + source: JSONWorker.uniqueWorkerId, + column: 'UNIQUEWORKERID', + error: + 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', + }); }; -const workerNotChanged = (JSONWorker) => !['NEW', 'UPDATE'].includes(JSONWorker.status); - module.exports = { crossValidate, _crossValidateMainJobRole, diff --git a/backend/server/models/classes/worker.js b/backend/server/models/classes/worker.js index 07f3484463..d9f0dc7f37 100644 --- a/backend/server/models/classes/worker.js +++ b/backend/server/models/classes/worker.js @@ -200,6 +200,10 @@ class Worker extends EntityValidator { return this._transferStaffRecord; } + get newWorkplaceId() { + return this._newWorkplaceId; + } + get contract() { return this._properties.get('Contract') ? this._properties.get('Contract').property : null; } @@ -366,9 +370,8 @@ class Worker extends EntityValidator { this._transferStaffRecord = document.transferStaffRecord; } - if (document._newWorkplaceId) { - console.log('_newWorkplaceId received on worker.load()'); - this._newWorkplaceId = document._newWorkplaceId; + if (document.newWorkplaceId) { + this._newWorkplaceId = document.newWorkplaceId; } // Consequential updates when one value means another should be empty or null @@ -1377,7 +1380,7 @@ class Worker extends EntityValidator { } if (this._newWorkplaceId !== null) { - myDefaultJSON._newWorkplaceId = this._newWorkplaceId; + myDefaultJSON.newWorkplaceId = this._newWorkplaceId; } // TODO: JSON schema validation diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index 58a81c56d3..b6b56392ae 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -1,3 +1,6 @@ +const sinon = require('sinon'); +const expect = require('chai').expect; + const WorkerCsvValidator = require('../../../../../../../lambdas/bulkUpload/classes/workerCSVValidator.js').WorkerCsvValidator; const mappings = require('../../../../../models/BulkImport/BUDI/index.js').mappings; @@ -6,10 +9,10 @@ const { crossValidate, _crossValidateMainJobRole, _isCQCRegulated, + crossValidateTransferStaffRecord, } = require('../../../../../models/BulkImport/csv/crossValidate'); -const sinon = require('sinon'); const models = require('../../../../../models'); -const expect = require('chai').expect; +const { Establishment } = require('../../../../../models/classes/establishment.js'); describe('crossValidate', () => { describe('_crossValidateMainJobRole', () => { @@ -199,4 +202,142 @@ describe('crossValidate', () => { expect(isCQCRegulated).to.not.deep.equal(false); }); }); + + describe('crossValidateTransferStaffRecord', () => { + const worker = new WorkerCsvValidator(null, null, null, mappings); + const buildMockJSONWorker = (override) => { + return { + ...worker.toJSON(), + status: 'UPDATE', + transferStaffRecord: 'target workplace', + uniqueWorkerId: 'mock_worker_ref', + lineNumber: 3, + ...override, + }; + }; + const myEstablishments = [ + { name: 'workplace A', id: 123 }, + { name: 'workplace B', id: 456 }, + { name: 'target workplace', id: 789 }, + ]; + + let stubEstablishmentFindOne; + let stubWorkerFindOne; + let myAPIEstablishments; + + beforeEach(() => { + stubEstablishmentFindOne = sinon.stub(models.establishment, 'findOne'); + stubEstablishmentFindOne.returns(myEstablishments[2]); + + stubWorkerFindOne = sinon.stub(models.worker, 'findOne'); + stubWorkerFindOne.returns(null); + + myAPIEstablishments = { + workplaceA: new Establishment(), + workplaceB: new Establishment(), + targetworkplace: new Establishment(), + }; + myAPIEstablishments.workplaceA.associateWorker('mock_worker_ref', {}); + }); + + afterEach(() => { + sinon.restore(); + }); + + it('should add an error to csvWorkerSchemaErrors if two workers with the same unique worker id are transferring into the same new workplace', async () => { + const JSONWorkerA = buildMockJSONWorker({ localId: 'workplace A', lineNumber: 3 }); + const JSONWorkerB = buildMockJSONWorker({ localId: 'workplace B', lineNumber: 4 }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + JSONWorkerA, + JSONWorkerB, + ]); + + const expectedError = { + column: 'UNIQUEWORKERID', + errCode: 1403, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: + 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', + worker: JSONWorkerB.uniqueWorkerId, + name: JSONWorkerB.localId, + lineNumber: JSONWorkerB.lineNumber, + source: JSONWorkerB.uniqueWorkerId, + }; + expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); + }); + + it('should add an error to csvWorkerSchemaErrors if the new workplace cannot be found', async () => { + stubEstablishmentFindOne.returns(null); + + const JSONWorker = buildMockJSONWorker({ transferStaffRecord: 'non_exist_workplace' }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + JSONWorker, + ]); + + const expectedError = { + column: 'TRANSFERSTAFFRECORD', + errCode: 1401, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: 'Cannot find an existing workplace with the localId provided in TRANSFERSTAFFRECORD', + worker: JSONWorker.uniqueWorkerId, + name: JSONWorker.localId, + lineNumber: JSONWorker.lineNumber, + source: JSONWorker.transferStaffRecord, + }; + + expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); + expect(stubEstablishmentFindOne).to.have.been.calledWith({ + where: { LocalIdentifierValue: 'non_exist_workplace', id: [123, 456, 789] }, + }); + }); + + it("should add an error to csvWorkerSchemaErrors if the worker's unique worker id is already used in the new workplace", async () => { + stubWorkerFindOne.returns({ nameOrId: 'another worker', LocalIdentifierValue: 'mock_worker_ref' }); + + const JSONWorker = buildMockJSONWorker({ uniqueWorkerId: 'mock_worker_ref' }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + JSONWorker, + ]); + + const expectedError = { + column: 'UNIQUEWORKERID', + errCode: 1402, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD.', + worker: JSONWorker.uniqueWorkerId, + name: JSONWorker.localId, + lineNumber: JSONWorker.lineNumber, + source: JSONWorker.uniqueWorkerId, + }; + + expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); + expect(stubWorkerFindOne).to.have.been.calledWith({ + where: { LocalIdentifierValue: 'mock_worker_ref', establishmentFk: 789 }, + }); + }); + + it('should add newWorkplaceId to the worker entity if all validations passed', async () => { + const JSONWorker = buildMockJSONWorker({ localId: 'workplace A', uniqueWorkerId: 'mock_worker_ref' }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + JSONWorker, + ]); + + expect(csvWorkerSchemaErrors).to.be.empty; + + const workerEntity = myAPIEstablishments['workplaceA']._workerEntities['mock_worker_ref']; + expect(workerEntity._newWorkplaceId).to.equal(789); + }); + }); }); diff --git a/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js b/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js index 70dc0bcc91..b2cd25b320 100644 --- a/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js +++ b/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js @@ -25,7 +25,7 @@ const workerHeadersWithoutCHGUNIQUEWRKID = 'NMCREG,NURSESPEC,AMHP,SCQUAL,NONSCQUAL,QUALACH01,QUALACH01NOTES,' + 'QUALACH02,QUALACH02NOTES,QUALACH03,QUALACH03NOTES'; -describe.only('server/routes/establishments/bulkUpload/validate/headers/worker', () => { +describe('server/routes/establishments/bulkUpload/validate/headers/worker', () => { describe('validateWorkerHeaders()', () => { it('should return true when headings match with CHGUNIQUEWRKID', async () => { expect(validateWorkerHeaders(workerHeadersWithCHGUNIQUEWRKID)).to.deep.equal(true); diff --git a/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js b/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js index f7836904e5..dce2c1c399 100644 --- a/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js +++ b/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js @@ -1430,7 +1430,7 @@ describe('/lambdas/bulkUpload/classes/workerCSVValidator', async () => { }); }); - describe.only('_validateTransferStaffRecord', () => { + describe('_validateTransferStaffRecord', () => { const worker = buildWorkerCsv({ overrides: { STATUS: 'UPDATE', From b88c14be9d0bc32a2ed15d1df5d121eebad51a17 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Wed, 6 Nov 2024 13:10:30 +0000 Subject: [PATCH 09/33] refactor validateWorkerHeaders --- .../bulkUpload/data/workerHeaders.js | 2 +- .../bulkUpload/validate/headers/worker.js | 31 ++++++------- .../validate/headers/worker.spec.js | 46 ++++++++++--------- 3 files changed, 38 insertions(+), 41 deletions(-) diff --git a/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js b/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js index 6b630b1843..4971d87f34 100644 --- a/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js +++ b/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js @@ -57,6 +57,6 @@ const workerHeadersWithTransferStaffRecordAsArray = [ ]; exports.workerHeadersWithCHGUNIQUEWRKID = workerHeadersWithChangeUniqueWorkerIdAsArray.join(','); -exports.workerHeadersWithTransferStaff = workerHeadersWithTransferStaffRecordAsArray.join(','); +exports.workerHeadersWithTRANSFERSTAFFRECORD = workerHeadersWithTransferStaffRecordAsArray.join(','); exports.workerHeadersWithoutCHGUNIQUEWRKID = workerHeaders.join(','); exports.getWorkerColumnIndex = (columnName) => workerHeaders.findIndex((header) => header === columnName); diff --git a/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js b/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js index db3a902305..f4717ff54b 100644 --- a/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js +++ b/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js @@ -1,28 +1,23 @@ const { workerHeadersWithCHGUNIQUEWRKID, workerHeadersWithoutCHGUNIQUEWRKID, - workerHeadersWithTransferStaff, + workerHeadersWithTRANSFERSTAFFRECORD, } = require('../../data/workerHeaders'); const validateWorkerHeaders = (headers) => { - const matchesWithChgUnique = headers.startsWith(workerHeadersWithCHGUNIQUEWRKID); - const matchesWithoutChgUnique = headers.startsWith(workerHeadersWithoutCHGUNIQUEWRKID); - const matchesWithTransferStaff = headers.startsWith(workerHeadersWithTransferStaff); - - if (!matchesWithChgUnique && !matchesWithoutChgUnique && !matchesWithTransferStaff) { - return false; - } - - if (matchesWithTransferStaff) { - // temp shortcut - return true; + const allowedWorkerHeaders = [ + workerHeadersWithoutCHGUNIQUEWRKID, + workerHeadersWithCHGUNIQUEWRKID, + workerHeadersWithTRANSFERSTAFFRECORD, + ]; + + for (const workerHeadersBeforeExtraQuals of allowedWorkerHeaders) { + if (headers.startsWith(workerHeadersBeforeExtraQuals)) { + const additionalQualsHeaders = headers.slice(workerHeadersBeforeExtraQuals.length); + return validateAdditionalQualificationsHeaders(additionalQualsHeaders); + } } - - const additionalQualsHeaders = matchesWithChgUnique - ? headers.slice(workerHeadersWithCHGUNIQUEWRKID.length) - : headers.slice(workerHeadersWithoutCHGUNIQUEWRKID.length); - - return validateAdditionalQualificationsHeaders(additionalQualsHeaders); + return false; }; const validateAdditionalQualificationsHeaders = (additionalQualsHeaders) => { diff --git a/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js b/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js index b2cd25b320..3a9c90af5f 100644 --- a/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js +++ b/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js @@ -35,7 +35,7 @@ describe('server/routes/establishments/bulkUpload/validate/headers/worker', () = expect(validateWorkerHeaders(workerHeadersWithTRANSFERSTAFFRECORD)).to.deep.equal(true); }); - it('should return true when headings match without CHGUNIQUEWRKID', async () => { + it('should return true when headings match without CHGUNIQUEWRKID OR TRANSFERSTAFFRECORD', async () => { expect(validateWorkerHeaders(workerHeadersWithoutCHGUNIQUEWRKID)).to.deep.equal(true); }); @@ -46,34 +46,36 @@ describe('server/routes/establishments/bulkUpload/validate/headers/worker', () = }); describe('Extra qualifications', () => { - it('should return true when headings match with headers for one extra QUAL', async () => { - const headersWithExtraQuals = workerHeadersWithoutCHGUNIQUEWRKID.concat(',QUALACH04,QUALACH04NOTES'); + const testCases = [ + workerHeadersWithCHGUNIQUEWRKID, + workerHeadersWithoutCHGUNIQUEWRKID, + workerHeadersWithTRANSFERSTAFFRECORD, + ]; - expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(true); - }); + testCases.forEach((workerHeaders) => { + it('should return true when headings match with headers for one extra QUAL', async () => { + const headersWithExtraQuals = workerHeaders.concat(',QUALACH04,QUALACH04NOTES'); - it('should return true when headings match with headers for two extra QUALs', async () => { - const headersWithExtraQuals = workerHeadersWithoutCHGUNIQUEWRKID.concat( - ',QUALACH04,QUALACH04NOTES,QUALACH05,QUALACH05NOTES', - ); + expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(true); + }); - expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(true); - }); + it('should return true when headings match with headers for two extra QUALs', async () => { + const headersWithExtraQuals = workerHeaders.concat(',QUALACH04,QUALACH04NOTES,QUALACH05,QUALACH05NOTES'); - it('should return false when invalid extra QUALs headers (wrong qual number)', async () => { - const headersWithExtraQuals = workerHeadersWithoutCHGUNIQUEWRKID.concat( - ',QUALACH04,QUALACH04NOTES,QUALACH04,QUALACH05NOTES', - ); + expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(true); + }); - expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(false); - }); + it('should return false when invalid extra QUALs headers (wrong qual number)', async () => { + const headersWithExtraQuals = workerHeaders.concat(',QUALACH04,QUALACH04NOTES,QUALACH04,QUALACH05NOTES'); + + expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(false); + }); - it('should return false when invalid extra QUALs headers (wrong qualNotes number)', async () => { - const headersWithExtraQuals = workerHeadersWithoutCHGUNIQUEWRKID.concat( - ',QUALACH04,QUALACH04NOTES,QUALACH05,QUALACH03NOTES', - ); + it('should return false when invalid extra QUALs headers (wrong qualNotes number)', async () => { + const headersWithExtraQuals = workerHeaders.concat(',QUALACH04,QUALACH04NOTES,QUALACH05,QUALACH03NOTES'); - expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(false); + expect(validateWorkerHeaders(headersWithExtraQuals)).to.deep.equal(false); + }); }); }); }); From fb2c31483c0929f3c4ce7c134c713b1fd0110ce1 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Wed, 6 Nov 2024 16:55:19 +0000 Subject: [PATCH 10/33] add validation to prevent the case of referenceId collision between NEW worker and TRANSFER worker --- .../models/BulkImport/csv/crossValidate.js | 34 +++++--- .../Bulkimport/csv/crossValidate.spec.js | 86 +++++++++++++------ .../bulkUpload/classes/workerCSVValidator.js | 1 + 3 files changed, 85 insertions(+), 36 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index b77fe6a754..21d97e0a7b 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -1,3 +1,4 @@ +const { chain } = require('lodash'); const models = require('../../../models'); const MAIN_JOB_ROLE_ERROR = () => 1280; @@ -65,8 +66,9 @@ const crossValidateTransferStaffRecord = async ( const relatedEstablishmentIds = myEstablishments.map((establishment) => establishment.id); const allMovingWorkers = myJSONWorkers.filter(isMovingToNewWorkplace); + const allNewWorkers = myJSONWorkers.filter((JSONWorker) => JSONWorker.status === 'NEW'); - _crossValidateWorkersWithSameRefMovingToSameWorkplace(csvWorkerSchemaErrors, allMovingWorkers); + _crossValidateWorkersWithSameRefMovingToSameWorkplace(csvWorkerSchemaErrors, allMovingWorkers, allNewWorkers); for (const JSONWorker of allMovingWorkers) { const newWorkplaceId = await _validateTransferIsPossible( @@ -128,7 +130,7 @@ const _checkDuplicateLocalIdInNewWorkplace = async (newWorkplaceId, uniqueWorker if (uniqueWorkerIdHasWhitespace) { // Handle special cases when uniqueWorkerId includes whitespace. - // As the legacy code does a /\s/g replacement in same places, we need to consider the case of local id collision with whitespaces stripped. + // As the legacy code does a /\s/g replacement in several different places, we need to ensure uniqueness even when whitespaces are stripped out. const allWorkersInNewWorkplace = await models.worker.findAll({ attributes: ['LocalIdentifierValue'], where: { @@ -195,28 +197,40 @@ const _addErrorForSameLocalIdExistInNewWorkplace = (csvWorkerSchemaErrors, JSONW }); }; -const _crossValidateWorkersWithSameRefMovingToSameWorkplace = (csvWorkerSchemaErrors, allMovingWorkers) => { - const destinations = {}; +const _crossValidateWorkersWithSameRefMovingToSameWorkplace = ( + csvWorkerSchemaErrors, + allMovingWorkers, + allNewWorkers, +) => { + const workplacesDict = _buildWorkplaceDictWithNewWorkers(allNewWorkers); for (const JSONWorker of allMovingWorkers) { - const newWorkplace = JSONWorker.transferStaffRecord; + const newWorkplaceRef = JSONWorker.transferStaffRecord; const workerRef = JSONWorker.uniqueWorkerId.replace(/\s/g, ''); - if (!destinations[newWorkplace]) { - destinations[newWorkplace] = new Set([workerRef]); + if (!workplacesDict[newWorkplaceRef]) { + workplacesDict[newWorkplaceRef] = new Set([workerRef]); continue; } - if (!destinations[newWorkplace].has(workerRef)) { - destinations[newWorkplace].add(workerRef); + if (!workplacesDict[newWorkplaceRef].has(workerRef)) { + workplacesDict[newWorkplaceRef].add(workerRef); continue; } - // if arrive at here, there is already another worker with that workerRef moving to the same new workplace + // if arrive at here, there is already another new or moving worker with that workerRef coming to the same new workplace _addErrorForWorkersWithSameRefsMovingToSameWorkplace(csvWorkerSchemaErrors, JSONWorker); } }; +const _buildWorkplaceDictWithNewWorkers = (allNewWorkers) => { + return chain(allNewWorkers) + .groupBy('localId') + .mapValues((JSONWorkers) => JSONWorkers.map((JSONWorker) => JSONWorker.uniqueWorkerId.replace(/\s/g, ''))) + .mapValues((workerRefs) => new Set(workerRefs)) + .value(); +}; + const _addErrorForWorkersWithSameRefsMovingToSameWorkplace = (csvWorkerSchemaErrors, JSONWorker) => { csvWorkerSchemaErrors.unshift({ worker: JSONWorker.uniqueWorkerId, diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index b6b56392ae..85b32c1905 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -14,7 +14,7 @@ const { const models = require('../../../../../models'); const { Establishment } = require('../../../../../models/classes/establishment.js'); -describe('crossValidate', () => { +describe.only('crossValidate', () => { describe('_crossValidateMainJobRole', () => { it('should add error to csvWorkerSchemaErrors if establishment not CQC regulated and main role ID is 4', () => { const csvWorkerSchemaErrors = []; @@ -244,31 +244,6 @@ describe('crossValidate', () => { sinon.restore(); }); - it('should add an error to csvWorkerSchemaErrors if two workers with the same unique worker id are transferring into the same new workplace', async () => { - const JSONWorkerA = buildMockJSONWorker({ localId: 'workplace A', lineNumber: 3 }); - const JSONWorkerB = buildMockJSONWorker({ localId: 'workplace B', lineNumber: 4 }); - - const csvWorkerSchemaErrors = []; - - await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ - JSONWorkerA, - JSONWorkerB, - ]); - - const expectedError = { - column: 'UNIQUEWORKERID', - errCode: 1403, - errType: 'TRANSFERSTAFFRECORD_ERROR', - error: - 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', - worker: JSONWorkerB.uniqueWorkerId, - name: JSONWorkerB.localId, - lineNumber: JSONWorkerB.lineNumber, - source: JSONWorkerB.uniqueWorkerId, - }; - expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); - }); - it('should add an error to csvWorkerSchemaErrors if the new workplace cannot be found', async () => { stubEstablishmentFindOne.returns(null); @@ -325,6 +300,65 @@ describe('crossValidate', () => { }); }); + it('should add an error to csvWorkerSchemaErrors if two workers with the same unique worker id are transferring into the same new workplace', async () => { + const JSONWorkerA = buildMockJSONWorker({ localId: 'workplace A', lineNumber: 3 }); + const JSONWorkerB = buildMockJSONWorker({ localId: 'workplace B', lineNumber: 4 }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + JSONWorkerA, + JSONWorkerB, + ]); + + const expectedError = { + column: 'UNIQUEWORKERID', + errCode: 1403, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: + 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', + worker: JSONWorkerB.uniqueWorkerId, + name: JSONWorkerB.localId, + lineNumber: JSONWorkerB.lineNumber, + source: JSONWorkerB.uniqueWorkerId, + }; + expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); + }); + + it('should add an error to csvWorkerSchemaErrors if a NEW worker and a transferring worker with the same ref are coming into the same new workplace', async () => { + const movingWorker = buildMockJSONWorker({ uniqueWorkerId: 'mock_worker_ref' }); + const newWorker = buildMockJSONWorker({ + uniqueWorkerId: 'mock_worker_ref', + status: 'NEW', + transferStaffRecord: null, + localId: 'target workplace', + }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + movingWorker, + newWorker, + ]); + + const expectedError = { + column: 'UNIQUEWORKERID', + errCode: 1403, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: + 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', + worker: movingWorker.uniqueWorkerId, + name: movingWorker.localId, + lineNumber: movingWorker.lineNumber, + source: movingWorker.uniqueWorkerId, + }; + + expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); + expect(stubWorkerFindOne).to.have.been.calledWith({ + where: { LocalIdentifierValue: 'mock_worker_ref', establishmentFk: 789 }, + }); + }); + it('should add newWorkplaceId to the worker entity if all validations passed', async () => { const JSONWorker = buildMockJSONWorker({ localId: 'workplace A', uniqueWorkerId: 'mock_worker_ref' }); diff --git a/lambdas/bulkUpload/classes/workerCSVValidator.js b/lambdas/bulkUpload/classes/workerCSVValidator.js index 4730bc623d..ee7c8f78ba 100644 --- a/lambdas/bulkUpload/classes/workerCSVValidator.js +++ b/lambdas/bulkUpload/classes/workerCSVValidator.js @@ -199,6 +199,7 @@ class WorkerCsvValidator { static get TRANSFERSTAFFRECORD_ERROR() { return 1400; + // NOTE: Reserve error code 1400 - 1409 for TRANSFERSTAFFRECORD errors } static get UNIQUE_WORKER_ID_WARNING() { From f0b989e520fcde8d55c024bf015f6b5dd3d26c38 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Wed, 6 Nov 2024 17:07:16 +0000 Subject: [PATCH 11/33] minor changes for clarity --- .../models/BulkImport/csv/crossValidate.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 21d97e0a7b..bf3e82669e 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -129,7 +129,7 @@ const _checkDuplicateLocalIdInNewWorkplace = async (newWorkplaceId, uniqueWorker const uniqueWorkerIdHasWhitespace = /\s/g.test(uniqueWorkerId); if (uniqueWorkerIdHasWhitespace) { - // Handle special cases when uniqueWorkerId includes whitespace. + // Handle the special case when uniqueWorkerId includes whitespace. // As the legacy code does a /\s/g replacement in several different places, we need to ensure uniqueness even when whitespaces are stripped out. const allWorkersInNewWorkplace = await models.worker.findAll({ attributes: ['LocalIdentifierValue'], @@ -142,24 +142,26 @@ const _checkDuplicateLocalIdInNewWorkplace = async (newWorkplaceId, uniqueWorker .filter((worker) => worker?.LocalIdentifierValue) .map((worker) => worker.LocalIdentifierValue.replace(/\s/g, '')); - return allRefsWithoutWhitespaces.includes(uniqueWorkerId.replace(/\s/g, '')); + const duplicationFound = allRefsWithoutWhitespaces.includes(uniqueWorkerId.replace(/\s/g, '')); + return duplicationFound; } - const sameLocalIdFound = await models.worker.findOne({ + // normal case, when uniqueWorkerId does not contain whitespace + const duplicationFound = await models.worker.findOne({ where: { LocalIdentifierValue: uniqueWorkerId, establishmentFk: newWorkplaceId, }, }); - return !!sameLocalIdFound; + return !!duplicationFound; }; const _workerPassedAllValidations = (csvWorkerSchemaErrors, JSONWorker) => { - return ( - csvWorkerSchemaErrors.find( - (error) => error?.lineNumber === JSONWorker.lineNumber && error?.errType === 'TRANSFERSTAFFRECORD_ERROR', - ) === undefined + const errorForThisWorker = csvWorkerSchemaErrors.find( + (error) => error?.lineNumber === JSONWorker.lineNumber && error?.errType === 'TRANSFERSTAFFRECORD_ERROR', ); + + return !errorForThisWorker; }; const _addNewWorkplaceIdToWorkerEntity = (myAPIEstablishments, JSONWorker, newWorkplaceId) => { From f67aaa8830c4583b30219e1581bdbd3422b53718 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Thu, 7 Nov 2024 12:34:50 +0000 Subject: [PATCH 12/33] add unit test for the special case of worker ref with whitespace --- .../models/BulkImport/csv/crossValidate.js | 7 +++-- .../Bulkimport/csv/crossValidate.spec.js | 29 ++++++++++++++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index bf3e82669e..6561393737 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -1,4 +1,5 @@ const { chain } = require('lodash'); +const { Op } = require('sequelize'); const models = require('../../../models'); const MAIN_JOB_ROLE_ERROR = () => 1280; @@ -138,11 +139,11 @@ const _checkDuplicateLocalIdInNewWorkplace = async (newWorkplaceId, uniqueWorker }, raw: true, }); - const allRefsWithoutWhitespaces = allWorkersInNewWorkplace + const allRefsWithWhitespacesStripped = allWorkersInNewWorkplace .filter((worker) => worker?.LocalIdentifierValue) .map((worker) => worker.LocalIdentifierValue.replace(/\s/g, '')); - const duplicationFound = allRefsWithoutWhitespaces.includes(uniqueWorkerId.replace(/\s/g, '')); + const duplicationFound = allRefsWithWhitespacesStripped.includes(uniqueWorkerId.replace(/\s/g, '')); return duplicationFound; } @@ -227,7 +228,7 @@ const _crossValidateWorkersWithSameRefMovingToSameWorkplace = ( const _buildWorkplaceDictWithNewWorkers = (allNewWorkers) => { return chain(allNewWorkers) - .groupBy('localId') + .groupBy('localId') // workplace ref .mapValues((JSONWorkers) => JSONWorkers.map((JSONWorker) => JSONWorker.uniqueWorkerId.replace(/\s/g, ''))) .mapValues((workerRefs) => new Set(workerRefs)) .value(); diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index 85b32c1905..eb17421a2a 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -14,7 +14,7 @@ const { const models = require('../../../../../models'); const { Establishment } = require('../../../../../models/classes/establishment.js'); -describe.only('crossValidate', () => { +describe('crossValidate', () => { describe('_crossValidateMainJobRole', () => { it('should add error to csvWorkerSchemaErrors if establishment not CQC regulated and main role ID is 4', () => { const csvWorkerSchemaErrors = []; @@ -300,6 +300,33 @@ describe.only('crossValidate', () => { }); }); + it("should add an error to csvWorkerSchemaErrors if the worker's unique worker id will collide with an existing worker in the new workplace given whitespaces are removed", async () => { + sinon + .stub(models.worker, 'findAll') + .returns([{ nameOrId: 'another worker', LocalIdentifierValue: 'mockWorker Ref' }]); + + const JSONWorker = buildMockJSONWorker({ uniqueWorkerId: 'mock Worker Ref' }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + JSONWorker, + ]); + + const expectedError = { + column: 'UNIQUEWORKERID', + errCode: 1402, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD.', + worker: JSONWorker.uniqueWorkerId, + name: JSONWorker.localId, + lineNumber: JSONWorker.lineNumber, + source: JSONWorker.uniqueWorkerId, + }; + + expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); + }); + it('should add an error to csvWorkerSchemaErrors if two workers with the same unique worker id are transferring into the same new workplace', async () => { const JSONWorkerA = buildMockJSONWorker({ localId: 'workplace A', lineNumber: 3 }); const JSONWorkerB = buildMockJSONWorker({ localId: 'workplace B', lineNumber: 4 }); From 00a4bf8de24fb281982a90dd59d8c4f14bdee7fc Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Thu, 7 Nov 2024 15:57:46 +0000 Subject: [PATCH 13/33] fix a bug of calling training.save() with wrong params --- backend/server/models/classes/worker.js | 3 +-- backend/server/test/unit/models/classes/worker.spec.js | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/backend/server/models/classes/worker.js b/backend/server/models/classes/worker.js index d9f0dc7f37..6aaff97a2d 100644 --- a/backend/server/models/classes/worker.js +++ b/backend/server/models/classes/worker.js @@ -568,7 +568,7 @@ class Worker extends EntityValidator { currentTrainingRecord.workerId = this._id; currentTrainingRecord.workerUid = this._uid; currentTrainingRecord.establishmentId = this._establishmentId; - newTrainingPromises.push(currentTrainingRecord.save(savedBy, bulkUploaded, 0, externalTransaction)); + newTrainingPromises.push(currentTrainingRecord.save(savedBy, bulkUploaded, externalTransaction)); }); } @@ -689,7 +689,6 @@ class Worker extends EntityValidator { if (associatedEntities) { await this.saveAssociatedEntities(savedBy, bulkUploaded, thisTransaction); } - if (this.nurseSpecialisms && this.nurseSpecialisms.value === 'Yes') { await models.workerNurseSpecialisms.bulkCreate( this.nurseSpecialisms.specialisms.map((thisSpecialism) => ({ diff --git a/backend/server/test/unit/models/classes/worker.spec.js b/backend/server/test/unit/models/classes/worker.spec.js index a6dc185a1a..bd780e4d87 100644 --- a/backend/server/test/unit/models/classes/worker.spec.js +++ b/backend/server/test/unit/models/classes/worker.spec.js @@ -529,7 +529,7 @@ describe('Worker Class', () => { where: { workerFk: mockWorker._id }, transaction: transaction, }); - expect(trainingSaveSpy).to.have.been.calledWith(savedBy, bulkUploaded, 0, transaction); + expect(trainingSaveSpy).to.have.been.calledWith(savedBy, bulkUploaded, transaction); }); it('should not make calls to delete certificates or destroy training records when no training records in trainingEntities', async () => { From f1a85d54fb2a0ec4df2d2519f585d1b472729ffe Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Thu, 7 Nov 2024 16:00:14 +0000 Subject: [PATCH 14/33] amend validate error msg --- backend/server/models/BulkImport/csv/crossValidate.js | 4 ++-- .../test/unit/models/Bulkimport/csv/crossValidate.spec.js | 6 +++--- lambdas/bulkUpload/classes/workerCSVValidator.js | 2 +- .../bulkUpload/test/unit/classes/workerCSVValidator.spec.js | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 6561393737..69b592ed29 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -183,7 +183,7 @@ const _addErrorForNewWorkplaceNotFound = (csvWorkerSchemaErrors, JSONWorker) => errType: 'TRANSFERSTAFFRECORD_ERROR', source: JSONWorker.transferStaffRecord, column: 'TRANSFERSTAFFRECORD', - error: 'Cannot find an existing workplace with the localId provided in TRANSFERSTAFFRECORD', + error: 'Cannot find an existing workplace with the reference provided in TRANSFERSTAFFRECORD', }); }; @@ -196,7 +196,7 @@ const _addErrorForSameLocalIdExistInNewWorkplace = (csvWorkerSchemaErrors, JSONW errType: 'TRANSFERSTAFFRECORD_ERROR', source: JSONWorker.uniqueWorkerId, column: 'UNIQUEWORKERID', - error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD.', + error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD', }); }; diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index eb17421a2a..368a85ea8e 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -259,7 +259,7 @@ describe('crossValidate', () => { column: 'TRANSFERSTAFFRECORD', errCode: 1401, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'Cannot find an existing workplace with the localId provided in TRANSFERSTAFFRECORD', + error: 'Cannot find an existing workplace with the reference provided in TRANSFERSTAFFRECORD', worker: JSONWorker.uniqueWorkerId, name: JSONWorker.localId, lineNumber: JSONWorker.lineNumber, @@ -287,7 +287,7 @@ describe('crossValidate', () => { column: 'UNIQUEWORKERID', errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD.', + error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD', worker: JSONWorker.uniqueWorkerId, name: JSONWorker.localId, lineNumber: JSONWorker.lineNumber, @@ -317,7 +317,7 @@ describe('crossValidate', () => { column: 'UNIQUEWORKERID', errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD.', + error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD', worker: JSONWorker.uniqueWorkerId, name: JSONWorker.localId, lineNumber: JSONWorker.lineNumber, diff --git a/lambdas/bulkUpload/classes/workerCSVValidator.js b/lambdas/bulkUpload/classes/workerCSVValidator.js index ee7c8f78ba..55f3f4c534 100644 --- a/lambdas/bulkUpload/classes/workerCSVValidator.js +++ b/lambdas/bulkUpload/classes/workerCSVValidator.js @@ -792,7 +792,7 @@ class WorkerCsvValidator { lineNumber: this._lineNumber, errCode: WorkerCsvValidator.TRANSFERSTAFFRECORD_ERROR, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: `TRANSFERSTAFFRECORD is provided but cannot find the worker in the old workplace`, + error: `TRANSFERSTAFFRECORD is provided but cannot find the worker in the given workplace`, source: this._currentLine.LOCALESTID, column: 'LOCALESTID', }); diff --git a/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js b/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js index dce2c1c399..ddac7f61bb 100644 --- a/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js +++ b/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js @@ -1448,7 +1448,7 @@ describe('/lambdas/bulkUpload/classes/workerCSVValidator', async () => { lineNumber: 2, errCode: WorkerCsvValidator.TRANSFERSTAFFRECORD_ERROR, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'TRANSFERSTAFFRECORD is provided but cannot find the worker in the old workplace', + error: 'TRANSFERSTAFFRECORD is provided but cannot find the worker in the given workplace', source: worker.LOCALESTID, column: 'LOCALESTID', name: 'MARMA', From 2ff8319b6fecef761543d98861e54d9eab15fb8c Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Fri, 8 Nov 2024 12:59:43 +0000 Subject: [PATCH 15/33] improve the way to detect conflicting worker local ref in new workplace --- .../models/BulkImport/csv/crossValidate.js | 33 +------------- backend/server/models/worker.js | 19 ++++++++ .../Bulkimport/csv/crossValidate.spec.js | 45 ++++--------------- 3 files changed, 28 insertions(+), 69 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 69b592ed29..c9140dad1e 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -97,7 +97,7 @@ const _validateTransferIsPossible = async (csvWorkerSchemaErrors, relatedEstabli return; } - const sameLocalIdExistInNewWorkplace = await _checkDuplicateLocalIdInNewWorkplace( + const sameLocalIdExistInNewWorkplace = await models.worker.findOneWithConflictingLocalRef( newWorkplaceId, JSONWorker.uniqueWorkerId, ); @@ -126,37 +126,6 @@ const _getNewWorkplaceId = async (newWorkplaceLocalRef, relatedEstablishmentIds) return null; }; -const _checkDuplicateLocalIdInNewWorkplace = async (newWorkplaceId, uniqueWorkerId) => { - const uniqueWorkerIdHasWhitespace = /\s/g.test(uniqueWorkerId); - - if (uniqueWorkerIdHasWhitespace) { - // Handle the special case when uniqueWorkerId includes whitespace. - // As the legacy code does a /\s/g replacement in several different places, we need to ensure uniqueness even when whitespaces are stripped out. - const allWorkersInNewWorkplace = await models.worker.findAll({ - attributes: ['LocalIdentifierValue'], - where: { - establishmentFk: newWorkplaceId, - }, - raw: true, - }); - const allRefsWithWhitespacesStripped = allWorkersInNewWorkplace - .filter((worker) => worker?.LocalIdentifierValue) - .map((worker) => worker.LocalIdentifierValue.replace(/\s/g, '')); - - const duplicationFound = allRefsWithWhitespacesStripped.includes(uniqueWorkerId.replace(/\s/g, '')); - return duplicationFound; - } - - // normal case, when uniqueWorkerId does not contain whitespace - const duplicationFound = await models.worker.findOne({ - where: { - LocalIdentifierValue: uniqueWorkerId, - establishmentFk: newWorkplaceId, - }, - }); - return !!duplicationFound; -}; - const _workerPassedAllValidations = (csvWorkerSchemaErrors, JSONWorker) => { const errorForThisWorker = csvWorkerSchemaErrors.find( (error) => error?.lineNumber === JSONWorker.lineNumber && error?.errType === 'TRANSFERSTAFFRECORD_ERROR', diff --git a/backend/server/models/worker.js b/backend/server/models/worker.js index 72626898b9..23b5096865 100644 --- a/backend/server/models/worker.js +++ b/backend/server/models/worker.js @@ -1401,5 +1401,24 @@ module.exports = function (sequelize, DataTypes) { }); }; + Worker.findOneWithConflictingLocalRef = async function (establishmentIds, localIdentifierValue) { + /** Check if there is a worker having the same localIdentifierValue when whitespaces are not considered. + * + * As the legacy code does a /\s/g replacement in several different places, + * this helps to ensure uniqueness and prevent unexpected reference collision. + */ + return await this.findOne({ + attributes: ['id', 'NameOrIdValue', 'LocalIdentifierValue'], + where: { + [Op.and]: [ + { establishmentFk: establishmentIds }, + sequelize.where(sequelize.fn('REPLACE', sequelize.col('LocalIdentifierValue'), ' ', ''), { + [Op.eq]: localIdentifierValue.replace(/\s/g, ''), + }), + ], + }, + }); + }; + return Worker; }; diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index 368a85ea8e..306a973789 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -222,15 +222,15 @@ describe('crossValidate', () => { ]; let stubEstablishmentFindOne; - let stubWorkerFindOne; + let stubWorkerFindOneWithLocalRef; let myAPIEstablishments; beforeEach(() => { stubEstablishmentFindOne = sinon.stub(models.establishment, 'findOne'); stubEstablishmentFindOne.returns(myEstablishments[2]); - stubWorkerFindOne = sinon.stub(models.worker, 'findOne'); - stubWorkerFindOne.returns(null); + stubWorkerFindOneWithLocalRef = sinon.stub(models.worker, 'findOneWithConflictingLocalRef'); + stubWorkerFindOneWithLocalRef.returns(null); myAPIEstablishments = { workplaceA: new Establishment(), @@ -273,39 +273,12 @@ describe('crossValidate', () => { }); it("should add an error to csvWorkerSchemaErrors if the worker's unique worker id is already used in the new workplace", async () => { - stubWorkerFindOne.returns({ nameOrId: 'another worker', LocalIdentifierValue: 'mock_worker_ref' }); - - const JSONWorker = buildMockJSONWorker({ uniqueWorkerId: 'mock_worker_ref' }); - - const csvWorkerSchemaErrors = []; - - await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ - JSONWorker, - ]); - - const expectedError = { - column: 'UNIQUEWORKERID', - errCode: 1402, - errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD', - worker: JSONWorker.uniqueWorkerId, - name: JSONWorker.localId, - lineNumber: JSONWorker.lineNumber, - source: JSONWorker.uniqueWorkerId, - }; - - expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); - expect(stubWorkerFindOne).to.have.been.calledWith({ - where: { LocalIdentifierValue: 'mock_worker_ref', establishmentFk: 789 }, + stubWorkerFindOneWithLocalRef.returns({ + NameOrIdValue: 'another worker', + LocalIdentifierValue: 'mock_worker_ref', }); - }); - it("should add an error to csvWorkerSchemaErrors if the worker's unique worker id will collide with an existing worker in the new workplace given whitespaces are removed", async () => { - sinon - .stub(models.worker, 'findAll') - .returns([{ nameOrId: 'another worker', LocalIdentifierValue: 'mockWorker Ref' }]); - - const JSONWorker = buildMockJSONWorker({ uniqueWorkerId: 'mock Worker Ref' }); + const JSONWorker = buildMockJSONWorker({ uniqueWorkerId: 'mock_worker_ref' }); const csvWorkerSchemaErrors = []; @@ -325,6 +298,7 @@ describe('crossValidate', () => { }; expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); + expect(stubWorkerFindOneWithLocalRef).to.have.been.calledWith(789, 'mock_worker_ref'); }); it('should add an error to csvWorkerSchemaErrors if two workers with the same unique worker id are transferring into the same new workplace', async () => { @@ -381,9 +355,6 @@ describe('crossValidate', () => { }; expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); - expect(stubWorkerFindOne).to.have.been.calledWith({ - where: { LocalIdentifierValue: 'mock_worker_ref', establishmentFk: 789 }, - }); }); it('should add newWorkplaceId to the worker entity if all validations passed', async () => { From d96980d665adb6d95942ae380ed2d0592950051b Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Fri, 8 Nov 2024 14:59:05 +0000 Subject: [PATCH 16/33] minor edit --- backend/server/models/BulkImport/csv/crossValidate.js | 3 +-- backend/server/models/classes/worker.js | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index c9140dad1e..69196b0adb 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -1,5 +1,4 @@ const { chain } = require('lodash'); -const { Op } = require('sequelize'); const models = require('../../../models'); const MAIN_JOB_ROLE_ERROR = () => 1280; @@ -53,7 +52,7 @@ const _isCQCRegulated = async (myEstablishments, JSONWorker) => { const _checkEstablishmentRegulatedInDatabase = async (establishmentId) => { const establishment = await models.establishment.findbyId(establishmentId); - return establishment.isRegulated; + return establishment?.isRegulated; }; const workerNotChanged = (JSONWorker) => !['NEW', 'UPDATE'].includes(JSONWorker.status); diff --git a/backend/server/models/classes/worker.js b/backend/server/models/classes/worker.js index 6aaff97a2d..e6fd09dde4 100644 --- a/backend/server/models/classes/worker.js +++ b/backend/server/models/classes/worker.js @@ -765,8 +765,8 @@ class Worker extends EntityValidator { updatedBy: savedBy.toLowerCase(), }; - if (bulkUploaded && this._status === 'UPDATE' && this.transferStaffRecord && this._newWorkplaceId) { - updateDocument.establishmentFk = this._newWorkplaceId; + if (bulkUploaded && this._status === 'UPDATE' && this.transferStaffRecord && this.newWorkplaceId) { + updateDocument.establishmentFk = this.newWorkplaceId; } if (this._changeLocalIdentifer) { From c651ab66191221009ccea4b9ab54522fc458e02b Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Mon, 11 Nov 2024 12:01:45 +0000 Subject: [PATCH 17/33] refactor the logic around adding errors to remove duplication --- .../models/BulkImport/csv/crossValidate.js | 69 ++++--------------- .../BulkImport/csv/crossValidateErrors.js | 57 +++++++++++++++ 2 files changed, 71 insertions(+), 55 deletions(-) create mode 100644 backend/server/models/BulkImport/csv/crossValidateErrors.js diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 69196b0adb..94e05399a9 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -1,9 +1,6 @@ const { chain } = require('lodash'); const models = require('../../../models'); - -const MAIN_JOB_ROLE_ERROR = () => 1280; - -const TRANSFERSTAFFRECORD_ERROR = () => 1400; +const { addCrossValidateError, MAIN_JOB_ERRORS, TRANSFER_STAFF_RECORD_ERRORS } = require('./crossValidateErrors'); const crossValidate = async (csvWorkerSchemaErrors, myEstablishments, JSONWorker) => { if (workerNotChanged(JSONWorker)) { @@ -17,17 +14,11 @@ const crossValidate = async (csvWorkerSchemaErrors, myEstablishments, JSONWorker const _crossValidateMainJobRole = (csvWorkerSchemaErrors, isCqcRegulated, JSONWorker) => { if (!isCqcRegulated && JSONWorker.mainJobRoleId === 4) { - csvWorkerSchemaErrors.unshift({ - worker: JSONWorker.uniqueWorkerId, - name: JSONWorker.localId, - lineNumber: JSONWorker.lineNumber, - errCode: MAIN_JOB_ROLE_ERROR(), - errType: 'MAIN_JOB_ROLE_ERROR', - source: JSONWorker.mainJobRoleId, - column: 'MAINJOBROLE', - error: - 'Workers MAINJOBROLE is Registered Manager but you are not providing a CQC regulated service. Please change to another Job Role', - }); + addCrossValidateError( + csvWorkerSchemaErrors, + MAIN_JOB_ERRORS.RegisteredManagerWithoutCqcRegulatedService, + JSONWorker, + ); } }; @@ -92,7 +83,7 @@ const _validateTransferIsPossible = async (csvWorkerSchemaErrors, relatedEstabli const newWorkplaceId = await _getNewWorkplaceId(newWorkplaceLocalRef, relatedEstablishmentIds); if (newWorkplaceId === null) { - _addErrorForNewWorkplaceNotFound(csvWorkerSchemaErrors, JSONWorker); + addCrossValidateError(csvWorkerSchemaErrors, TRANSFER_STAFF_RECORD_ERRORS.NewWorkplaceNotFound, JSONWorker); return; } @@ -102,7 +93,11 @@ const _validateTransferIsPossible = async (csvWorkerSchemaErrors, relatedEstabli ); if (sameLocalIdExistInNewWorkplace) { - _addErrorForSameLocalIdExistInNewWorkplace(csvWorkerSchemaErrors, JSONWorker); + addCrossValidateError( + csvWorkerSchemaErrors, + TRANSFER_STAFF_RECORD_ERRORS.SameLocalIdExistInNewWorkplace, + JSONWorker, + ); return; } @@ -142,32 +137,6 @@ const _addNewWorkplaceIdToWorkerEntity = (myAPIEstablishments, JSONWorker, newWo workerEntity._newWorkplaceId = newWorkplaceId; }; -const _addErrorForNewWorkplaceNotFound = (csvWorkerSchemaErrors, JSONWorker) => { - csvWorkerSchemaErrors.unshift({ - worker: JSONWorker.uniqueWorkerId, - name: JSONWorker.localId, - lineNumber: JSONWorker.lineNumber, - errCode: TRANSFERSTAFFRECORD_ERROR() + 1, - errType: 'TRANSFERSTAFFRECORD_ERROR', - source: JSONWorker.transferStaffRecord, - column: 'TRANSFERSTAFFRECORD', - error: 'Cannot find an existing workplace with the reference provided in TRANSFERSTAFFRECORD', - }); -}; - -const _addErrorForSameLocalIdExistInNewWorkplace = (csvWorkerSchemaErrors, JSONWorker) => { - csvWorkerSchemaErrors.unshift({ - worker: JSONWorker.uniqueWorkerId, - name: JSONWorker.localId, - lineNumber: JSONWorker.lineNumber, - errCode: TRANSFERSTAFFRECORD_ERROR() + 2, - errType: 'TRANSFERSTAFFRECORD_ERROR', - source: JSONWorker.uniqueWorkerId, - column: 'UNIQUEWORKERID', - error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD', - }); -}; - const _crossValidateWorkersWithSameRefMovingToSameWorkplace = ( csvWorkerSchemaErrors, allMovingWorkers, @@ -190,7 +159,7 @@ const _crossValidateWorkersWithSameRefMovingToSameWorkplace = ( } // if arrive at here, there is already another new or moving worker with that workerRef coming to the same new workplace - _addErrorForWorkersWithSameRefsMovingToSameWorkplace(csvWorkerSchemaErrors, JSONWorker); + addCrossValidateError(csvWorkerSchemaErrors, TRANSFER_STAFF_RECORD_ERRORS.SameRefsMovingToWorkplace, JSONWorker); } }; @@ -203,17 +172,7 @@ const _buildWorkplaceDictWithNewWorkers = (allNewWorkers) => { }; const _addErrorForWorkersWithSameRefsMovingToSameWorkplace = (csvWorkerSchemaErrors, JSONWorker) => { - csvWorkerSchemaErrors.unshift({ - worker: JSONWorker.uniqueWorkerId, - name: JSONWorker.localId, - lineNumber: JSONWorker.lineNumber, - errCode: TRANSFERSTAFFRECORD_ERROR() + 3, - errType: 'TRANSFERSTAFFRECORD_ERROR', - source: JSONWorker.uniqueWorkerId, - column: 'UNIQUEWORKERID', - error: - 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', - }); + addCrossValidateError(csvWorkerSchemaErrors, TRANSFER_STAFF_RECORD_ERRORS.SameRefsMovingToWorkplace, JSONWorker); }; module.exports = { diff --git a/backend/server/models/BulkImport/csv/crossValidateErrors.js b/backend/server/models/BulkImport/csv/crossValidateErrors.js new file mode 100644 index 0000000000..aa04e2414a --- /dev/null +++ b/backend/server/models/BulkImport/csv/crossValidateErrors.js @@ -0,0 +1,57 @@ +const MAIN_JOB_ROLE_ERROR_CODE = 1280; +const TRANSFER_STAFF_RECORD_BASE_ERROR_CODE = 1400; + +const MAIN_JOB_ERRORS = Object.freeze({ + RegisteredManagerWithoutCqcRegulatedService: { + errCode: MAIN_JOB_ROLE_ERROR_CODE, + errType: 'MAIN_JOB_ROLE_ERROR', + column: 'MAINJOBROLE', + _sourceFieldName: 'mainJobRoleId', + error: + 'Workers MAINJOBROLE is Registered Manager but you are not providing a CQC regulated service. Please change to another Job Role', + }, +}); + +const TRANSFER_STAFF_RECORD_ERRORS = Object.freeze({ + NewWorkplaceNotFound: { + errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 1, + errType: 'TRANSFERSTAFFRECORD_ERROR', + column: 'TRANSFERSTAFFRECORD', + _sourceFieldName: 'transferStaffRecord', + error: 'Cannot find an existing workplace with the reference provided in TRANSFERSTAFFRECORD', + }, + SameLocalIdExistInNewWorkplace: { + errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 2, + errType: 'TRANSFERSTAFFRECORD_ERROR', + column: 'UNIQUEWORKERID', + _sourceFieldName: 'uniqueWorkerId', + error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD', + }, + SameRefsMovingToWorkplace: { + errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 3, + errType: 'TRANSFERSTAFFRECORD_ERROR', + column: 'UNIQUEWORKERID', + _sourceFieldName: 'uniqueWorkerId', + error: + 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', + }, +}); + +const addCrossValidateError = (errorsArray, errorType, JSONWorker) => { + const newErrorObject = { + ...errorType, + worker: JSONWorker.uniqueWorkerId, + name: JSONWorker.localId, + lineNumber: JSONWorker.lineNumber, + source: JSONWorker[errorType._sourceFieldName], + }; + delete newErrorObject._sourceFieldName; + + errorsArray.unshift(newErrorObject); +}; + +module.exports = { + addCrossValidateError, + MAIN_JOB_ERRORS, + TRANSFER_STAFF_RECORD_ERRORS, +}; From 18244be2f86081470e01c9df8052cf1e4e2403e9 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Mon, 11 Nov 2024 12:21:03 +0000 Subject: [PATCH 18/33] remove unused code, minor fix --- .../models/BulkImport/csv/crossValidate.js | 4 ---- .../BulkImport/csv/crossValidateErrors.js | 24 +++++++++---------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 94e05399a9..8da6dc14c7 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -171,10 +171,6 @@ const _buildWorkplaceDictWithNewWorkers = (allNewWorkers) => { .value(); }; -const _addErrorForWorkersWithSameRefsMovingToSameWorkplace = (csvWorkerSchemaErrors, JSONWorker) => { - addCrossValidateError(csvWorkerSchemaErrors, TRANSFER_STAFF_RECORD_ERRORS.SameRefsMovingToWorkplace, JSONWorker); -}; - module.exports = { crossValidate, _crossValidateMainJobRole, diff --git a/backend/server/models/BulkImport/csv/crossValidateErrors.js b/backend/server/models/BulkImport/csv/crossValidateErrors.js index aa04e2414a..560254e91b 100644 --- a/backend/server/models/BulkImport/csv/crossValidateErrors.js +++ b/backend/server/models/BulkImport/csv/crossValidateErrors.js @@ -1,41 +1,41 @@ const MAIN_JOB_ROLE_ERROR_CODE = 1280; const TRANSFER_STAFF_RECORD_BASE_ERROR_CODE = 1400; -const MAIN_JOB_ERRORS = Object.freeze({ - RegisteredManagerWithoutCqcRegulatedService: { +const MAIN_JOB_ERRORS = { + RegisteredManagerWithoutCqcRegulatedService: Object.freeze({ errCode: MAIN_JOB_ROLE_ERROR_CODE, errType: 'MAIN_JOB_ROLE_ERROR', column: 'MAINJOBROLE', _sourceFieldName: 'mainJobRoleId', error: 'Workers MAINJOBROLE is Registered Manager but you are not providing a CQC regulated service. Please change to another Job Role', - }, -}); + }), +}; -const TRANSFER_STAFF_RECORD_ERRORS = Object.freeze({ - NewWorkplaceNotFound: { +const TRANSFER_STAFF_RECORD_ERRORS = { + NewWorkplaceNotFound: Object.freeze({ errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 1, errType: 'TRANSFERSTAFFRECORD_ERROR', column: 'TRANSFERSTAFFRECORD', _sourceFieldName: 'transferStaffRecord', error: 'Cannot find an existing workplace with the reference provided in TRANSFERSTAFFRECORD', - }, - SameLocalIdExistInNewWorkplace: { + }), + SameLocalIdExistInNewWorkplace: Object.freeze({ errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 2, errType: 'TRANSFERSTAFFRECORD_ERROR', column: 'UNIQUEWORKERID', _sourceFieldName: 'uniqueWorkerId', error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD', - }, - SameRefsMovingToWorkplace: { + }), + SameRefsMovingToWorkplace: Object.freeze({ errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 3, errType: 'TRANSFERSTAFFRECORD_ERROR', column: 'UNIQUEWORKERID', _sourceFieldName: 'uniqueWorkerId', error: 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', - }, -}); + }), +}; const addCrossValidateError = (errorsArray, errorType, JSONWorker) => { const newErrorObject = { From 56ee36cd6dbe5019f4b8dfe41b256041594cbd09 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Tue, 12 Nov 2024 12:33:30 +0000 Subject: [PATCH 19/33] disallow CHGSUB as a status, handle transfer staff record for _crossValidateMainJobRole --- .../models/BulkImport/csv/crossValidate.js | 11 +- .../BulkImport/csv/workplaceCSVValidator.js | 2 +- .../validate/validateDuplicateLocations.js | 2 +- .../Bulkimport/csv/crossValidate.spec.js | 223 +++++++++--------- .../csv/workplaceCSVValidator.spec.js | 33 --- .../bulkUpload/classes/workerCSVValidator.js | 17 +- .../unit/classes/workerCSVValidator.spec.js | 28 +++ 7 files changed, 154 insertions(+), 162 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 8da6dc14c7..a5e5c26c89 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -23,9 +23,14 @@ const _crossValidateMainJobRole = (csvWorkerSchemaErrors, isCqcRegulated, JSONWo }; const _isCQCRegulated = async (myEstablishments, JSONWorker) => { - const workerEstablishment = myEstablishments.find( - (establishment) => JSONWorker.establishmentKey === establishment.key, - ); + let workerEstablishmentKey; + if (isMovingToNewWorkplace(JSONWorker)) { + workerEstablishmentKey = JSONWorker.transferStaffRecord.replace(/\s/g, ''); + } else { + workerEstablishmentKey = JSONWorker.establishmentKey; + } + + const workerEstablishment = myEstablishments.find((establishment) => workerEstablishmentKey === establishment.key); if (workerEstablishment) { switch (workerEstablishment.status) { diff --git a/backend/server/models/BulkImport/csv/workplaceCSVValidator.js b/backend/server/models/BulkImport/csv/workplaceCSVValidator.js index 39549a9ae9..ad7244bbe7 100644 --- a/backend/server/models/BulkImport/csv/workplaceCSVValidator.js +++ b/backend/server/models/BulkImport/csv/workplaceCSVValidator.js @@ -2780,7 +2780,7 @@ class WorkplaceCSVValidator { let registeredManagers = 0; - const dataInCSV = ['NEW', 'UPDATE', 'CHGSUB']; //For theses statuses trust the data in the CSV + const dataInCSV = ['NEW', 'UPDATE']; //For theses statuses trust the data in the CSV myJSONWorkers.forEach((worker) => { if (this.key === worker.establishmentKey && dataInCSV.includes(worker.status)) { diff --git a/backend/server/routes/establishments/bulkUpload/validate/validateDuplicateLocations.js b/backend/server/routes/establishments/bulkUpload/validate/validateDuplicateLocations.js index 00d563b8e4..89dae8b227 100644 --- a/backend/server/routes/establishments/bulkUpload/validate/validateDuplicateLocations.js +++ b/backend/server/routes/establishments/bulkUpload/validate/validateDuplicateLocations.js @@ -31,7 +31,7 @@ const validateDuplicateLocations = async (myEstablishments, csvEstablishmentSche myEstablishments .filter((thisEstablishment) => thisEstablishment._currentLine.LOCATIONID) - .filter((thisEstablishment) => filterByStatus(thisEstablishment, ['NEW', 'UPDATE', 'CHGSUB'])) + .filter((thisEstablishment) => filterByStatus(thisEstablishment, ['NEW', 'UPDATE'])) .forEach((thisEstablishment) => { return checkDuplicate(thisEstablishment, thisEstablishment._currentLine.LOCATIONID); }); diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index 306a973789..c11eb2594a 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -84,122 +84,129 @@ describe('crossValidate', () => { sinon.restore(); }); - it('should return true when unchecked establishment with matching key is regulated (according to database)', async () => { - const myEstablishments = [ - { - id: 1, - status: 'UNCHECKED', - key: 'HELLO', - }, - ]; + const newWorker = (establishmentKey = 'HELLO') => { const worker = new WorkerCsvValidator(null, null, null, mappings); worker._status = 'NEW'; const JSONWorker = worker.toJSON(); - JSONWorker.establishmentKey = 'HELLO'; - - sinon.stub(models.establishment, 'findbyId').returns({ isRegulated: true }); - - const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); - - expect(isCQCRegulated).to.deep.equal(true); - }); - - it('should return false when unchecked establishment with matching key is not regulated (according to database)', async () => { - const myEstablishments = [ - { - id: 1, - status: 'UNCHECKED', - key: 'HELLO', - }, - ]; - const worker = new WorkerCsvValidator(null, null, null, mappings); - worker._status = 'NEW'; - const JSONWorker = worker.toJSON(); - JSONWorker.establishmentKey = 'HELLO'; - - sinon.stub(models.establishment, 'findbyId').returns({ isRegulated: false }); - - const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); - - expect(isCQCRegulated).to.deep.equal(false); - }); - - it('should return true when updated establishment with matching key is regulated (according to file)', async () => { - const myEstablishments = [ - { - id: 1, - status: 'UPDATE', - key: 'HELLO', - regType: 1, - }, - ]; - const worker = new WorkerCsvValidator(null, null, null, mappings); - worker._status = 'NEW'; - const JSONWorker = worker.toJSON(); - JSONWorker.establishmentKey = 'HELLO'; - - const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); - - expect(isCQCRegulated).to.deep.equal(false); - }); - - it('should return false when updated establishment with matching key is not regulated (according to file)', async () => { - const myEstablishments = [ - { - id: 1, - status: 'UPDATE', - key: 'HELLO', - regType: 2, - }, - ]; - const worker = new WorkerCsvValidator(null, null, null, mappings); - worker._status = 'NEW'; - const JSONWorker = worker.toJSON(); - JSONWorker.establishmentKey = 'HELLO'; - - const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); - - expect(isCQCRegulated).to.deep.equal(true); - }); - - it('should not return anything if the establishment is set to DELETE', async () => { - const myEstablishments = [ - { - id: 1, - status: 'DELETE', - key: 'HELLO', - regType: 2, - }, - ]; - const worker = new WorkerCsvValidator(null, null, null, mappings); - worker._status = 'NEW'; - const JSONWorker = worker.toJSON(); - JSONWorker.establishmentKey = 'HELLO'; - - const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); - - expect(isCQCRegulated).to.not.deep.equal(true); - expect(isCQCRegulated).to.not.deep.equal(false); - }); + JSONWorker.establishmentKey = establishmentKey; + return JSONWorker; + }; - it('should not return anything if the establishment is not found', async () => { - const myEstablishments = [ - { - id: 1, - status: 'UPDATE', - key: 'HELLO', - regType: 2, - }, - ]; + const transferringWorker = (establishmentKey = 'HELLO') => { const worker = new WorkerCsvValidator(null, null, null, mappings); - worker._status = 'NEW'; + worker._status = 'UPDATE'; + worker._transferStaffRecord = establishmentKey; const JSONWorker = worker.toJSON(); - JSONWorker.establishmentKey = 'HELLO1'; + return JSONWorker; + }; - const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); + const testCases = [ + { workertype: 'New worker', workerBuilder: newWorker }, + { workertype: 'Transferring worker', workerBuilder: transferringWorker }, + ]; - expect(isCQCRegulated).to.not.deep.equal(true); - expect(isCQCRegulated).to.not.deep.equal(false); + testCases.forEach(({ workertype, workerBuilder }) => { + describe(`Case of ${workertype}`, () => { + it('should return true when unchecked establishment with matching key is regulated (according to database)', async () => { + const myEstablishments = [ + { + id: 1, + status: 'UNCHECKED', + key: 'HELLO', + }, + ]; + const JSONWorker = workerBuilder(); + + sinon.stub(models.establishment, 'findbyId').returns({ isRegulated: true }); + + const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); + + expect(isCQCRegulated).to.deep.equal(true); + }); + + it('should return false when unchecked establishment with matching key is not regulated (according to database)', async () => { + const myEstablishments = [ + { + id: 1, + status: 'UNCHECKED', + key: 'HELLO', + }, + ]; + const JSONWorker = workerBuilder(); + + sinon.stub(models.establishment, 'findbyId').returns({ isRegulated: false }); + + const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); + + expect(isCQCRegulated).to.deep.equal(false); + }); + + it('should return true when updated establishment with matching key is regulated (according to file)', async () => { + const myEstablishments = [ + { + id: 1, + status: 'UPDATE', + key: 'HELLO', + regType: 1, + }, + ]; + const JSONWorker = workerBuilder(); + + const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); + + expect(isCQCRegulated).to.deep.equal(false); + }); + + it('should return false when updated establishment with matching key is not regulated (according to file)', async () => { + const myEstablishments = [ + { + id: 1, + status: 'UPDATE', + key: 'HELLO', + regType: 2, + }, + ]; + const JSONWorker = workerBuilder(); + + const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); + + expect(isCQCRegulated).to.deep.equal(true); + }); + + it('should not return anything if the establishment is set to DELETE', async () => { + const myEstablishments = [ + { + id: 1, + status: 'DELETE', + key: 'HELLO', + regType: 2, + }, + ]; + const JSONWorker = workerBuilder(); + + const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); + + expect(isCQCRegulated).to.not.deep.equal(true); + expect(isCQCRegulated).to.not.deep.equal(false); + }); + + it('should not return anything if the establishment is not found', async () => { + const myEstablishments = [ + { + id: 1, + status: 'UPDATE', + key: 'HELLO', + regType: 2, + }, + ]; + const JSONWorker = workerBuilder('HELLO1'); + + const isCQCRegulated = await _isCQCRegulated(myEstablishments, JSONWorker); + + expect(isCQCRegulated).to.not.deep.equal(true); + expect(isCQCRegulated).to.not.deep.equal(false); + }); + }); }); }); diff --git a/backend/server/test/unit/models/Bulkimport/csv/workplaceCSVValidator.spec.js b/backend/server/test/unit/models/Bulkimport/csv/workplaceCSVValidator.spec.js index cec00d5a46..f76fbe3624 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/workplaceCSVValidator.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/workplaceCSVValidator.spec.js @@ -1764,40 +1764,7 @@ describe('Bulk Upload - Establishment CSV', () => { databaseWorkers, ); }); - it('should NOT show error if not Head Office and registered manager is moving into workplace ', async () => { - const establishmentRow = buildEstablishmentCSV({ - overrides: { - STATUS: 'NEW', - TOTALPERMTEMP: 1, - VACANCIES: '0;0;0', - }, - }); - const workerRow = buildWorkerCSV({ - overrides: { - LOCALESTID: establishmentRow.LOCALESTID, - MAINJOBROLE: 4, - STATUS: 'CHGSUB', - UNIQUEWORKERID: 'bob', - }, - }); - const databaseWorkers = [ - { - uniqueWorker: 'bob', - contractTypeId: 'Permanent', - mainJobRoleId: 22, - otherJobIds: '', - }, - ]; - crossValidate( - establishmentRow, - workerRow, - (csvEstablishmentSchemaErrors) => { - expect(csvEstablishmentSchemaErrors.length).to.equal(0); - }, - databaseWorkers, - ); - }); it('should show error if not Head Office and registered manager is DELETE ', async () => { const establishmentRow = buildEstablishmentCSV({ overrides: { diff --git a/lambdas/bulkUpload/classes/workerCSVValidator.js b/lambdas/bulkUpload/classes/workerCSVValidator.js index 55f3f4c534..f925f02f32 100644 --- a/lambdas/bulkUpload/classes/workerCSVValidator.js +++ b/lambdas/bulkUpload/classes/workerCSVValidator.js @@ -691,7 +691,7 @@ class WorkerCsvValidator { } _validateStatus() { - const statusValues = ['DELETE', 'UPDATE', 'UNCHECKED', 'NOCHANGE', 'NEW', 'CHGSUB']; + const statusValues = ['DELETE', 'UPDATE', 'UNCHECKED', 'NOCHANGE', 'NEW']; const myStatus = this._currentLine.STATUS ? this._currentLine.STATUS.toUpperCase() : this._currentLine.STATUS; if (!statusValues.includes(myStatus)) { @@ -759,21 +759,6 @@ class WorkerCsvValidator { }); } break; - case 'CHGSUB': - // note - the LOCALESTID here is that of the target sub - not the current sub - if (thisWorkerExists()) { - this._validationErrors.push({ - name: this._currentLine.LOCALESTID, - worker: this._currentLine.UNIQUEWORKERID, - lineNumber: this._lineNumber, - errCode: WorkerCsvValidator.STATUS_ERROR, - errType: 'STATUS_ERROR', - error: 'STATUS is CHGSUB but staff already exists in the new workplace', - source: myStatus, - column: 'STATUS', - }); - } - break; } this._status = myStatus; diff --git a/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js b/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js index ddac7f61bb..5df9127687 100644 --- a/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js +++ b/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js @@ -1470,5 +1470,33 @@ describe('/lambdas/bulkUpload/classes/workerCSVValidator', async () => { expect(validator._transferStaffRecord).to.equal('workplace B'); }); }); + + describe('_validateStatus', () => { + it('should give an error if CHGSUB (a deprecated option) is supplied in STATUS column', async () => { + const worker = buildWorkerCsv({ + overrides: { + STATUS: 'CHGSUB', + }, + }); + const expectedWarning = { + column: 'STATUS', + lineNumber: 2, + name: 'MARMA', + source: 'CHGSUB', + errCode: WorkerCsvValidator.STATUS_ERROR, + errType: 'STATUS_ERROR', + error: 'The STATUS you have supplied is incorrect', + worker: '3', + }; + + const validator = new WorkerCsvValidator(worker, 2, null, mappings); + + validator.validate(); + validator.transform(); + + expect(validator._validationErrors).to.deep.equal([expectedWarning]); + expect(validator._validationErrors.length).to.equal(1); + }); + }); }); }); From a43e4e6db0f83345efdc7a210dfc85cc669414a2 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Thu, 14 Nov 2024 12:50:10 +0000 Subject: [PATCH 20/33] amend error messages related to TRANSFERSTAFFRECORD --- .../models/BulkImport/csv/crossValidateErrors.js | 7 +++---- .../unit/models/Bulkimport/csv/crossValidate.spec.js | 10 ++++------ lambdas/bulkUpload/classes/workerCSVValidator.js | 2 +- .../test/unit/classes/workerCSVValidator.spec.js | 2 +- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidateErrors.js b/backend/server/models/BulkImport/csv/crossValidateErrors.js index 560254e91b..c5a8ae864b 100644 --- a/backend/server/models/BulkImport/csv/crossValidateErrors.js +++ b/backend/server/models/BulkImport/csv/crossValidateErrors.js @@ -18,22 +18,21 @@ const TRANSFER_STAFF_RECORD_ERRORS = { errType: 'TRANSFERSTAFFRECORD_ERROR', column: 'TRANSFERSTAFFRECORD', _sourceFieldName: 'transferStaffRecord', - error: 'Cannot find an existing workplace with the reference provided in TRANSFERSTAFFRECORD', + error: 'The LOCALESTID in TRANSFERSTAFFRECORD does not exist', }), SameLocalIdExistInNewWorkplace: Object.freeze({ errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 2, errType: 'TRANSFERSTAFFRECORD_ERROR', column: 'UNIQUEWORKERID', _sourceFieldName: 'uniqueWorkerId', - error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD', + error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', }), SameRefsMovingToWorkplace: Object.freeze({ errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 3, errType: 'TRANSFERSTAFFRECORD_ERROR', column: 'UNIQUEWORKERID', _sourceFieldName: 'uniqueWorkerId', - error: - 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', + error: 'Duplicate UNIQUEWORKERID’s are being moved to the same LOCALESTID in TRANSFERSTAFFRECORD', }), }; diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index c11eb2594a..1b43ef1ae7 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -266,7 +266,7 @@ describe('crossValidate', () => { column: 'TRANSFERSTAFFRECORD', errCode: 1401, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'Cannot find an existing workplace with the reference provided in TRANSFERSTAFFRECORD', + error: 'The LOCALESTID in TRANSFERSTAFFRECORD does not exist', worker: JSONWorker.uniqueWorkerId, name: JSONWorker.localId, lineNumber: JSONWorker.lineNumber, @@ -297,7 +297,7 @@ describe('crossValidate', () => { column: 'UNIQUEWORKERID', errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'The UNIQUEWORKERID for this worker is already used in the new workplace given in TRANSFERSTAFFRECORD', + error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', worker: JSONWorker.uniqueWorkerId, name: JSONWorker.localId, lineNumber: JSONWorker.lineNumber, @@ -323,8 +323,7 @@ describe('crossValidate', () => { column: 'UNIQUEWORKERID', errCode: 1403, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: - 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', + error: 'Duplicate UNIQUEWORKERID’s are being moved to the same LOCALESTID in TRANSFERSTAFFRECORD', worker: JSONWorkerB.uniqueWorkerId, name: JSONWorkerB.localId, lineNumber: JSONWorkerB.lineNumber, @@ -353,8 +352,7 @@ describe('crossValidate', () => { column: 'UNIQUEWORKERID', errCode: 1403, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: - 'There are more than one worker with this UNIQUEWORKERID moving into the new workplace given in TRANSFERSTAFFRECORD.', + error: 'Duplicate UNIQUEWORKERID’s are being moved to the same LOCALESTID in TRANSFERSTAFFRECORD', worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, diff --git a/lambdas/bulkUpload/classes/workerCSVValidator.js b/lambdas/bulkUpload/classes/workerCSVValidator.js index f925f02f32..1fa6292470 100644 --- a/lambdas/bulkUpload/classes/workerCSVValidator.js +++ b/lambdas/bulkUpload/classes/workerCSVValidator.js @@ -777,7 +777,7 @@ class WorkerCsvValidator { lineNumber: this._lineNumber, errCode: WorkerCsvValidator.TRANSFERSTAFFRECORD_ERROR, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: `TRANSFERSTAFFRECORD is provided but cannot find the worker in the given workplace`, + error: 'Staff record has TRANSFERSTAFFRECORD given but does not exist', source: this._currentLine.LOCALESTID, column: 'LOCALESTID', }); diff --git a/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js b/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js index 5df9127687..a15f178c1d 100644 --- a/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js +++ b/lambdas/bulkUpload/test/unit/classes/workerCSVValidator.spec.js @@ -1448,7 +1448,7 @@ describe('/lambdas/bulkUpload/classes/workerCSVValidator', async () => { lineNumber: 2, errCode: WorkerCsvValidator.TRANSFERSTAFFRECORD_ERROR, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'TRANSFERSTAFFRECORD is provided but cannot find the worker in the given workplace', + error: 'Staff record has TRANSFERSTAFFRECORD given but does not exist', source: worker.LOCALESTID, column: 'LOCALESTID', name: 'MARMA', From e700124d392cab257b508ef9ee101b507c0ded2d Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Fri, 22 Nov 2024 13:59:03 +0000 Subject: [PATCH 21/33] remove "CHGSUB" status --- backend/server/models/classes/worker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/server/models/classes/worker.js b/backend/server/models/classes/worker.js index e6fd09dde4..52fbeb0ef7 100644 --- a/backend/server/models/classes/worker.js +++ b/backend/server/models/classes/worker.js @@ -572,7 +572,7 @@ class Worker extends EntityValidator { }); } - if (bulkUploaded && ['NEW', 'UPDATE', 'CHGSUB'].includes(this.status)) { + if (bulkUploaded && ['NEW', 'UPDATE'].includes(this.status)) { const qualificationHelper = new BulkUploadQualificationHelper({ workerId: this._id, workerUid: this._uid, From 85afbf24352c2981b34ad8defc4669cd5cf7a092 Mon Sep 17 00:00:00 2001 From: Duncan Carter Date: Mon, 25 Nov 2024 11:25:52 +0000 Subject: [PATCH 22/33] Accept both CHGUNIQUEWRKID and TRANSFERSTAFFRECORD columns in header validation --- .../establishments/bulkUpload/data/workerHeaders.js | 9 +++++++++ .../bulkUpload/validate/headers/worker.js | 2 ++ .../bulkUpload/validate/headers/worker.spec.js | 11 +++++++++++ 3 files changed, 22 insertions(+) diff --git a/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js b/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js index 4971d87f34..4f4c601bb3 100644 --- a/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js +++ b/backend/server/routes/establishments/bulkUpload/data/workerHeaders.js @@ -56,7 +56,16 @@ const workerHeadersWithTransferStaffRecordAsArray = [ ...workerHeaders.slice(2), ]; +const workerHeadersWithChangeUniqueWorkerIdAndTransferStaffRecordAsArray = [ + ...workerHeaders.slice(0, 2), + 'CHGUNIQUEWRKID', + 'TRANSFERSTAFFRECORD', + ...workerHeaders.slice(2), +]; + exports.workerHeadersWithCHGUNIQUEWRKID = workerHeadersWithChangeUniqueWorkerIdAsArray.join(','); exports.workerHeadersWithTRANSFERSTAFFRECORD = workerHeadersWithTransferStaffRecordAsArray.join(','); exports.workerHeadersWithoutCHGUNIQUEWRKID = workerHeaders.join(','); +exports.workerHeadersWithCHGUNIQUEWRKIDAndTRANSFERSTAFFRECORD = + workerHeadersWithChangeUniqueWorkerIdAndTransferStaffRecordAsArray.join(','); exports.getWorkerColumnIndex = (columnName) => workerHeaders.findIndex((header) => header === columnName); diff --git a/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js b/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js index f4717ff54b..9eb6545404 100644 --- a/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js +++ b/backend/server/routes/establishments/bulkUpload/validate/headers/worker.js @@ -2,6 +2,7 @@ const { workerHeadersWithCHGUNIQUEWRKID, workerHeadersWithoutCHGUNIQUEWRKID, workerHeadersWithTRANSFERSTAFFRECORD, + workerHeadersWithCHGUNIQUEWRKIDAndTRANSFERSTAFFRECORD, } = require('../../data/workerHeaders'); const validateWorkerHeaders = (headers) => { @@ -9,6 +10,7 @@ const validateWorkerHeaders = (headers) => { workerHeadersWithoutCHGUNIQUEWRKID, workerHeadersWithCHGUNIQUEWRKID, workerHeadersWithTRANSFERSTAFFRECORD, + workerHeadersWithCHGUNIQUEWRKIDAndTRANSFERSTAFFRECORD, ]; for (const workerHeadersBeforeExtraQuals of allowedWorkerHeaders) { diff --git a/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js b/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js index 3a9c90af5f..ecf510b22e 100644 --- a/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js +++ b/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js @@ -17,6 +17,11 @@ const workerHeadersWithTRANSFERSTAFFRECORD = workerHeadersWithCHGUNIQUEWRKID.rep 'TRANSFERSTAFFRECORD', ); +const workerHeadersWithCHGUNIQUEWRKIDAndTRANSFERSTAFFRECORD = workerHeadersWithCHGUNIQUEWRKID.replace( + 'CHGUNIQUEWRKID', + 'CHGUNIQUEWRKID,TRANSFERSTAFFRECORD', +); + const workerHeadersWithoutCHGUNIQUEWRKID = 'LOCALESTID,UNIQUEWORKERID,STATUS,DISPLAYID,NINUMBER,' + 'POSTCODE,DOB,GENDER,ETHNICITY,NATIONALITY,BRITISHCITIZENSHIP,COUNTRYOFBIRTH,YEAROFENTRY,' + @@ -39,6 +44,11 @@ describe('server/routes/establishments/bulkUpload/validate/headers/worker', () = expect(validateWorkerHeaders(workerHeadersWithoutCHGUNIQUEWRKID)).to.deep.equal(true); }); + it('should return true when headings match with CHGUNIQUEWRKID and TRANSFERSTAFFRECORD', async () => { + console.log(workerHeadersWithCHGUNIQUEWRKIDAndTRANSFERSTAFFRECORD); + expect(validateWorkerHeaders(workerHeadersWithCHGUNIQUEWRKIDAndTRANSFERSTAFFRECORD)).to.equal(true); + }); + it('should return false when header (NATIONALITY) missing', async () => { const invalidHeaders = workerHeadersWithoutCHGUNIQUEWRKID.replace('NATIONALITY,', ''); @@ -50,6 +60,7 @@ describe('server/routes/establishments/bulkUpload/validate/headers/worker', () = workerHeadersWithCHGUNIQUEWRKID, workerHeadersWithoutCHGUNIQUEWRKID, workerHeadersWithTRANSFERSTAFFRECORD, + workerHeadersWithCHGUNIQUEWRKIDAndTRANSFERSTAFFRECORD, ]; testCases.forEach((workerHeaders) => { From 108f0b166b19b286a3702d7a796fb925ac624a55 Mon Sep 17 00:00:00 2001 From: Duncan Carter Date: Mon, 25 Nov 2024 13:01:29 +0000 Subject: [PATCH 23/33] Update whichFile to recognise worker file typeon upload when it has both CHGUNIQUEWRKID and TRANSFERSTAFFRECORD --- .../routes/establishments/bulkUpload/whichFile.js | 10 +++++++++- .../routes/establishments/bulkUpload/whichFile.spec.js | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/backend/server/routes/establishments/bulkUpload/whichFile.js b/backend/server/routes/establishments/bulkUpload/whichFile.js index aac8f697a2..03b796a8ef 100644 --- a/backend/server/routes/establishments/bulkUpload/whichFile.js +++ b/backend/server/routes/establishments/bulkUpload/whichFile.js @@ -4,7 +4,15 @@ const isWorkerFile = (fileAsString) => { const headersRegexBaseCase = /LOCALESTID,UNIQUEWORKERID,STATUS,DISPLAYID,/; const headersRegexChangeUniqueWorkerId = /LOCALESTID,UNIQUEWORKERID,CHGUNIQUEWRKID,STATUS,DISPLAYID,/; const headersRegexTransferStaffRecord = /LOCALESTID,UNIQUEWORKERID,TRANSFERSTAFFRECORD,STATUS,DISPLAYID,/; - const regexToCheckHeaders = [headersRegexBaseCase, headersRegexChangeUniqueWorkerId, headersRegexTransferStaffRecord]; + const headersRegexChangeUniqueWorkerIdTransferStaffRecord = + /LOCALESTID,UNIQUEWORKERID,CHGUNIQUEWRKID,TRANSFERSTAFFRECORD,STATUS,DISPLAYID,/; + + const regexToCheckHeaders = [ + headersRegexBaseCase, + headersRegexChangeUniqueWorkerId, + headersRegexTransferStaffRecord, + headersRegexChangeUniqueWorkerIdTransferStaffRecord, + ]; const headerRow = fileAsString.split('\n')[0]; diff --git a/backend/server/test/unit/routes/establishments/bulkUpload/whichFile.spec.js b/backend/server/test/unit/routes/establishments/bulkUpload/whichFile.spec.js index 8e7d3e13d1..386e69430a 100644 --- a/backend/server/test/unit/routes/establishments/bulkUpload/whichFile.spec.js +++ b/backend/server/test/unit/routes/establishments/bulkUpload/whichFile.spec.js @@ -22,6 +22,11 @@ describe('whichFile', () => { expect(isWorkerFile(header)).to.deep.equal(true); }); + it('return true when headings match with CHGUNIQUEWRKID and TRANSFERSTAFFRECORD', async () => { + const header = 'LOCALESTID,UNIQUEWORKERID,CHGUNIQUEWRKID,TRANSFERSTAFFRECORD,STATUS,DISPLAYID,NINUMBER'; + expect(isWorkerFile(header)).to.deep.equal(true); + }); + it("return false when headings don't match", async () => { const header = 'NOTATALLWHATWEEXPECT,HOWCOULDYOUUPLOADTHISFILE,'; expect(isWorkerFile(header)).to.deep.equal(false); From 1f0a9e36d6842987587d6943cbd1db166015a417 Mon Sep 17 00:00:00 2001 From: Duncan Carter Date: Tue, 26 Nov 2024 09:05:05 +0000 Subject: [PATCH 24/33] Add checks for changeUniqueWorker to allow users to update unique IDs and move staff at same time --- .../models/BulkImport/csv/crossValidate.js | 73 +++++++-- .../server/models/classes/establishment.js | 4 +- .../Bulkimport/csv/crossValidate.spec.js | 147 ++++++++++++++++-- 3 files changed, 197 insertions(+), 27 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index a5e5c26c89..635ed30065 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -62,15 +62,33 @@ const crossValidateTransferStaffRecord = async ( const relatedEstablishmentIds = myEstablishments.map((establishment) => establishment.id); const allMovingWorkers = myJSONWorkers.filter(isMovingToNewWorkplace); - const allNewWorkers = myJSONWorkers.filter((JSONWorker) => JSONWorker.status === 'NEW'); + const allNewWorkers = myJSONWorkers.filter((worker) => worker.status === 'NEW'); + const allOtherWorkers = myJSONWorkers.filter((worker) => !isMovingToNewWorkplace(worker) && worker.status !== 'NEW'); + + const newWorkerWithDuplicateIdErrorAdded = _crossValidateWorkersWithSameRefMovingToSameWorkplace( + csvWorkerSchemaErrors, + allMovingWorkers, + allNewWorkers, + TRANSFER_STAFF_RECORD_ERRORS.SameRefsMovingToWorkplace, + ); + + if (newWorkerWithDuplicateIdErrorAdded) return; + + const existingWorkerWithDuplicateIdErrorAdded = _crossValidateWorkersWithSameRefMovingToSameWorkplace( + csvWorkerSchemaErrors, + allMovingWorkers, + allOtherWorkers, + TRANSFER_STAFF_RECORD_ERRORS.SameLocalIdExistInNewWorkplace, + ); - _crossValidateWorkersWithSameRefMovingToSameWorkplace(csvWorkerSchemaErrors, allMovingWorkers, allNewWorkers); + if (existingWorkerWithDuplicateIdErrorAdded) return; for (const JSONWorker of allMovingWorkers) { const newWorkplaceId = await _validateTransferIsPossible( csvWorkerSchemaErrors, relatedEstablishmentIds, JSONWorker, + allOtherWorkers, ); if (newWorkplaceId) { @@ -83,7 +101,12 @@ const isMovingToNewWorkplace = (JSONWorker) => { return JSONWorker.status === 'UPDATE' && JSONWorker.transferStaffRecord; }; -const _validateTransferIsPossible = async (csvWorkerSchemaErrors, relatedEstablishmentIds, JSONWorker) => { +const _validateTransferIsPossible = async ( + csvWorkerSchemaErrors, + relatedEstablishmentIds, + JSONWorker, + allOtherWorkers, +) => { const newWorkplaceLocalRef = JSONWorker.transferStaffRecord; const newWorkplaceId = await _getNewWorkplaceId(newWorkplaceLocalRef, relatedEstablishmentIds); @@ -92,17 +115,23 @@ const _validateTransferIsPossible = async (csvWorkerSchemaErrors, relatedEstabli return; } - const sameLocalIdExistInNewWorkplace = await models.worker.findOneWithConflictingLocalRef( + const sameLocalIdExistsInNewWorkplace = await models.worker.findOneWithConflictingLocalRef( newWorkplaceId, JSONWorker.uniqueWorkerId, ); - if (sameLocalIdExistInNewWorkplace) { - addCrossValidateError( - csvWorkerSchemaErrors, - TRANSFER_STAFF_RECORD_ERRORS.SameLocalIdExistInNewWorkplace, - JSONWorker, + if (sameLocalIdExistsInNewWorkplace) { + const workerWithSameLocalIdInFile = allOtherWorkers.find( + (worker) => worker.uniqueWorkerId == JSONWorker.uniqueWorkerId, ); + + if (!workerWithSameLocalIdInFile?.changeUniqueWorker) { + addCrossValidateError( + csvWorkerSchemaErrors, + TRANSFER_STAFF_RECORD_ERRORS.SameLocalIdExistInNewWorkplace, + JSONWorker, + ); + } return; } @@ -145,13 +174,19 @@ const _addNewWorkplaceIdToWorkerEntity = (myAPIEstablishments, JSONWorker, newWo const _crossValidateWorkersWithSameRefMovingToSameWorkplace = ( csvWorkerSchemaErrors, allMovingWorkers, - allNewWorkers, + otherWorkers, + errorType, ) => { - const workplacesDict = _buildWorkplaceDictWithNewWorkers(allNewWorkers); + const workplacesDict = _buildWorkplaceDictWithNewWorkers(otherWorkers); + + let errorAdded = false; for (const JSONWorker of allMovingWorkers) { const newWorkplaceRef = JSONWorker.transferStaffRecord; - const workerRef = JSONWorker.uniqueWorkerId.replace(/\s/g, ''); + + const workerRef = JSONWorker.changeUniqueWorker + ? JSONWorker.changeUniqueWorker.replace(/\s/g, '') + : JSONWorker.uniqueWorkerId.replace(/\s/g, ''); if (!workplacesDict[newWorkplaceRef]) { workplacesDict[newWorkplaceRef] = new Set([workerRef]); @@ -162,16 +197,24 @@ const _crossValidateWorkersWithSameRefMovingToSameWorkplace = ( workplacesDict[newWorkplaceRef].add(workerRef); continue; } + // worker's ID exists in workplace in file + addCrossValidateError(csvWorkerSchemaErrors, errorType, JSONWorker); - // if arrive at here, there is already another new or moving worker with that workerRef coming to the same new workplace - addCrossValidateError(csvWorkerSchemaErrors, TRANSFER_STAFF_RECORD_ERRORS.SameRefsMovingToWorkplace, JSONWorker); + errorAdded = true; } + + return errorAdded; }; const _buildWorkplaceDictWithNewWorkers = (allNewWorkers) => { return chain(allNewWorkers) .groupBy('localId') // workplace ref - .mapValues((JSONWorkers) => JSONWorkers.map((JSONWorker) => JSONWorker.uniqueWorkerId.replace(/\s/g, ''))) + .mapValues((JSONWorkers) => + JSONWorkers.map((JSONWorker) => { + if (JSONWorker.changeUniqueWorker) return JSONWorker.changeUniqueWorker.replace(/\s/g, ''); + return JSONWorker.uniqueWorkerId.replace(/\s/g, ''); + }), + ) .mapValues((workerRefs) => new Set(workerRefs)) .value(); }; diff --git a/backend/server/models/classes/establishment.js b/backend/server/models/classes/establishment.js index a8bf7edae6..b7331945ea 100644 --- a/backend/server/models/classes/establishment.js +++ b/backend/server/models/classes/establishment.js @@ -723,7 +723,9 @@ class Establishment extends EntityValidator { // new and updated Workers const starterSavePromise = Promise.resolve(null); - await workersAsArray.reduce( + console.log('workersAsArray: ', workersAsArray); + const sortedWorkersAsArray = workersAsArray.sort((worker) => worker.changeLocalIdentifier !== null); + await sortedWorkersAsArray.reduce( (p, thisWorkerToSave) => p.then(() => thisWorkerToSave.save(savedBy, bulkUploaded, 0, externalTransaction, true)).then(log), starterSavePromise, diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index 1b43ef1ae7..b662e9a90f 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -211,8 +211,8 @@ describe('crossValidate', () => { }); describe('crossValidateTransferStaffRecord', () => { - const worker = new WorkerCsvValidator(null, null, null, mappings); const buildMockJSONWorker = (override) => { + const worker = new WorkerCsvValidator(null, null, null, mappings); return { ...worker.toJSON(), status: 'UPDATE', @@ -279,18 +279,56 @@ describe('crossValidate', () => { }); }); - it("should add an error to csvWorkerSchemaErrors if the worker's unique worker id is already used in the new workplace", async () => { + it("should add an error to csvWorkerSchemaErrors if the worker's unique worker id is already in the new workplace in file", async () => { + const movingWorker = buildMockJSONWorker({ + uniqueWorkerId: 'mock_worker_ref', + localId: 'workplace A', + }); + + const existingWorkerInWorkplace = buildMockJSONWorker({ + uniqueWorkerId: 'mock_worker_ref', + status: 'UPDATE', + transferStaffRecord: null, + localId: 'target workplace', + }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + movingWorker, + existingWorkerInWorkplace, + ]); + + const expectedError = { + column: 'UNIQUEWORKERID', + errCode: 1402, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', + worker: movingWorker.uniqueWorkerId, + name: movingWorker.localId, + lineNumber: movingWorker.lineNumber, + source: movingWorker.uniqueWorkerId, + }; + + expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); + }); + + it("should add an error to csvWorkerSchemaErrors if the worker's unique worker id is not in file but is found in database", async () => { stubWorkerFindOneWithLocalRef.returns({ - NameOrIdValue: 'another worker', + id: 123, + NameOrIdValue: 'Mock Worker', LocalIdentifierValue: 'mock_worker_ref', }); - const JSONWorker = buildMockJSONWorker({ uniqueWorkerId: 'mock_worker_ref' }); + const movingWorker = buildMockJSONWorker({ + uniqueWorkerId: 'mock_worker_ref', + localId: 'workplace A', + }); const csvWorkerSchemaErrors = []; await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ - JSONWorker, + movingWorker, ]); const expectedError = { @@ -298,14 +336,14 @@ describe('crossValidate', () => { errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', - worker: JSONWorker.uniqueWorkerId, - name: JSONWorker.localId, - lineNumber: JSONWorker.lineNumber, - source: JSONWorker.uniqueWorkerId, + worker: movingWorker.uniqueWorkerId, + name: movingWorker.localId, + lineNumber: movingWorker.lineNumber, + source: movingWorker.uniqueWorkerId, }; - expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); expect(stubWorkerFindOneWithLocalRef).to.have.been.calledWith(789, 'mock_worker_ref'); + expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); }); it('should add an error to csvWorkerSchemaErrors if two workers with the same unique worker id are transferring into the same new workplace', async () => { @@ -362,7 +400,57 @@ describe('crossValidate', () => { expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); }); - it('should add newWorkplaceId to the worker entity if all validations passed', async () => { + it("should not add an error to csvWorkerSchemaErrors and add newWorkplaceId to worker entity if transferring worker to workplace with worker with same ref but that worker's ID is being changed", async () => { + const movingWorker = buildMockJSONWorker({ uniqueWorkerId: 'mock_worker_ref', localId: 'workplace A' }); + const existingWorkerInWorkplace = buildMockJSONWorker({ + uniqueWorkerId: 'mock_worker_ref', + status: 'UPDATE', + changeUniqueWorker: 'new_unique_worker_ref', + transferStaffRecord: null, + localId: 'target workplace', + }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + movingWorker, + existingWorkerInWorkplace, + ]); + + expect(csvWorkerSchemaErrors).to.be.empty; + + const workerEntity = myAPIEstablishments['workplaceA']._workerEntities['mock_worker_ref']; + expect(workerEntity._newWorkplaceId).to.equal(789); + }); + + it("should not add an error to csvWorkerSchemaErrors and add newWorkplaceId to worker entity if transferring worker to workplace with worker with same ref but moving worker's ID is being changed", async () => { + const movingWorker = buildMockJSONWorker({ + uniqueWorkerId: 'mock_worker_ref', + localId: 'workplace A', + changeUniqueWorker: 'new_unique_worker_ref', + }); + + const existingWorkerInWorkplace = buildMockJSONWorker({ + uniqueWorkerId: 'mock_worker_ref', + status: 'UPDATE', + transferStaffRecord: null, + localId: 'target workplace', + }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + movingWorker, + existingWorkerInWorkplace, + ]); + + expect(csvWorkerSchemaErrors).to.be.empty; + + const workerEntity = myAPIEstablishments['workplaceA']._workerEntities['mock_worker_ref']; + expect(workerEntity._newWorkplaceId).to.equal(789); + }); + + it('should add newWorkplaceId to the worker entity if all validations pass for worker with transferStaffRecord', async () => { const JSONWorker = buildMockJSONWorker({ localId: 'workplace A', uniqueWorkerId: 'mock_worker_ref' }); const csvWorkerSchemaErrors = []; @@ -376,5 +464,42 @@ describe('crossValidate', () => { const workerEntity = myAPIEstablishments['workplaceA']._workerEntities['mock_worker_ref']; expect(workerEntity._newWorkplaceId).to.equal(789); }); + + it('should not add errors if no workers with transferStaffRecord', async () => { + const buildMockJSONWorkerWithoutTransferStaffRecord = (override) => { + const worker = new WorkerCsvValidator(null, null, null, mappings); + return { + ...worker.toJSON(), + status: 'UPDATE', + transferStaffRecord: null, + uniqueWorkerId: 'mock_worker_ref', + lineNumber: 3, + ...override, + }; + }; + + const worker1 = buildMockJSONWorkerWithoutTransferStaffRecord({ + localId: 'workplace A', + uniqueWorkerId: 'mock_worker_ref', + }); + const worker2 = buildMockJSONWorkerWithoutTransferStaffRecord({ + localId: 'workplace B', + uniqueWorkerId: 'mock_worker_ref', + }); + const worker3 = buildMockJSONWorkerWithoutTransferStaffRecord({ + localId: 'workplace A', + uniqueWorkerId: 'mock_worker_ref2', + }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + worker1, + worker2, + worker3, + ]); + + expect(csvWorkerSchemaErrors).to.be.empty; + }); }); }); From 8cf4484f871d218e07181415850311144e128b5e Mon Sep 17 00:00:00 2001 From: Duncan Carter Date: Tue, 26 Nov 2024 09:10:35 +0000 Subject: [PATCH 25/33] Remove console log --- backend/server/models/classes/establishment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/server/models/classes/establishment.js b/backend/server/models/classes/establishment.js index b7331945ea..63179c1c05 100644 --- a/backend/server/models/classes/establishment.js +++ b/backend/server/models/classes/establishment.js @@ -723,7 +723,7 @@ class Establishment extends EntityValidator { // new and updated Workers const starterSavePromise = Promise.resolve(null); - console.log('workersAsArray: ', workersAsArray); + const sortedWorkersAsArray = workersAsArray.sort((worker) => worker.changeLocalIdentifier !== null); await sortedWorkersAsArray.reduce( (p, thisWorkerToSave) => From b6ed1aeea768673274a2ce8d5e68469194403d86 Mon Sep 17 00:00:00 2001 From: Duncan Carter Date: Tue, 26 Nov 2024 09:20:01 +0000 Subject: [PATCH 26/33] Add test for changing ref for transferred staff record to another existing ref in workplace --- .../Bulkimport/csv/crossValidate.spec.js | 43 +++++++++++++++++++ .../validate/headers/worker.spec.js | 1 - 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index b662e9a90f..6202f10900 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -450,6 +450,49 @@ describe('crossValidate', () => { expect(workerEntity._newWorkplaceId).to.equal(789); }); + it("should add an error to csvWorkerSchemaErrors if transferring worker to workplace with worker with same ref and moving worker's ID is being changed to existing ref", async () => { + const movingWorker = buildMockJSONWorker({ + uniqueWorkerId: 'mock_worker_ref', + localId: 'workplace A', + changeUniqueWorker: 'changed_but_still_duplicate_worker_ref', + }); + + const existingWorkerInWorkplace = buildMockJSONWorker({ + uniqueWorkerId: 'mock_worker_ref', + status: 'UPDATE', + transferStaffRecord: null, + localId: 'target workplace', + }); + + const existingWorker2InWorkplace = buildMockJSONWorker({ + uniqueWorkerId: 'changed_but_still_duplicate_worker_ref', + status: 'UPDATE', + transferStaffRecord: null, + localId: 'target workplace', + }); + + const csvWorkerSchemaErrors = []; + + await crossValidateTransferStaffRecord(csvWorkerSchemaErrors, myAPIEstablishments, myEstablishments, [ + movingWorker, + existingWorkerInWorkplace, + existingWorker2InWorkplace, + ]); + + const expectedError = { + column: 'UNIQUEWORKERID', + errCode: 1402, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', + worker: movingWorker.uniqueWorkerId, + name: movingWorker.localId, + lineNumber: movingWorker.lineNumber, + source: movingWorker.uniqueWorkerId, + }; + + expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); + }); + it('should add newWorkplaceId to the worker entity if all validations pass for worker with transferStaffRecord', async () => { const JSONWorker = buildMockJSONWorker({ localId: 'workplace A', uniqueWorkerId: 'mock_worker_ref' }); diff --git a/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js b/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js index ecf510b22e..cc1007af46 100644 --- a/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js +++ b/backend/server/test/unit/routes/establishments/bulkUpload/validate/headers/worker.spec.js @@ -45,7 +45,6 @@ describe('server/routes/establishments/bulkUpload/validate/headers/worker', () = }); it('should return true when headings match with CHGUNIQUEWRKID and TRANSFERSTAFFRECORD', async () => { - console.log(workerHeadersWithCHGUNIQUEWRKIDAndTRANSFERSTAFFRECORD); expect(validateWorkerHeaders(workerHeadersWithCHGUNIQUEWRKIDAndTRANSFERSTAFFRECORD)).to.equal(true); }); From a87c1735166857d37f74c10849f36287379536c0 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Tue, 26 Nov 2024 12:58:06 +0000 Subject: [PATCH 27/33] change cross validate to give error if transferring worker to workplace with worker with same ref, even if the worker's ID is being changed --- .../models/BulkImport/csv/crossValidate.js | 8 +++++--- .../BulkImport/csv/crossValidateErrors.js | 3 ++- .../Bulkimport/csv/crossValidate.spec.js | 19 +++++++++++++++---- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 635ed30065..e0844285e8 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -211,11 +211,13 @@ const _buildWorkplaceDictWithNewWorkers = (allNewWorkers) => { .groupBy('localId') // workplace ref .mapValues((JSONWorkers) => JSONWorkers.map((JSONWorker) => { - if (JSONWorker.changeUniqueWorker) return JSONWorker.changeUniqueWorker.replace(/\s/g, ''); - return JSONWorker.uniqueWorkerId.replace(/\s/g, ''); + if (JSONWorker.changeUniqueWorker) { + return [JSONWorker.changeUniqueWorker.replace(/\s/g, ''), JSONWorker.uniqueWorkerId.replace(/\s/g, '')]; + } + return [JSONWorker.uniqueWorkerId.replace(/\s/g, '')]; }), ) - .mapValues((workerRefs) => new Set(workerRefs)) + .mapValues((workerRefs) => new Set(workerRefs.flat())) .value(); }; diff --git a/backend/server/models/BulkImport/csv/crossValidateErrors.js b/backend/server/models/BulkImport/csv/crossValidateErrors.js index c5a8ae864b..96af382827 100644 --- a/backend/server/models/BulkImport/csv/crossValidateErrors.js +++ b/backend/server/models/BulkImport/csv/crossValidateErrors.js @@ -25,7 +25,8 @@ const TRANSFER_STAFF_RECORD_ERRORS = { errType: 'TRANSFERSTAFFRECORD_ERROR', column: 'UNIQUEWORKERID', _sourceFieldName: 'uniqueWorkerId', - error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', + error: + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", }), SameRefsMovingToWorkplace: Object.freeze({ errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 3, diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index 6202f10900..048693cc2d 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -400,7 +400,7 @@ describe('crossValidate', () => { expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); }); - it("should not add an error to csvWorkerSchemaErrors and add newWorkplaceId to worker entity if transferring worker to workplace with worker with same ref but that worker's ID is being changed", async () => { + it("should add an error to csvWorkerSchemaErrors if transferring worker to workplace with worker with same ref but that worker's ID is being changed", async () => { const movingWorker = buildMockJSONWorker({ uniqueWorkerId: 'mock_worker_ref', localId: 'workplace A' }); const existingWorkerInWorkplace = buildMockJSONWorker({ uniqueWorkerId: 'mock_worker_ref', @@ -417,10 +417,21 @@ describe('crossValidate', () => { existingWorkerInWorkplace, ]); - expect(csvWorkerSchemaErrors).to.be.empty; + const expectedError = { + column: 'UNIQUEWORKERID', + errCode: 1402, + errType: 'TRANSFERSTAFFRECORD_ERROR', + error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', + worker: movingWorker.uniqueWorkerId, + name: movingWorker.localId, + lineNumber: movingWorker.lineNumber, + source: movingWorker.uniqueWorkerId, + }; - const workerEntity = myAPIEstablishments['workplaceA']._workerEntities['mock_worker_ref']; - expect(workerEntity._newWorkplaceId).to.equal(789); + expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); + + // const workerEntity = myAPIEstablishments['workplaceA']._workerEntities['mock_worker_ref']; + // expect(workerEntity._newWorkplaceId).to.equal(789); }); it("should not add an error to csvWorkerSchemaErrors and add newWorkplaceId to worker entity if transferring worker to workplace with worker with same ref but moving worker's ID is being changed", async () => { From 8b4c4dbb2f96abd2220c0d98b8b85088ff513ccd Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Tue, 26 Nov 2024 15:16:40 +0000 Subject: [PATCH 28/33] change the worker reference that we pass to findOneWithConflictingLocalRef() --- .../models/BulkImport/csv/crossValidate.js | 29 ++++++++++--------- .../Bulkimport/csv/crossValidate.spec.js | 18 +++++++----- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index e0844285e8..61058ddd59 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -115,23 +115,26 @@ const _validateTransferIsPossible = async ( return; } - const sameLocalIdExistsInNewWorkplace = await models.worker.findOneWithConflictingLocalRef( + const workerReferenceToLookup = JSONWorker.changeUniqueWorker + ? JSONWorker.changeUniqueWorker + : JSONWorker.uniqueWorkerId; + + // if worker with duplicated reference found in database but not in csv file, + // changes to unique worker ID are applied before deleting workers not in file, + // which would cause bulk upload to break + // the code below prevents this issue + + const uniqueWorkerIdFoundInWorkplaceInDatabase = await models.worker.findOneWithConflictingLocalRef( newWorkplaceId, - JSONWorker.uniqueWorkerId, + workerReferenceToLookup, ); - if (sameLocalIdExistsInNewWorkplace) { - const workerWithSameLocalIdInFile = allOtherWorkers.find( - (worker) => worker.uniqueWorkerId == JSONWorker.uniqueWorkerId, + if (uniqueWorkerIdFoundInWorkplaceInDatabase) { + addCrossValidateError( + csvWorkerSchemaErrors, + TRANSFER_STAFF_RECORD_ERRORS.SameLocalIdExistInNewWorkplace, + JSONWorker, ); - - if (!workerWithSameLocalIdInFile?.changeUniqueWorker) { - addCrossValidateError( - csvWorkerSchemaErrors, - TRANSFER_STAFF_RECORD_ERRORS.SameLocalIdExistInNewWorkplace, - JSONWorker, - ); - } return; } diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index 048693cc2d..7f33db759d 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -303,7 +303,8 @@ describe('crossValidate', () => { column: 'UNIQUEWORKERID', errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', + error: + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, @@ -335,7 +336,8 @@ describe('crossValidate', () => { column: 'UNIQUEWORKERID', errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', + error: + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, @@ -400,7 +402,7 @@ describe('crossValidate', () => { expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); }); - it("should add an error to csvWorkerSchemaErrors if transferring worker to workplace with worker with same ref but that worker's ID is being changed", async () => { + it("should add an error to csvWorkerSchemaErrors if transferring worker to workplace with worker with same ref, even if the worker's ID is being changed", async () => { const movingWorker = buildMockJSONWorker({ uniqueWorkerId: 'mock_worker_ref', localId: 'workplace A' }); const existingWorkerInWorkplace = buildMockJSONWorker({ uniqueWorkerId: 'mock_worker_ref', @@ -421,7 +423,8 @@ describe('crossValidate', () => { column: 'UNIQUEWORKERID', errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', + error: + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, @@ -429,9 +432,6 @@ describe('crossValidate', () => { }; expect(csvWorkerSchemaErrors).to.deep.equal([expectedError]); - - // const workerEntity = myAPIEstablishments['workplaceA']._workerEntities['mock_worker_ref']; - // expect(workerEntity._newWorkplaceId).to.equal(789); }); it("should not add an error to csvWorkerSchemaErrors and add newWorkplaceId to worker entity if transferring worker to workplace with worker with same ref but moving worker's ID is being changed", async () => { @@ -459,6 +459,7 @@ describe('crossValidate', () => { const workerEntity = myAPIEstablishments['workplaceA']._workerEntities['mock_worker_ref']; expect(workerEntity._newWorkplaceId).to.equal(789); + expect(stubWorkerFindOneWithLocalRef).to.have.been.calledWith(789, 'new_unique_worker_ref'); }); it("should add an error to csvWorkerSchemaErrors if transferring worker to workplace with worker with same ref and moving worker's ID is being changed to existing ref", async () => { @@ -494,7 +495,8 @@ describe('crossValidate', () => { column: 'UNIQUEWORKERID', errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', - error: 'The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD', + error: + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, From e3b9d4d228ba4baa1f0eaeba0ec24c3568cbbdf3 Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Tue, 26 Nov 2024 15:45:00 +0000 Subject: [PATCH 29/33] add a if statement to handle the case of duplicated worker ref causing workerEntity not found in establishment --- backend/server/models/BulkImport/csv/crossValidate.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 61058ddd59..992a58353a 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -171,7 +171,9 @@ const _addNewWorkplaceIdToWorkerEntity = (myAPIEstablishments, JSONWorker, newWo const workerEntity = myAPIEstablishments[oldWorkplaceKey].theWorker(workerEntityKey); - workerEntity._newWorkplaceId = newWorkplaceId; + if (workerEntity) { + workerEntity._newWorkplaceId = newWorkplaceId; + } }; const _crossValidateWorkersWithSameRefMovingToSameWorkplace = ( From d35532a0b2ed2796de7bb17e9a5372390127de7d Mon Sep 17 00:00:00 2001 From: Joe Fong Date: Tue, 26 Nov 2024 15:53:01 +0000 Subject: [PATCH 30/33] refactoring --- .../models/BulkImport/csv/crossValidate.js | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidate.js b/backend/server/models/BulkImport/csv/crossValidate.js index 992a58353a..274a2b2dfb 100644 --- a/backend/server/models/BulkImport/csv/crossValidate.js +++ b/backend/server/models/BulkImport/csv/crossValidate.js @@ -65,7 +65,7 @@ const crossValidateTransferStaffRecord = async ( const allNewWorkers = myJSONWorkers.filter((worker) => worker.status === 'NEW'); const allOtherWorkers = myJSONWorkers.filter((worker) => !isMovingToNewWorkplace(worker) && worker.status !== 'NEW'); - const newWorkerWithDuplicateIdErrorAdded = _crossValidateWorkersWithSameRefMovingToSameWorkplace( + const newWorkerWithDuplicateIdErrorAdded = _crossValidateWorkersWithDuplicateRefsMovingToWorkplace( csvWorkerSchemaErrors, allMovingWorkers, allNewWorkers, @@ -74,7 +74,7 @@ const crossValidateTransferStaffRecord = async ( if (newWorkerWithDuplicateIdErrorAdded) return; - const existingWorkerWithDuplicateIdErrorAdded = _crossValidateWorkersWithSameRefMovingToSameWorkplace( + const existingWorkerWithDuplicateIdErrorAdded = _crossValidateWorkersWithDuplicateRefsMovingToWorkplace( csvWorkerSchemaErrors, allMovingWorkers, allOtherWorkers, @@ -88,7 +88,6 @@ const crossValidateTransferStaffRecord = async ( csvWorkerSchemaErrors, relatedEstablishmentIds, JSONWorker, - allOtherWorkers, ); if (newWorkplaceId) { @@ -101,12 +100,7 @@ const isMovingToNewWorkplace = (JSONWorker) => { return JSONWorker.status === 'UPDATE' && JSONWorker.transferStaffRecord; }; -const _validateTransferIsPossible = async ( - csvWorkerSchemaErrors, - relatedEstablishmentIds, - JSONWorker, - allOtherWorkers, -) => { +const _validateTransferIsPossible = async (csvWorkerSchemaErrors, relatedEstablishmentIds, JSONWorker) => { const newWorkplaceLocalRef = JSONWorker.transferStaffRecord; const newWorkplaceId = await _getNewWorkplaceId(newWorkplaceLocalRef, relatedEstablishmentIds); @@ -176,13 +170,13 @@ const _addNewWorkplaceIdToWorkerEntity = (myAPIEstablishments, JSONWorker, newWo } }; -const _crossValidateWorkersWithSameRefMovingToSameWorkplace = ( +const _crossValidateWorkersWithDuplicateRefsMovingToWorkplace = ( csvWorkerSchemaErrors, allMovingWorkers, otherWorkers, errorType, ) => { - const workplacesDict = _buildWorkplaceDictWithNewWorkers(otherWorkers); + const workplacesDict = _buildWorkplaceDictWithOtherWorkers(otherWorkers); let errorAdded = false; @@ -211,8 +205,8 @@ const _crossValidateWorkersWithSameRefMovingToSameWorkplace = ( return errorAdded; }; -const _buildWorkplaceDictWithNewWorkers = (allNewWorkers) => { - return chain(allNewWorkers) +const _buildWorkplaceDictWithOtherWorkers = (otherWorkers) => { + return chain(otherWorkers) .groupBy('localId') // workplace ref .mapValues((JSONWorkers) => JSONWorkers.map((JSONWorker) => { From d5ef454606f19d7d9020f8464707320c4b4b142c Mon Sep 17 00:00:00 2001 From: Duncan Carter Date: Tue, 26 Nov 2024 16:51:29 +0000 Subject: [PATCH 31/33] Revert change to sort workers before saving as unnecessary and not working as sort takes a function with two params for comparison --- backend/server/models/classes/establishment.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/server/models/classes/establishment.js b/backend/server/models/classes/establishment.js index 63179c1c05..ea6f1cd267 100644 --- a/backend/server/models/classes/establishment.js +++ b/backend/server/models/classes/establishment.js @@ -724,8 +724,7 @@ class Establishment extends EntityValidator { // new and updated Workers const starterSavePromise = Promise.resolve(null); - const sortedWorkersAsArray = workersAsArray.sort((worker) => worker.changeLocalIdentifier !== null); - await sortedWorkersAsArray.reduce( + await workersAsArray.reduce( (p, thisWorkerToSave) => p.then(() => thisWorkerToSave.save(savedBy, bulkUploaded, 0, externalTransaction, true)).then(log), starterSavePromise, From 433f4e969ea68f5bea4581134a1a3000b2445c43 Mon Sep 17 00:00:00 2001 From: Duncan Carter Date: Tue, 26 Nov 2024 16:54:10 +0000 Subject: [PATCH 32/33] Add full stop to error message with two sentences --- .../server/models/BulkImport/csv/crossValidateErrors.js | 2 +- backend/server/models/classes/establishment.js | 1 - .../test/unit/models/Bulkimport/csv/crossValidate.spec.js | 8 ++++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidateErrors.js b/backend/server/models/BulkImport/csv/crossValidateErrors.js index 96af382827..e17163368c 100644 --- a/backend/server/models/BulkImport/csv/crossValidateErrors.js +++ b/backend/server/models/BulkImport/csv/crossValidateErrors.js @@ -26,7 +26,7 @@ const TRANSFER_STAFF_RECORD_ERRORS = { column: 'UNIQUEWORKERID', _sourceFieldName: 'uniqueWorkerId', error: - "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID.", }), SameRefsMovingToWorkplace: Object.freeze({ errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 3, diff --git a/backend/server/models/classes/establishment.js b/backend/server/models/classes/establishment.js index ea6f1cd267..a8bf7edae6 100644 --- a/backend/server/models/classes/establishment.js +++ b/backend/server/models/classes/establishment.js @@ -723,7 +723,6 @@ class Establishment extends EntityValidator { // new and updated Workers const starterSavePromise = Promise.resolve(null); - await workersAsArray.reduce( (p, thisWorkerToSave) => p.then(() => thisWorkerToSave.save(savedBy, bulkUploaded, 0, externalTransaction, true)).then(log), diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index 7f33db759d..17ed75223c 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -304,7 +304,7 @@ describe('crossValidate', () => { errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', error: - "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID.", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, @@ -337,7 +337,7 @@ describe('crossValidate', () => { errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', error: - "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID.", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, @@ -424,7 +424,7 @@ describe('crossValidate', () => { errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', error: - "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID.", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, @@ -496,7 +496,7 @@ describe('crossValidate', () => { errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', error: - "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID.", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, From 03d1d29d9b4c02e3dc3438265622d408899fc960 Mon Sep 17 00:00:00 2001 From: Duncan Carter Date: Tue, 26 Nov 2024 17:10:31 +0000 Subject: [PATCH 33/33] Remove full stops from end of error message as colon follows in error report --- .../server/models/BulkImport/csv/crossValidateErrors.js | 2 +- .../test/unit/models/Bulkimport/csv/crossValidate.spec.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/backend/server/models/BulkImport/csv/crossValidateErrors.js b/backend/server/models/BulkImport/csv/crossValidateErrors.js index e17163368c..96af382827 100644 --- a/backend/server/models/BulkImport/csv/crossValidateErrors.js +++ b/backend/server/models/BulkImport/csv/crossValidateErrors.js @@ -26,7 +26,7 @@ const TRANSFER_STAFF_RECORD_ERRORS = { column: 'UNIQUEWORKERID', _sourceFieldName: 'uniqueWorkerId', error: - "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID.", + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", }), SameRefsMovingToWorkplace: Object.freeze({ errCode: TRANSFER_STAFF_RECORD_BASE_ERROR_CODE + 3, diff --git a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js index 17ed75223c..7f33db759d 100644 --- a/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js +++ b/backend/server/test/unit/models/Bulkimport/csv/crossValidate.spec.js @@ -304,7 +304,7 @@ describe('crossValidate', () => { errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', error: - "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID.", + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, @@ -337,7 +337,7 @@ describe('crossValidate', () => { errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', error: - "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID.", + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, @@ -424,7 +424,7 @@ describe('crossValidate', () => { errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', error: - "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID.", + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber, @@ -496,7 +496,7 @@ describe('crossValidate', () => { errCode: 1402, errType: 'TRANSFERSTAFFRECORD_ERROR', error: - "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID.", + "The UNIQUEWORKERID already exists in the LOCALESTID given in TRANSFERSTAFFRECORD. Use CHGUNIQUEWRKID to change this worker's UNIQUEWORKERID", worker: movingWorker.uniqueWorkerId, name: movingWorker.localId, lineNumber: movingWorker.lineNumber,