From a236317e2fff20fb6f63e5c05882e76e537b63b7 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Fri, 3 Jan 2025 19:33:00 +0000 Subject: [PATCH] Rename Import job as Licence Changes and refactor (#1593) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://eaflood.atlassian.net/browse/WATER-4728 After completing [Remove the replacement legacy import code](https://github.com/DEFRA/water-abstraction-system/pull/1585) we could review the replacement 'import job' we created in a clearer light. The immediate thing that rang out was that we are not importing a license! The job is connected to the import licence process, but specifically, it cares about changes to the licence. Hence the name change. After that, we could see the job and the two downstream processes it kicks off from a bird's eye view. What jumped out is they were both doing there own variation of the same thing: working out what the 'change date' is. Plus, in the case of return logs it was re-quering the DB for information that could be retrieved in the job's `FetchLicenceService`. So, in this change we have refactored the job to handle both - determining that a licence has a changed 'end date' - determining what that changed date is As the job has this information, it can then pass it to the downstream services, which means we can delete the redundant code from them. Along the way, we do some housekeeping: updating the job to match the existing jobs, enhancing the comments, and fixing a whoopsie in the query `FetchLicencesServices` uses! 😱 😁 --- .env.example | 3 + app/controllers/check.controller.js | 30 +- app/controllers/jobs.controller.js | 8 +- app/routes/jobs.routes.js | 4 +- ...ermine-licence-end-date-changed.service.js | 73 ----- .../jobs/import/fetch-licences.service.js | 40 --- .../import/generate-return-logs.service.js | 46 --- .../import/process-import-licence.service.js | 44 --- ...e-earliest-licence-changed-date.service.js | 120 ++++++++ .../licence-changes/fetch-licences.service.js | 44 +++ .../process-licence-changes.service.js} | 52 ++-- .../process-licence.service.js | 48 +++ ...etermine-imported-licence-flags.service.js | 56 +--- .../process-billing-flag.service.js | 4 +- config/jobs.config.js | 4 +- test/controllers/jobs.controller.test.js | 36 +-- ...e-licence-end-date-changed.service.test.js | 118 -------- .../generate-return-logs.service.test.js | 206 ------------- .../import/import-licences.service.test.js | 129 -------- .../process-import-licence.service.test.js | 148 --------- ...liest-licence-changed-date.service.test.js | 283 ++++++++++++++++++ .../process-licence-changes.service.test.js | 150 ++++++++++ .../process-licence.service.test.js | 124 ++++++++ ...ine-imported-licence-flags.service.test.js | 52 ++-- .../process-billing-flag.service.test.js | 10 +- 25 files changed, 864 insertions(+), 968 deletions(-) delete mode 100644 app/services/jobs/import/determine-licence-end-date-changed.service.js delete mode 100644 app/services/jobs/import/fetch-licences.service.js delete mode 100644 app/services/jobs/import/generate-return-logs.service.js delete mode 100644 app/services/jobs/import/process-import-licence.service.js create mode 100644 app/services/jobs/licence-changes/determine-earliest-licence-changed-date.service.js create mode 100644 app/services/jobs/licence-changes/fetch-licences.service.js rename app/services/jobs/{import/import-licences.service.js => licence-changes/process-licence-changes.service.js} (50%) create mode 100644 app/services/jobs/licence-changes/process-licence.service.js delete mode 100644 test/services/jobs/import/determine-licence-end-date-changed.service.test.js delete mode 100644 test/services/jobs/import/generate-return-logs.service.test.js delete mode 100644 test/services/jobs/import/import-licences.service.test.js delete mode 100644 test/services/jobs/import/process-import-licence.service.test.js create mode 100644 test/services/jobs/licence-changes/determine-earliest-licence-changed-date.service.test.js create mode 100644 test/services/jobs/licence-changes/process-licence-changes.service.test.js create mode 100644 test/services/jobs/licence-changes/process-licence.service.test.js diff --git a/.env.example b/.env.example index 28407148c8..a291e82b85 100644 --- a/.env.example +++ b/.env.example @@ -61,6 +61,9 @@ REDIS_DISABLE_TLS=false BILLING_ANNUAL_BATCH_SIZE=5 BILLING_WAIT_FOR_STATUS_PAUSE_IN_MS=1000 +# Jobs config +JOBS_LICENCE_CHANGES_BATCH_SIZE=10 + # Cookie secret for authenticating requests coming via the UI. Note that the value should be the same as `COOKIE_SECRET` # on the UI side COOKIE_SECRET= diff --git a/app/controllers/check.controller.js b/app/controllers/check.controller.js index f9c1bb430c..38b463f217 100644 --- a/app/controllers/check.controller.js +++ b/app/controllers/check.controller.js @@ -11,11 +11,13 @@ const ProcessLicenceReturnLogsService = require('../services/return-logs/process const NO_CONTENT_STATUS_CODE = 204 /** - * A test end point for the licence supplementary billing flags process + * A test end point for checking licences with changed end dates on import are flagged for supplementary billing * - * This endpoint takes a licenceId and an expired, lapsed and revoked date. It passes this onto the - * `DetermineSupplementaryBillingFlagsService` to test if it correctly flags the licence for supplementary billing. This - * normally happens during the licence import process. + * This endpoint takes a licenceId and 'changeDate'. It passes this onto the + * `ProcessBillingFlagService` to test if it correctly flags the licence for supplementary billing. This would normally + * be done as part of `/jobs/licence-changes`. But that checks all licences and is driven by comparing NALD and WRLS. + * + * This endpoint allows us to test a licence in isolation, with a 'change date' of our choosing. * * @param request - the hapi request object * @param h - the hapi response object @@ -23,22 +25,18 @@ const NO_CONTENT_STATUS_CODE = 204 * @returns {Promise} - A promise that resolves to an HTTP response object with a 204 status code */ async function flagForBilling(request, h) { - const { licenceId, expiredDate, lapsedDate, revokedDate } = request.payload - - const payload = { - expiredDate: expiredDate ? new Date(expiredDate) : null, - lapsedDate: lapsedDate ? new Date(lapsedDate) : null, - licenceId, - revokedDate: revokedDate ? new Date(revokedDate) : null - } + const { licenceId, changeDate } = request.payload - await ProcessBillingFlagService.go(payload) + await ProcessBillingFlagService.go({ licenceId, changeDate: new Date(changeDate) }) return h.response().code(NO_CONTENT_STATUS_CODE) } /** - * A test end point to create return logs for a given licence reference + * A test end point for checking we correctly reissue the return logs for a given licence + * + * We always expect a `licenceId` and an optional `changeDate` to be passed in the request. If a `changeDate` is not + * provided, the current date will be used. * * @param request - the hapi request object * @param h - the hapi response object @@ -46,9 +44,9 @@ async function flagForBilling(request, h) { * @returns {Promise} - A promise that resolves to an HTTP response object with a 204 status code */ async function licenceReturnLogs(request, h) { - const { licenceId } = request.params + const { licenceId, changeDate } = request.payload - await ProcessLicenceReturnLogsService.go(licenceId) + await ProcessLicenceReturnLogsService.go(licenceId, changeDate ? new Date(changeDate) : null) return h.response().code(NO_CONTENT_STATUS_CODE) } diff --git a/app/controllers/jobs.controller.js b/app/controllers/jobs.controller.js index ef346a9c3c..0d4a174e1f 100644 --- a/app/controllers/jobs.controller.js +++ b/app/controllers/jobs.controller.js @@ -6,7 +6,7 @@ */ const ExportService = require('../services/jobs/export/export.service.js') -const ImportLicence = require('../services/jobs/import/import-licences.service.js') +const ProcessLicenceChanges = require('../services/jobs/licence-changes/process-licence-changes.service.js') const ProcessLicenceUpdates = require('../services/jobs/licence-updates/process-licence-updates.js') const ProcessReturnLogsService = require('../services/jobs/return-logs/process-return-logs.service.js') const ProcessSessionStorageCleanupService = require('../services/jobs/session-cleanup/process-session-storage-cleanup.service.js') @@ -30,8 +30,8 @@ async function exportDb(_request, h) { return h.response().code(NO_CONTENT_STATUS_CODE) } -async function ImportLicences(_request, h) { - ImportLicence.go() +async function licenceChanges(_request, h) { + ProcessLicenceChanges.go() return h.response().code(NO_CONTENT_STATUS_CODE) } @@ -68,7 +68,7 @@ async function returnLogs(request, h) { module.exports = { exportDb, - ImportLicences, + licenceChanges, licenceUpdates, returnLogs, sessionCleanup, diff --git a/app/routes/jobs.routes.js b/app/routes/jobs.routes.js index ef3eafea0e..28557db2f7 100644 --- a/app/routes/jobs.routes.js +++ b/app/routes/jobs.routes.js @@ -19,9 +19,9 @@ const routes = [ }, { method: 'POST', - path: '/jobs/import-licences', + path: '/jobs/licence-changes', options: { - handler: JobsController.ImportLicences, + handler: JobsController.licenceChanges, app: { plainOutput: true }, diff --git a/app/services/jobs/import/determine-licence-end-date-changed.service.js b/app/services/jobs/import/determine-licence-end-date-changed.service.js deleted file mode 100644 index 01b4c1748e..0000000000 --- a/app/services/jobs/import/determine-licence-end-date-changed.service.js +++ /dev/null @@ -1,73 +0,0 @@ -'use strict' - -/** - * Determines if an imported licence has a new end date - * @module DetermineLicenceEndDateChangedService - */ - -const LicenceModel = require('../../../models/licence.model.js') - -/** - * Determines if an imported licence has a new end date. - * - * This service is responsible for determining whether a licence imported has a new end date. - * - * It compares the licences end dates (such as lapsed, revoked or expired dates) between WRLS licence and the imported - * data. - * - * @param {object} importedLicence - The imported licence - * @param {string} licenceId - The UUID of the licence being updated by the import - * - * @returns {Promise} - */ -async function go(importedLicence, licenceId) { - const query = LicenceModel.query().select(['id']).where('id', licenceId) - - _whereClauses(query, importedLicence) - - const result = await query - - return result.length === 0 -} - -/** - * Adds where clauses to compare the end dates (expired, revoked, lapsed) of the imported licence with those stored - * in the database. It handles where the end dates can be null. - * - * In SQL, comparing `null` values using a regular `where` clause does not work as expected because - * `null` represents the absence of a value and `null = null` returns false. To address this, we use - * `whereNull` to explicitly check for null values in the database. - * - * If an end date is present on the imported licence, the query uses a standard `where` clause to check - * for a match. If the end date is null, the query uses `whereNull` to compare against the null values. - * - * This ensures that value types (dates and null) can be correctly compared, allowing us to detect changes - * between the imported licence and the existing WRLS licence data. - * - * @private - */ -function _whereClauses(query, importedLicence) { - const { expiredDate, lapsedDate, revokedDate } = importedLicence - - if (expiredDate) { - query.where('expiredDate', expiredDate) - } else { - query.whereNull('expiredDate') - } - - if (revokedDate) { - query.where('revokedDate', revokedDate) - } else { - query.whereNull('revokedDate') - } - - if (lapsedDate) { - query.where('lapsedDate', lapsedDate) - } else { - query.whereNull('lapsedDate') - } -} - -module.exports = { - go -} diff --git a/app/services/jobs/import/fetch-licences.service.js b/app/services/jobs/import/fetch-licences.service.js deleted file mode 100644 index cb98ec54b7..0000000000 --- a/app/services/jobs/import/fetch-licences.service.js +++ /dev/null @@ -1,40 +0,0 @@ -'use strict' - -/** - * Fetches all licences that are in NALD and WRLS - * @module FetchLicences - */ - -const { db } = require('../../../../db/db.js') - -/** - * Fetches all licences that are in NALD and WRLS - * - * A licence is valid if at least one licence version is not in draft - * - * @returns {Promise} - An array of licences - */ -async function go() { - const query = ` - SELECT DISTINCT ON (nal."LIC_NO") - l.id as id, - TO_DATE(NULLIF(nal."EXPIRY_DATE", 'null'), 'DD/MM/YYYY') AS expired_date, - TO_DATE(NULLIF(nal."LAPSED_DATE", 'null'), 'DD/MM/YYYY') AS lapsed_date, - TO_DATE(NULLIF(nal."EXPIRY_DATE", 'null'), 'DD/MM/YYYY') AS revoked_date - FROM import."NALD_ABS_LIC_VERSIONS" nalv - INNER JOIN import."NALD_ABS_LICENCES" nal - ON nal."ID" = nalv."AABL_ID" - AND nal."FGAC_REGION_CODE" = nalv."FGAC_REGION_CODE" - INNER JOIN - public.licences l ON l.licence_ref = nal."LIC_NO" - WHERE nalv."STATUS" <> 'DRAFT' - ` - - const { rows } = await db.raw(query) - - return rows -} - -module.exports = { - go -} diff --git a/app/services/jobs/import/generate-return-logs.service.js b/app/services/jobs/import/generate-return-logs.service.js deleted file mode 100644 index b30bba9fc4..0000000000 --- a/app/services/jobs/import/generate-return-logs.service.js +++ /dev/null @@ -1,46 +0,0 @@ -'use strict' - -/** - * Generate the return logs for a changed imported licence - * @module GenerateReturnLogsService - */ - -const { determineEarliestDate } = require('../../../lib/dates.lib.js') -const LicenceModel = require('../../../models/licence.model.js') -const ProcessLicenceReturnLogsService = require('../../return-logs/process-licence-return-logs.service.js') - -/** - * Determines if an imported licence has a changed end date. - * - * This service is responsible for determining whether a licence imported has a changed end date and therefore should - * generate new return logs. - * - * @param {string} licenceId - The UUID of the licence being updated by the import - * @param {object} importedLicence - The imported licence - */ -async function go(licenceId, importedLicence) { - try { - const { expiredDate, lapsedDate, revokedDate } = importedLicence - const existingEarliestEndDate = await _fetchLicenceEndDate(licenceId) - const earliestEndDate = determineEarliestDate([existingEarliestEndDate, expiredDate, lapsedDate, revokedDate]) - - await ProcessLicenceReturnLogsService.go(licenceId, earliestEndDate) - } catch (error) { - global.GlobalNotifier.omfg('Generate return logs on import failed ', { licenceId }, error) - } -} - -async function _fetchLicenceEndDate(licenceId) { - const { expiredDate, lapsedDate, revokedDate } = await LicenceModel.query() - .select(['expiredDate', 'lapsedDate', 'revokedDate']) - .where('id', licenceId) - .first() - - const earliestDate = determineEarliestDate([expiredDate, lapsedDate, revokedDate]) - - return earliestDate -} - -module.exports = { - go -} diff --git a/app/services/jobs/import/process-import-licence.service.js b/app/services/jobs/import/process-import-licence.service.js deleted file mode 100644 index d6f71d4eda..0000000000 --- a/app/services/jobs/import/process-import-licence.service.js +++ /dev/null @@ -1,44 +0,0 @@ -'use strict' - -/** - * Process licence for the licences imported from NALD - * @module ProcessImportLicence - */ - -const DetermineLicenceEndDateChangedService = require('./determine-licence-end-date-changed.service.js') -const ProcessBillingFlagService = require('../../licences/supplementary/process-billing-flag.service.js') -const GenerateReturnLogsService = require('./generate-return-logs.service.js') - -/** - * Process licence for the licences imported from NALD - * - * When the licence exists in the WRLS databse and we are improting from NALD. - * - * We need to check if the licence needs to be flagged for supplementary billing and - * determine if any new return logs need to be created depending on the current cycle. - * - * @param {object} licence - a licence - */ -async function go(licence) { - try { - const payload = { - expiredDate: licence.expired_date, - lapsedDate: licence.lapsed_date, - revokedDate: licence.revoked_date, - licenceId: licence.id - } - - const licenceEndDateChanged = await DetermineLicenceEndDateChangedService.go(payload, licence.id) - - if (licenceEndDateChanged) { - ProcessBillingFlagService.go(payload) - GenerateReturnLogsService.go(licence.id, payload) - } - } catch (error) { - global.GlobalNotifier.omfg(`Importing licence ${licence.id}`, null, error) - } -} - -module.exports = { - go -} diff --git a/app/services/jobs/licence-changes/determine-earliest-licence-changed-date.service.js b/app/services/jobs/licence-changes/determine-earliest-licence-changed-date.service.js new file mode 100644 index 0000000000..6191da992a --- /dev/null +++ b/app/services/jobs/licence-changes/determine-earliest-licence-changed-date.service.js @@ -0,0 +1,120 @@ +'use strict' + +/** + * Determines if a licence's 'end dates' have changed, and if more than one has, which was the earliest + * @module DetermineEarliestLicenceChangedDateService + */ + +const { determineEarliestDate } = require('../../../lib/dates.lib.js') + +/** + * Determines if a licence's 'end dates' have changed, and if more than one has, which was the earliest + * + * We need to know two things for each licence we process in the job. + * + * - has _any_ 'end date' changed? + * - if more than one has changed, which was the earliest? + * + * The answer to the first question determines if we bother to process the licence at all. If nothing has changed, we + * can skip the licence and move on to the next one. + * + * If something has changed, those downstream services will need to know what the 'change date' was. + * + * When it comes to determining the 'change date', we should always come back to a date, because if both were null then + * there would not be a change! + * + * - when the NALD date is less than WRLS; `changeDate` = `naldDate` + * - when the NALD date is greater than WRLS; `changeDate` = `wrlsDate` + * - else `changeDate` is whichever date is populated + * + * On that last example, if the NALD date is null (a user has unset it), then the 'change date' is whatever we have in + * WRLS. We are effectively saying, whatever decisions we made about the licence from 'x', are now open to change + * because it is no longer 'ended'. + * + * @param {object} licence - The licence object containing NALD and WRLS details to check for changed dates + * + * @returns {object|null} The earliest changed date if any changes are found, otherwise null + */ +function go(licence) { + const changedDates = _changedDates(licence) + + if (changedDates.length === 0) { + return null + } + + return _earliestChangedDate(changedDates) +} + +/** + * Handles the complexity of comparing dates and then determining what the 'change date' is + * + * In JavaScript, comparing date objects directly can lead to incorrect results, as two date objects, even with the same + * date and time, are treated as different objects. To avoid this, we convert the dates to strings for comparison. + * Normally, you might use getTime() to compare dates, but since any of these values can be null, calling getTime() on a + * null value would result in an error. Using strings safely handles null values. + * + * @private + */ +function _changedDate(dateType, naldDate, wrlsDate) { + if (String(naldDate) === String(wrlsDate)) { + return null + } + + const changeDate = determineEarliestDate([naldDate, wrlsDate]) + + return { changeDate, dateType, naldDate, wrlsDate } +} + +function _changedDates(licence) { + const { + nald_expired_date: naldExpiredDate, + nald_lapsed_date: naldLapsedDate, + nald_revoked_date: naldRevokedDate, + wrls_expired_date: wrlsExpiredDate, + wrls_lapsed_date: wrlsLapsedDate, + wrls_revoked_date: wrlsRevokedDate + } = licence + const changedDates = [] + + // Expired + let changedDate = _changedDate('expired', naldExpiredDate, wrlsExpiredDate) + if (changedDate) { + changedDates.push(changedDate) + } + + // Lapsed + changedDate = _changedDate('lapsed', naldLapsedDate, wrlsLapsedDate) + if (changedDate) { + changedDates.push(changedDate) + } + + // Revoked + changedDate = _changedDate('revoked', naldRevokedDate, wrlsRevokedDate) + if (changedDate) { + changedDates.push(changedDate) + } + + return changedDates +} + +function _earliestChangedDate(changedDates) { + // NOTE: sort() sorts the elements 'in place' + // + // It should return a number where: + // + // - a negative number means `a` should come before `b` + // - a positive number means `a` should come after `b` + // - 0 means they are equal and the order should not change + // + // The MDN docs say to memorise this use `(a, b) => a - b` to sort in ascending order. Hence, we can simplify our + // date sort by converting the dates to milliseconds and then subtracting them to get the result sort() needs. + changedDates.sort((changedDateA, changedDateB) => { + return changedDateA.changeDate.getTime() - changedDateB.changeDate.getTime() + }) + + return changedDates[0] +} + +module.exports = { + go +} diff --git a/app/services/jobs/licence-changes/fetch-licences.service.js b/app/services/jobs/licence-changes/fetch-licences.service.js new file mode 100644 index 0000000000..b6fc42da3b --- /dev/null +++ b/app/services/jobs/licence-changes/fetch-licences.service.js @@ -0,0 +1,44 @@ +'use strict' + +/** + * Fetches all licences that are in NALD, joins them to those in WRLS, and returns the end dates for both + * @module FetchLicences + */ + +const { db } = require('../../../../db/db.js') + +/** + * Fetches all licences that are in NALD, joins them to those in WRLS, and returns the end dates for both + * + * A licence is valid if at least one licence version is not in draft + * + * @returns {Promise} - An array of objects representing each licence, including end dates for both and the + * WRLS ID + */ +async function go() { + const query = ` + SELECT DISTINCT ON (nal."LIC_NO") + l.id AS id, + TO_DATE(NULLIF(nal."EXPIRY_DATE", 'null'), 'DD/MM/YYYY') AS nald_expired_date, + TO_DATE(NULLIF(nal."LAPSED_DATE", 'null'), 'DD/MM/YYYY') AS nald_lapsed_date, + TO_DATE(NULLIF(nal."REV_DATE", 'null'), 'DD/MM/YYYY') AS nald_revoked_date, + l.expired_date AS wrls_expired_date, + l.lapsed_date AS wrls_lapsed_date, + l.revoked_date AS wrls_revoked_date + FROM "import"."NALD_ABS_LIC_VERSIONS" nalv + INNER JOIN "import"."NALD_ABS_LICENCES" nal + ON nal."ID" = nalv."AABL_ID" + AND nal."FGAC_REGION_CODE" = nalv."FGAC_REGION_CODE" + INNER JOIN + public.licences l ON l.licence_ref = nal."LIC_NO" + WHERE nalv."STATUS" = 'CURR'; + ` + + const { rows } = await db.raw(query) + + return rows +} + +module.exports = { + go +} diff --git a/app/services/jobs/import/import-licences.service.js b/app/services/jobs/licence-changes/process-licence-changes.service.js similarity index 50% rename from app/services/jobs/import/import-licences.service.js rename to app/services/jobs/licence-changes/process-licence-changes.service.js index 65f11827ba..b299c1cc9e 100644 --- a/app/services/jobs/import/import-licences.service.js +++ b/app/services/jobs/licence-changes/process-licence-changes.service.js @@ -1,24 +1,18 @@ 'use strict' /** - * Extracts and imports licence from NALD - * @module ImportLicenceService + * Compares all licences in the NALD extract with those in WRLS and processes any with changed end dates + * @module ProcessLicenceChangesService */ -let pMap -;(async () => { - pMap = (await import('p-map')).default -})() - const FetchLicences = require('./fetch-licences.service.js') -const ProcessImportLicence = require('./process-import-licence.service.js') -const config = require('../../../../config/jobs.config.js') const { calculateAndLogTimeTaken, currentTimeInNanoseconds } = require('../../../lib/general.lib.js') +const ProcessLicenceService = require('./process-licence.service.js') -const { batchSize } = config.importLicence +const JobConfig = require('../../../../config/jobs.config.js') /** - * Processes NALD licences due for import on a nightly basis + * Compares all licences in the NALD extract with those in WRLS and processes any with changed end dates * * Overnight the {@link https://github.com/DEFRA/water-abstraction-import | water-abstraction-import} app imports each * abstraction licence found in NALD. New licences are added, existing ones are updated. @@ -38,44 +32,48 @@ const { batchSize } = config.importLicence * runs. This is so these processes can see the differences between the NALD licence record and ours, to determine * if and what they need to do. * - * > If a licence in NALD does not have a status of DRAFT, and at least one non-draft licence version then it will be + * > If a licence in NALD does not have a status of DRAFT, and at least one current licence version then it will be * excluded - * */ async function go() { try { const startTime = currentTimeInNanoseconds() + // Fetch all licences from NALD and WRLS const licences = await FetchLicences.go() + // Process any licences that have changed end dates await _processLicences(licences) - calculateAndLogTimeTaken(startTime, `Importing ${licences.length} licences from NALD`) + // Log the time it took to complete the job + calculateAndLogTimeTaken(startTime, 'Licence changes job complete', { count: licences.length }) } catch (error) { - global.GlobalNotifier.omfg('Importing Licence job failed', null, error) + // Log any errors that occur + global.GlobalNotifier.omfg('Licence changes job failed', null, error) } } /** - * Process Licences + * Iterate over the licences in batches and process them + * + * {@link https://github.com/sindresorhus/p-map | p-map} is a dependency built by the same person who built + * {@link https://github.com/sindresorhus/got | Got}. * - * When we process the licences we want to batch them into smaller chunks in an attempt to make the - * import process as efficient as possible + * > Useful when you need to run promise-returning & async functions multiple times with different inputs concurrently. + * > This is different from Promise.all() in that you can control the concurrency * - * @param {object[]} licences - An array of licences + * We had previously crafted something similar, but that involved iterating an array in slices, and then passing the + * 'slices' to Promise.all(). p-map does the same thing, but much cleaner and the intent is clearer. * * @private */ async function _processLicences(licences) { - if (!pMap) { - throw new Error('pMap is not yet loaded.') - } - - const mapper = async (licence) => { - return ProcessImportLicence.go(licence) - } + // The pMap dependency does not support CJS modules. This causes us a problem as we are locked into + // using these for the time being. We've used the same workaround we used for Got (built by the same person) in + // app/requests/base.request.js + const pMap = (await import('p-map')).default - await pMap(licences, mapper, { concurrency: batchSize }) + await pMap(licences, ProcessLicenceService.go, { concurrency: JobConfig.licenceChanges.batchSize }) } module.exports = { diff --git a/app/services/jobs/licence-changes/process-licence.service.js b/app/services/jobs/licence-changes/process-licence.service.js new file mode 100644 index 0000000000..c77722ab59 --- /dev/null +++ b/app/services/jobs/licence-changes/process-licence.service.js @@ -0,0 +1,48 @@ +'use strict' + +/** + * Check the end dates of the licence between NALD and WRLS and process if necessary + * @module ProcessLicenceChangesService + */ + +const DetermineEarliestLicenceChangedDateService = require('./determine-earliest-licence-changed-date.service.js') +const ProcessBillingFlagService = require('../../licences/supplementary/process-billing-flag.service.js') +const ProcessLicenceReturnLogsService = require('../../return-logs/process-licence-return-logs.service.js') + +const FeatureFlags = require('../../../../config/feature-flags.config.js') + +/** + * Check the end dates of the licence between NALD and WRLS and process if necessary + * + * @param {object} licence - The licence to process + */ +async function go(licence) { + try { + const changedDateDetails = DetermineEarliestLicenceChangedDateService.go(licence) + + if (!changedDateDetails) { + return + } + + await _supplementaryBillingFlags(licence, changedDateDetails) + await _returnLogs(licence, changedDateDetails) + } catch (error) { + global.GlobalNotifier.omfg('Licence changes processing failed', { id: licence.id }, error) + } +} + +async function _returnLogs(licence, changedDateDetails) { + if (FeatureFlags.enableRequirementsForReturns) { + await ProcessLicenceReturnLogsService.go(licence.id, changedDateDetails.changeDate) + } +} + +async function _supplementaryBillingFlags(licence, changedDateDetails) { + const payload = { licenceId: licence.id, changedDateDetails } + + await ProcessBillingFlagService.go(payload) +} + +module.exports = { + go +} diff --git a/app/services/licences/supplementary/determine-imported-licence-flags.service.js b/app/services/licences/supplementary/determine-imported-licence-flags.service.js index 140f5f5bd3..3e5094ae17 100644 --- a/app/services/licences/supplementary/determine-imported-licence-flags.service.js +++ b/app/services/licences/supplementary/determine-imported-licence-flags.service.js @@ -1,8 +1,7 @@ 'use strict' /** - * Determines if a licence should be flagged for supplementary billing based on changes to the expired/lapsed/revoked - * dates + * Determines if a licence should be flagged for supplementary billing based on changes to its 'end dates' * @module DetermineImportedLicenceFlagsService */ @@ -12,8 +11,7 @@ const { determineCurrentFinancialYear } = require('../../../lib/general.lib.js') const SROC_START_DATE = new Date('2022-04-01') /** - * Determines if a licence should be flagged for supplementary billing based on changes to the expired/lapsed/revoked - * dates + * Determines if a licence should be flagged for supplementary billing based on changes to its 'end dates' * * This service is triggered when an end date change has already been confirmed and determines the appropriate * supplementary billing flags (Pre-SROC, SROC, or two-part tariff) for the licence. It compares the updated end @@ -34,21 +32,20 @@ const SROC_START_DATE = new Date('2022-04-01') * licence doesn't have the correct charge versions to get picked up in a bill run. The service this passes * back to always persists the flags it receives so we do this check before we check the dates. * - * @param {object} importedLicence - Object representing the updated licence being imported - * @param {string} licenceId - The UUID of the licence being imported + * @param {string} licenceId - The UUID of the imported licence with a changed end date + * @param {Date} changeDate - The date of the change * * @returns {Promise} A promise is returned but it does not resolve to anything we expect the caller to use */ -async function go(importedLicence, licenceId) { +async function go(licenceId, changeDate) { const existingLicenceDetails = await FetchExistingLicenceDetailsService.go(licenceId) const { endDate } = determineCurrentFinancialYear() - const earliestChangedDate = _earliestChangedDate(importedLicence, existingLicenceDetails, endDate) const { flagForSrocSupplementary, flagForPreSrocSupplementary } = _determineExistingFlags(existingLicenceDetails) const result = { licenceId: existingLicenceDetails.id, regionId: existingLicenceDetails.region_id, - startDate: earliestChangedDate, + startDate: changeDate > endDate ? null : changeDate, endDate, flagForPreSrocSupplementary, flagForSrocSupplementary, @@ -57,11 +54,11 @@ async function go(importedLicence, licenceId) { // If not set it means none of the dates changed were before the current financial year end so there is no reason // to change anything on the flags - if (!earliestChangedDate) { + if (!result.startDate) { return result } - _updateFlags(earliestChangedDate, existingLicenceDetails, flagForPreSrocSupplementary, result) + _updateFlags(existingLicenceDetails, flagForPreSrocSupplementary, result) return result } @@ -87,39 +84,6 @@ function _determineExistingFlags(existingLicenceDetails) { return { flagForSrocSupplementary, flagForPreSrocSupplementary } } -function _earliestChangedDate(importedLicence, existingLicenceDetails, currentFinancialYearEndDate) { - const changedDates = [] - - let date - - // NOTE: In JavaScript, comparing date objects directly can lead to incorrect results, as two date objects, even with - // the same date and time, are treated as different objects. To avoid this, we convert the dates to strings for - // comparison. Normally, you might use getTime() to compare dates, but since any of these values can be null, calling - // getTime() on a null value would result in an error. Using strings safely handles null values. - if (String(importedLicence.expiredDate) !== String(existingLicenceDetails.expired_date)) { - date = importedLicence.expiredDate ?? existingLicenceDetails.expired_date - changedDates.push(date) - } - - if (String(importedLicence.lapsedDate) !== String(existingLicenceDetails.lapsed_date)) { - date = importedLicence.lapsedDate ?? existingLicenceDetails.lapsed_date - changedDates.push(date) - } - - if (String(importedLicence.revokedDate) !== String(existingLicenceDetails.revoked_date)) { - date = importedLicence.revokedDate ?? existingLicenceDetails.revoked_date - changedDates.push(date) - } - - // Filter out those greater than the current financial year end date - const filteredDates = changedDates.filter((changedDate) => { - return changedDate < currentFinancialYearEndDate - }) - - // Now work out the earliest end date from those that have changed - return filteredDates.length > 0 ? new Date(Math.min(...filteredDates)) : null -} - /** * Updates the supplementary billing flags for a licence based on the earliest changed date and existing licence details * @@ -139,11 +103,11 @@ function _earliestChangedDate(importedLicence, existingLicenceDetails, currentFi * * @private */ -function _updateFlags(earliestChangedDate, existingLicenceDetails, flagForPreSrocSupplementary, result) { +function _updateFlags(existingLicenceDetails, flagForPreSrocSupplementary, result) { if (!flagForPreSrocSupplementary) { const { pre_sroc_charge_versions: chargeVersions } = existingLicenceDetails - result.flagForPreSrocSupplementary = chargeVersions && earliestChangedDate < SROC_START_DATE + result.flagForPreSrocSupplementary = chargeVersions && result.startDate < SROC_START_DATE } result.flagForSrocSupplementary = existingLicenceDetails.sroc_charge_versions diff --git a/app/services/licences/supplementary/process-billing-flag.service.js b/app/services/licences/supplementary/process-billing-flag.service.js index 6df42616c7..3d5696e3f8 100644 --- a/app/services/licences/supplementary/process-billing-flag.service.js +++ b/app/services/licences/supplementary/process-billing-flag.service.js @@ -60,8 +60,8 @@ async function go(payload) { * @private */ async function _determineFlags(payload) { - if (payload.importedLicence) { - return DetermineImportedLicenceFlagsService.go(payload.importedLicence, payload.licenceId) + if (payload.changedDateDetails) { + return DetermineImportedLicenceFlagsService.go(payload.licenceId, payload.changedDateDetails.changeDate) } if (payload.chargeVersionId) { return DetermineChargeVersionFlagsService.go(payload.chargeVersionId) diff --git a/config/jobs.config.js b/config/jobs.config.js index b3ea3f5568..907e5799b4 100644 --- a/config/jobs.config.js +++ b/config/jobs.config.js @@ -10,8 +10,8 @@ require('dotenv').config() const config = { - importLicence: { - batchSize: parseInt(process.env.IMORT_LICENCE_BATCH_SIZE) || 10 + licenceChanges: { + batchSize: parseInt(process.env.JOBS_LICENCE_CHANGES_BATCH_SIZE) || 10 } } diff --git a/test/controllers/jobs.controller.test.js b/test/controllers/jobs.controller.test.js index f288de20c1..e74dac6ef6 100644 --- a/test/controllers/jobs.controller.test.js +++ b/test/controllers/jobs.controller.test.js @@ -10,7 +10,7 @@ const { expect } = Code // Things we need to stub const ExportService = require('../../app/services/jobs/export/export.service.js') -const ImportLicence = require('../../app/services/jobs/import/import-licences.service.js') +const ProcessLicenceChanges = require('../../app/services/jobs/licence-changes/process-licence-changes.service.js') const ProcessLicenceUpdatesService = require('../../app/services/jobs/licence-updates/process-licence-updates.js') const ProcessReturnLogsService = require('../../app/services/jobs/return-logs/process-return-logs.service.js') const ProcessSessionStorageCleanupService = require('../../app/services/jobs/session-cleanup/process-session-storage-cleanup.service.js') @@ -29,9 +29,9 @@ describe('Jobs controller', () => { }) beforeEach(async () => { - // We silence any calls to server.logger.error made in the plugin to try and keep the test output as clean as - // possible + // We silence any calls to server.logger.error and info to try and keep the test output as clean as possible Sinon.stub(server.logger, 'error') + Sinon.stub(server.logger, 'info') // We silence sending a notification to our Errbit instance using Airbrake Sinon.stub(server.app.airbrake, 'notify').resolvesThis() @@ -61,15 +61,15 @@ describe('Jobs controller', () => { }) }) - describe('/jobs/import-licence', () => { + describe('/jobs/licence-changes', () => { describe('POST', () => { beforeEach(() => { - options = { method: 'POST', url: '/jobs/import-licences' } + options = { method: 'POST', url: '/jobs/licence-changes' } }) describe('when the request succeeds', () => { beforeEach(async () => { - Sinon.stub(ImportLicence, 'go').resolves() + Sinon.stub(ProcessLicenceChanges, 'go').resolves() }) it('returns a 204 response', async () => { @@ -162,30 +162,6 @@ describe('Jobs controller', () => { }) }) - describe('when the licence reference is known', () => { - describe('POST', () => { - beforeEach(() => { - options = { - method: 'POST', - url: '/jobs/return-logs/summer', - payload: { licenceReference: 'AT/Test' } - } - }) - - describe('when the request succeeds', () => { - beforeEach(async () => { - Sinon.stub(ProcessReturnLogsService, 'go').resolves() - }) - - it('returns a 204 response', async () => { - const response = await server.inject(options) - - expect(response.statusCode).to.equal(204) - }) - }) - }) - }) - describe('when the requested cycle is all-year', () => { describe('POST', () => { beforeEach(() => { diff --git a/test/services/jobs/import/determine-licence-end-date-changed.service.test.js b/test/services/jobs/import/determine-licence-end-date-changed.service.test.js deleted file mode 100644 index 388ccda2f8..0000000000 --- a/test/services/jobs/import/determine-licence-end-date-changed.service.test.js +++ /dev/null @@ -1,118 +0,0 @@ -'use strict' - -// Test framework dependencies -const Lab = require('@hapi/lab') -const Code = require('@hapi/code') -const Sinon = require('sinon') - -const { describe, it, before, beforeEach, afterEach } = (exports.lab = Lab.script()) -const { expect } = Code - -// Test helpers -const LicenceHelper = require('../../../support/helpers/licence.helper.js') - -// Thing under test -const DetermineLicenceEndDateChangedService = require('../../../../app/services/jobs/import/determine-licence-end-date-changed.service.js') - -describe('Determine Licence End Date Changed Service', () => { - const lapsedDate = new Date('2023-01-01') - const revokedDate = new Date('2023-01-01') - const expiredDate = new Date('2023-01-01') - - let existingLicenceNullDates - let existingLicencePopulatedDates - let notifierStub - let importedLicence - - before(async () => { - existingLicenceNullDates = await LicenceHelper.add() - existingLicencePopulatedDates = await LicenceHelper.add({ expiredDate, lapsedDate, revokedDate }) - }) - - beforeEach(() => { - // The service depends on GlobalNotifier to have been set. This happens in app/plugins/global-notifier.plugin.js - // when the app starts up and the plugin is registered. As we're not creating an instance of Hapi server in this - // test we recreate the condition by setting it directly with our own stub - notifierStub = { omg: Sinon.stub(), omfg: Sinon.stub() } - global.GlobalNotifier = notifierStub - }) - - afterEach(async () => { - Sinon.restore() - }) - - describe('when the existing version of the licence', () => { - describe('matches the imported version of the licence', () => { - describe('because all the dates are null', () => { - before(() => { - importedLicence = { expiredDate: null, lapsedDate: null, revokedDate: null } - }) - - it('does not call ProcessLicenceReturnLogsService', async () => { - const result = await DetermineLicenceEndDateChangedService.go(importedLicence, existingLicenceNullDates.id) - - expect(result).to.be.false() - }) - }) - - describe('because all the dates match', () => { - before(() => { - importedLicence = { expiredDate, lapsedDate, revokedDate } - }) - - it('does not call ProcessLicenceReturnLogsService', async () => { - const result = await DetermineLicenceEndDateChangedService.go( - importedLicence, - existingLicencePopulatedDates.id - ) - - expect(result).to.be.false() - }) - }) - }) - - describe('does not match the imported version of the licence', () => { - describe('because the imported version has an end date where the existing version has null', () => { - before(() => { - importedLicence = { expiredDate, lapsedDate: null, revokedDate: null } - }) - - it('calls ProcessImportedLicenceService to handle what supplementary flags are needed', async () => { - const result = await DetermineLicenceEndDateChangedService.go(importedLicence, existingLicenceNullDates.id) - - expect(result).to.be.true() - }) - }) - - describe('because the imported version has a null end date where the existing version has one', () => { - before(() => { - importedLicence = { expiredDate, lapsedDate: null, revokedDate } - }) - - it('calls ProcessImportedLicenceService to handle what supplementary flags are needed', async () => { - const result = await DetermineLicenceEndDateChangedService.go( - importedLicence, - existingLicencePopulatedDates.id - ) - - expect(result).to.be.true() - }) - }) - - describe('because the imported version has a different end date to the existing version', () => { - before(() => { - importedLicence = { expiredDate, lapsedDate, revokedDate: new Date('2023-02-02') } - }) - - it('calls ProcessImportedLicenceService to handle what supplementary flags are needed', async () => { - const result = await DetermineLicenceEndDateChangedService.go( - importedLicence, - existingLicencePopulatedDates.id - ) - - expect(result).to.be.true() - }) - }) - }) - }) -}) diff --git a/test/services/jobs/import/generate-return-logs.service.test.js b/test/services/jobs/import/generate-return-logs.service.test.js deleted file mode 100644 index dd47d88f39..0000000000 --- a/test/services/jobs/import/generate-return-logs.service.test.js +++ /dev/null @@ -1,206 +0,0 @@ -'use strict' - -// Test framework dependencies -const Lab = require('@hapi/lab') -const Code = require('@hapi/code') -const Sinon = require('sinon') - -const { describe, it, before, beforeEach, afterEach } = (exports.lab = Lab.script()) -const { expect } = Code - -// Test helpers -const LicenceHelper = require('../../../support/helpers/licence.helper.js') - -// Things we need to stub -const ProcessLicenceReturnLogsService = require('../../../../app/services/return-logs/process-licence-return-logs.service.js') - -// Thing under test -const GenerateReturnLogsService = require('../../../../app/services/jobs/import/generate-return-logs.service.js') - -describe('Generate Return Logs Service', () => { - const changeDate = new Date('2024-05-26') - const olderChangeDate = new Date('2024-05-20') - const currentDate = new Date('2024-07-15') - - let clock - let importedLicence - let licence - let notifierStub - let processLicenceReturnLogsStub - - beforeEach(() => { - // We control what the 'current' date is, so we can assert what the service does when not provided with `changeDate` - clock = Sinon.useFakeTimers(currentDate) - processLicenceReturnLogsStub = Sinon.stub(ProcessLicenceReturnLogsService, 'go').resolves() - notifierStub = { omg: Sinon.stub(), omfg: Sinon.stub() } - global.GlobalNotifier = notifierStub - }) - - afterEach(() => { - Sinon.restore() - clock.restore() - delete global.GlobalNotifier - }) - - describe('when the imported licence has no end date and the existing licence has an expiredDate', () => { - before(async () => { - licence = await LicenceHelper.add({ expiredDate: changeDate, lapsedDate: null, revokedDate: null }) - importedLicence = { expiredDate: null, lapsedDate: null, revokedDate: null } - }) - - it('should call the GenerateReturnLogsService with the expiredDate', async () => { - await GenerateReturnLogsService.go(licence.id, importedLicence) - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - - const processReturnLogArgs = processLicenceReturnLogsStub.firstCall.args - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - expect(processReturnLogArgs[1]).to.equal(changeDate) - }) - }) - - describe('when the imported licence has no end date and the existing licence has a lapsedDate', () => { - before(async () => { - licence = await LicenceHelper.add({ expiredDate: null, lapsedDate: changeDate, revokedDate: null }) - importedLicence = { expiredDate: null, lapsedDate: null, revokedDate: null } - }) - - it('should call the GenerateReturnLogsService with the lapsedDate', async () => { - await GenerateReturnLogsService.go(licence.id, importedLicence) - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - - const processReturnLogArgs = processLicenceReturnLogsStub.firstCall.args - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - expect(processReturnLogArgs[1]).to.equal(changeDate) - }) - }) - - describe('when the imported licence has no end date and the existing licence has a revokedDate', () => { - before(async () => { - licence = await LicenceHelper.add({ expiredDate: null, lapsedDate: null, revokedDate: changeDate }) - importedLicence = { expiredDate: null, lapsedDate: null, revokedDate: null } - }) - - it('should call the GenerateReturnLogsService with the revokedDate', async () => { - await GenerateReturnLogsService.go(licence.id, importedLicence) - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - - const processReturnLogArgs = processLicenceReturnLogsStub.firstCall.args - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - expect(processReturnLogArgs[1]).to.equal(changeDate) - }) - }) - - describe('when the imported licence has an expiredDate and the existing licence has no end date', () => { - before(async () => { - licence = await LicenceHelper.add({ expiredDate: null, lapsedDate: null, revokedDate: null }) - importedLicence = { expiredDate: changeDate, lapsedDate: null, revokedDate: null } - }) - - it('should call the GenerateReturnLogsService with the expiredDate', async () => { - await GenerateReturnLogsService.go(licence.id, importedLicence) - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - - const processReturnLogArgs = processLicenceReturnLogsStub.firstCall.args - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - expect(processReturnLogArgs[1]).to.equal(changeDate) - }) - }) - - describe('when the imported licence has a lapsedDate and the existing licence has no end date', () => { - before(async () => { - licence = await LicenceHelper.add({ expiredDate: null, lapsedDate: null, revokedDate: null }) - importedLicence = { expiredDate: null, lapsedDate: changeDate, revokedDate: null } - }) - - it('should call the GenerateReturnLogsService with the lapsedDate', async () => { - await GenerateReturnLogsService.go(licence.id, importedLicence) - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - - const processReturnLogArgs = processLicenceReturnLogsStub.firstCall.args - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - expect(processReturnLogArgs[1]).to.equal(changeDate) - }) - }) - - describe('when the imported licence has a revokedDate and the existing licence has no end date', () => { - before(async () => { - licence = await LicenceHelper.add({ expiredDate: null, lapsedDate: null, revokedDate: null }) - importedLicence = { expiredDate: null, lapsedDate: null, revokedDate: changeDate } - }) - - it('should call the GenerateReturnLogsService with the revokedDate', async () => { - await GenerateReturnLogsService.go(licence.id, importedLicence) - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - - const processReturnLogArgs = processLicenceReturnLogsStub.firstCall.args - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - expect(processReturnLogArgs[1]).to.equal(changeDate) - }) - }) - - describe('when the imported licence has a revokedDate and the existing licence has an older revokedDate', () => { - before(async () => { - licence = await LicenceHelper.add({ expiredDate: null, lapsedDate: null, revokedDate: olderChangeDate }) - importedLicence = { expiredDate: null, lapsedDate: null, revokedDate: changeDate } - }) - - it('should call the GenerateReturnLogsService with the earlier revokedDate', async () => { - await GenerateReturnLogsService.go(licence.id, importedLicence) - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - - const processReturnLogArgs = processLicenceReturnLogsStub.firstCall.args - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - expect(processReturnLogArgs[1]).to.equal(olderChangeDate) - }) - }) - - describe('when the imported licence has an older revokedDate and the existing licence has a revokedDate', () => { - before(async () => { - licence = await LicenceHelper.add({ expiredDate: null, lapsedDate: null, revokedDate: changeDate }) - importedLicence = { expiredDate: null, lapsedDate: null, revokedDate: olderChangeDate } - }) - - it('should call the GenerateReturnLogsService with the earlier revokedDate', async () => { - await GenerateReturnLogsService.go(licence.id, importedLicence) - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - - const processReturnLogArgs = processLicenceReturnLogsStub.firstCall.args - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - expect(processReturnLogArgs[1]).to.equal(olderChangeDate) - }) - }) - - describe('when the imported licence has an older revokedDate and the existing licence has a revokedDate, expiredDate and lapsedDate', () => { - before(async () => { - licence = await LicenceHelper.add({ expiredDate: changeDate, lapsedDate: changeDate, revokedDate: changeDate }) - importedLicence = { expiredDate: null, lapsedDate: null, revokedDate: olderChangeDate } - }) - - it('should call the GenerateReturnLogsService with the earlier revokedDate', async () => { - await GenerateReturnLogsService.go(licence.id, importedLicence) - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - - const processReturnLogArgs = processLicenceReturnLogsStub.firstCall.args - - expect(processLicenceReturnLogsStub.callCount).to.equal(1) - expect(processReturnLogArgs[1]).to.equal(olderChangeDate) - }) - }) -}) diff --git a/test/services/jobs/import/import-licences.service.test.js b/test/services/jobs/import/import-licences.service.test.js deleted file mode 100644 index b1b2b5e2f3..0000000000 --- a/test/services/jobs/import/import-licences.service.test.js +++ /dev/null @@ -1,129 +0,0 @@ -'use strict' - -// Test framework dependencies -const Lab = require('@hapi/lab') -const Code = require('@hapi/code') -const Sinon = require('sinon') - -const { describe, it, beforeEach, afterEach } = (exports.lab = Lab.script()) -const { expect } = Code - -// Things we need to stub -const FetchLicences = require('../../../../app/services/jobs/import/fetch-licences.service.js') -const ProcessImportLicence = require('../../../../app/services/jobs/import/process-import-licence.service.js') -const config = require('../../../../config/jobs.config.js') -const { generateUUID } = require('../../../../app/lib/general.lib.js') - -// Thing under test -const ImportLicenceService = require('../../../../app/services/jobs/import/import-licences.service.js') - -describe('Import Licence Service', () => { - const batchSize = 10 - - let stubFetchLicences - let stubProcessImportLicence - let notifierStub - let licences - - beforeEach(async () => { - config.importLicence.batchSize = batchSize - - licences = _licences() - - stubFetchLicences = Sinon.stub(FetchLicences, 'go').resolves(licences) - - notifierStub = { omg: Sinon.stub(), omfg: Sinon.stub() } - global.GlobalNotifier = notifierStub - }) - - afterEach(() => { - Sinon.restore() - delete global.GlobalNotifier - }) - - describe('when processing the import licences', () => { - beforeEach(() => { - stubProcessImportLicence = Sinon.stub(ProcessImportLicence, 'go').resolves() - }) - - it('fetches the nald licence data', async () => { - await ImportLicenceService.go() - - expect(stubFetchLicences.calledOnce).to.be.true() - }) - - it('logs to highlight the amount of licences being imported', async () => { - await ImportLicenceService.go() - - const args = notifierStub.omg.firstCall.args - - expect(args[0]).to.equal('Importing 100 licences from NALD') - expect(args[1].timeTakenMs).to.exist() - }) - - describe('when batching the licences to process', () => { - it('should call the process licence service with the first licence', async () => { - await ImportLicenceService.go() - - const firstLicence = licences[0] - - expect(stubProcessImportLicence.getCall(0).calledWithExactly(firstLicence)).to.be.true() - }) - - it('should call the process licence service with the last licence', async () => { - await ImportLicenceService.go() - - const lastLicence = licences[licences.length - 1] - - expect(stubProcessImportLicence.getCall(licences.length - 1).calledWithExactly(lastLicence)).to.be.true() - }) - - it('should process all the fetched licences', async () => { - await ImportLicenceService.go() - - expect(stubProcessImportLicence.callCount).to.equal(licences.length) - }) - - it('should process the expected number of batches', async () => { - await ImportLicenceService.go() - - // Check the expected number of batches (100 items / 10 per batch = 10 batches) - const expectedBatches = Math.ceil(licences.length / batchSize) - - expect(stubProcessImportLicence.getCalls().length / batchSize).to.equal(expectedBatches) - }) - }) - }) - - describe('when handling the batch size', { timeout: 15000 }, () => { - const delayInSeconds = 1 - - beforeEach(() => { - stubProcessImportLicence = Sinon.stub(ProcessImportLicence, 'go').callsFake(() => { - return new Promise((resolve) => { - setTimeout(() => { - resolve() - }, delayInSeconds * 1000) - }) - }) - }) - - it('should handle the batch as efficiently as possible', async () => { - await ImportLicenceService.go() - - const args = notifierStub.omg.firstCall.args - - expect(args[1].timeTakenSs).to.equal(10n) - }) - }) -}) - -function _licences() { - const licences = [] - - for (let i = 0; i < 100; i++) { - licences.push({ id: generateUUID(), expired_date: null, lapsed_date: null, revoked_date: null }) - } - - return licences -} diff --git a/test/services/jobs/import/process-import-licence.service.test.js b/test/services/jobs/import/process-import-licence.service.test.js deleted file mode 100644 index 7f26606660..0000000000 --- a/test/services/jobs/import/process-import-licence.service.test.js +++ /dev/null @@ -1,148 +0,0 @@ -'use strict' - -// Test framework dependencies -const Lab = require('@hapi/lab') -const Code = require('@hapi/code') -const Sinon = require('sinon') - -const { describe, it, afterEach, beforeEach } = (exports.lab = Lab.script()) -const { expect } = Code - -const { generateUUID } = require('../../../../app/lib/general.lib.js') - -// Things we need to stub -const DetermineLicenceEndDateChangedService = require('../../../../app/services/jobs/import/determine-licence-end-date-changed.service.js') -const ProcessBillingFlagService = require('../../../../app/services/licences/supplementary/process-billing-flag.service.js') -const GenerateReturnLogsService = require('../../../../app/services/jobs/import/generate-return-logs.service.js') - -// Thing under test -const ProcessImportLicence = require('../../../../app/services/jobs/import/process-import-licence.service.js') - -describe('Process Import Licence Service', () => { - let licence - let notifierStub - let stubDetermineLicenceEndDateChangedService - let stubProcessBillingFlagService - let stubGenerateReturnLogsService - - beforeEach(async () => { - licence = _licence() - - notifierStub = { omg: Sinon.stub(), omfg: Sinon.stub() } - global.GlobalNotifier = notifierStub - }) - - afterEach(() => { - Sinon.restore() - delete global.GlobalNotifier - }) - - describe('when a licence has been process successfully', () => { - describe('and the licence end date has changed', () => { - beforeEach(() => { - stubDetermineLicenceEndDateChangedService = Sinon.stub(DetermineLicenceEndDateChangedService, 'go').resolves( - true - ) - stubProcessBillingFlagService = Sinon.stub(ProcessBillingFlagService, 'go').resolves() - stubGenerateReturnLogsService = Sinon.stub(GenerateReturnLogsService, 'go').resolves() - }) - - it('should call the "DetermineLicenceEndDateChangedService" service ', async () => { - await ProcessImportLicence.go(licence) - - const expectedPayload = { - expiredDate: licence.expired_date, - lapsedDate: licence.lapsed_date, - revokedDate: licence.revoked_date, - licenceId: licence.id - } - - expect(stubDetermineLicenceEndDateChangedService.calledWithExactly(expectedPayload, licence.id)).to.be.true() - }) - - it('should call the "ProcessBillingFlagService" service ', async () => { - await ProcessImportLicence.go(licence) - - const expectedPayload = { - expiredDate: licence.expired_date, - lapsedDate: licence.lapsed_date, - revokedDate: licence.revoked_date, - licenceId: licence.id - } - - expect(stubProcessBillingFlagService.calledWithExactly(expectedPayload)).to.be.true() - }) - - it('should call the "stubDetermineLicenceEndDateChangedService" service ', async () => { - await ProcessImportLicence.go(licence) - - const expectedPayload = { - expiredDate: licence.expired_date, - lapsedDate: licence.lapsed_date, - revokedDate: licence.revoked_date, - licenceId: licence.id - } - - expect(stubGenerateReturnLogsService.calledWithExactly(licence.id, expectedPayload)).to.be.true() - }) - }) - describe('and the licence end date has not changed', () => { - beforeEach(() => { - stubDetermineLicenceEndDateChangedService = Sinon.stub(DetermineLicenceEndDateChangedService, 'go').resolves( - false - ) - stubProcessBillingFlagService = Sinon.stub(ProcessBillingFlagService, 'go').resolves() - stubGenerateReturnLogsService = Sinon.stub(GenerateReturnLogsService, 'go').resolves() - }) - - it('should call the "DetermineLicenceEndDateChangedService" service ', async () => { - await ProcessImportLicence.go(licence) - - const expectedPayload = { - expiredDate: licence.expired_date, - lapsedDate: licence.lapsed_date, - revokedDate: licence.revoked_date, - licenceId: licence.id - } - - expect(stubDetermineLicenceEndDateChangedService.calledWithExactly(expectedPayload, licence.id)).to.be.true() - }) - - it('should not call the "ProcessBillingFlagService" service ', async () => { - await ProcessImportLicence.go(licence) - - expect(stubProcessBillingFlagService.called).to.be.false() - }) - - it('should not call the "stubDetermineLicenceEndDateChangedService" service ', async () => { - await ProcessImportLicence.go(licence) - - expect(stubGenerateReturnLogsService.called).to.be.false() - }) - }) - }) - - describe('when processing a licence fails', () => { - beforeEach(() => { - stubDetermineLicenceEndDateChangedService = Sinon.stub(DetermineLicenceEndDateChangedService, 'go').rejects( - new Error('Test error') - ) - stubProcessBillingFlagService = Sinon.stub(ProcessBillingFlagService, 'go').resolves() - stubGenerateReturnLogsService = Sinon.stub(GenerateReturnLogsService, 'go').resolves() - }) - - it('should catch the error', async () => { - await ProcessImportLicence.go(licence) - - expect(notifierStub.omfg.firstCall.args).to.equal([ - `Importing licence ${licence.id}`, - null, - new Error('Test error') - ]) - }) - }) -}) - -function _licence() { - return { id: generateUUID(), expired_date: null, lapsed_date: null, revoked_date: null } -} diff --git a/test/services/jobs/licence-changes/determine-earliest-licence-changed-date.service.test.js b/test/services/jobs/licence-changes/determine-earliest-licence-changed-date.service.test.js new file mode 100644 index 0000000000..e129864c74 --- /dev/null +++ b/test/services/jobs/licence-changes/determine-earliest-licence-changed-date.service.test.js @@ -0,0 +1,283 @@ +'use strict' + +// Test framework dependencies +const Lab = require('@hapi/lab') +const Code = require('@hapi/code') + +const { describe, it, beforeEach } = (exports.lab = Lab.script()) +const { expect } = Code + +// Thing under test +const DetermineEarliestLicenceChangedDateService = require('../../../../app/services/jobs/licence-changes/determine-earliest-licence-changed-date.service.js') + +describe('Jobs - Licence Changes - Determine Earliest Licence Changed Date service', () => { + let licence + + beforeEach(() => { + licence = { + id: '614d2443-7f26-4d5d-a3ca-918e3cd53faa', + nald_expired_date: null, + nald_lapsed_date: null, + nald_revoked_date: null, + wrls_expired_date: null, + wrls_lapsed_date: null, + wrls_revoked_date: null + } + }) + + describe('when the NALD and WRLS end dates are the same', () => { + beforeEach(() => { + // NOTE: We set a date to show the logic determines matching dates as unchanged + licence.nald_revoked_date = new Date('2023-01-01') + licence.wrls_revoked_date = new Date('2023-01-01') + }) + + it('returns null', async () => { + const result = await DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal(null) + }) + }) + + describe('when the NALD and WRLS "expired" dates are different', () => { + describe('because the NALD date is now set but the WRLS date is not', () => { + beforeEach(() => { + licence.nald_expired_date = new Date('2023-01-01') + licence.wrls_expired_date = null + }) + + it('returns details of the changed date with "changeDate" set as the NALD date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.nald_expired_date, + dateType: 'expired', + naldDate: licence.nald_expired_date, + wrlsDate: licence.wrls_expired_date + }) + }) + }) + + describe('because the NALD date has been unset and the WRLS date is still set', () => { + beforeEach(() => { + licence.nald_expired_date = null + licence.wrls_expired_date = new Date('2023-01-01') + }) + + it('returns details of the changed date with "changeDate" set as the WRLS date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.wrls_expired_date, + dateType: 'expired', + naldDate: licence.nald_expired_date, + wrlsDate: licence.wrls_expired_date + }) + }) + }) + + describe('because the NALD date is now earlier than the WRLS date', () => { + beforeEach(() => { + licence.nald_expired_date = new Date('2022-12-31') + licence.wrls_expired_date = new Date('2023-01-01') + }) + + it('returns details of the changed date with "changeDate" set as the NALD date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.nald_expired_date, + dateType: 'expired', + naldDate: licence.nald_expired_date, + wrlsDate: licence.wrls_expired_date + }) + }) + }) + + describe('because the NALD date is now later than the WRLS date', () => { + beforeEach(() => { + licence.nald_expired_date = new Date('2023-01-02') + licence.wrls_expired_date = new Date('2023-01-01') + }) + + it('returns details of the changed date with "changeDate" set as the WRLS date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.wrls_expired_date, + dateType: 'expired', + naldDate: licence.nald_expired_date, + wrlsDate: licence.wrls_expired_date + }) + }) + }) + }) + + describe('when the NALD and WRLS "lapsed" dates are different', () => { + describe('because the NALD date is now set but the WRLS date is not', () => { + beforeEach(() => { + licence.nald_lapsed_date = new Date('2023-01-01') + licence.wrls_lapsed_date = null + }) + + it('returns details of the changed date with "changeDate" set as the NALD date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.nald_lapsed_date, + dateType: 'lapsed', + naldDate: licence.nald_lapsed_date, + wrlsDate: licence.wrls_lapsed_date + }) + }) + }) + + describe('because the NALD date has been unset and the WRLS date is still set', () => { + beforeEach(() => { + licence.nald_lapsed_date = null + licence.wrls_lapsed_date = new Date('2023-01-01') + }) + + it('returns details of the changed date with "changeDate" set as the WRLS date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.wrls_lapsed_date, + dateType: 'lapsed', + naldDate: licence.nald_lapsed_date, + wrlsDate: licence.wrls_lapsed_date + }) + }) + }) + + describe('because the NALD date is now earlier than the WRLS date', () => { + beforeEach(() => { + licence.nald_lapsed_date = new Date('2022-12-31') + licence.wrls_lapsed_date = new Date('2023-01-01') + }) + + it('returns details of the changed date with "changeDate" set as the NALD date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.nald_lapsed_date, + dateType: 'lapsed', + naldDate: licence.nald_lapsed_date, + wrlsDate: licence.wrls_lapsed_date + }) + }) + }) + + describe('because the NALD date is now later than the WRLS date', () => { + beforeEach(() => { + licence.nald_lapsed_date = new Date('2023-01-02') + licence.wrls_lapsed_date = new Date('2023-01-01') + }) + + it('returns details of the changed date with "changeDate" set as the WRLS date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.wrls_lapsed_date, + dateType: 'lapsed', + naldDate: licence.nald_lapsed_date, + wrlsDate: licence.wrls_lapsed_date + }) + }) + }) + }) + + describe('when the NALD and WRLS "revoked" dates are different', () => { + describe('because the NALD date is now set but the WRLS date is not', () => { + beforeEach(() => { + licence.nald_revoked_date = new Date('2023-01-01') + licence.wrls_revoked_date = null + }) + + it('returns details of the changed date with "changeDate" set as the NALD date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.nald_revoked_date, + dateType: 'revoked', + naldDate: licence.nald_revoked_date, + wrlsDate: licence.wrls_revoked_date + }) + }) + }) + + describe('because the NALD date has been unset and the WRLS date is still set', () => { + beforeEach(() => { + licence.nald_revoked_date = null + licence.wrls_revoked_date = new Date('2023-01-01') + }) + + it('returns details of the changed date with "changeDate" set as the WRLS date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.wrls_revoked_date, + dateType: 'revoked', + naldDate: licence.nald_revoked_date, + wrlsDate: licence.wrls_revoked_date + }) + }) + }) + + describe('because the NALD date is now earlier than the WRLS date', () => { + beforeEach(() => { + licence.nald_revoked_date = new Date('2022-12-31') + licence.wrls_revoked_date = new Date('2023-01-01') + }) + + it('returns details of the changed date with "changeDate" set as the NALD date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.nald_revoked_date, + dateType: 'revoked', + naldDate: licence.nald_revoked_date, + wrlsDate: licence.wrls_revoked_date + }) + }) + }) + + describe('because the NALD date is now later than the WRLS date', () => { + beforeEach(() => { + licence.nald_revoked_date = new Date('2023-01-02') + licence.wrls_revoked_date = new Date('2023-01-01') + }) + + it('returns details of the changed date with "changeDate" set as the WRLS date', () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.wrls_revoked_date, + dateType: 'revoked', + naldDate: licence.nald_revoked_date, + wrlsDate: licence.wrls_revoked_date + }) + }) + }) + }) + + describe('when multiple NALD and WRLS end dates are different', () => { + beforeEach(() => { + licence.nald_expired_date = new Date('2026-01-01') + licence.wrls_expired_date = new Date('2037-01-01') + licence.nald_revoked_date = new Date('2023-01-01') + licence.wrls_revoked_date = null + }) + + it('returns just the details of the earliest changed date', async () => { + const result = DetermineEarliestLicenceChangedDateService.go(licence) + + expect(result).to.equal({ + changeDate: licence.nald_revoked_date, + dateType: 'revoked', + naldDate: licence.nald_revoked_date, + wrlsDate: licence.wrls_revoked_date + }) + }) + }) +}) diff --git a/test/services/jobs/licence-changes/process-licence-changes.service.test.js b/test/services/jobs/licence-changes/process-licence-changes.service.test.js new file mode 100644 index 0000000000..b0b642491f --- /dev/null +++ b/test/services/jobs/licence-changes/process-licence-changes.service.test.js @@ -0,0 +1,150 @@ +'use strict' + +// Test framework dependencies +const Lab = require('@hapi/lab') +const Code = require('@hapi/code') +const Sinon = require('sinon') + +const { describe, it, beforeEach, afterEach } = (exports.lab = Lab.script()) +const { expect } = Code + +// Things we need to stub +const FetchLicencesService = require('../../../../app/services/jobs/licence-changes/fetch-licences.service.js') +const { generateUUID } = require('../../../../app/lib/general.lib.js') +const JobsConfig = require('../../../../config/jobs.config.js') +const ProcessLicenceService = require('../../../../app/services/jobs/licence-changes/process-licence.service.js') + +// Thing under test +const ProcessLicenceChangesService = require('../../../../app/services/jobs/licence-changes/process-licence-changes.service.js') + +describe('Jobs - Licence Changes - Process Licence Changes service', () => { + const batchSize = 10 + + let licences + let notifierStub + let processLicenceStub + + beforeEach(() => { + licences = _licences() + + // NOTE: We set our batch size to ensure consistency within the tests. Depending on who or where the tests are being + // run, might mean this value is different. + Sinon.stub(JobsConfig, 'licenceChanges').value({ batchSize }) + + Sinon.stub(FetchLicencesService, 'go').resolves(licences) + + // The service depends on GlobalNotifier to have been set. This happens in app/plugins/global-notifier.plugin.js + // when the app starts up and the plugin is registered. As we're not creating an instance of Hapi server in this + // test we recreate the condition by setting it directly with our own stub + notifierStub = { omg: Sinon.stub(), omfg: Sinon.stub() } + global.GlobalNotifier = notifierStub + }) + + afterEach(() => { + Sinon.restore() + delete global.GlobalNotifier + }) + + describe('when processing the licences', () => { + beforeEach(() => { + processLicenceStub = Sinon.stub(ProcessLicenceService, 'go').resolves() + }) + + it('processes all licences with a current licence version in NALD and a matching record in WRLS', async () => { + await ProcessLicenceChangesService.go() + + const firstLicence = licences[0] + const lastLicence = licences[licences.length - 1] + + expect(processLicenceStub.callCount).to.equal(licences.length) + expect(processLicenceStub.getCall(0).firstArg).to.equal(firstLicence) + expect(processLicenceStub.getCall(licences.length - 1).firstArg).to.equal(lastLicence) + }) + + it('processes them in batches', async () => { + await ProcessLicenceChangesService.go() + + // Check the expected number of batches (100 items / 10 per batch = 10 batches) + const expectedBatches = Math.ceil(licences.length / batchSize) + + expect(processLicenceStub.getCalls().length / batchSize).to.equal(expectedBatches) + }) + + it('logs the time taken in milliseconds and seconds', async () => { + await ProcessLicenceChangesService.go() + + const logDataArg = notifierStub.omg.firstCall.args[1] + + expect(notifierStub.omg.calledWith('Licence changes job complete')).to.be.true() + expect(logDataArg.timeTakenMs).to.exist() + expect(logDataArg.timeTakenSs).to.exist() + expect(logDataArg.count).to.exist() + }) + }) + + // NOTE: We've gone to the effort of batching the work to ensure the total time is not 'number of licences * time to + // process one licence'! + // + // This handy test is a great way of demonstrating that batching the work reduces the overall time it takes to + // complete the job. We make it so that each licence takes 0.25 seconds to complete. Processing 100 licences in + // batches of 10 should mean the overall time is around 2.5 seconds. + // + // This is because with the help of p-map each batch is processing 10 licences asynchronously. This means a batch + // should take approximately the same time to complete as processing a single licence. So, the overall time is + // 10 x 0.25 seconds. + describe('when batching the processing of the licences', () => { + beforeEach(() => { + const delayInMilliseconds = 250 // 0.25 seconds + + processLicenceStub = Sinon.stub(ProcessLicenceService, 'go').callsFake(() => { + return new Promise((resolve) => { + setTimeout(() => { + resolve() + }, delayInMilliseconds) + }) + }) + }) + + it('takes less time to complete the job than doing them one at a time', { timeout: 4000 }, async () => { + await ProcessLicenceChangesService.go() + + const args = notifierStub.omg.firstCall.args + + expect(args[1].timeTakenSs).to.be.lessThan(3n) + }) + }) + + describe('when there is an error', () => { + beforeEach(() => { + Sinon.stub(ProcessLicenceService, 'go').rejects() + }) + + it('handles the error', async () => { + await ProcessLicenceChangesService.go() + + const args = notifierStub.omfg.firstCall.args + + expect(args[0]).to.equal('Licence changes job failed') + expect(args[1]).to.be.null() + expect(args[2]).to.be.an.error() + }) + }) +}) + +function _licences() { + const licences = [] + + for (let i = 0; i < 100; i++) { + licences.push({ + id: generateUUID(), + nald_expired_date: null, + nald_lapsed_date: null, + nald_revoked_date: null, + wrls_expired_date: null, + wrls_lapsed_date: null, + wrls_revoked_date: null + }) + } + + return licences +} diff --git a/test/services/jobs/licence-changes/process-licence.service.test.js b/test/services/jobs/licence-changes/process-licence.service.test.js new file mode 100644 index 0000000000..9dc93a3b2b --- /dev/null +++ b/test/services/jobs/licence-changes/process-licence.service.test.js @@ -0,0 +1,124 @@ +'use strict' + +// Test framework dependencies +const Lab = require('@hapi/lab') +const Code = require('@hapi/code') +const Sinon = require('sinon') + +const { describe, it, beforeEach, afterEach } = (exports.lab = Lab.script()) +const { expect } = Code + +// Things we need to stub +const FeatureFlagsConfig = require('../../../../config/feature-flags.config.js') +const ProcessBillingFlagService = require('../../../../app/services/licences/supplementary/process-billing-flag.service.js') +const ProcessLicenceReturnLogsService = require('../../../../app/services/return-logs/process-licence-return-logs.service.js') + +// Thing under test +const ProcessLicenceService = require('../../../../app/services/jobs/licence-changes/process-licence.service.js') + +describe('Jobs - Licence Changes - Process Licence service', () => { + let licence + let notifierStub + let processBillingFlagsStub + let processReturnLogsStub + + beforeEach(() => { + licence = { + id: '614d2443-7f26-4d5d-a3ca-918e3cd53faa', + nald_expired_date: null, + nald_lapsed_date: null, + nald_revoked_date: null, + wrls_expired_date: null, + wrls_lapsed_date: null, + wrls_revoked_date: null + } + + // The service depends on GlobalNotifier to have been set. This happens in app/plugins/global-notifier.plugin.js + // when the app starts up and the plugin is registered. As we're not creating an instance of Hapi server in this + // test we recreate the condition by setting it directly with our own stub + notifierStub = { omg: Sinon.stub(), omfg: Sinon.stub() } + global.GlobalNotifier = notifierStub + }) + + afterEach(() => { + Sinon.restore() + delete global.GlobalNotifier + }) + + describe('when the "end dates" on the licence have not changed', () => { + beforeEach(() => { + processBillingFlagsStub = Sinon.stub(ProcessBillingFlagService, 'go').resolves() + + // NOTE: We set our feature flag to true to ensure that this is not the reason ProcessLicenceReturnLogsService was + // not called + Sinon.stub(FeatureFlagsConfig, 'enableRequirementsForReturns').value(true) + processReturnLogsStub = Sinon.stub(ProcessLicenceReturnLogsService, 'go').resolves() + }) + + it('does not process the licence', async () => { + await ProcessLicenceService.go(licence) + + expect(processBillingFlagsStub.called).to.be.false() + expect(processReturnLogsStub.called).to.be.false() + }) + }) + + describe('when the "end dates" on the licence have changed', () => { + beforeEach(() => { + licence.nald_revoked_date = new Date('2023-01-01') + licence.wrls_revoked_date = null + + processBillingFlagsStub = Sinon.stub(ProcessBillingFlagService, 'go').resolves() + processReturnLogsStub = Sinon.stub(ProcessLicenceReturnLogsService, 'go').resolves() + }) + + it('processes the licence for supplementary billing flags', async () => { + await ProcessLicenceService.go(licence) + + expect(processBillingFlagsStub.calledOnce).to.be.true() + }) + + describe('and the app is now managing "requirements for returns"', () => { + beforeEach(() => { + Sinon.stub(FeatureFlagsConfig, 'enableRequirementsForReturns').value(true) + }) + + it('processes the licence for reissuing return logs', async () => { + await ProcessLicenceService.go(licence) + + expect(processReturnLogsStub.calledOnce).to.be.true() + }) + }) + + describe('and the app is not yet managing "requirements for returns"', () => { + beforeEach(() => { + Sinon.stub(FeatureFlagsConfig, 'enableRequirementsForReturns').value(false) + }) + + it('does not process the licence for reissuing return logs', async () => { + await ProcessLicenceService.go(licence) + + expect(processReturnLogsStub.called).to.be.false() + }) + }) + }) + + describe('when an error is thrown whilst processing the licence', () => { + beforeEach(() => { + licence.nald_revoked_date = new Date('2023-01-01') + licence.wrls_revoked_date = null + + processBillingFlagsStub = Sinon.stub(ProcessBillingFlagService, 'go').rejects(new Error('failed')) + }) + + it('handles the error', async () => { + await ProcessLicenceService.go(licence) + + const errorLogArgs = notifierStub.omfg.firstCall.args + + expect(notifierStub.omfg.calledWith('Licence changes processing failed')).to.be.true() + expect(errorLogArgs[1]).to.equal({ id: licence.id }) + expect(errorLogArgs[2]).to.be.instanceOf(Error) + }) + }) +}) diff --git a/test/services/licences/supplementary/determine-imported-licence-flags.service.test.js b/test/services/licences/supplementary/determine-imported-licence-flags.service.test.js index d8cfa6321d..e094ea9cd2 100644 --- a/test/services/licences/supplementary/determine-imported-licence-flags.service.test.js +++ b/test/services/licences/supplementary/determine-imported-licence-flags.service.test.js @@ -14,7 +14,7 @@ const FetchExistingLicenceDetailsService = require('../../../../app/services/lic // Thing under test const DetermineImportedLicenceFlagsService = require('../../../../app/services/licences/supplementary/determine-imported-licence-flags.service.js') -describe('Determine Imported Licence Flags Service', () => { +describe('Licences - Supplementary - Determine Imported Licence Flags service', () => { const licenceId = 'aad74a3d-59ea-4c18-8091-02b0f8b0a147' let clock @@ -30,16 +30,12 @@ describe('Determine Imported Licence Flags Service', () => { Sinon.restore() }) - describe('when given an imported licence', () => { - let importedLicence + describe('when processing an imported licence', () => { + let changeDate describe('with a future revoked date', () => { before(() => { - importedLicence = { - expiredDate: null, - lapsedDate: null, - revokedDate: new Date('2030-04-01') - } + changeDate = new Date('2030-04-01') }) describe('and a licenceId', () => { @@ -50,7 +46,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('always returns the licenceId, regionId, startDate and endDate', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.licenceId).to.equal('aad74a3d-59ea-4c18-8091-02b0f8b0a147') expect(result.regionId).to.equal('ff92e0b1-3934-430b-8b16-5b89a3ea258f') @@ -59,7 +55,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(false) expect(result.flagForSrocSupplementary).to.equal(false) @@ -73,7 +69,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('always returns the licenceId, regionId, startDate and endDate', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.licenceId).to.equal('aad74a3d-59ea-4c18-8091-02b0f8b0a147') expect(result.regionId).to.equal('ff92e0b1-3934-430b-8b16-5b89a3ea258f') @@ -82,7 +78,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(false) expect(result.flagForSrocSupplementary).to.equal(false) @@ -98,7 +94,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(true) expect(result.flagForSrocSupplementary).to.equal(true) @@ -112,7 +108,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(false) expect(result.flagForSrocSupplementary).to.equal(false) @@ -125,11 +121,7 @@ describe('Determine Imported Licence Flags Service', () => { describe('with an sroc lapsed date of "2022-04-01"', () => { before(() => { - importedLicence = { - expiredDate: null, - lapsedDate: new Date('2022-04-01'), - revokedDate: null - } + changeDate = new Date('2022-04-01') }) describe('for a licence with no charge versions', () => { @@ -139,7 +131,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(false) expect(result.flagForSrocSupplementary).to.equal(false) @@ -153,7 +145,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(false) expect(result.flagForSrocSupplementary).to.equal(false) @@ -169,7 +161,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(true) expect(result.flagForSrocSupplementary).to.equal(true) @@ -183,7 +175,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(false) expect(result.flagForSrocSupplementary).to.equal(true) @@ -195,11 +187,7 @@ describe('Determine Imported Licence Flags Service', () => { describe('with a pre-sroc expired date of "2019-01-01"', () => { before(() => { - importedLicence = { - expiredDate: new Date('2019-01-01'), - lapsedDate: null, - revokedDate: null - } + changeDate = new Date('2019-01-01') }) describe('for a licence with no charge versions', () => { @@ -209,7 +197,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(false) expect(result.flagForSrocSupplementary).to.equal(false) @@ -223,7 +211,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(false) expect(result.flagForSrocSupplementary).to.equal(false) @@ -239,7 +227,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(true) expect(result.flagForSrocSupplementary).to.equal(true) @@ -253,7 +241,7 @@ describe('Determine Imported Licence Flags Service', () => { }) it('returns the correct flags', async () => { - const result = await DetermineImportedLicenceFlagsService.go(importedLicence, licenceId) + const result = await DetermineImportedLicenceFlagsService.go(licenceId, changeDate) expect(result.flagForPreSrocSupplementary).to.equal(true) expect(result.flagForSrocSupplementary).to.equal(true) diff --git a/test/services/licences/supplementary/process-billing-flag.service.test.js b/test/services/licences/supplementary/process-billing-flag.service.test.js index 5bc7da0a5d..6db9d16437 100644 --- a/test/services/licences/supplementary/process-billing-flag.service.test.js +++ b/test/services/licences/supplementary/process-billing-flag.service.test.js @@ -21,7 +21,7 @@ const PersistSupplementaryBillingFlagsService = require('../../../../app/service // Thing under test const ProcessBillingFlagService = require('../../../../app/services/licences/supplementary/process-billing-flag.service.js') -describe('Process Billing Flag Service', () => { +describe('Licences - Supplementary - Process Billing Flag service', () => { let notifierStub let payload @@ -254,8 +254,12 @@ describe('Process Billing Flag Service', () => { describe('with an importedLicence', () => { before(() => { payload = { - importedLicence: { - licenceData: true + licenceId: 'b5f81330-bec5-4c3e-95dd-267c10836fea', + changedDateDetails: { + changeDate: new Date('2024-07-01'), + dateType: 'revoked', + naldDate: new Date('2024-07-01'), + wrlsDate: null } } })