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
Show file tree
Hide file tree
Changes from 9 commits
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
5 changes: 3 additions & 2 deletions app/presenters/licences/view-licence-returns.presenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* @module ViewLicenceReturnsPresenter
*/

const { formatLongDate } = require('../base.presenter')
const { formatLongDate } = require('../base.presenter.js')

/**
* Formats common data for the `/licences/{id}/*` view licence pages
Expand All @@ -17,7 +17,8 @@ function go (returnsData) {

return {
activeTab: 'returns',
returns
returns,
hasReturns: returns.length > 0
}
}

Expand Down
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
}
14 changes: 10 additions & 4 deletions app/services/licences/view-licence-returns.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,18 @@
* @module ViewLicenceSummaryService
*/

const FetchLicenceReturnsService = require('./fetch-licence-returns.service')
const ViewLicenceService = require('./view-licence.service')
const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.presenter')
const PaginatorPresenter = require('../../presenters/paginator.presenter')
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 ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.presenter.js')
const ViewLicenceService = require('./view-licence.service.js')

/**
* Orchestrates fetching and presenting the data needed for the licence summary page
*
* @param {string} licenceId - The UUID of the licence
* @param {Object} auth - The auth object taken from `request.auth` containing user details
* @param {Object} page - The current page for the pagination service
*
* @returns {Promise<Object>} an object representing the `pageData` needed by the licence summary template.
*/
Expand All @@ -23,11 +26,14 @@ async function go (licenceId, auth, page) {
const returnsData = await FetchLicenceReturnsService.go(licenceId, page)
const pageData = ViewLicenceReturnsPresenter.go(returnsData)

const hasRequirements = await FetchLicenceHasRequirementsService.go(licenceId)

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

return {
...pageData,
...commonData,
hasRequirements,
pagination
}
}
Expand Down
2 changes: 1 addition & 1 deletion app/services/licences/view-licence.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const ViewLicencePresenter = require('../../presenters/licences/view-licence.pre
* Orchestrates fetching and presenting the data needed for the licence summary page
*
* @param {string} licenceId - The UUID of the licence
* @param {string} auth - Auth object
* @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 summary template.
*/
async function go (licenceId, auth) {
Expand Down
11 changes: 7 additions & 4 deletions app/views/licences/tabs/returns.njk
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

<table class="govuk-table">
<h2 class="govuk-heading-l">Returns</h2>
{% if returns.length > 0 %}
{% if hasReturns %}
<thead class="govuk-table__head">
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
<tr class="govuk-table__row">
<th scope="col" class="govuk-table__header">Return reference and dates</th>
Expand All @@ -43,12 +43,15 @@
</tr>
{% endfor %}
</tbody>
{% else %}
<p>No returns found</p>

{% elseif hasReturns == false and hasRequirements == false %}
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
<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>

{% if returns and pagination.numberOfPages > 1 %}
{% if hasReturns and pagination.numberOfPages > 1 %}
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
{{ govukPagination(pagination.component) }}
{% endif %}

31 changes: 26 additions & 5 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
Expand Up @@ -421,21 +421,41 @@ describe('Licences controller', () => {
})
})

