Merge pull request #6421 from NMDSdevopsServiceAdm/fix/delete-certificates-when-deleting-training-record

Fix/delete certificates when deleting training record

Author: duncanc19

backend/server/routes/establishments/training/index.js

@@ -154,44 +154,39 @@ const updateTrainingRecord = async (req, res) => {
   }
 };
 
-// deletes requested training record using the training uid
-const deleteTrainingRecord = async (req, res) => {
-  const establishmentId = req.establishmentId;
-  const trainingUid = req.params.trainingUid;
-  const workerUid = req.params.workerId;
-  const establishmentUid = req.params.id;
+const deleteTrainingRecordEndpoint = async (req, res) => {
+  const trainingRecord = new Training(req.establishmentId, req.params.workerId);
+  const trainingCertificateService = WorkerCertificateService.initialiseTraining();
 
-  const thisTrainingRecord = new Training(establishmentId, workerUid);
+  return await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
+};
 
+const deleteTrainingRecord = async (req, res, trainingRecord, trainingCertificateService) => {
   try {
-    // before updating a Worker, we need to be sure the Worker is
-    //  available to the given establishment. The best way of doing that
-    //  is to restore from given UID
-    if (await thisTrainingRecord.restore(trainingUid)) {
-      // TODO: JSON validation
+    const trainingUid = req.params.trainingUid;
+    const workerUid = req.params.workerId;
+    const establishmentUid = req.params.id;
 
-      const trainingCertificates = thisTrainingRecord?._trainingCertificates;
+    const trainingRecordFound = await trainingRecord.restore(trainingUid);
+    if (!trainingRecordFound) {
+      return res.status(404).send('Not Found');
+    }
 
-      const trainingCertificateService = WorkerCertificateService.initialiseTraining();
+    const trainingCertificates = trainingRecord?._trainingCertificates;
 
-      if (this.trainingCertificates?.length) {
-        await trainingCertificateService.deleteCertificates(
-          trainingCertificates,
-          establishmentUid,
-          workerUid,
-          trainingUid,
-        );
-      }
+    if (trainingCertificates?.length) {
+      await trainingCertificateService.deleteCertificates(
+        trainingCertificates,
+        establishmentUid,
+        workerUid,
+        trainingUid,
+      );
+    }
 
-      // by deleting after the restore we can be sure this training record belongs to the given worker
-      const deleteSuccess = await thisTrainingRecord.delete();
-      if (deleteSuccess) {
-        return res.status(204).json();
-      } else {
-        return res.status(404).send('Not Found');
-      }
+    const deleteSuccess = await trainingRecord.delete();
+    if (deleteSuccess) {
+      return res.status(204).json();
     } else {
-      // not found worker
       return res.status(404).send('Not Found');
     }
   } catch (err) {
@@ -204,7 +199,7 @@ router.route('/').get(hasPermission('canViewWorker'), getTrainingListWithMissing
 router.route('/').post(hasPermission('canEditWorker'), createTrainingRecord);
 router.route('/:trainingUid').get(hasPermission('canViewWorker'), viewTrainingRecord);
 router.route('/:trainingUid').put(hasPermission('canEditWorker'), updateTrainingRecord);
-router.route('/:trainingUid').delete(hasPermission('canEditWorker'), deleteTrainingRecord);
+router.route('/:trainingUid').delete(hasPermission('canEditWorker'), deleteTrainingRecordEndpoint);
 router.use('/:trainingUid/certificate', TrainingCertificateRoute);
 
 module.exports = router;

backend/server/test/unit/routes/establishments/training/index.spec.js

@@ -2,14 +2,12 @@ const expect = require('chai').expect;
 const sinon = require('sinon');
 const httpMocks = require('node-mocks-http');
 const buildUser = require('../../../../factories/user');
-const models = require('../../../../../models');
 const Training = require('../../../../../models/classes/training').Training;
 const { deleteTrainingRecord } = require('../../../../../routes/establishments/training/index');
 const {
   mockTrainingRecordWithCertificates,
   mockTrainingRecordWithoutCertificates,
 } = require('../../../mockdata/training');
-const WorkerCertificateService = require('../../../../../routes/establishments/workerCertificate/workerCertificateService');
 const HttpError = require('../../../../../utils/errors/httpError');
 
 describe('server/routes/establishments/training/index.js', () => {
@@ -25,8 +23,8 @@ describe('server/routes/establishments/training/index.js', () => {
     let workerUid = mockTrainingRecordWithCertificates.workerUid;
     let trainingUid = mockTrainingRecordWithCertificates.uid;
     let establishmentUid = user.establishment.uid;
-
-    let stubs = {};
+    let trainingRecord;
+    let trainingCertificateService;
 
     beforeEach(() => {
       req = httpMocks.createRequest({
@@ -37,76 +35,110 @@ describe('server/routes/establishments/training/index.js', () => {
       });
       res = httpMocks.createResponse();
 
-      stubs = {
-        trainingRecord: sinon.createStubInstance(Training),
-        restoreTrainingRecord: sinon.stub(Training.prototype, 'restore'),
-        getWorkerCertificateServiceInstance: sinon.stub(WorkerCertificateService, 'initialiseTraining').returns(new WorkerCertificateService()),
-        deleteCertificates: sinon.stub(WorkerCertificateService.prototype, 'deleteCertificates'),
-        destroyTrainingRecord: sinon.stub(models.workerTraining, 'destroy'),
-      }
+      trainingRecord = sinon.createStubInstance(Training);
+      trainingRecord.restore.resolves(true);
+
+      trainingCertificateService = {
+        deleteCertificates: sinon.stub(),
+      };
     });
 
-    it('should return with a status of 204 when the training record is deleted with training certificates', async () => {
-      stubs.restoreTrainingRecord.returns(mockTrainingRecordWithCertificates);
-      stubs.destroyTrainingRecord.returns(1);
+    describe('With training certificates', () => {
+      beforeEach(() => {
+        trainingRecord.trainingCertificates = mockTrainingRecordWithCertificates.trainingCertificates;
+        trainingRecord.delete.resolves(1);
+      });
 
-      await deleteTrainingRecord(req, res);
+      it('should return with a status of 204', async () => {
+        await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
+
+        expect(res.statusCode).to.equal(204);
+      });
 
-      expect(res.statusCode).to.equal(204);
+      it('should call delete on training instance', async () => {
+        await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
+
+        expect(trainingRecord.delete).to.have.been.calledOnce;
+      });
+
+      it('should call deleteCertificates', async () => {
+        await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
+
+        expect(trainingCertificateService.deleteCertificates.calledOnce).to.be.true;
+        expect(
+          trainingCertificateService.deleteCertificates.calledWith(
+            mockTrainingRecordWithCertificates.trainingCertificates,
+            establishmentUid,
+            workerUid,
+            trainingUid,
+          ),
+        ).to.be.true;
+      });
     });
 
-    it('should return with a status of 204 when the training record is deleted with no training certificates', async () => {
-      stubs.restoreTrainingRecord.returns(mockTrainingRecordWithoutCertificates);
-      stubs.destroyTrainingRecord.returns(1);
+    describe('Without training certificates', () => {
+      beforeEach(() => {
+        trainingRecord.trainingCertificates = null;
+        trainingRecord.delete.resolves(1);
+      });
+
+      it('should return with a status of 204 when successful', async () => {
+        await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
+
+        expect(res.statusCode).to.equal(204);
+      });
 
-      await deleteTrainingRecord(req, res);
+      it('should call delete on training instance', async () => {
+        await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
 
-      expect(res.statusCode).to.equal(204);
+        expect(trainingRecord.delete).to.have.been.calledOnce;
+      });
+
+      it('should not call deleteCertificates when no training certificates', async () => {
+        await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
+
+        expect(trainingCertificateService.deleteCertificates.called).to.be.false;
+      });
     });
 
     describe('errors', () => {
       it('should pass through status code if one is provided', async () => {
-        stubs.restoreTrainingRecord.throws(new HttpError('Test error message', 123));
-        req.params.trainingUid = 'mockTrainingUid';
+        trainingRecord.restore.throws(new HttpError('Test error message', 123));
 
-        await deleteTrainingRecord(req, res);
+        await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
 
         expect(res.statusCode).to.equal(123);
       });
 
       it('should default to status code 500 if no error code is provided', async () => {
-        stubs.restoreTrainingRecord.throws(new Error());
-        req.params.trainingUid = 'mockTrainingUid';
+        trainingRecord.restore.throws(new Error());
 
-        await deleteTrainingRecord(req, res);
+        await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
 
         expect(res.statusCode).to.equal(500);
       });
 
-      describe('restoring training record', () => {
-        it('should return a 404 status code if there is an unknown worker uid', async () => {
-          trainingRecord_workerUid = 'mockWorkerUid';
+      it('should return a 404 status code when failed to restore training (e.g. unknown worker uid)', async () => {
+        trainingRecord.restore.resolves(false);
 
-          await deleteTrainingRecord(req, res);
+        await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
 
-          const response = res._getData();
+        const response = res._getData();
 
-          expect(res.statusCode).to.equal(404);
-          expect(response).to.equal('Not Found');
-        });
+        expect(res.statusCode).to.equal(404);
+        expect(response).to.equal('Not Found');
       });
 
-      describe('deleting training record', () => {
-        it('should return with a status of 404 when there is an error deleting the training record from the database', async () => {
-          stubs.destroyTrainingRecord.returns(0);
+      it('should return with a status of 404 when there is an error deleting the training record from the database', async () => {
+        trainingRecord.restore.resolves(true);
+        trainingRecord.delete.resolves(0);
 
-          await deleteTrainingRecord(req, res);
+        await deleteTrainingRecord(req, res, trainingRecord, trainingCertificateService);
 
-          const response = res._getData();
+        const response = res._getData();
 
-          expect(res.statusCode).to.equal(404);
-          expect(response).to.equal('Not Found');
-        });
+        expect(res.statusCode).to.equal(404);
+        expect(response).to.equal('Not Found');
       });
     });
   });