Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FORMS-1563: auto-approve new ext api when same ministry/url already a… #1543

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
28 changes: 24 additions & 4 deletions app/src/forms/admin/service.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const { ExternalAPIStatuses } = require('../common/constants');
const { Form, FormVersion, User, UserFormAccess, FormComponentsProactiveHelp, AdminExternalAPI, ExternalAPI, ExternalAPIStatusCode } = require('../common/models');
const { queryUtils } = require('../common/utils');
const { v4: uuidv4 } = require('uuid');
Expand Down Expand Up @@ -148,14 +149,33 @@ const service = {
upd['userTokenHeader'] = null;
upd['userTokenBearer'] = false;
}

await ExternalAPI.query().patchAndFetchById(id, upd);

return ExternalAPI.query().findById(id);
let trx;
try {
trx = await ExternalAPI.startTransaction();
await ExternalAPI.query(trx).patchAndFetchById(id, upd);
await service._approveMany(id, data, trx);
await trx.commit();
return ExternalAPI.query().findById(id);
} catch (err) {
if (trx) await trx.rollback();
throw err;
}
},
getExternalAPIStatusCodes: async () => {
return ExternalAPIStatusCode.query();
},
_approveMany: async (id, data, trx) => {
// if we are setting to approved, approve all similar endpoints.
// same ministry, same base url...
if (data.code === ExternalAPIStatuses.APPROVED) {
const adminExternalAPI = await AdminExternalAPI.query(trx).findById(id);
const delimiter = '?';
const baseUrl = data.endpointUrl.split(delimiter)[0];
await ExternalAPI.query(trx)
.patch({ code: ExternalAPIStatuses.APPROVED })
.whereRaw(`"endpointUrl" like '${baseUrl}%' and code = 'SUBMITTED' and "formId" in (select id from form where ministry = '${adminExternalAPI.ministry}')`);
WalterMoar marked this conversation as resolved.
Show resolved Hide resolved
}
},

