Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amend no returns message in view licence Returns tab #1087

Merged
Merged
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
0e9117b
View Licence Returns no return messages
jonathangoulding Jun 11, 2024
b2535dc
feat: add fetch for returns requirements check
jonathangoulding Jun 11, 2024
bb96e66
chore: pre pr check
jonathangoulding Jun 11, 2024
f523b2b
chore: pre pr check
jonathangoulding Jun 11, 2024
f64f88e
chore: pre pr check
jonathangoulding Jun 11, 2024
219424d
chore: pre pr check
jonathangoulding Jun 11, 2024
6672c29
fix: old style test
jonathangoulding Jun 11, 2024
01596a9
chore: pre pr check
jonathangoulding Jun 11, 2024
552c5ed
chore: pre pr check
jonathangoulding Jun 11, 2024
120586e
Merge branch 'main' into feature-licence-returns-summary-no-returns-m…
jonathangoulding Jun 12, 2024
ddf7ea9
refactor: has requirements into the returns service
jonathangoulding Jun 12, 2024
6c51c5f
chore: rename to private method
jonathangoulding Jun 12, 2024
3fe3d75
chore: remove .only
jonathangoulding Jun 12, 2024
a5ba5ad
chore: remove .only
jonathangoulding Jun 12, 2024
ace05de
refactor: error message implementation
jonathangoulding Jun 12, 2024
ed310ed
refactor: rename to DetermineLicenceHasReturnVersionsService
jonathangoulding Jun 12, 2024
b1d5eec
Merge branch 'main' into feature-licence-returns-summary-no-returns-m…
jonathangoulding Jun 12, 2024
26d04aa
Merge branch 'main' into feature-licence-returns-summary-no-returns-m…
jonathangoulding Jun 12, 2024
d774fb2
Update app/views/licences/tabs/returns.njk
jonathangoulding Jun 12, 2024
705fd1c
Update app/views/licences/tabs/returns.njk
jonathangoulding Jun 12, 2024
e46c807
Update app/services/licences/determine-licence-has-return-versions.se…
jonathangoulding Jun 12, 2024
f5b2ec3
Update app/services/licences/determine-licence-has-return-versions.se…
jonathangoulding Jun 12, 2024
d6e80c2
Update app/services/licences/determine-licence-has-return-versions.se…
jonathangoulding Jun 12, 2024
c639d27
Update app/views/licences/tabs/returns.njk
jonathangoulding Jun 12, 2024
6eef66e
fix: pr isssues
jonathangoulding Jun 12, 2024
8b158a3
Merge remote-tracking branch 'origin/feature-licence-returns-summary-…
jonathangoulding Jun 12, 2024
30bab8d
Update test/services/licences/determine-licence-has-return-versions.s…
jonathangoulding Jun 12, 2024
1b1f240
Merge branch 'main' into feature-licence-returns-summary-no-returns-m…
jonathangoulding Jun 12, 2024
d835aff
Merge branch 'main' into feature-licence-returns-summary-no-returns-m…
jonathangoulding Jun 13, 2024
245e718
Merge branch 'main' into feature-licence-returns-summary-no-returns-m…
jonathangoulding Jun 13, 2024
ee066a7
Merge branch 'main' into feature-licence-returns-summary-no-returns-m…
jonathangoulding Jun 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refactor: error message implementation
use njk component for table
jonathangoulding committed Jun 12, 2024

Verified

