From 6090ad5a3f832ec6cf1c032552c59654aa1ca50b Mon Sep 17 00:00:00 2001 From: Matthew O'Donoghue Date: Mon, 23 Dec 2024 14:17:10 +0000 Subject: [PATCH] fix(AHWR-321): use organisation, not farmer, name (#256) * fix(AHWR-321): use organisation, not farmer, name - stop allowing 404s from contact history (it now returns [] instead) - remove debug logging for auth, just in case we go to prod --- app/api/contact-history.js | 26 ++-- app/constants/contact-history.js | 2 - app/routes/agreement.js | 2 +- app/routes/models/application-organisation.js | 2 +- app/routes/view-agreement.js | 2 +- app/routes/view-claim.js | 2 +- app/views/agreement.njk | 125 +++++++++--------- app/views/view-agreement.njk | 2 +- package-lock.json | 4 +- package.json | 2 +- test/acceptance/wdio.bs.config.js | 5 - .../narrow/routes/view-agreement.test.js | 2 +- .../view-application-date-of-testing.test.js | 4 +- .../narrow/routes/view-claim.test.js | 8 +- test/unit/api/contact-history.test.js | 35 +++-- 15 files changed, 102 insertions(+), 121 deletions(-) diff --git a/app/api/contact-history.js b/app/api/contact-history.js index 0172a1a1..8e601a80 100644 --- a/app/api/contact-history.js +++ b/app/api/contact-history.js @@ -1,5 +1,4 @@ const wreck = require('@hapi/wreck') -const _ = require('lodash') const { applicationApiUri } = require('../config') const { fieldsNames, labels, notAvailable } = require('./../constants/contact-history') @@ -10,42 +9,39 @@ async function getContactHistory (reference, logger) { return payload } catch (err) { - if (err.output.statusCode === 404) { - return null - } else { - logger.setBindings({ err, endpoint }) - throw err - } + logger.setBindings({ err, endpoint }) + throw err } } const getContactFieldData = (contactHistoryData, field) => { - const filteredData = contactHistoryData.filter(contact => contact?.data?.field === field) - if (filteredData.length) { - const oldValue = _.sortBy(filteredData, [function (data) { return new Date(data.createdAt) }])[0].data.oldValue - return `${labels[field]} ${oldValue}` - } else { + const filteredData = contactHistoryData.filter(contact => contact.data?.field === field) + + if (filteredData.length === 0) { return 'NA' } + + const [firstUpdate] = filteredData.sort((a, b) => + new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime() + ) + + return `${labels[field]} ${firstUpdate.data.oldValue}` } const displayContactHistory = (contactHistory) => { if (contactHistory) { const orgEmail = getContactFieldData(contactHistory, fieldsNames.orgEmail) const email = getContactFieldData(contactHistory, fieldsNames.email) - const farmerName = getContactFieldData(contactHistory, fieldsNames.farmerName) const address = getContactFieldData(contactHistory, fieldsNames.address) return { orgEmail, email, - farmerName, address } } else { return { orgEmail: notAvailable, email: notAvailable, - farmerName: notAvailable, address: notAvailable } } diff --git a/app/constants/contact-history.js b/app/constants/contact-history.js index 5d97c268..9b7d0a12 100644 --- a/app/constants/contact-history.js +++ b/app/constants/contact-history.js @@ -2,13 +2,11 @@ module.exports = { fieldsNames: { orgEmail: 'orgEmail', email: 'email', - farmerName: 'farmerName', address: 'address' }, labels: { orgEmail: 'Organisation email at start of agreement:', email: 'User email at start of agreement:', - farmerName: 'Name at start of agreement:', address: 'Address at start of agreement:' }, notAvailable: 'NA' diff --git a/app/routes/agreement.js b/app/routes/agreement.js index 77b0fbf1..cd606aa5 100644 --- a/app/routes/agreement.js +++ b/app/routes/agreement.js @@ -51,7 +51,7 @@ module.exports = [{ const summaryDetails = [ { field: 'Agreement number', newValue: request.params.reference, oldValue: null }, { field: 'Agreement date', newValue: formatedDateToUk(application.createdAt), oldValue: null }, - { field: 'Agreement holder', newValue: organisation?.farmerName, oldValue: contactHistoryDetails.farmerName }, + { field: 'Agreement holder', newValue: organisation?.name, oldValue: null }, { field: 'Agreement holder email', newValue: organisation?.email, oldValue: contactHistoryDetails.email }, { field: 'SBI number', newValue: organisation?.sbi, oldValue: null }, { field: 'Address', newValue: organisation?.address, oldValue: contactHistoryDetails.address }, diff --git a/app/routes/models/application-organisation.js b/app/routes/models/application-organisation.js index d8d22202..41c709b5 100644 --- a/app/routes/models/application-organisation.js +++ b/app/routes/models/application-organisation.js @@ -1,6 +1,6 @@ module.exports = (organisation) => { return [ - { key: { text: 'Name:' }, value: { text: organisation?.farmerName } }, + { key: { text: 'Name:' }, value: { text: organisation?.name } }, { key: { text: 'SBI number:' }, value: { text: organisation?.sbi } }, { key: { text: 'Address:' }, value: { text: organisation?.address } }, { key: { text: 'User Email address:' }, value: { text: organisation?.email } }, diff --git a/app/routes/view-agreement.js b/app/routes/view-agreement.js index 015819b4..411ffc1f 100644 --- a/app/routes/view-agreement.js +++ b/app/routes/view-agreement.js @@ -78,7 +78,7 @@ module.exports = { const contactHistoryDetails = displayContactHistory(contactHistory) const organisation = application.data?.organisation const listData = [ - { field: 'Name', newValue: organisation?.farmerName, oldValue: contactHistoryDetails.farmerName }, + { field: 'Name', newValue: organisation?.name, oldValue: null }, { field: 'SBI number', newValue: organisation?.sbi, oldValue: null }, { field: 'Address', newValue: organisation?.address, oldValue: contactHistoryDetails.address }, { field: 'Email address', newValue: organisation?.email, oldValue: contactHistoryDetails.email }, diff --git a/app/routes/view-claim.js b/app/routes/view-claim.js index 8379cc24..29383400 100644 --- a/app/routes/view-claim.js +++ b/app/routes/view-claim.js @@ -64,7 +64,7 @@ module.exports = { const applicationSummaryDetails = [ { key: { text: 'Agreement number' }, value: { text: applicationReference } }, { key: { text: 'Agreement date' }, value: { text: formatedDateToUk(application.createdAt) } }, - { key: { text: 'Agreement holder' }, value: { text: organisation?.farmerName } }, + { key: { text: 'Agreement holder' }, value: { text: organisation?.name } }, { key: { text: 'Agreement holder email' }, value: { text: organisation?.email } }, { key: { text: 'SBI number' }, value: { text: organisation?.sbi } }, { key: { text: 'Address' }, value: { text: organisation?.address } }, diff --git a/app/views/agreement.njk b/app/views/agreement.njk index c050d7a2..ae87b659 100644 --- a/app/views/agreement.njk +++ b/app/views/agreement.njk @@ -1,70 +1,65 @@ {% extends './layouts/layout.njk'%} -{%block pageTitle %}{{ siteTitle }} - {{ businessName }}{%endblock%} +{% block pageTitle %} + {{ siteTitle }} - {{ businessName }} +{% endblock %} - {% block beforeContent %} - {{ govukBackLink({ - text: "Back", - href: backLink - }) }} - {% endblock %} +{% block beforeContent %} + {{ govukBackLink({ + text: "Back", + href: backLink + }) }} +{% endblock %} - {% block content %} -
-
-

