Skip to content

Commit

Permalink
refactor: FORMS-1407 move orm code to service (#1454)
Browse files Browse the repository at this point in the history
* refactor: FORMS-1407 move orm code to service

* revert the reordering to make the PR clearer
  • Loading branch information
WalterMoar authored Aug 1, 2024
1 parent 2653749 commit b58ee10
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 2 deletions.
4 changes: 2 additions & 2 deletions app/src/forms/common/middleware/validateParameter.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const Problem = require('api-problem');
const uuid = require('uuid');

const externalApiService = require('../../form/externalApi/service');
const formService = require('../../form/service');
const submissionService = require('../../submission/service');
const { ExternalAPI } = require('../models');

/**
* Throws a 400 problem if the parameter is not a valid UUID.
Expand Down Expand Up @@ -161,7 +161,7 @@ const validateExternalAPIId = async (req, _res, next, externalAPIId) => {
try {
_validateUuid(externalAPIId, 'externalAPIId');

const externalApi = await ExternalAPI.query().findById(externalAPIId);
const externalApi = await externalApiService.readExternalAPI(externalAPIId);
if (!externalApi || externalApi.formId !== req.params.formId) {
throw new Problem(404, {
detail: 'externalAPIId does not exist on this form',
Expand Down
4 changes: 4 additions & 0 deletions app/src/forms/form/externalApi/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ const service = {
await ExternalAPI.query().modify('findByIdAndFormId', externalAPIId, formId).first().throwIfNotFound();
await ExternalAPI.query().deleteById(externalAPIId);
},

readExternalAPI: (externalAPIId) => {
return ExternalAPI.query().findById(externalAPIId);
},
};

module.exports = service;
118 changes: 118 additions & 0 deletions app/tests/unit/forms/common/middleware/validateParameter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const { getMockReq, getMockRes } = require('@jest-mock/express');
const { v4: uuidv4 } = require('uuid');

const validateParameter = require('../../../../../src/forms/common/middleware/validateParameter');
const externalApiService = require('../../../../../src/forms/form/externalApi/service');
const formService = require('../../../../../src/forms/form/service');
const submissionService = require('../../../../../src/forms/submission/service');

Expand Down Expand Up @@ -198,6 +199,123 @@ describe('validateDocumentTemplateId', () => {
});
});

describe('validateExternalApiId', () => {
const externalApiId = uuidv4();

const mockReadExternalApiResponse = {
formId: formId,
id: externalApiId,
};

externalApiService.readExternalAPI = jest.fn().mockReturnValue(mockReadExternalApiResponse);

describe('400 response when', () => {
const expectedStatus = { status: 400 };

test('externalAPIId is missing', async () => {
const req = getMockReq({
params: {
formId: formId,
},
});
const { res, next } = getMockRes();

await validateParameter.validateExternalAPIId(req, res, next);

expect(externalApiService.readExternalAPI).toBeCalledTimes(0);
expect(next).toBeCalledWith(expect.objectContaining(expectedStatus));
});

test.each(invalidUuids)('externalAPIId is "%s"', async (eachExternalApiId) => {
const req = getMockReq({
params: {
formId: formId,
externalAPIId: eachExternalApiId,
},
});
const { res, next } = getMockRes();

await validateParameter.validateExternalAPIId(req, res, next, eachExternalApiId);

expect(externalApiService.readExternalAPI).toBeCalledTimes(0);
expect(next).toBeCalledWith(expect.objectContaining(expectedStatus));
});
});

describe('404 response when', () => {
const expectedStatus = { status: 404 };

test('formId is missing', async () => {
const req = getMockReq({
params: {
externalAPIId: externalApiId,
},
});
const { res, next } = getMockRes();

await validateParameter.validateExternalAPIId(req, res, next, externalApiId);

expect(externalApiService.readExternalAPI).toBeCalledTimes(1);
expect(next).toBeCalledWith(expect.objectContaining(expectedStatus));
});

test('formId does not match', async () => {
externalApiService.readExternalAPI.mockReturnValueOnce({
formId: uuidv4(),
id: externalApiId,
});
const req = getMockReq({
params: {
externalAPIId: externalApiId,
formId: formId,
},
});
const { res, next } = getMockRes();

await validateParameter.validateExternalAPIId(req, res, next, externalApiId);

expect(externalApiService.readExternalAPI).toBeCalledTimes(1);
expect(next).toBeCalledWith(expect.objectContaining(expectedStatus));
});
});

describe('handles error thrown by', () => {
test('readVersion', async () => {
const error = new Error();
externalApiService.readExternalAPI.mockRejectedValueOnce(error);
const req = getMockReq({
params: {
externalAPIId: externalApiId,
formId: formId,
},
});
const { res, next } = getMockRes();

await validateParameter.validateExternalAPIId(req, res, next, externalApiId);

expect(externalApiService.readExternalAPI).toBeCalledTimes(1);
expect(next).toBeCalledWith(error);
});
});

describe('allows', () => {
test('external api with matching form id', async () => {
const req = getMockReq({
params: {
externalAPIId: externalApiId,
formId: formId,
},
});
const { res, next } = getMockRes();

await validateParameter.validateExternalAPIId(req, res, next, externalApiId);

expect(externalApiService.readExternalAPI).toBeCalledTimes(1);
expect(next).toBeCalledWith();
});
});
});

describe('validateFileId', () => {
describe('400 response when', () => {
const expectedStatus = { status: 400 };
Expand Down

0 comments on commit b58ee10

Please sign in to comment.