This commit was signed with the committer’s verified signature.
DanielaE Daniela Engert
commit ace05de256935040a41efa798b5d0489fc1d6c52
23 changes: 20 additions & 3 deletions app/presenters/licences/view-licence-returns.presenter.js
Original file line number Diff line number Diff line change
@@ -10,15 +10,20 @@ const { formatLongDate } = require('../base.presenter.js')
/**
* Formats common data for the `/licences/{id}/*` view licence pages
*
* @param {module:ReturnLogModel[]} returnsData - The session for the return requirement journey
* @param {boolean} hasRequirements - If the licence has return versions then it has requirements
*
* @returns {Object} The data formatted for the view template
*/
function go (returnsData) {
const returns = _formatReturnToTableRow(returnsData.returns)
function go (returnsData, hasRequirements) {
const returns = _formatReturnToTableRow(returnsData)

const hasReturns = returns.length > 0

return {
activeTab: 'returns',
returns,
hasReturns: returns.length > 0
noReturnsMessage: _noReturnsMessage(hasReturns, hasRequirements)
}
}

@@ -54,6 +59,18 @@ function _formatStatus (status) {
return 'NO STATUS'
}

function _noReturnsMessage (hasReturns, hasRequirements) {
if (!hasReturns && !hasRequirements) {
return 'No requirements for returns have been set up for this licence.'
}

if (hasRequirements && !hasReturns) {
return 'No returns for this licence.'
}

return null
}

module.exports = {
go
}
34 changes: 34 additions & 0 deletions app/services/licences/fetch-licence-has-requirements.service.js
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
'use strict'

/**
* Fetches requirements for a licence and returns and returns a boolean if the licence has requirements for returns
* @module FetchLicenceHasRequirementsService
*/

const ReturnVersionModel = require('../../models/return-version.model.js')

/**
* Fetches requirements for a licence and returns a boolean if the licence has requirements for returns
*
* @param {string} licenceId - The UUID for the licence id to fetch
*
* @returns {Promise<Boolean>} the result of check if a licence has requirements for returns
*/
async function go (licenceId) {
const requirement = await _fetch(licenceId)

return !!requirement
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
}

async function _fetch (licenceId) {
return ReturnVersionModel.query()
.select([
'id'
])
.where('licenceId', licenceId)
.first()
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
}

module.exports = {
go
}
21 changes: 4 additions & 17 deletions app/services/licences/view-licence-returns.service.js
Original file line number Diff line number Diff line change
@@ -6,8 +6,8 @@
*/

const FetchLicenceReturnsService = require('./fetch-licence-returns.service.js')
const FetchLicenceHasRequirementsService = require('./fetch-licence-has-requirements.service.js')
const PaginatorPresenter = require('../../presenters/paginator.presenter.js')
const ReturnVersionModel = require('../../models/return-version.model.js')
const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.presenter.js')
const ViewLicenceService = require('./view-licence.service.js')

@@ -23,33 +23,20 @@ const ViewLicenceService = require('./view-licence.service.js')
async function go (licenceId, auth, page) {
const commonData = await ViewLicenceService.go(licenceId, auth)

const returnsData = await FetchLicenceReturnsService.go(licenceId, page)
const pageData = ViewLicenceReturnsPresenter.go(returnsData)
const hasRequirements = await FetchLicenceHasRequirementsService.go(licenceId)

const hasRequirements = await _licenceHasRequirements(licenceId)
const returnsData = await FetchLicenceReturnsService.go(licenceId, page)
const pageData = ViewLicenceReturnsPresenter.go(returnsData.returns, hasRequirements)

const pagination = PaginatorPresenter.go(returnsData.pagination.total, Number(page), `/system/licences/${licenceId}/returns`)

return {
...pageData,
...commonData,
hasRequirements,
pagination
}
}

async function _licenceHasRequirements (licenceId) {
const requirement = await ReturnVersionModel.query()
.select([
'id'
])
.where('licenceId', licenceId)
.first()
.limit(1)

return !!requirement
}

module.exports = {
go
}
79 changes: 48 additions & 31 deletions app/views/licences/tabs/returns.njk
Original file line number Diff line number Diff line change
@@ -19,37 +19,54 @@
{% endif %}
{% endmacro %}

<table class="govuk-table">
<h2 class="govuk-heading-l">Returns</h2>
{% if hasReturns %}
<thead class="govuk-table__head">
<tr class="govuk-table__row">
<th scope="col" class="govuk-table__header">Return reference and dates</th>
<th scope="col" class="govuk-table__header">Purpose and description</th>
<th scope="col" class="govuk-table__header">Due date</th>
<th scope="col" class="govuk-table__header">Status</th>
</tr>
</thead>
<tbody class="govuk-table__body">
{% for return in returns %}
<tr class="govuk-table__row govuk-body-s">
<th class="govuk-table__cell">
<a href="/return/internal?returnId={{ return.id }}" class="govuk-link">{{ return.reference }} </a>
<p class="govuk-body-s"> {{ return.dates }}</p>
</th>
<td class="govuk-table__cell">{{ return.purpose }} <p class="govuk-body-s"> {{ return.description }}</p></td>
<td class="govuk-table__cell">{{ return.dueDate }}</td>
<td class="govuk-table__cell">{{ formatStatus(return.status) }}</td>
</tr>
{% endfor %}
</tbody>

{% elseif hasReturns == false and hasRequirements == false %}
<p> No requirements for returns have been set up for this licence. </p>
{% elseif hasRequirements and hasReturns == false %}
<p> No returns for this licence. </p>
{% endif %}
</table>
<h2 class="govuk-heading-l">Returns</h2>

{% if noReturnsMessage %}
<p> {{ noReturnsMessage }} </p>
{% else %}

{% macro referneceColum(return) %}
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
<a href="/return/internal?returnId={{ return.id }}" class="govuk-link">{{ return.reference }} </a>
<p class="govuk-body-s"> {{ return.dates }}</p>
{% endmacro %}

{% set returnRows = [] %}

{% for returnItem in returns %}
{% set returnRows = (returnRows.push([
{
html: referneceColum(returnItem)
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
},
{
html: returnItem.purpose + "<p class=\"govuk-body-s\">" + returnItem.description + "</p>"
},
{
text: returnItem.dueDate
},
{
html: formatStatus(returnItem.status)
}
]), returnRows) %}
{% endfor %}

{{ govukTable({
head: [
{
text: 'Return reference and dates'
},
{
text: 'Purpose and description'
},
{
text: 'Due date'
},
{
text: 'Status'
}
],
rows: returnRows
}) }}
{% endif %}

{% if hasReturns and pagination.numberOfPages > 1 %}
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
{{ govukPagination(pagination.component) }}
40 changes: 9 additions & 31 deletions test/controllers/licences.controller.test.js
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -403,9 +403,15 @@ describe('Licences controller', () => {
}
})

