Skip to content

Commit

Permalink
Move licence end date logic to model (#681)
Browse files Browse the repository at this point in the history
https://eaflood.atlassian.net/browse/WATER-4325
https://eaflood.atlassian.net/browse/WATER-4263

We have had two tickets come through that both need to determine if and when a licence 'ends'.

A licence can 'end' for 3 reasons:

- because it is _revoked_
- because it is _lapsed_
- because it is _expired_

The previous delivery team chose to encode these as 3 separate date fields on the licence record. So, if a field is populated it means the licence 'ends' for that reason on that day. But it's not that simple! 😁

More than one of these fields may be populated. For example, a licence was due to expire on 2023-08-10 but was then revoked on 2022-04-27. So, to determine the reason you need to select the _earliest_ date.

But there's more! 😲

We have examples where 2 of the fields might be populated with the same date (and 1 licence where all 3 have the same date!) So, how do you determine the 'end' reason then? If more than one date field is populated and they hold the earliest date value then we select based on priority; revoked -> lapsed -> expired.

Both tickets are connected to PRs that have to handle all or some of this logic.

- [Returns required journey - Select start date page iteration 2 (1 of 7)](#646)
- [Add warning text to view licence page](#670)

Both PRs will also have to write numerous tests to check the right date is being used, and we have a strong suspicion we are already doing something similar in supplementary billing and that we'll need the 'end' in future tickets.

So, enough is enough! This refactoring change moves the logic to the model, specifically, as a [custom Objection.js instance method](https://vincit.github.io/objection.js/api/model/instance-methods.html). This is something we've done in a [previous project](https://github.com/DEFRA/sroc-charging-module-api) and in this project to determine a `ContactModel`s name.

We can then remove the duplication both in logic and unit tests.

This change updates the model, adds the new tests, and then refactors existing usages to use the model's `$ends()` method.
  • Loading branch information
Cruikshanks authored and robertparkinson committed Jan 24, 2024
1 parent 05c40d0 commit 9298c3d
Show file tree
Hide file tree
Showing 6 changed files with 333 additions and 254 deletions.
58 changes: 58 additions & 0 deletions app/models/licence.model.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,64 @@ class LicenceModel extends BaseModel {
}
}
}

/**
* Determine the 'end' date for the licence
*
* A licence can 'end' for 3 reasons:
*
* - because it is _revoked_
* - because it is _lapsed_
* - because it is _expired_
*
* The previous delivery team chose to encode these as 3 separate date fields on the licence record. So, if a field is
* populated it means the licence 'ends' for that reason on that day.
*
* More than one of these fields may be populated. For example, a licence was due to expire on 2023-08-10 but was then
* revoked on 2022-04-27. So, to determine the reason you need to select the _earliest_ date.
*
* But are examples where 2 of the fields might be populated with the same date (and 1 licence where all 3 have the
* same date!) If more than one date field is populated and they hold the earliest date value then we select based on
* priority; _revoked_ -> _lapsed_ -> _expired_.
*
* @returns `null` if no 'end' dates are set else an object containing the date, priority and reason for either the
* earliest or highest priority end date
*/
$ends () {
const endDates = [
{ date: this.revokedDate, priority: 1, reason: 'revoked' },
{ date: this.lapsedDate, priority: 2, reason: 'lapsed' },
{ date: this.expiredDate, priority: 3, reason: 'expired' }
]

const filteredDates = endDates.filter((endDate) => endDate.date)

if (filteredDates.length === 0) {
return null
}

// NOTE: For date comparisons you cannot use !== with just the date values. Using < or > will coerce the values into
// numbers for comparison. But equality operators are checking that the two operands are referring to the same
// Object. So, where we have matching dates and expect !== to return 'false' we get 'true' instead
// Thanks to https://stackoverflow.com/a/493018/6117745 for explaining the problem and providing the solution
filteredDates.sort((firstDate, secondDate) => {
if (firstDate.date.getTime() !== secondDate.date.getTime()) {
if (firstDate.date.getTime() < secondDate.date.getTime()) {
return -1
}

return 1
}

if (firstDate.priority < secondDate.priority) {
return -1
}

return 1
})

return filteredDates[0]
}
}

module.exports = LicenceModel
58 changes: 15 additions & 43 deletions app/presenters/licences/view-licence.presenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ const { formatLongDate } = require('../base.presenter.js')
* @returns {Object} The data formatted for the view template
*/
function go (licence) {
const { expiredDate, id, lapsedDate, licenceRef, region, revokedDate, startDate } = licence
const warning = _generateWarningMessage(expiredDate, lapsedDate, revokedDate)
const { expiredDate, id, licenceRef, region, startDate } = licence
const warning = _generateWarningMessage(licence)

return {
id,
Expand All @@ -28,17 +28,6 @@ function go (licence) {
}
}

function _compareEndDates (firstEndDate, secondEndDate) {
if (firstEndDate.date.getTime() === secondEndDate.date.getTime()) {
if (firstEndDate.name === 'revoked') return firstEndDate
if (secondEndDate.name === 'revoked') return secondEndDate
if (firstEndDate.name === 'lapsed') return firstEndDate
} else if (firstEndDate.date < secondEndDate.date) {
return firstEndDate
}
return secondEndDate
}

function _endDate (expiredDate) {
if (!expiredDate || expiredDate < Date.now()) {
return null
Expand All @@ -47,46 +36,29 @@ function _endDate (expiredDate) {
return formatLongDate(expiredDate)
}

function _generateWarningMessage (expiredDate, lapsedDate, revokedDate) {
const endDates = []

if (lapsedDate) {
endDates.push({
name: 'lapsed',
message: `This licence lapsed on ${formatLongDate(lapsedDate)}`,
date: lapsedDate
})
}
function _generateWarningMessage (licence) {
const ends = licence.$ends()

if (expiredDate) {
endDates.push({
name: 'expired',
message: `This licence expired on ${formatLongDate(expiredDate)}`,
date: expiredDate
})
if (!ends) {
return null
}

if (revokedDate) {
endDates.push({
name: 'revoked',
message: `This licence was revoked on ${formatLongDate(revokedDate)}`,
date: revokedDate
})
}
const { date, reason } = ends
const today = new Date()

if (endDates.length === 0) {
if (date > today) {
return null
}

if (endDates.length === 1) {
return endDates[0].message
if (reason === 'revoked') {
return `This licence was revoked on ${formatLongDate(date)}`
}

const earliestPriorityEndDate = endDates.reduce((result, endDate) => {
return _compareEndDates(result, endDate)
})
if (reason === 'lapsed') {
return `This licence lapsed on ${formatLongDate(date)}`
}

return earliestPriorityEndDate.message
return `This licence expired on ${formatLongDate(date)}`
}

module.exports = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,12 @@ async function _createSession (data) {

function _data (licence, journey) {
const { id, licenceDocument, licenceRef, licenceVersions } = licence
const ends = licence.$ends()

return {
licence: {
id,
endDate: ends ? ends.date : null,
licenceRef,
licenceHolder: _licenceHolder(licenceDocument),
startDate: _startDate(licenceVersions)
Expand All @@ -62,7 +64,10 @@ async function _fetchLicence (licenceId) {
.findById(licenceId)
.select([
'id',
'licenceRef'
'expiredDate',
'lapsedDate',
'licenceRef',
'revokedDate'
])
.withGraphFetched('licenceVersions')
.modifyGraph('licenceVersions', (builder) => {
Expand Down
185 changes: 185 additions & 0 deletions test/models/licence.model.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,189 @@ describe('Licence model', () => {
})
})
})

describe('$ends', () => {
let expiredDate
let lapsedDate
let revokedDate

describe('when no end dates are set', () => {
it('returns null', () => {
testRecord = LicenceModel.fromJson({})

expect(testRecord.$ends()).to.be.null()
})
})

describe('when all the end dates are null', () => {
beforeEach(() => {
expiredDate = null
lapsedDate = null
revokedDate = null
})

it('returns null', () => {
testRecord = LicenceModel.fromJson({ expiredDate, lapsedDate, revokedDate })

expect(testRecord.$ends()).to.be.null()
})
})

describe('when only the revoked date is set', () => {
beforeEach(() => {
revokedDate = new Date('2023-03-07')
})

it("returns 'revoked' as the reason and '2023-03-07' as the date", () => {
const result = LicenceModel.fromJson({ revokedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-07'), priority: 1, reason: 'revoked' })
})
})

describe('when only the lapsed date is set', () => {
beforeEach(() => {
lapsedDate = new Date('2023-03-08')
})

it("returns 'lapsed' as the reason and '2023-03-08' as the date", () => {
const result = LicenceModel.fromJson({ lapsedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-08'), priority: 2, reason: 'lapsed' })
})
})

describe('when only the expired date is set', () => {
beforeEach(() => {
expiredDate = new Date('2023-03-09')
})

it("returns 'lapsed' as the reason and '2023-03-09' as the date", () => {
const result = LicenceModel.fromJson({ expiredDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-09'), priority: 3, reason: 'expired' })
})
})

describe('when two dates are set', () => {
describe('that have different dates', () => {
beforeEach(() => {
expiredDate = new Date('2023-03-09')
lapsedDate = new Date('2023-03-08')
})

it('returns the one with the earliest date', () => {
const result = LicenceModel.fromJson({ expiredDate, lapsedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-08'), priority: 2, reason: 'lapsed' })
})
})

describe('that have the same date', () => {
beforeEach(() => {
expiredDate = new Date('2023-03-09')
lapsedDate = new Date('2023-03-09')
revokedDate = new Date('2023-03-09')
})

describe("and they are 'lapsed' and 'expired'", () => {
it("returns 'lapsed' as the end date", () => {
const result = LicenceModel.fromJson({ expiredDate, lapsedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-09'), priority: 2, reason: 'lapsed' })
})
})

describe("and they are 'lapsed' and 'revoked'", () => {
it("returns 'revoked' as the end date", () => {
const result = LicenceModel.fromJson({ lapsedDate, revokedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-09'), priority: 1, reason: 'revoked' })
})
})

describe("and they are 'expired' and 'revoked'", () => {
it("returns 'revoked' as the end date", () => {
const result = LicenceModel.fromJson({ expiredDate, revokedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-09'), priority: 1, reason: 'revoked' })
})
})
})
})

describe('when all dates are set', () => {
describe('and all have different dates', () => {
beforeEach(() => {
expiredDate = new Date('2023-03-09')
lapsedDate = new Date('2023-03-08')
revokedDate = new Date('2023-03-07')
})

it('returns the one with the earliest date', () => {
const result = LicenceModel.fromJson({ expiredDate, lapsedDate, revokedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-07'), priority: 1, reason: 'revoked' })
})
})

describe('and two have the same earliest date', () => {
describe("and they are 'lapsed' and 'expired'", () => {
beforeEach(() => {
expiredDate = new Date('2023-03-09')
lapsedDate = new Date('2023-03-09')
revokedDate = new Date('2023-03-10')
})

it("returns 'lapsed' as the end date", () => {
const result = LicenceModel.fromJson({ expiredDate, lapsedDate, revokedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-09'), priority: 2, reason: 'lapsed' })
})
})

describe("and they are 'lapsed' and 'revoked'", () => {
beforeEach(() => {
expiredDate = new Date('2023-03-10')
lapsedDate = new Date('2023-03-09')
revokedDate = new Date('2023-03-09')
})

it("returns 'revoked' as the end date", () => {
const result = LicenceModel.fromJson({ expiredDate, lapsedDate, revokedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-09'), priority: 1, reason: 'revoked' })
})
})

describe("and they are 'expired' and 'revoked'", () => {
beforeEach(() => {
expiredDate = new Date('2023-03-09')
lapsedDate = new Date('2023-03-10')
revokedDate = new Date('2023-03-09')
})

it("returns 'revoked' as the end date", () => {
const result = LicenceModel.fromJson({ expiredDate, lapsedDate, revokedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-09'), priority: 1, reason: 'revoked' })
})
})
})

describe('and all have the same date', () => {
beforeEach(() => {
expiredDate = new Date('2023-03-09')
lapsedDate = new Date('2023-03-09')
revokedDate = new Date('2023-03-09')
})

it("returns 'revoked' as the end date", () => {
const result = LicenceModel.fromJson({ expiredDate, lapsedDate, revokedDate }).$ends()

expect(result).to.equal({ date: new Date('2023-03-09'), priority: 1, reason: 'revoked' })
})
})
})
})
})
Loading

0 comments on commit 9298c3d

Please sign in to comment.