Skip to content

Commit

Permalink
Suggested View contact details changes
Browse files Browse the repository at this point in the history
https://eaflood.atlassian.net/browse/WATER-4433

This relates to [Add View Licence contact details page](#993). What started as experimenting with an alternate fetch implementation has grown into a detailed comparison of the original changes and our current patterns and conventions.

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

These are suggested and can be challenged. But we felt in this case "show don't tell" would be more appropriate to support the conversation!
  • Loading branch information
Cruikshanks committed May 13, 2024
1 parent fc84d51 commit 0854af3
Show file tree
Hide file tree
Showing 12 changed files with 395 additions and 426 deletions.
12 changes: 6 additions & 6 deletions app/controllers/licences.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
*/

const InitiateSessionService = require('../services/return-requirements/initiate-session.service.js')
const ViewLicenceBillsService = require('../services/licences/view-licence-bills.service')
const ViewLicenceContactDetailsService = require('../services/licences/view-licence-contact-details.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 @@ -40,9 +40,9 @@ async function viewBills (request, h) {
}

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

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

return h.view(ViewLicencePage, {
...data
Expand Down
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
}
55 changes: 0 additions & 55 deletions app/presenters/licences/view-licence-contact-details.presenter.js

This file was deleted.

62 changes: 0 additions & 62 deletions app/services/licences/fetch-licence-contact-details.service.js

This file was deleted.

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
}
19 changes: 11 additions & 8 deletions app/services/licences/view-licence-contact-details.service.js
Original file line number Diff line number Diff line change
@@ -1,30 +1,33 @@
'use strict'

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

const FetchLicenceContactDetailsService = require('./fetch-licence-contact-details.service')
const ViewLicenceContactDetailsPresenter = require('../../presenters/licences/view-licence-contact-details.presenter')
const ViewLicenceService = require('./view-licence.service')
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.
* @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)

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

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

Expand Down
40 changes: 20 additions & 20 deletions app/views/licences/tabs/contact-details.njk
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<h2 class="govuk-heading-l">Contact Details</h2>

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

{% if licenceContacts.length > 0 %}
Expand All @@ -15,34 +15,34 @@
</tr>
</thead>
<tbody class="govuk-table__body">
{% for contact in licenceContacts %}
{% for licenceContact in licenceContacts %}
<tr class="govuk-table__row">
<td class="govuk-table__cell"> {{ contact.name }}</td>
<td class="govuk-table__cell"> {{ contact.communicationType }}</td>
<td class="govuk-table__cell">{{ licenceContact.name }}</td>
<td class="govuk-table__cell">{{ licenceContact.communicationType }}</td>
<td class="govuk-table__cell">
{% if contact.address.address1 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ contact.address.address1 }}</p>
{% if licenceContact.address.address1 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address1 }}</p>
{% endif %}
{% if contact.address.address2 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ contact.address.address2 }}</p>
{% if licenceContact.address.address2 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address2 }}</p>
{% endif %}
{% if contact.address.address3 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ contact.address.address3 }}</p>
{% if licenceContact.address.address3 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address3 }}</p>
{% endif %}
{% if contact.address.address4 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ contact.address.address4 }}</p>
{% if licenceContact.address.address4 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address4 }}</p>
{% endif %}
{% if contact.address.address5 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ contact.address.address5 }}</p>
{% if licenceContact.address.address5 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address5 }}</p>
{% endif %}
{% if contact.address.address6 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ contact.address.address6 }}</p>
{% if licenceContact.address.address6 %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.address6 }}</p>
{% endif %}
{% if contact.address.country %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ contact.address.country }}</p>
{% if licenceContact.address.country %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.country }}</p>
{% endif %}
{% if contact.address.postcode %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ contact.address.postcode }}</p>
{% if licenceContact.address.postcode %}
<p class="govuk-body govuk-!-margin-bottom-0">{{ licenceContact.address.postcode }}</p>
{% endif %}
</tr>
{% endfor %}
Expand Down
Loading

0 comments on commit 0854af3

Please sign in to comment.