describe('when a request is valid and has returns', () => {
describe('when a request is valid and has returns and the licence has requirements', () => {
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
beforeEach(async () => {
Sinon.stub(ViewLicenceReturnsService, 'go').resolves(_viewLicenceReturns())
Sinon.stub(ViewLicenceReturnsService, 'go').resolves({
activeTab: 'returns',
returns: [
{ id: 'returns-id' }
],
noReturnsMessage: null
})
})

it('returns the page successfully', async () => {
@@ -426,8 +432,7 @@ describe('Licences controller', () => {
Sinon.stub(ViewLicenceReturnsService, 'go').resolves({
activeTab: 'returns',
returns: [],
hasReturns: false,
hasRequirements: true
noReturnsMessage: 'No returns for this licence.'
})
})

@@ -439,25 +444,6 @@ describe('Licences controller', () => {
expect(response.payload).to.contain('No returns for this licence.')
})
})

describe('when a request is valid and has NO requirements and NO returns', () => {
beforeEach(async () => {
Sinon.stub(ViewLicenceReturnsService, 'go').resolves({
activeTab: 'returns',
returns: [],
hasReturns: false,
hasRequirements: false
})
})

it('returns the page successfully with the message \'No returns for this licence.\'', async () => {
const response = await server.inject(options)

expect(response.statusCode).to.equal(200)
expect(response.payload).to.contain('Returns')
expect(response.payload).to.contain('No requirements for returns have been set up for this licence.')
})
})
})
})

@@ -494,14 +480,6 @@ function _viewLicenceContactDetails () {
}
}

function _viewLicenceReturns () {
return {
activeTab: 'returns',
hasReturns: true,
returns: [{ id: 'returns-id' }]
}
}

function _viewLicenceSummary () {
return {
id: '7861814c-ca19-43f2-be11-3c612f0d744b',
67 changes: 42 additions & 25 deletions test/presenters/licences/view-licence-returns-presenter.test.js
Original file line number Diff line number Diff line change
@@ -11,7 +11,8 @@ const { expect } = Code
const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter.js')

describe('View Licence returns presenter', () => {
let returnData
let returnsData
let hasRequirements

const returnItem = {
id: 'mock-id-1',
@@ -43,26 +44,25 @@ describe('View Licence returns presenter', () => {
}

beforeEach(() => {
returnData = {
returns: [
{ ...returnItem },
{
...returnItem,
id: 'mock-id-2',
status: 'due',
returnReference: '1069'
}
]
}
hasRequirements = true
returnsData = [
{ ...returnItem },
{
...returnItem,
id: 'mock-id-2',
status: 'due',
returnReference: '1069'
}
]
})

describe('when provided with returns data', () => {
it('correctly presents the data', () => {
const result = ViewLicenceReturnsPresenter.go(returnData)
const result = ViewLicenceReturnsPresenter.go(returnsData, hasRequirements)

expect(result).to.equal({
activeTab: 'returns',
hasReturns: true,
noReturnsMessage: null,
returns: [
{
id: 'mock-id-1',
@@ -87,20 +87,37 @@ describe('View Licence returns presenter', () => {
})
})

describe('when provided with NO returns data', () => {
beforeEach(() => {
returnData = {
returns: []
}
describe('the "noReturnsMessage" property', () => {
describe('when a licence has returns and requirements', () => {
it('returns null', () => {
const result = ViewLicenceReturnsPresenter.go(returnsData, hasRequirements)

expect(result.noReturnsMessage).to.be.null()
})
})

it('correctly returns no returns data ', () => {
const result = ViewLicenceReturnsPresenter.go(returnData)
describe('when a licence has NO returns and NO requirements', () => {
beforeEach(() => {
returnsData = []
hasRequirements = false
})

expect(result).to.equal({
activeTab: 'returns',
hasReturns: false,
returns: []
it('presents the No returns and No requirements message "No requirements for returns have been set up for this licence."', () => {
const result = ViewLicenceReturnsPresenter.go(returnsData, hasRequirements)

expect(result.noReturnsMessage).to.equal('No requirements for returns have been set up for this licence.')
})
})

describe('when a licence has returns but no requirements', () => {
beforeEach(() => {
returnsData = []
})

it('presents the returns but no requirements message "No returns for this licence."', () => {
const result = ViewLicenceReturnsPresenter.go(returnsData, hasRequirements)

expect(result.noReturnsMessage).to.equal('No returns for this licence.')
})
})
})
Loading