Skip to content

Commit

Permalink
Add View Licence contact details page (#993)
Browse files Browse the repository at this point in the history
* Add View Licence contact details page

https://eaflood.atlassian.net/browse/WATER-4433
https://eaflood.atlassian.net/browse/WATER-4434

The existing service handling view licence is slow because it loads all the data for the tabs in one render. Work has been done previously to refactor the summary page to load only the summary information.

This change will introduce a contact details controller, service and presenter to handle the view licence contact details page.

This will share the same view as the summary page and load the same 'common data' established in [previous work](#957).

PR made to optimise the fetch service for the licence contact details
Suggested View contact details changes (#1011)

In this commit, we have tried to highlight how we think the changes would be implemented if they followed our current norms.

- corrections - adding line breaks, extensions to module imports, parens and block body for arrow functions
- fetch logic - switching from Objection to Knex directly. Where we are fetching data from various tables to provide a summarised view, and not intending to interact with the instances other than that, dropping to Knex can make things easier and remove the need for additional logic in presenters services
- single responsibility - switching the basis for the modules to be single-responsibility. The ViewService will be responsible for fetching and presenting the data for the whole tab. But assume fetching and presenter licence contacts versus customer contacts will be different hence rename/update the fetch and presenter to be licence contact-specific

---------

Co-authored-by: Alan Cruikshanks <[email protected]>
  • Loading branch information
jonathangoulding and Cruikshanks authored May 13, 2024
1 parent 8c8964c commit 1e94502
Show file tree
Hide file tree
Showing 11 changed files with 552 additions and 3 deletions.
18 changes: 15 additions & 3 deletions app/controllers/licences.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
*/

const InitiateSessionService = require('../services/return-requirements/initiate-session.service.js')
const ViewLicenceBillsService = require('../services/licences/view-licence-bills.service')
const ViewLicenceReturnsService = require('../services/licences/view-licence-returns.service')
const ViewLicenceSummaryService = require('../services/licences/view-licence-summary.service')
const ViewLicenceBillsService = require('../services/licences/view-licence-bills.service.js')
const ViewLicenceContactDetailsService = require('../services/licences/view-licence-contact-details.service.js')
const ViewLicenceReturnsService = require('../services/licences/view-licence-returns.service.js')
const ViewLicenceSummaryService = require('../services/licences/view-licence-summary.service.js')

const ViewLicencePage = 'licences/view.njk'

Expand Down Expand Up @@ -38,6 +39,16 @@ async function viewBills (request, h) {
})
}

async function viewContacts (request, h) {
const { params: { id }, auth } = request

const data = await ViewLicenceContactDetailsService.go(id, auth)

return h.view(ViewLicencePage, {
...data
})
}

async function viewSummary (request, h) {
const { params: { id }, auth } = request

Expand All @@ -62,6 +73,7 @@ module.exports = {
noReturnsRequired,
returnsRequired,
viewBills,
viewContacts,
viewReturns,
viewSummary
}
61 changes: 61 additions & 0 deletions app/presenters/licences/licence-contacts.presenter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
'use strict'

/**
* Formats data for the `/licences/{id}/contact-details` view licence contact details page
* @module ViewLicenceContactDetailsPresenter
*/

/**
* Formats data for the `/licences/{id}/contact-details` view licence contact details page
*
* @returns {Object} The data formatted for the view template
*/
function go (contacts) {
return {
customerId: _findCustomerId(contacts),
licenceContacts: _licenceContacts(contacts)
}
}

function _findCustomerId (contacts) {
const customerContact = contacts.find((contact) => {
return contact.communicationType === 'Licence Holder'
})

if (customerContact) {
return customerContact.companyId
}

return null
}

function _licenceContactName (contact) {
if (contact.contactId) {
return `${contact.firstName || ''} ${contact.lastName}`.trim()
}

return contact.companyName
}

function _licenceContacts (contacts) {
return contacts.map((contact) => {
return {
address: {
address1: contact.address1,
address2: contact.address2,
address3: contact.address3,
address4: contact.address4,
address5: contact.address5,
address6: contact.address6,
country: contact.country,
postcode: contact.postcode
},
communicationType: contact.communicationType,
name: _licenceContactName(contact)
}
})
}

module.exports = {
go
}
8 changes: 8 additions & 0 deletions app/routes/licence.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ const routes = [
description: 'View a licence bills page'
}
},
{
method: 'GET',
path: '/licences/{id}/contact-details',
handler: LicencesController.viewContacts,
options: {
description: 'View a licence contacts page'
}
},
{
method: 'GET',
path: '/licences/{id}/summary',
Expand Down
55 changes: 55 additions & 0 deletions app/services/licences/fetch-licence-contacts.service.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
'use strict'

/**
* Fetches all return logs for a licence which is needed for the view '/licences/{id}/contact-details` page
* @module FetchLicenceContactDetailsService
*/

const { db } = require('../../../db/db.js')

/**
* Fetches all contact details for a licence which is needed for the view '/licences/{id}/contact-details` page
*
* @param {string} licenceId - The UUID for the licence to fetch
*
* @returns {Promise<Object>} the data needed to populate the view licence page's contact details tab
*/
async function go (licenceId) {
return _fetch(licenceId)
}

async function _fetch (licenceId) {
return db.withSchema('public')
.select([
'lr.label AS communicationType',
'cmp.id AS companyId',
'cmp.name AS companyName',
'con.id AS contactId',
'con.firstName',
'con.lastName',
'a.address1',
'a.address2',
'a.address3',
'a.address4',
'a.address5',
'a.address6',
'a.postcode',
'a.country'
])
.from('licenceDocuments AS ld')
.innerJoin('licences AS l', 'l.licenceRef', '=', 'ld.licenceRef')
.innerJoin('licenceDocumentRoles AS ldr', 'ldr.licenceDocumentId', '=', 'ld.id')
.innerJoin('licenceRoles AS lr', 'lr.id', '=', 'ldr.licenceRoleId')
.innerJoin('companies AS cmp', 'cmp.id', '=', 'ldr.companyId')
.leftJoin('contacts AS con', 'con.id', '=', 'ldr.contactId')
.innerJoin('addresses AS a', 'a.id', '=', 'ldr.addressId')
.where('l.id', '=', licenceId)
.andWhere((builder) => {
builder.whereNull('ldr.end_date').orWhere('ldr.end_date', '>', db.raw('NOW()'))
})
.orderBy('lr.label', 'desc')
}

module.exports = {
go
}
36 changes: 36 additions & 0 deletions app/services/licences/view-licence-contact-details.service.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict'

/**
* Orchestrates fetching and presenting the data needed for the view licence contact details tab
* @module ViewLicenceContactDetailsService
*/

const FetchLicenceContactsService = require('./fetch-licence-contacts.service.js')
const LicenceContactsPresenter = require('../../presenters/licences/licence-contacts.presenter.js')
const ViewLicenceService = require('./view-licence.service.js')

/**
* Orchestrates fetching and presenting the data needed for the licence contact details page
*
* @param {string} licenceId - The UUID of the licence
* @param {Object} auth - The auth object taken from `request.auth` containing user details
*
* @returns {Promise<Object>} an object representing the `pageData` needed by the licence contact details template.
*/
async function go (licenceId, auth) {
const commonData = await ViewLicenceService.go(licenceId, auth)

// Licence contact details
const licenceContacts = await FetchLicenceContactsService.go(licenceId)
const licenceContactsData = LicenceContactsPresenter.go(licenceContacts)

return {
activeTab: 'contact-details',
...commonData,
...licenceContactsData
}
}

module.exports = {
go
}
53 changes: 53 additions & 0 deletions app/views/licences/tabs/contact-details.njk
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<h2 class="govuk-heading-l">Contact Details</h2>

{% if customerId %}
<p><a href='/customer/{{ customerId }}/#contacts'>Go to customer contacts</a></p>
{% endif %}

{% if licenceContacts.length > 0 %}
<h2 class="govuk-heading-m">Licence contacts</h2>
<table class="govuk-table">
<thead class="govuk-table__head">
<tr class="govuk-table__row">
<th scope="col" class="govuk-table__header">Name</th>
<th scope="col" class="govuk-table__header">Communication type</th>
<th scope="col" class="govuk-table__header">Send to</th>
</tr>
</thead>
<tbody class="govuk-table__body">
{% for licenceContact in licenceContacts %}
<tr class="govuk-table__row">
<td class="govuk-table__cell">{{ licenceContact.name }}</td>
<td class="govuk-table__cell">{{ licenceContact.communicationType }}</td>
<td class="govuk-table__cell">
{% if licenceContact.address.address1 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address1 }}</p>
{% endif %}
{% if licenceContact.address.address2 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address2 }}</p>
{% endif %}
{% if licenceContact.address.address3 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address3 }}</p>
{% endif %}
{% if licenceContact.address.address4 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address4 }}</p>
{% endif %}
{% if licenceContact.address.address5 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address5 }}</p>
{% endif %}
{% if licenceContact.address.address6 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address6 }}</p>
{% endif %}
{% if licenceContact.address.country %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.country }}</p>
{% endif %}
{% if licenceContact.address.postcode %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.postcode }}</p>
{% endif %}
</tr>
{% endfor %}
</tbody>
</table>
{% else %}
<p>No contacts found.</p>
{% endif %}
3 changes: 3 additions & 0 deletions app/views/licences/view.njk
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@
{% if activeTab === 'summary' %}
{% include "licences/tabs/summary.njk" %}
{% endif %}
{% if activeTab === 'contact-details' %}
{% include "licences/tabs/contact-details.njk" %}
{% endif %}
{% if activeTab === 'returns' %}
{% include "licences/tabs/returns.njk" %}
{% endif %}
Expand Down
52 changes: 52 additions & 0 deletions test/controllers/licences.controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const Boom = require('@hapi/boom')
// Things we need to stub
const InitiateSessionService = require('../../app/services/return-requirements/initiate-session.service.js')
const ViewLicenceBillsService = require('../../app/services/licences/view-licence-bills.service')
const ViewLicenceContactDetailsService = require('../../app/services/licences/view-licence-contact-details.service')
const ViewLicenceSummaryService = require('../../app/services/licences/view-licence-summary.service')
const ViewLicenceReturnsService = require('../../app/services/licences/view-licence-returns.service')