- Agreement details and claims -

-

{{ businessName }}

-
- {% for item in applicationSummaryDetails %} -
-
{{item.field}}
-
-

{{item.newValue}}

- {% if (item.oldValue) and (item.oldValue != "NA") %} -
- - View change - -
- {{ item.oldValue }} -
-
- {% endif %} -
-
- {% endfor %} -
-
-
-
- {% if claimsRowsTotal %} -
-

- Claims by this business -

- {% endif %} -
-
-
- {{ govukTable({ - attributes: { - 'data-module': 'moj-sortable-table' - }, - firstCellIsHeader: false, - head: claimTable.header, - rows: claimTable.claims - }) }} -
- {{ govukPagination({ - previous: model.previous, - next: model.next, - items: model.pages - }) - }} -
-
+{% block content %} +
+
+

Agreement details and claims

+

{{ businessName }}

+
+ {% for item in applicationSummaryDetails %} +
+
{{item.field}}
+
+

{{item.newValue}}

+ {% if (item.oldValue) and (item.oldValue != "NA") %} +
+ + View change + +
+ {{ item.oldValue }} +
+
+ {% endif %} +
+
+ {% endfor %} +
+
+
+
+ {% if claimsRowsTotal %} +
+

Claims by this business

+ {% endif %} +
+
+
+ {{ govukTable({ + attributes: { + 'data-module': 'moj-sortable-table' + }, + firstCellIsHeader: false, + head: claimTable.header, + rows: claimTable.claims + }) }}
-
-{%endblock%} \ No newline at end of file + {{ govukPagination({ + previous: model.previous, + next: model.next, + items: model.pages + }) + }} +
+{%endblock%} diff --git a/app/views/view-agreement.njk b/app/views/view-agreement.njk index 855cd680..72f5f737 100644 --- a/app/views/view-agreement.njk +++ b/app/views/view-agreement.njk @@ -98,4 +98,4 @@ {% endif %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/package-lock.json b/package-lock.json index dd4679f9..781a1ae1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "ffc-ahwr-backoffice", - "version": "1.24.85", + "version": "1.24.86", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "ffc-ahwr-backoffice", - "version": "1.24.85", + "version": "1.24.86", "license": "OGL-UK-3.0", "dependencies": { "@azure/msal-node": "1.14.5", diff --git a/package.json b/package.json index 78458987..0fdd756d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ffc-ahwr-backoffice", - "version": "1.24.85", + "version": "1.24.86", "description": "Back office of the health and welfare of your livestock", "homepage": "https://github.com/DEFRA/ffc-ahwr-backoffice", "main": "app/index.js", diff --git a/test/acceptance/wdio.bs.config.js b/test/acceptance/wdio.bs.config.js index 2642dd90..de15f967 100644 --- a/test/acceptance/wdio.bs.config.js +++ b/test/acceptance/wdio.bs.config.js @@ -1,10 +1,5 @@ const browserstack = require('browserstack-local') -//const { ReportAggregator, HtmlReporter } = require('@rpii/wdio-html-reporter') -//const log4js = require('@log4js-node/log4js-api') const allureReporter = require('@wdio/allure-reporter') -//const cucumberJson = require('wdio-cucumberjs-json-reporter') -//const logger = log4js.getLogger('default') -const _ = require('lodash') const timeStamp = new Date().toLocaleString() const envRoot = (process.env.TEST_ENVIRONMENT_ROOT_URL || 'http://host.docker.internal:3004') const chromeArgs = process.env.CHROME_ARGS ? process.env.CHROME_ARGS.split(' ') : [] diff --git a/test/integration/narrow/routes/view-agreement.test.js b/test/integration/narrow/routes/view-agreement.test.js index 05fcdee9..4f72b188 100644 --- a/test/integration/narrow/routes/view-agreement.test.js +++ b/test/integration/narrow/routes/view-agreement.test.js @@ -24,7 +24,7 @@ function expectApplicationDetails ($) { expect($('.govuk-summary-list__row').length).toEqual(5) expect($('.govuk-summary-list__key').eq(0).text()).toMatch('Name') - expect($('.govuk-summary-list__value').eq(0).text()).toMatch('Farmer name') + expect($('.govuk-summary-list__value').eq(0).text()).toMatch('My Farm') expect($('.govuk-summary-list__key').eq(1).text()).toMatch('SBI number') expect($('.govuk-summary-list__value').eq(1).text()).toMatch('333333333') diff --git a/test/integration/narrow/routes/view-application-date-of-testing.test.js b/test/integration/narrow/routes/view-application-date-of-testing.test.js index 1cdcca61..bf42e67f 100644 --- a/test/integration/narrow/routes/view-application-date-of-testing.test.js +++ b/test/integration/narrow/routes/view-application-date-of-testing.test.js @@ -80,7 +80,7 @@ describe('View Application test with Date of Testing enabled', () => { expect($('title').text()).toContain('Administration: User Agreement') expect($('.govuk-summary-list__row').length).toEqual(5) expect($('.govuk-summary-list__key').eq(0).text()).toMatch('Name') - expect($('.govuk-summary-list__value').eq(0).text()).toMatch('Farmer name') + expect($('.govuk-summary-list__value').eq(0).text()).toMatch('My Farm') expect($('.govuk-summary-list__key').eq(1).text()).toMatch('SBI number') expect($('.govuk-summary-list__value').eq(1).text()).toMatch('333333333') @@ -138,7 +138,7 @@ describe('View Application test with Date of Testing enabled', () => { expect($('title').text()).toContain('Administration: User Agreement') expect($('.govuk-summary-list__row').length).toEqual(5) expect($('.govuk-summary-list__key').eq(0).text()).toMatch('Name') - expect($('.govuk-summary-list__value').eq(0).text()).toMatch('Farmer name') + expect($('.govuk-summary-list__value').eq(0).text()).toMatch('My Farm') expect($('.govuk-summary-list__key').eq(1).text()).toMatch('SBI number') expect($('.govuk-summary-list__value').eq(1).text()).toMatch('333333333') diff --git a/test/integration/narrow/routes/view-claim.test.js b/test/integration/narrow/routes/view-claim.test.js index cf337dbe..5886b23e 100644 --- a/test/integration/narrow/routes/view-claim.test.js +++ b/test/integration/narrow/routes/view-claim.test.js @@ -30,7 +30,7 @@ describe('View claim test', () => { offerStatus: 'accepted', organisation: { sbi: '113494460', - name: 'Mrs S Clark', + name: 'Test Farm Lodge', email: 'russelldaviese@seivadllessurm.com.test', orgEmail: 'orgEmail@gmail.com', address: @@ -216,14 +216,14 @@ describe('View claim test', () => { const content = [ { key: 'Agreement number', value: 'AHWR-1234-APP1' }, { key: 'Agreement date', value: '22/03/2024' }, - { key: 'Agreement holder', value: 'Russell Paul Davies' }, + { key: 'Agreement holder', value: 'Test Farm Lodge' }, { key: 'Agreement holder email', value: 'russelldaviese@seivadllessurm.com.test' }, { key: 'SBI number', value: '113494460' }, { key: 'Address', value: 'Tesco Stores Ltd,Harwell,Betton,WHITE HOUSE FARM,VINCENT CLOSE,LEIGHTON BUZZARD,HR2 8AN,United Kingdom' }, { key: 'Business email', value: 'orgEmail@gmail.com' }, { key: 'Status', value: 'Paid' }, { key: 'Claim date', value: '25/03/2024' }, - { key: 'Business name', value: 'Mrs S Clark' }, + { key: 'Business name', value: 'Test Farm Lodge' }, { key: 'Livestock', value: 'Pigs' }, { key: 'Type of visit', value: 'Animal health and welfare review' }, { key: 'Date of visit', value: '22/03/2024' }, @@ -281,7 +281,7 @@ describe('View claim test', () => { null, { key: 'Status', value: 'Recommended to pay' }, { key: 'Claim date', value: '20/03/2024' }, - { key: 'Business name', value: 'Mrs S Clark' }, + { key: 'Business name', value: 'Test Farm Lodge' }, { key: 'Livestock', value: 'Sheep' }, { key: 'Type of visit', value: 'Endemic disease follow-ups' }, { key: 'Date of visit', value: '22/03/2024' }, diff --git a/test/unit/api/contact-history.test.js b/test/unit/api/contact-history.test.js index 55bd772a..fa3f4bb0 100644 --- a/test/unit/api/contact-history.test.js +++ b/test/unit/api/contact-history.test.js @@ -6,19 +6,6 @@ jest.mock('../../../app/config') describe('contact-history', () => { describe('getContactHistory', () => { - test('returns null if response status is 404', async () => { - const reference = '123' - wreck.get.mockRejectedValueOnce({ - output: { - statusCode: 404 - } - }) - - const result = await getContactHistory(reference) - - expect(result).toBeNull() - }) - test('returns contact history payload on 200 response', async () => { const reference = '123' const payload = [{ createdAt: '2020-01-01', data: { field: 'email', oldValue: 'test@example.com' } }] @@ -59,7 +46,6 @@ describe('contact-history', () => { expect(result).toEqual({ orgEmail: 'NA', email: 'NA', - farmerName: 'NA', address: 'NA' }) }) @@ -69,22 +55,33 @@ describe('contact-history', () => { createdAt: '2020-01-01', data: { field: 'orgEmail', - oldValue: 'org@example.com' + oldValue: 'more-recent-org@example.com' + } + }, { + createdAt: '2019-12-01', + data: { + field: 'orgEmail', + oldValue: 'original-org@example.com' } }, { createdAt: '2020-02-01', data: { field: 'email', - oldValue: 'test@example.com' + oldValue: 'more-recent-test@example.com' + } + }, { + createdAt: '2020-01-01', + data: { + field: 'email', + oldValue: 'original-test@example.com' } }] const result = displayContactHistory(contactHistory) expect(result).toEqual({ - orgEmail: 'Organisation email at start of agreement: org@example.com', - email: 'User email at start of agreement: test@example.com', - farmerName: 'NA', + orgEmail: 'Organisation email at start of agreement: original-org@example.com', + email: 'User email at start of agreement: original-test@example.com', address: 'NA' }) })