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

Stop adding link on due returns for unauth users #1378

Merged
merged 7 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 12 additions & 6 deletions app/presenters/licences/view-licence-returns.presenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ const { formatLongDate } = require('../base.presenter.js')
*
* @param {module:ReturnLogModel[]} returnLogs - The results from `FetchLicenceReturnsService` to be formatted
* @param {boolean} hasRequirements - True if the licence has return requirements else false
* @param {object} auth - The auth object taken from `request.auth` containing user details
*
* @returns {object} The data formatted for the view template
*/
function go (returnLogs, hasRequirements) {
const returns = _returns(returnLogs)
function go (returnLogs, hasRequirements, auth) {
const canManageReturns = auth.credentials.scope.includes('returns')
const returns = _returns(returnLogs, canManageReturns)

const hasReturns = returns.length > 0

Expand All @@ -26,12 +28,16 @@ function go (returnLogs, hasRequirements) {
}
}

function _link (status, returnLogId) {
function _link (status, returnLogId, canManageReturns) {
if (['completed', 'void'].includes(status)) {
return `/returns/return?id=${returnLogId}`
}

return `/return/internal?returnId=${returnLogId}`
if (canManageReturns) {
return `/return/internal?returnId=${returnLogId}`
}

return null
}

function _noReturnsMessage (hasReturns, hasRequirements) {
Expand All @@ -52,15 +58,15 @@ function _purpose (purpose) {
return firstPurpose.alias ? firstPurpose.alias : firstPurpose.tertiary.description
}

function _returns (returns) {
function _returns (returns, canManageReturns) {
return returns.map((returnLog) => {
const { endDate, dueDate, id: returnLogId, metadata, returnReference, startDate, status } = returnLog

return {
dates: `${formatLongDate(new Date(startDate))} to ${formatLongDate(new Date(endDate))}`,
description: metadata.description,
dueDate: formatLongDate(new Date(dueDate)),
link: _link(status, returnLogId),
link: _link(status, returnLogId, canManageReturns),
purpose: _purpose(metadata.purposes),
reference: returnReference,
returnLogId,
Expand Down
2 changes: 1 addition & 1 deletion app/services/licences/view-licence-returns.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ async function go (licenceId, auth, page) {
const hasRequirements = await DetermineLicenceHasReturnVersionsService.go(licenceId)

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

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

Expand Down
7 changes: 6 additions & 1 deletion app/views/licences/tabs/returns.njk
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
{% from "macros/return-status-tag.njk" import statusTag %}

{% macro referenceColumn(return) %}
<a href="{{ return.link }}" class="govuk-link">{{ return.reference }} </a>
{% if return.link %}
<a href="{{ return.link }}" class="govuk-link">{{ return.reference }} </a>
{% else %}
<div>{{ return.reference }}</div>
{% endif %}

<p class="govuk-body-s">{{ return.dates }}</p>
{% endmacro %}

Expand Down
80 changes: 60 additions & 20 deletions test/presenters/licences/view-licence-returns.presenter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,29 @@ const { expect } = Code
const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter.js')

describe('View Licence returns presenter', () => {
let auth
let returnLogs
let hasRequirements

beforeEach(() => {
auth = {
isValid: true,
credentials: {
user: { id: 123 },
roles: ['returns'],
groups: [],
scope: ['returns'],
permissions: { abstractionReform: false, billRuns: true, manage: true }
}
}

hasRequirements = true
returnLogs = _returnLogs()
})

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

expect(result).to.equal({
noReturnsMessage: null,
Expand Down Expand Up @@ -52,7 +64,7 @@ describe('View Licence returns presenter', () => {

describe('the "dates" property', () => {
it('returns the start and end date in long format (2 January 2020 to 1 February 2020)', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[0].dates).to.equal('2 January 2020 to 1 February 2020')
})
Expand All @@ -61,17 +73,31 @@ describe('View Licence returns presenter', () => {
describe('the "link" property', () => {
describe('when the return log has a status of "completed"', () => {
it('returns a link to the view return log page', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[0].link).to.equal('/returns/return?id=v1:1:01/123:10046821:2020-01-02:2020-02-01')
})
})

describe('when the return log has a status of "due"', () => {
it('returns a link to the edit return log page', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
describe('and the user is permitted to submit and edit returns', () => {
it('returns a link to the edit return log page', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].link).to.equal('/return/internal?returnId=v1:1:01/123:10046820:2020-01-02:2020-02-01')
})
})

describe('and the user is not permitted to submit and edit returns', () => {
beforeEach(() => {
auth.credentials.scope = []
})

it('returns null', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].link).to.equal('/return/internal?returnId=v1:1:01/123:10046820:2020-01-02:2020-02-01')
expect(result.returns[1].link).to.be.null()
})
})
})

Expand All @@ -80,10 +106,24 @@ describe('View Licence returns presenter', () => {
returnLogs[1].status = 'received'
})

it('returns a link to the edit return log page', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
describe('and the user is permitted to submit and edit returns', () => {
it('returns a link to the edit return log page', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].link).to.equal('/return/internal?returnId=v1:1:01/123:10046820:2020-01-02:2020-02-01')
expect(result.returns[1].link).to.equal('/return/internal?returnId=v1:1:01/123:10046820:2020-01-02:2020-02-01')
})
})

describe('and the user is not permitted to submit and edit returns', () => {
beforeEach(() => {
auth.credentials.scope = []
})

it('returns null', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].link).to.be.null()
})
})
})

Expand All @@ -93,7 +133,7 @@ describe('View Licence returns presenter', () => {
})

it('returns a link to the view return log page', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].link).to.equal('/returns/return?id=v1:1:01/123:10046820:2020-01-02:2020-02-01')
})
Expand All @@ -103,7 +143,7 @@ describe('View Licence returns presenter', () => {
describe('the "purpose" property', () => {
describe("when the first purpose in the return log's metadata does not have an alias", () => {
it("returns the purpose's tertiary description", () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].purpose).to.equal('SPRAY IRRIGATION')
})
Expand All @@ -115,7 +155,7 @@ describe('View Licence returns presenter', () => {
})

it("returns the purpose's alias", () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].purpose).to.equal('Spray irrigation - top field only')
})
Expand All @@ -125,7 +165,7 @@ describe('View Licence returns presenter', () => {
describe('the "status" property', () => {
describe('when the return log has a status of "completed"', () => {
it('returns "complete"', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[0].status).to.equal('complete')
})
Expand All @@ -134,7 +174,7 @@ describe('View Licence returns presenter', () => {
describe('when the return log has a status of "due"', () => {
describe('and the due date is less than today', () => {
it('returns "overdue"', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].status).to.equal('overdue')
})
Expand All @@ -147,7 +187,7 @@ describe('View Licence returns presenter', () => {
})

it('returns "due"', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].status).to.equal('due')
})
Expand All @@ -160,7 +200,7 @@ describe('View Licence returns presenter', () => {
})

it('returns "received"', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].status).to.equal('received')
})
Expand All @@ -172,7 +212,7 @@ describe('View Licence returns presenter', () => {
})

it('returns "void"', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.returns[1].status).to.equal('void')
})
Expand All @@ -183,7 +223,7 @@ describe('View Licence returns presenter', () => {
describe('the "noReturnsMessage" property', () => {
describe('when a licence has returns and requirements', () => {
it('returns null', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.noReturnsMessage).to.be.null()
})
Expand All @@ -196,7 +236,7 @@ describe('View Licence returns presenter', () => {
})

it('returns the message "No requirements for returns have been set up for this licence."', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.noReturnsMessage).to.equal('No requirements for returns have been set up for this licence.')
})
Expand All @@ -208,7 +248,7 @@ describe('View Licence returns presenter', () => {
})

it('returns the message "No returns for this licence."', () => {
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements)
const result = ViewLicenceReturnsPresenter.go(returnLogs, hasRequirements, auth)

expect(result.noReturnsMessage).to.equal('No returns for this licence.')
})
Expand Down
14 changes: 13 additions & 1 deletion test/services/licences/view-licence-returns.service.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,21 @@ const ViewLicenceReturnsService = require('../../../app/services/licences/view-l
describe('View Licence Returns service', () => {
const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de'
const page = 1
const auth = {}

let auth

beforeEach(async () => {
auth = {
isValid: true,
credentials: {
user: { id: 123 },
roles: ['returns'],
groups: [],
scope: ['returns'],
permissions: { abstractionReform: false, billRuns: true, manage: true }
}
}

Sinon.stub(DetermineLicenceHasReturnVersionsService, 'go').returns(true)

Sinon.stub(FetchLicenceReturnsService, 'go').resolves({
Expand Down
Loading