describe('when a request is valid and has NO returns', () => {
describe('when a request is valid and has requirements but NO returns', () => {
jonathangoulding marked this conversation as resolved.
Show resolved Hide resolved
beforeEach(async () => {
Sinon.stub(ViewLicenceReturnsService, 'go').resolves({
activeTab: 'returns',
returns: []
returns: [],
hasReturns: false,
hasRequirements: true
})
})

it('returns the page successfully', async () => {
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')
// Check the table titles
expect(response.payload).to.contain('No returns found')
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.')
})
})
})
Expand Down Expand Up @@ -477,6 +497,7 @@ function _viewLicenceContactDetails () {
function _viewLicenceReturns () {
return {
activeTab: 'returns',
hasReturns: true,
returns: [{ id: 'returns-id' }]
}
}
Expand Down
127 changes: 61 additions & 66 deletions test/presenters/licences/view-licence-returns-presenter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,61 @@ const { describe, it, beforeEach } = exports.lab = Lab.script()
const { expect } = Code

// Thing under test
const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter')
const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter.js')

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

const returnItem = {
id: 'mock-id-1',
dueDate: '2012-11-28T00:00:00.000Z',
status: 'completed',
startDate: '2020/01/02',
endDate: '2020/02/01',
metadata: {
purposes: [
{
alias: 'SPRAY IRRIGATION',
primary: {
code: 'A',
description: 'Agriculture'
},
tertiary: {
code: '400',
description: 'Spray Irrigation - Direct'
},
secondary: {
code: 'AGR',
description: 'General Agriculture'
}
}
],
description: 'empty description'
},
returnReference: '1068'
}

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

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

expect(result).to.equal({
activeTab: 'returns',
hasReturns: true,
returns: [
{
id: 'mock-id-1',
Expand All @@ -37,7 +77,7 @@ describe('View Licence returns presenter', () => {
id: 'mock-id-2',
reference: '1069',
purpose: 'SPRAY IRRIGATION',
dueDate: '28 November 2019',
dueDate: '28 November 2012',
status: 'OVERDUE',
dates: '2 January 2020 to 1 February 2020',
description: 'empty description'
Expand All @@ -46,67 +86,22 @@ describe('View Licence returns presenter', () => {
})
})
})
})

function _returnData () {
return {
returns: [
{
id: 'mock-id-1',
dueDate: '2012-11-28T00:00:00.000Z',
status: 'completed',
startDate: '2020/01/02',
endDate: '2020/02/01',
metadata: {
purposes: [
{
alias: 'SPRAY IRRIGATION',
primary: {
code: 'A',
description: 'Agriculture'
},
tertiary: {
code: '400',
description: 'Spray Irrigation - Direct'
},
secondary: {
code: 'AGR',
description: 'General Agriculture'
}
}
],
description: 'empty description'
},
returnReference: '1068'
},
{
id: 'mock-id-2',
dueDate: '2019-11-28T00:00:00.000Z',
status: 'due',
startDate: '2020/01/02',
endDate: '2020/02/01',
metadata: {
description: 'empty description',
purposes: [
{
alias: 'SPRAY IRRIGATION',
primary: {
code: 'A',
description: 'Agriculture'
},
tertiary: {
code: '400',
description: 'Spray Irrigation - Direct'
},
secondary: {
code: 'AGR',
description: 'General Agriculture'
}
}
]
},
returnReference: '1069'
describe('when provided with NO returns data', () => {
beforeEach(() => {
returnData = {
returns: []
}
]
}
}
})

it('correctly returns no returns data ', () => {
const result = ViewLicenceReturnsPresenter.go(returnData)

expect(result).to.equal({
activeTab: 'returns',
hasReturns: false,
returns: []
})
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict'

// Test framework dependencies
const Lab = require('@hapi/lab')
const Code = require('@hapi/code')

const { describe, it, beforeEach } = exports.lab = Lab.script()
const { expect } = Code

// Test helpers
const DatabaseSupport = require('../../support/database.js')
const ReturnVersionHelper = require('../../support/helpers/return-version.helper.js')

// Thing under test
const FetchLicenceHasRequirementsService =
require('../../../app/services/licences/fetch-licence-has-requirements.service.js')

describe('Fetch Licence Has Requirements service', () => {
let returnVersion

beforeEach(async () => {
await DatabaseSupport.clean()
})

describe('when the licence has return versions data', () => {
beforeEach(async () => {
returnVersion = await ReturnVersionHelper.add()
})

it('returns true a licence has any return versions', async () => {
const result = await FetchLicenceHasRequirementsService.go(returnVersion.licenceId)

expect(result).to.be.true()
})

it('returns false no return versions for licence', async () => {
const result = await FetchLicenceHasRequirementsService.go('ed3b9b1a-94e0-480c-8ad6-60e05f5fa9f4')

expect(result).to.be.false()
})
})
})
Loading
Loading