/**
* @function createFormComponentsProactiveHelp
Expand Down
72 changes: 55 additions & 17 deletions app/src/forms/form/externalApi/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const Problem = require('api-problem');
const { v4: uuidv4 } = require('uuid');
const { ExternalAPIStatuses } = require('../../common/constants');

const { ExternalAPI, ExternalAPIStatusCode } = require('../../common/models');
const { ExternalAPI, ExternalAPIStatusCode, Form, AdminExternalAPI } = require('../../common/models');

const { ENCRYPTION_ALGORITHMS } = require('../../../components/encryptionService');

Expand Down Expand Up @@ -60,20 +60,45 @@ const service = {
}
},

_updateAllPreApproved: async (formId, data, trx) => {
let result = 0;
const form = await Form.query().findById(formId);
const delimiter = '?';
const baseUrl = data.endpointUrl.split(delimiter)[0];
// check if there are matching api endpoints for the same ministry as our form that have been previously approved.
const approvedApis = await AdminExternalAPI.query(trx).whereRaw(`"endpointUrl" like '${baseUrl}%' and ministry = '${form.ministry}' and code = 'APPROVED'`);
if (approvedApis && approvedApis.length) {
// ok, since we've already approved a matching api endpoint, make sure others on this form are approved too.
result = await ExternalAPI.query(trx)
.patch({ code: ExternalAPIStatuses.APPROVED })
.whereRaw(`"endpointUrl" like '${baseUrl}%' and code = 'SUBMITTED' and "formId" = '${formId}'`);
}
return result;
},

createExternalAPI: async (formId, data, currentUser) => {
service.validateExternalAPI(data);

data.id = uuidv4();
// set status to SUBMITTED
// always create as SUBMITTED.
data.code = ExternalAPIStatuses.SUBMITTED;
// ensure that new records don't send user tokens.
service.checkAllowSendUserToken(data, false);
await ExternalAPI.query().insert({
...data,
createdBy: currentUser.usernameIdp,
});

return ExternalAPI.query().findById(data.id);
let trx;
try {
trx = await ExternalAPI.startTransaction();
await ExternalAPI.query(trx).insert({
...data,
createdBy: currentUser.usernameIdp,
});
// any urls on this form pre-approved?
await service._updateAllPreApproved(formId, data, trx);
await trx.commit();
return ExternalAPI.query().findById(data.id);
} catch (err) {
if (trx) await trx.rollback();
throw err;
}
},

updateExternalAPI: async (formId, externalAPIId, data, currentUser) => {
Expand All @@ -82,17 +107,30 @@ const service = {
const existing = await ExternalAPI.query().modify('findByIdAndFormId', externalAPIId, formId).first().throwIfNotFound();

// let's use a different method for the administrators to update status code and allow send user token
// this method should not change the status code.
// this method should not change the status code
data.code = existing.code;
if (existing.endpointUrl.split('?')[0] !== data.endpointUrl.split('?')[0]) {
// url changed, so save as SUBMITTED.
data.code = ExternalAPIStatuses.SUBMITTED;
}
service.checkAllowSendUserToken(data, existing.allowSendUserToken);
await ExternalAPI.query()
.modify('findByIdAndFormId', externalAPIId, formId)
.update({
...data,
updatedBy: currentUser.usernameIdp,
});

return ExternalAPI.query().findById(externalAPIId);
let trx;
try {
trx = await ExternalAPI.startTransaction();
await ExternalAPI.query(trx)
.modify('findByIdAndFormId', externalAPIId, formId)
.update({
...data,
WalterMoar marked this conversation as resolved.
Show resolved Hide resolved
updatedBy: currentUser.usernameIdp,
});
// any urls on this form pre-approved?
await service._updateAllPreApproved(formId, data, trx);
await trx.commit();
return ExternalAPI.query().findById(data.id);
} catch (err) {
if (trx) await trx.rollback();
throw err;
}
},

deleteExternalAPI: async (formId, externalAPIId) => {
Expand Down
4 changes: 4 additions & 0 deletions app/tests/unit/forms/admin/service.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ describe('Admin service', () => {
});

it('updateExternalAPI should patch and fetch', async () => {
service._approveMany = jest.fn().mockResolvedValueOnce();
await service.updateExternalAPI('id', { code: 'APPROVED', allowSendUserToken: true });
expect(MockModel.query).toBeCalledTimes(3);
expect(MockModel.patchAndFetchById).toBeCalledTimes(1);
Expand All @@ -145,9 +146,11 @@ describe('Admin service', () => {
code: 'APPROVED',
allowSendUserToken: true,
});
expect(service._approveMany).toBeCalledWith('id', { code: 'APPROVED', allowSendUserToken: true }, expect.anything());
});

it('updateExternalAPI should patch and fetch and update user token fields', async () => {
service._approveMany = jest.fn().mockResolvedValueOnce();
await service.updateExternalAPI('id', { code: 'APPROVED', allowSendUserToken: false });
expect(MockModel.query).toBeCalledTimes(3);
expect(MockModel.patchAndFetchById).toBeCalledTimes(1);
Expand All @@ -160,6 +163,7 @@ describe('Admin service', () => {
userTokenHeader: null,
userTokenBearer: false,
});
expect(service._approveMany).toBeCalledWith('id', { code: 'APPROVED', allowSendUserToken: false }, expect.anything());
});

it('getExternalAPIStatusCodes should fetch data', async () => {
Expand Down
6 changes: 6 additions & 0 deletions app/tests/unit/forms/form/externalApi/service.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ describe('createExternalAPI', () => {
});

it('should insert valid data', async () => {
service._updateAllPreApproved = jest.fn().mockResolvedValueOnce(0);
validData.id = null;
validData.code = null;
await service.createExternalAPI(validData.formId, validData, user);
Expand All @@ -164,6 +165,7 @@ describe('createExternalAPI', () => {
code: ExternalAPIStatuses.SUBMITTED,
...validData,
});
expect(service._updateAllPreApproved).toBeCalledWith(validData.formId, validData, expect.anything());
});

it('should raise errors', async () => {
Expand Down Expand Up @@ -205,6 +207,7 @@ describe('updateExternalAPI', () => {
});

it('should update valid data', async () => {
service._updateAllPreApproved = jest.fn().mockResolvedValueOnce(0);
MockModel.throwIfNotFound = jest.fn().mockResolvedValueOnce(Object.assign({}, validData));

// we do not update (status) code - must stay SUBMITTED
Expand All @@ -217,6 +220,7 @@ describe('updateExternalAPI', () => {
code: ExternalAPIStatuses.SUBMITTED,
...validData,
});
expect(service._updateAllPreApproved).toBeCalledWith(validData.formId, validData, expect.anything());
});

it('should update user token fields when allowed', async () => {
Expand All @@ -226,6 +230,7 @@ describe('updateExternalAPI', () => {
validData.userTokenHeader = 'Authorization';
validData.userTokenBearer = true;
MockModel.throwIfNotFound = jest.fn().mockResolvedValueOnce(Object.assign({}, validData));
service._updateAllPreApproved = jest.fn().mockResolvedValueOnce(0);

await service.updateExternalAPI(validData.formId, validData.id, validData, user);
expect(MockModel.update).toBeCalledTimes(1);
Expand All @@ -243,6 +248,7 @@ describe('updateExternalAPI', () => {
validData.userTokenHeader = 'Authorization';
validData.userTokenBearer = true;
MockModel.throwIfNotFound = jest.fn().mockResolvedValueOnce(Object.assign({}, validData));
service._updateAllPreApproved = jest.fn().mockResolvedValueOnce(0);

await service.updateExternalAPI(validData.formId, validData.id, validData, user);
expect(MockModel.update).toBeCalledTimes(1);
Expand Down
Loading