Expand Down Expand Up @@ -199,6 +200,50 @@ describe('Licences controller', () => {
})
})

describe('GET /licences/{id}/contact-details', () => {
beforeEach(async () => {
options = {
method: 'GET',
url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/contact-details',
auth: {
strategy: 'session',
credentials: { scope: [] }
}
}
})

describe('when a request is valid and has contacts', () => {
beforeEach(async () => {
Sinon.stub(ViewLicenceContactDetailsService, 'go').resolves(_viewLicenceContactDetails())
})

it('returns the page successfully', async () => {
const response = await server.inject(options)

expect(response.statusCode).to.equal(200)
expect(response.payload).to.contain('Contact Details')
// Table row titles
expect(response.payload).to.contain('Name')
expect(response.payload).to.contain('Communication type')
expect(response.payload).to.contain('Send to')
})
})

describe('when a request is valid and has no contact details', () => {
beforeEach(async () => {
Sinon.stub(ViewLicenceContactDetailsService, 'go').resolves({ activeTab: 'contact-details' })
})

it('returns the page successfully', async () => {
const response = await server.inject(options)

expect(response.statusCode).to.equal(200)
expect(response.payload).to.contain('Contact Details')
expect(response.payload).to.contain('No contacts found.')
})
})
})

describe('GET /licences/{id}/summary', () => {
beforeEach(async () => {
options = {
Expand Down Expand Up @@ -284,6 +329,13 @@ function _viewLicenceBills () {
}
}

function _viewLicenceContactDetails () {
return {
activeTab: 'contact-details',
licenceContacts: [{}]
}
}

function _viewLicenceReturns () {
return {
activeTab: 'returns',
Expand Down
Loading

0 comments on commit 1e94502

Please sign in to comment.