diff --git a/packages/gafl-webapp-service/src/__mocks__/data/permits.js b/packages/gafl-webapp-service/src/__mocks__/data/permits.js index 32cc18d4bb..7c488e75c4 100644 --- a/packages/gafl-webapp-service/src/__mocks__/data/permits.js +++ b/packages/gafl-webapp-service/src/__mocks__/data/permits.js @@ -34,6 +34,8 @@ export default [ isForFulfilment: false, isCounterSales: true, cost: 6, + newCost: 7, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42289' }, { @@ -71,6 +73,8 @@ export default [ isForFulfilment: false, isCounterSales: true, cost: 6, + newCost: 7, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42290' }, { @@ -108,6 +112,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 12, + newCost: 13, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42361' }, { @@ -145,6 +151,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 12, + newCost: 13, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42363' }, { @@ -182,6 +190,8 @@ export default [ isForFulfilment: false, isCounterSales: true, cost: 12, + newCost: 13, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42305' }, { @@ -219,6 +229,8 @@ export default [ isForFulfilment: false, isCounterSales: true, cost: 12, + newCost: 13, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42307' }, { @@ -256,6 +268,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 27, + newCost: 28, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42365' }, { @@ -293,6 +307,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 27, + newCost: 28, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42370' }, { @@ -330,6 +346,8 @@ export default [ isForFulfilment: false, isCounterSales: false, cost: 0, + newCost: 0, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: null }, { @@ -367,6 +385,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 30, + newCost: 31, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42309' }, { @@ -404,6 +424,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 20, + newCost: 21, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42335' }, { @@ -441,6 +463,8 @@ export default [ isForFulfilment: false, isCounterSales: false, cost: 0, + newCost: 0, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: null }, { @@ -478,6 +502,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 20, + newCost: 21, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42337' }, { @@ -515,6 +541,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 20, + newCost: 21, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42339' }, { @@ -552,6 +580,8 @@ export default [ isForFulfilment: false, isCounterSales: false, cost: 0, + newCost: 0, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: null }, { @@ -589,6 +619,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 45, + newCost: 46, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42341' }, { @@ -626,6 +658,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 30, + newCost: 31, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42343' }, { @@ -663,6 +697,8 @@ export default [ isForFulfilment: false, isCounterSales: false, cost: 0, + newCost: 0, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: null }, { @@ -700,6 +736,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 30, + newCost: 31, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42345' }, { @@ -737,6 +775,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 30, + newCost: 31, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42347' }, { @@ -774,6 +814,8 @@ export default [ isForFulfilment: true, isCounterSales: false, cost: 0, + newCost: 0, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: null }, { @@ -811,6 +853,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 82, + newCost: 83, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42372' }, { @@ -848,6 +892,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 54, + newCost: 55, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42374' }, { @@ -885,6 +931,8 @@ export default [ isForFulfilment: true, isCounterSales: false, cost: 0, + newCost: 0, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: null }, { @@ -922,6 +970,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 54, + newCost: 55, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42376' }, { @@ -959,6 +1009,8 @@ export default [ isForFulfilment: true, isCounterSales: true, cost: 54, + newCost: 55, + newCostStartDate: '2023-04-01T00:00:00.000Z', itemId: '42378' } ] diff --git a/packages/gafl-webapp-service/src/pages/licence-details/licence-length/__tests__/update-transaction.spec.js b/packages/gafl-webapp-service/src/pages/licence-details/licence-length/__tests__/update-transaction.spec.js index 0ee63f839d..e9611dfe0c 100644 --- a/packages/gafl-webapp-service/src/pages/licence-details/licence-length/__tests__/update-transaction.spec.js +++ b/packages/gafl-webapp-service/src/pages/licence-details/licence-length/__tests__/update-transaction.spec.js @@ -1,19 +1,26 @@ import updateTransaction from '../update-transaction' import { LICENCE_FULFILMENT } from '../../../../uri.js' import * as concessionHelper from '../../../../processors/concession-helper.js' -import { findPermit } from '../../../../processors/find-permit.js' +import findPermit from '../../../../processors/find-permit.js' import { isPhysical } from '../../../../processors/licence-type-display.js' import * as mappings from '../../../../processors/mapping-constants.js' import moment from 'moment-timezone' +import hashPermission from '../../../../processors/hash-permission' jest.mock('../../../../processors/concession-helper.js') jest.mock('../../../../processors/find-permit.js') jest.mock('../../../../processors/licence-type-display.js') +jest.mock('../../../../processors/hash-permission.js', () => jest.fn(() => 'abcde12345')) describe('licence-details > update-transaction', () => { describe('default', () => { beforeEach(jest.clearAllMocks) - const getMockRequest = (licenceLength = '12M', transactionPageGet = { licenceLength: '' }) => ({ + const getMockRequest = ({ + permission = { licenceLength: '12M' }, + licenceLength = permission.licenceLength, + transactionSet = () => {}, + statusSet = () => {} + } = {}) => ({ cache: jest.fn(() => ({ helpers: { page: { @@ -21,11 +28,11 @@ describe('licence-details > update-transaction', () => { }, status: { getCurrentPermission: () => {}, - setCurrentPermission: jest.fn() + setCurrentPermission: statusSet }, transaction: { - getCurrentPermission: () => transactionPageGet, - setCurrentPermission: jest.fn() + getCurrentPermission: () => permission, + setCurrentPermission: transactionSet } } })) @@ -33,71 +40,100 @@ describe('licence-details > update-transaction', () => { it('should find the permit with the available data', async () => { const permission = { licenceLength: '1D' } - const request = getMockRequest('1D', permission) + const request = getMockRequest({ permission }) await updateTransaction(request) - expect(findPermit).toHaveBeenCalledWith(permission, request) + expect(findPermit).toHaveBeenCalledWith(permission) + }) + + it('attaches the permit to the permission', async () => { + const transactionSet = jest.fn() + const permit = Symbol('permit') + findPermit.mockReturnValueOnce(permit) + await updateTransaction(getMockRequest({ transactionSet })) + expect(transactionSet).toHaveBeenCalledWith(expect.objectContaining({ permit })) + }) + + it('hashes the permission', async () => { + const transactionSet = jest.fn() + const hash = Symbol('hash') + console.log('hashPermission', hashPermission) + hashPermission.mockReturnValueOnce(hash) + await updateTransaction(getMockRequest({ transactionSet })) + expect(transactionSet).toHaveBeenCalledWith(expect.objectContaining({ hash })) + }) + + it('passes the permission to be hashed', async () => { + const permission = {} + await updateTransaction(getMockRequest({ permission })) + expect(hashPermission).toHaveBeenCalledWith(permission) + }) + + it('only retrieves the permit if the hash has changed', async () => { + const hash = Symbol('hash') + const permission = { property: 'value', permit: Symbol('permit'), hash } + hashPermission.mockReturnValueOnce(hash) + await updateTransaction(getMockRequest({ permission })) + expect(findPermit).not.toHaveBeenCalled() }) it('should set the licence fulfilment page to false on the status', async () => { - const request = getMockRequest() + const statusSet = jest.fn() + const request = getMockRequest({ statusSet }) await updateTransaction(request) - expect(request.cache.mock.results[2].value.helpers.status.setCurrentPermission).toHaveBeenCalledWith({ + expect(statusSet).toHaveBeenCalledWith({ [LICENCE_FULFILMENT.page]: false }) }) describe('checkLicenceToStart', () => { it('should set licenceStartTime to null when the length is 12M', async () => { - const request = getMockRequest() + const transactionSet = jest.fn() + const request = getMockRequest({ transactionSet }) await updateTransaction(request) - expect(request.cache.mock.results[3].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( - expect.objectContaining({ licenceStartTime: null }) - ) + expect(transactionSet).toHaveBeenCalledWith(expect.objectContaining({ licenceStartTime: null })) }) it('should set licenceToStart to after-payment when the length is 12M and the start date matches the current day', async () => { + const transactionSet = jest.fn() const permission = { licenceLength: '12M', licenceStartDate: moment() } - const request = getMockRequest('12M', permission) + const request = getMockRequest({ permission, transactionSet }) await updateTransaction(request) - expect(request.cache.mock.results[3].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( - expect.objectContaining({ licenceToStart: 'after-payment' }) - ) + expect(transactionSet).toHaveBeenCalledWith(expect.objectContaining({ licenceToStart: 'after-payment' })) }) it('should not modify licenceToStart when the length is 12M and the start date is a later day', async () => { + const transactionSet = jest.fn() const licenceToStart = Symbol('licenceToStart') const permission = { licenceLength: '12M', licenceStartDate: '2100-01-01', licenceToStart } - const request = getMockRequest('12M', permission) + const request = getMockRequest({ permission, transactionSet }) await updateTransaction(request) - expect(request.cache.mock.results[3].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( - expect.objectContaining({ licenceToStart }) - ) + expect(transactionSet).toHaveBeenCalledWith(expect.objectContaining({ licenceToStart })) }) it('should not modify licenceStartTime when the length is not 12M', async () => { + const transactionSet = jest.fn() const licenceStartTime = Symbol('licenceStartTime') const permission = { licenceLength: '1D', licenceStartTime } - const request = getMockRequest('1D', permission) + const request = getMockRequest({ permission, transactionSet }) await updateTransaction(request) - expect(request.cache.mock.results[3].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( - expect.objectContaining({ licenceStartTime }) - ) + expect(transactionSet).toHaveBeenCalledWith(expect.objectContaining({ licenceStartTime })) }) }) describe('checkContactDetails', () => { it('should set the correct values when the permit is physical and preferredMethodOfConfirmation and/or preferredMethodOfReminder are set to none', async () => { + const transactionSet = jest.fn() isPhysical.mockReturnValueOnce(true) const permission = { licensee: { preferredMethodOfConfirmation: mappings.HOW_CONTACTED.none } } - const request = getMockRequest('12M', permission) + const request = getMockRequest({ licenceLength: '12M', permission, transactionSet }) await updateTransaction(request) - expect(request.cache.mock.results[3].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( + expect(transactionSet).toHaveBeenCalledWith( expect.objectContaining({ licensee: { postalFulfilment: true, @@ -110,11 +146,12 @@ describe('licence-details > update-transaction', () => { it('should set the correct values the permit is not physical and preferredMethodOfConfirmation and/or preferredMethodOfReminder are set to letter', async () => { isPhysical.mockReturnValueOnce(false) + const transactionSet = jest.fn() const permission = { licensee: { preferredMethodOfConfirmation: mappings.HOW_CONTACTED.letter } } - const request = getMockRequest('12M', permission) + const request = getMockRequest({ licenceLength: '12M', permission, transactionSet }) await updateTransaction(request) - expect(request.cache.mock.results[3].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( + expect(transactionSet).toHaveBeenCalledWith( expect.objectContaining({ licensee: { postalFulfilment: false, @@ -128,13 +165,14 @@ describe('licence-details > update-transaction', () => { describe('checkDisabledConcessions', () => { it('should store the concession to previouslyDisabled when the licenceLength is not 12M and the permission has a disabled concession', async () => { + const transactionSet = jest.fn() concessionHelper.hasDisabled.mockReturnValueOnce(true) const concession = { type: mappings.CONCESSION.DISABLED } const permission = { licenceLength: '1D', concessions: [concession] } - const request = getMockRequest('1D', permission) + const request = getMockRequest({ permission, transactionSet }) await updateTransaction(request) - expect(request.cache.mock.results[3].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( + expect(transactionSet).toHaveBeenCalledWith( expect.objectContaining({ previouslyDisabled: concession }) @@ -145,20 +183,21 @@ describe('licence-details > update-transaction', () => { concessionHelper.hasDisabled.mockReturnValueOnce(true) const concession = { type: mappings.CONCESSION.DISABLED } const permission = { licenceLength: '1D', concessions: [concession] } - const request = getMockRequest('1D', permission) + const request = getMockRequest({ permission }) await updateTransaction(request) expect(concessionHelper.removeDisabled).toHaveBeenCalledWith(permission) }) it('should re-add the disabled concession and clear previouslyDisabled when the licenceLength is 12M and the permission previously had a disabled concession', async () => { + const transactionSet = jest.fn() concessionHelper.hasDisabled.mockReturnValueOnce(false) const previousConcession = Symbol('previousConcession') const permission = { licenceLength: '12M', concessions: [], previouslyDisabled: previousConcession } - const request = getMockRequest('12M', permission) + const request = getMockRequest({ permission, transactionSet }) await updateTransaction(request) - expect(request.cache.mock.results[3].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( + expect(transactionSet).toHaveBeenCalledWith( expect.objectContaining({ concessions: [previousConcession], previouslyDisabled: null @@ -167,13 +206,12 @@ describe('licence-details > update-transaction', () => { }) it('should set the number of rods to 2 when the licenceLength is not 12M and the licenceType is trout and course', async () => { + const transactionSet = jest.fn() const permission = { licenceLength: '1D', licenceType: mappings.LICENCE_TYPE['trout-and-coarse'], numberOfRods: '10' } - const request = getMockRequest('1D', permission) + const request = getMockRequest({ permission, transactionSet }) await updateTransaction(request) - expect(request.cache.mock.results[3].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( - expect.objectContaining({ numberOfRods: '2' }) - ) + expect(transactionSet).toHaveBeenCalledWith(expect.objectContaining({ numberOfRods: '2' })) }) }) }) diff --git a/packages/gafl-webapp-service/src/pages/licence-details/licence-length/update-transaction.js b/packages/gafl-webapp-service/src/pages/licence-details/licence-length/update-transaction.js index 7d3acaf243..e50afc569c 100644 --- a/packages/gafl-webapp-service/src/pages/licence-details/licence-length/update-transaction.js +++ b/packages/gafl-webapp-service/src/pages/licence-details/licence-length/update-transaction.js @@ -6,7 +6,8 @@ import { cacheDateFormat } from '../../../processors/date-and-time-display.js' import { SERVICE_LOCAL_TIME } from '@defra-fish/business-rules-lib' import { isPhysical } from '../../../processors/licence-type-display.js' import { licenceToStart } from '../licence-to-start/update-transaction.js' -import { findPermit } from '../../../processors/find-permit.js' +import findPermit from '../../../processors/find-permit.js' +import hashPermission from '../../../processors/hash-permission.js' /** * If we have set the licence type to physical change the method of contact from 'none' to 'letter' and vice-versa @@ -88,7 +89,11 @@ export default async request => { const permission = await request.cache().helpers.transaction.getCurrentPermission() permission.licenceLength = payload['licence-length'] - await findPermit(permission, request) + const hash = hashPermission(permission) + if (permission.hash !== hash) { + permission.hash = hash + permission.permit = await findPermit(permission) + } onLengthChange(permission) // Clear the licence fulfilment here otherwise it can end up being set incorrectly diff --git a/packages/gafl-webapp-service/src/pages/summary/licence-summary/__tests__/route.spec.js b/packages/gafl-webapp-service/src/pages/summary/licence-summary/__tests__/route.spec.js index dc81d6ebcf..e9e8691970 100644 --- a/packages/gafl-webapp-service/src/pages/summary/licence-summary/__tests__/route.spec.js +++ b/packages/gafl-webapp-service/src/pages/summary/licence-summary/__tests__/route.spec.js @@ -1,7 +1,8 @@ import { getData } from '../route' import { LICENCE_SUMMARY_SEEN } from '../../../../constants.js' import { DATE_OF_BIRTH, LICENCE_LENGTH, LICENCE_TO_START, LICENCE_TYPE, NAME, NEW_TRANSACTION, NEW_PRICES } from '../../../../uri.js' -import { findPermit } from '../../../../processors/find-permit.js' +import findPermit from '../../../../processors/find-permit.js' +import hashPermission from '../../../../processors/hash-permission.js' import { licenceTypeDisplay } from '../../../../processors/licence-type-display.js' import { addLanguageCodeToUri } from '../../../../processors/uri-helper.js' import mappingConstants from '../../../../processors/mapping-constants.js' @@ -40,14 +41,18 @@ jest.mock('../../../../processors/mapping-constants.js', () => ({ none: 'Not Proof' } })) -jest.mock('../../../../processors/find-permit.js', () => ({ - findPermit: jest.fn((_permission, _request) => {}) -})) +jest.mock('../../../../processors/find-permit.js') +jest.mock('../../../../processors/hash-permission.js') jest.mock('../../../../processors/price-display.js', () => ({ displayPermissionPrice: jest.fn(() => '#6') })) -const getMockRequest = ({ currentPermission = getMockPermission(), statusCache = {}, statusCacheSet = () => {} } = {}) => ({ +const getMockRequest = ({ + currentPermission = getMockPermission(), + statusCache = {}, + statusCacheSet = () => {}, + transactionCacheSet = () => {} +} = {}) => ({ cache: () => ({ helpers: { status: { @@ -56,7 +61,7 @@ const getMockRequest = ({ currentPermission = getMockPermission(), statusCache = }, transaction: { getCurrentPermission: async () => currentPermission, - setCurrentPermission: () => {} + setCurrentPermission: transactionCacheSet } } }), @@ -168,6 +173,9 @@ const getMockContinuingPermission = () => ({ }) describe('licence-summary > route', () => { + beforeAll(() => { + hashPermission.mockReturnValue('lkjhgfdertyu0987654rftghj') + }) beforeEach(jest.clearAllMocks) describe('sets from summary on status cache', () => { @@ -224,10 +232,74 @@ describe('licence-summary > route', () => { it.each([ { desc: 'renewal', currentPermission: getMockPermission() }, { desc: 'new', currentPermission: getMockNewPermission() } - ])('calls findPermit with permission and request where permission is a $desc permission', async ({ currentPermission }) => { + ])('calls findPermit with permission where permission is a $desc permission', async ({ currentPermission }) => { + const mockRequest = getMockRequest({ currentPermission }) + await getData(mockRequest) + expect(findPermit).toHaveBeenCalledWith(currentPermission) + }) + + it('attaches the permit to the permission', async () => { + const currentPermisison = getMockPermission() + const mockRequest = getMockRequest({ currentPermisison }) + const permit = { cost: 10 } + hashPermission.mockReturnValueOnce('dfghj3456789') + findPermit.mockReturnValueOnce(permit) + + await getData(mockRequest) + + expect(displayPermissionPrice).toHaveBeenCalledWith(expect.objectContaining({ permit }), expect.any(Object)) + }) + + it('hashes the permission', async () => { + const currentPermission = getMockPermission() + const mockRequest = getMockRequest({ currentPermission }) + const hash = Symbol('hash') + hashPermission.mockReturnValueOnce(hash) + + await getData(mockRequest) + + expect(licenceTypeDisplay).toHaveBeenCalledWith(expect.objectContaining({ hash }), expect.any(Object)) + }) + + it('passes the permission to the hash function', async () => { + const currentPermission = getMockPermission() + await getData(getMockRequest({ currentPermission })) + expect(hashPermission).toHaveBeenCalledWith(currentPermission) + }) + + it('only retrieves the permit if the hash has changed', async () => { + const hash = Symbol('hash') + const currentPermission = { ...getMockPermission(), hash } const mockRequest = getMockRequest({ currentPermission }) + hashPermission.mockReturnValueOnce(hash) await getData(mockRequest) - expect(findPermit).toHaveBeenCalledWith(currentPermission, mockRequest) + + expect(findPermit).not.toHaveBeenCalled() + }) + + it('persists modified permission', async () => { + const hash = Symbol('hash') + const currentPermission = getMockPermission() + const transactionCacheSet = jest.fn() + const mockRequest = getMockRequest({ currentPermission, transactionCacheSet }) + const permit = { cost: 10 } + hashPermission.mockReturnValueOnce(hash) + findPermit.mockReturnValueOnce(permit) + + await getData(mockRequest) + + expect(transactionCacheSet).toHaveBeenCalledWith(expect.objectContaining({ ...currentPermission, permit, hash })) + }) + + it("doesn't persist modified permission if hash hasn't changed", async () => { + const currentPermission = getMockPermission() + currentPermission.hash = hashPermission() + const transactionCacheSet = jest.fn() + const mockRequest = getMockRequest({ currentPermission, transactionCacheSet }) + + await getData(mockRequest) + + expect(transactionCacheSet).not.toHaveBeenCalled() }) it.each([[NEW_TRANSACTION.uri], [NEW_PRICES.uri]])('addLanguageCodeToUri is called with request and %s', async uri => { diff --git a/packages/gafl-webapp-service/src/pages/summary/licence-summary/route.js b/packages/gafl-webapp-service/src/pages/summary/licence-summary/route.js index 9075390b90..f0b6a18b16 100644 --- a/packages/gafl-webapp-service/src/pages/summary/licence-summary/route.js +++ b/packages/gafl-webapp-service/src/pages/summary/licence-summary/route.js @@ -1,7 +1,8 @@ import moment from 'moment-timezone' import pageRoute from '../../../routes/page-route.js' import GetDataRedirect from '../../../handlers/get-data-redirect.js' -import { findPermit } from '../../../processors/find-permit.js' +import findPermit from '../../../processors/find-permit.js' +import hashPermission from '../../../processors/hash-permission.js' import { displayStartTime } from '../../../processors/date-and-time-display.js' import { licenceTypeDisplay } from '../../../processors/licence-type-display.js' import { @@ -189,7 +190,12 @@ export const getData = async request => { status.fromSummary = getFromSummary(status.fromSummary, permission.isRenewal) await request.cache().helpers.status.setCurrentPermission(status) debug('retrieving permit info') - await findPermit(permission, request) + const hash = hashPermission(permission) + if (permission.hash !== hash) { + permission.permit = await findPermit(permission) + permission.hash = hash + await request.cache().helpers.transaction.setCurrentPermission(permission) + } debug('retrieved permit', JSON.stringify(permission)) return { diff --git a/packages/gafl-webapp-service/src/processors/__tests__/find-permit.spec.js b/packages/gafl-webapp-service/src/processors/__tests__/find-permit.spec.js index 4247e12375..124f142965 100644 --- a/packages/gafl-webapp-service/src/processors/__tests__/find-permit.spec.js +++ b/packages/gafl-webapp-service/src/processors/__tests__/find-permit.spec.js @@ -1,132 +1,94 @@ -import { findPermit } from '../find-permit.js' -import filterPermits from '../filter-permits.js' -import crypto from 'crypto' -import db from 'debug' - -jest.mock('../filter-permits.js') -jest.mock('debug', () => jest.fn(() => jest.fn())) -const debugMock = db.mock.results[0].value - -const getMockHashImplementation = (overrides = {}) => - jest.fn(() => ({ - update: () => {}, - digest: () => 'abc123', - ...overrides - })) - -jest.mock('crypto', () => ({ - createHash: getMockHashImplementation() +import findPermit from '../find-permit' + +jest.mock('@defra-fish/connectors-lib', () => ({ + salesApi: { + permits: { + getAll: async () => [ + { + id: 'prm-111', + numberOfRods: 1, + durationMagnitude: '8', + durationDesignator: { description: 'M' }, + permitSubtype: { label: 'type-1' } + }, + { + id: 'prm-222', + numberOfRods: 1, + durationMagnitude: '17', + durationDesignator: { description: 'Seconds' }, + permitSubtype: { label: 'type-2' } + }, + { + id: 'prm-333', + numberOfRods: 1, + durationMagnitude: '17', + durationDesignator: { description: 'Seconds' }, + permitSubtype: { label: 'type-3' } + }, + { + id: 'prm-444', + numberOfRods: 1, + durationMagnitude: '10', + durationDesignator: { description: 'Hours' }, + permitSubtype: { label: 'type-3' } + }, + { + id: 'prm-555', + numberOfRods: 3, + durationMagnitude: '10', + durationDesignator: { description: 'Hours' }, + permitSubtype: { label: 'type-3' } + } + ] + }, + permitConcessions: { + getAll: async () => [{ permitId: 'prm-111', concessionId: 'con-111' }] + }, + concessions: { + getAll: async () => [ + { id: 'con-111', name: 'concession-type-1' }, + { id: 'con-222', name: 'concession-type-2' } + ] + } + } })) describe('findPermit', () => { - beforeEach(jest.clearAllMocks) - - describe('debug', () => { - it('logs permit missing', async () => { - await findPermit(getMockPermission(), getMockRequest()) - expect(debugMock).toHaveBeenCalledWith("permit wasn't retrieved", expect.any(Object)) - }) - - it.each([ - ['newCostStartDate', { newCost: 1 }], - ['newCost', { newCostStartDate: '2023-04-01' }], - ['newCost and newCostStartDate', {}] - ])('logs permit missing %s', async (_d, p) => { - filterPermits.mockReturnValueOnce(p) - await findPermit(getMockPermission(), getMockRequest()) - expect(debugMock).toHaveBeenCalledWith('permit missing new cost details', expect.any(Object)) - }) - - it("doesn't log if newCost and newCostStartDate are defined", async () => { - filterPermits.mockReturnValueOnce({ newCost: 1, newCostStartDate: '2023-04-01' }) - await findPermit(getMockPermission(), getMockRequest()) - expect(debugMock).not.toHaveBeenCalledWith('permit missing new cost details', expect.any(Object)) - }) - - it('logs permit data present and up to date', async () => { - const mockPermission = getMockPermission() - mockPermission.hash = 'abc123' - await findPermit(mockPermission, getMockRequest()) - expect(debugMock).toHaveBeenCalledWith("permit data present and doesn't need updating") - }) - }) - - describe('when the permission has no hash', () => { - it('generates a new hash', async () => { - await findPermit(getMockPermission(), getMockRequest()) - - expect(crypto.createHash).toHaveBeenCalledWith('sha256') - }) - - it('updates the permission with the right permit', async () => { - const filteredPermit = Symbol('permit') - filterPermits.mockReturnValueOnce(filteredPermit) - - const permission = getMockPermission() - const request = getMockRequest() - await findPermit(permission, request) - - expect(request.cache.mock.results[0].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( - expect.objectContaining({ permit: filteredPermit }) - ) - }) + const getSamplePermission = overrides => ({ + licenceLength: '10Hours', + licenceType: 'type-3', + numberOfRods: '1', + ...overrides }) - - describe('when the permission already has a hash', () => { - it('generates a new hash', async () => { - const oldHash = crypto.createHash() - - const permission = getMockPermission({ hash: oldHash }) - await findPermit(permission, getMockRequest()) - - expect(crypto.createHash).toHaveBeenCalledWith('sha256') - }) - - describe('when the new hash does not match the existing hash', () => { - it('updates the permission with the permit and new hash', async () => { - const filteredPermit = Symbol('permit') - filterPermits.mockReturnValueOnce(filteredPermit) - const newHash = Symbol('new hash') - crypto.createHash.mockImplementation(getMockHashImplementation({ digest: () => newHash })) - - const permission = getMockPermission({ hash: Symbol('old hash') }) - const request = getMockRequest() - await findPermit(permission, request) - - const expectedData = { permit: filteredPermit, hash: newHash } - expect(request.cache.mock.results[0].value.helpers.transaction.setCurrentPermission).toHaveBeenCalledWith( - expect.objectContaining(expectedData) - ) - }) - }) - - describe('when the new hash matches the existing hash', () => { - it('does not update the permission', async () => { - const sameHash = Symbol('same hash') - crypto.createHash.mockImplementationOnce(getMockHashImplementation({ digest: () => sameHash })) - - const permission = getMockPermission({ hash: sameHash }) - const request = getMockRequest() - await findPermit(permission, request) - - expect(request.cache).not.toHaveBeenCalled() - }) - }) + const getSamplePermissionWithConcession = () => ({ + concessions: [{ type: 'concession-type-1' }], + licenceLength: '8M', + licenceType: 'type-1', + numberOfRods: '1' }) - const getMockPermission = (overrides = {}) => ({ - hash: null, - permit: jest.fn(), + const createExpectedPermit = ({ + numberOfRods = 1, + durationMagnitude = '10', + durationDescription = 'Hours', + permitSubtypeLabel = 'type-3', + ...overrides + } = {}) => ({ + numberOfRods, + durationMagnitude, + durationDesignator: { description: durationDescription }, + permitSubtype: { label: permitSubtypeLabel }, ...overrides }) - const getMockRequest = () => ({ - cache: jest.fn(() => ({ - helpers: { - transaction: { - setCurrentPermission: jest.fn() - } - } - })) + it.each` + description | expectedPermit | permission + ${'concessions'} | ${createExpectedPermit({ id: 'prm-111', durationMagnitude: '8', durationDescription: 'M', permitSubtypeLabel: 'type-1' })} | ${getSamplePermissionWithConcession()} + ${'licence type'} | ${createExpectedPermit({ id: 'prm-222', durationMagnitude: '17', durationDescription: 'Seconds', permitSubtypeLabel: 'type-2' })} | ${getSamplePermission({ licenceLength: '17Seconds', licenceType: 'type-2' })} + ${'licence length'} | ${createExpectedPermit({ id: 'prm-444' })} | ${getSamplePermission()} + ${'number of rods'} | ${createExpectedPermit({ id: 'prm-555', numberOfRods: 3 })} | ${getSamplePermission({ numberOfRods: '3' })} + `('matches a permission to a permit on $description', async ({ expectedPermit, permission }) => { + const permit = await findPermit(permission) + expect(permit).toStrictEqual(expect.objectContaining(expectedPermit)) }) }) diff --git a/packages/gafl-webapp-service/src/processors/__tests__/hash-permission.spec.js b/packages/gafl-webapp-service/src/processors/__tests__/hash-permission.spec.js new file mode 100644 index 0000000000..ea224f0d05 --- /dev/null +++ b/packages/gafl-webapp-service/src/processors/__tests__/hash-permission.spec.js @@ -0,0 +1,54 @@ +import crypto from 'crypto' +import hashPermission from '../hash-permission.js' + +jest.mock('crypto', () => ({ + createHash: jest.fn(() => ({ digest: () => 'aaa111', update: () => {} })) +})) + +describe('hash permission', () => { + beforeEach(jest.clearAllMocks) + it.each(['abc-123', 'def-910'])('creates hash (%s) for permission', hash => { + crypto.createHash.mockReturnValueOnce({ digest: () => hash, update: () => {} }) + const hashedPermission = hashPermission({}) + expect(hashedPermission).toBe(hash) + }) + + it('uses sha256 algorithm for hashing', () => { + hashPermission({}) + expect(crypto.createHash).toHaveBeenCalledWith('sha256') + }) + + it('uses hex encoding for hashing', () => { + const digest = jest.fn() + crypto.createHash.mockReturnValueOnce({ digest, update: () => {} }) + hashPermission({}) + expect(digest).toHaveBeenCalledWith('hex') + }) + + it.each([ + { isLicenceForYou: true, licenceToStart: 'after-payment', licenceStartDate: '2023-09-18' }, + { licenceType: 'Trout and coarse', numberOfRods: '2', licenceLength: '12M' }, + { licenceStartTime: null, numberOfRods: '3', licenceLength: '8D' } + ])('passes string representation of permission (%o) to hash ', samplePermission => { + const update = jest.fn() + crypto.createHash.mockReturnValueOnce({ digest: () => '', update }) + const jsonPermission = JSON.stringify(samplePermission) + hashPermission(samplePermission) + expect(update).toHaveBeenCalledWith(jsonPermission) + }) + + it.each` + keyToOmit | samplePermission + ${'hash'} | ${{ property1: '1', property2: 'Two', property3: 'Trois', hash: 'sdfghj345678' }} + ${'permit'} | ${{ property1: '1', property2: 'Two', property3: 'Trois', permit: { id: 'permit-1' } }} + ${'licensee'} | ${{ property1: '1', property2: 'Two', property3: 'Trois', licensee: { name: 'Brenin Pysgotwr' } }} + `("doesn't pass $keyToOmit to hash", ({ keyToOmit, samplePermission }) => { + const permissionCopy = { ...samplePermission } + delete permissionCopy[keyToOmit] + const update = jest.fn() + crypto.createHash.mockReturnValueOnce({ digest: () => '', update }) + const jsonPermission = JSON.stringify(permissionCopy) + hashPermission(samplePermission) + expect(update).toHaveBeenCalledWith(jsonPermission) + }) +}) diff --git a/packages/gafl-webapp-service/src/processors/__tests__/pricing-summary.spec.js b/packages/gafl-webapp-service/src/processors/__tests__/pricing-summary.spec.js index dc776c2eaf..bb5f1660f8 100644 --- a/packages/gafl-webapp-service/src/processors/__tests__/pricing-summary.spec.js +++ b/packages/gafl-webapp-service/src/processors/__tests__/pricing-summary.spec.js @@ -1,6 +1,6 @@ import { pricingDetail } from '../pricing-summary.js' -jest.mock('../filter-permits.js', () => ({ +jest.mock('../find-permit.js', () => ({ getPermitsJoinPermitConcessions: () => [ { id: '9d1b34a0-0c66-e611-80dc-c4346bad0190', diff --git a/packages/gafl-webapp-service/src/processors/filter-permits.js b/packages/gafl-webapp-service/src/processors/filter-permits.js deleted file mode 100644 index c4cef38bfe..0000000000 --- a/packages/gafl-webapp-service/src/processors/filter-permits.js +++ /dev/null @@ -1,38 +0,0 @@ -import { salesApi } from '@defra-fish/connectors-lib' - -export const getPermitsJoinPermitConcessions = async () => { - const permits = await salesApi.permits.getAll() - const permitConcessions = await salesApi.permitConcessions.getAll() - const concessions = await salesApi.concessions.getAll() - return permits.map(p => ({ - ...p, - concessions: permitConcessions.filter(pc => pc.permitId === p.id).map(pc => concessions.find(c => c.id === pc.concessionId)) - })) -} - -const filterPermits = async permission => { - const licenseeConcessions = permission.concessions || [] - const permitsJoinPermitConcessions = await getPermitsJoinPermitConcessions() - - // Filter the joined list to include every and only those concessions in licenseeConcessions - const filteredPermitsJoinPermitConcessions = permitsJoinPermitConcessions.filter( - pjpc => - licenseeConcessions.map(lc => lc.type).every(t => pjpc.concessions.map(c => c.name).includes(t)) && - pjpc.concessions.length === licenseeConcessions.length - ) - - // Filter by the licence length - const byLicenceLength = filteredPermitsJoinPermitConcessions.filter( - p => String(p.durationMagnitude + p.durationDesignator.description) === permission.licenceLength - ) - - // Filter by the licence (sub) type - const byLicenceType = byLicenceLength.filter(p => p.permitSubtype.label === permission.licenceType) - - // Filter by the number of rods - const byNumberOfRods = byLicenceType.filter(r => String(r.numberOfRods) === permission.numberOfRods) - - return byNumberOfRods[0] -} - -export default filterPermits diff --git a/packages/gafl-webapp-service/src/processors/find-permit.js b/packages/gafl-webapp-service/src/processors/find-permit.js index 5538bb451d..0347378e8a 100644 --- a/packages/gafl-webapp-service/src/processors/find-permit.js +++ b/packages/gafl-webapp-service/src/processors/find-permit.js @@ -1,45 +1,36 @@ -import filterPermits from './filter-permits.js' -import crypto from 'crypto' -import db from 'debug' -const debug = db('webapp:find-permit') +import { salesApi } from '@defra-fish/connectors-lib' -export const findPermit = async (permission, request) => { - /* - * To stop repeated reads of the API with users repeatably refreshing the page, the transaction cache stores - * a hash of itself. If the transaction cache has not changed the permit is not recalculated. - * - * The section of the transaction cache subject to the hashing algorithm excludes - * name, address, or anything not effecting permit filter - */ - const hashOperand = (({ hash: _hash, permit: _permit, licensee: _licensee, ...p }) => p)(permission) +export const getPermitsJoinPermitConcessions = async () => { + const permits = await salesApi.permits.getAll() + const permitConcessions = await salesApi.permitConcessions.getAll() + const concessions = await salesApi.concessions.getAll() + return permits.map(p => ({ + ...p, + concessions: permitConcessions.filter(pc => pc.permitId === p.id).map(pc => concessions.find(c => c.id === pc.concessionId)) + })) +} + +export default async permission => { + const licenseeConcessions = permission.concessions || [] + const permitsJoinPermitConcessions = await getPermitsJoinPermitConcessions() + + // Filter the joined list to include every and only those concessions in licenseeConcessions + const filteredPermitsJoinPermitConcessions = permitsJoinPermitConcessions.filter( + pjpc => + licenseeConcessions.map(lc => lc.type).every(t => pjpc.concessions.map(c => c.name).includes(t)) && + pjpc.concessions.length === licenseeConcessions.length + ) + + // Filter by the licence length + const byLicenceLength = filteredPermitsJoinPermitConcessions.filter( + p => String(p.durationMagnitude + p.durationDesignator.description) === permission.licenceLength + ) - // To calculate a permit, hash and save - const addHashAndPermit = async () => { - const permit = await filterPermits(permission) - permission.permit = permit - if (permit) { - if (!permit.newCostStartDate || !permit.newCost) { - debug('permit missing new cost details', permission) - } - } else { - debug("permit wasn't retrieved", permission) - } + // Filter by the licence (sub) type + const byLicenceType = byLicenceLength.filter(p => p.permitSubtype.label === permission.licenceType) - const hash = crypto.createHash('sha256') - hash.update(JSON.stringify(hashOperand)) - permission.hash = hash.digest('hex') - await request.cache().helpers.transaction.setCurrentPermission(permission) - } + // Filter by the number of rods + const byNumberOfRods = byLicenceType.filter(r => String(r.numberOfRods) === permission.numberOfRods) - if (!permission.hash) { - await addHashAndPermit() - } else { - const hash = crypto.createHash('sha256') - hash.update(JSON.stringify(hashOperand)) - if (hash.digest('hex') !== permission.hash) { - await addHashAndPermit() - } else { - debug("permit data present and doesn't need updating") - } - } + return byNumberOfRods[0] } diff --git a/packages/gafl-webapp-service/src/processors/hash-permission.js b/packages/gafl-webapp-service/src/processors/hash-permission.js new file mode 100644 index 0000000000..50d36e3b00 --- /dev/null +++ b/packages/gafl-webapp-service/src/processors/hash-permission.js @@ -0,0 +1,9 @@ +import crypto from 'crypto' + +const hashOperand = ({ hash: _hash, permit: _permit, licensee: _licensee, ...p }) => p + +export default permission => { + const hash = crypto.createHash('sha256') + hash.update(JSON.stringify(hashOperand(permission))) + return hash.digest('hex') +} diff --git a/packages/gafl-webapp-service/src/processors/pricing-summary.js b/packages/gafl-webapp-service/src/processors/pricing-summary.js index a5d179c8ed..3030d1db4f 100644 --- a/packages/gafl-webapp-service/src/processors/pricing-summary.js +++ b/packages/gafl-webapp-service/src/processors/pricing-summary.js @@ -3,7 +3,7 @@ */ import { licenseTypes } from '../pages/licence-details/licence-type/route.js' import { LICENCE_TYPE } from '../uri.js' -import { getPermitsJoinPermitConcessions } from './filter-permits.js' +import { getPermitsJoinPermitConcessions } from './find-permit.js' import * as concessionHelper from '../processors/concession-helper.js' import * as constants from './mapping-constants.js' import moment from 'moment'