From ff796ccf8f5b15ef5fff6ccac70fefc07cfc7af7 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 13:23:45 +0100 Subject: [PATCH 01/34] add breadcrumbs to find page --- src/views/organisations/find.html | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/views/organisations/find.html b/src/views/organisations/find.html index eb577631..2ac1bc7b 100644 --- a/src/views/organisations/find.html +++ b/src/views/organisations/find.html @@ -1,3 +1,5 @@ +{% from "govuk/components/breadcrumbs/macro.njk" import govukBreadcrumbs %} + {% extends "layouts/main.html" %} {% set serviceType = 'Submit' %} @@ -7,6 +9,15 @@ {{ super() }} +{{ govukBreadcrumbs({ + items: [ + { + text: "Home", + href: "/manage" + } + ] +}) }} + {% endblock %} {% block content %} From 39077da002b131e54435ca1fb3b8fb24aadb977c Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 13:27:42 +0100 Subject: [PATCH 02/34] add more to breadcrumbs --- src/views/organisations/find.html | 3 +++ src/views/organisations/overview.html | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/views/organisations/find.html b/src/views/organisations/find.html index 2ac1bc7b..384cebe2 100644 --- a/src/views/organisations/find.html +++ b/src/views/organisations/find.html @@ -14,6 +14,9 @@ { text: "Home", href: "/manage" + }, + { + text: "Organisations" } ] }) }} diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index e83a6064..1325780e 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -13,11 +13,14 @@ items: [ { text: "Home", - href: "/overview/start" + href: "/manage" }, { text: "Organisations", - href: "/overview/organisations" + href: "/organisations" + }, + { + text: organisation.name } ] }) }} From b6004d3cb779cf172773aed54586dbd7d91374c4 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Fri, 9 Aug 2024 15:28:19 +0100 Subject: [PATCH 03/34] fix wording for status messagaes --- src/views/organisations/overview.html | 6 +++--- test/unit/lpaOverviewPage.test.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index e83a6064..cf413784 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -40,17 +40,17 @@

{{datasetsWithEndpoints}}/{{totalDatasets}} - datasets provided + datasets submitted
{{datasetsWithErrors}} - datasets with errors + data URL with errors
{{datasetsWithIssues}} - datasets with issues + datasets need fixing
diff --git a/test/unit/lpaOverviewPage.test.js b/test/unit/lpaOverviewPage.test.js index 0f1518ce..e9817d26 100644 --- a/test/unit/lpaOverviewPage.test.js +++ b/test/unit/lpaOverviewPage.test.js @@ -91,17 +91,17 @@ describe('LPA Overview Page', () => { const statsBoxes = document.querySelector('.dataset-status').children it('Datasets provided gives the correct value', () => { expect(statsBoxes[0].textContent).toContain('2/8') - expect(statsBoxes[0].textContent).toContain('datasets provided') + expect(statsBoxes[0].textContent).toContain('datasets submitted') }) it('Datasets with errors gives the correct value', () => { expect(statsBoxes[1].textContent).toContain('2') - expect(statsBoxes[1].textContent).toContain('datasets with errors') + expect(statsBoxes[1].textContent).toContain('data URL with errors') }) it('Datasets with issues gives the correct value', () => { expect(statsBoxes[2].textContent).toContain('2') - expect(statsBoxes[2].textContent).toContain('datasets with issues') + expect(statsBoxes[2].textContent).toContain('datasets need fixing') }) const datasetCards = document.querySelector('.govuk-task-list').children From eea521991cd42547ca393b84d205dec4820eef67 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Fri, 9 Aug 2024 16:30:32 +0100 Subject: [PATCH 04/34] remove secondary links from dataset cards, update wording --- src/views/organisations/overview.html | 30 ++++----------------------- test/unit/lpaOverviewPage.test.js | 9 +------- 2 files changed, 5 insertions(+), 34 deletions(-) diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index cf413784..d0251bcb 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -81,36 +81,14 @@

{% else %}

Endpoint provided

{% endif %} - -
    - {% if dataset.endpoint is null %} -
  • - Add endpoint -
    • - {% endif %} - {% if dataset.error %} -
  • - Fix errors -
    • - {% elseif dataset.issue %} -
  • - Fix issues -
    • - {% endif %} - {% if dataset.endpoint %} -
  • - View data -
    • - {% endif %} -
- +
{% if dataset.endpoint is null %} {{govukTag({ - text: "Not provided", + text: "Not submitted", classes: "govuk-tag--grey" })}} {% elif dataset.error %} @@ -120,12 +98,12 @@

})}} {% elif dataset.issue %} {{govukTag({ - text: "Issues", + text: "Needs fixing", classes: "govuk-tag--yellow" })}} {% else %} {{govukTag({ - text: "No issues", + text: "Live", classes: "govuk-tag--green" })}} {% endif %} diff --git a/test/unit/lpaOverviewPage.test.js b/test/unit/lpaOverviewPage.test.js index e9817d26..73fff999 100644 --- a/test/unit/lpaOverviewPage.test.js +++ b/test/unit/lpaOverviewPage.test.js @@ -136,19 +136,12 @@ describe('LPA Overview Page', () => { if (dataset.endpoint) { expectedActions.push({ text: 'View data', href: '/taskLists/taskChecklist' }) } - - const actions = datasetCards[i].querySelector('.planning-data-actions').children - expectedActions.forEach((expectedAction, j) => { - expect(actions[j].textContent, `expect action ${expectedAction.text} for dataset ${dataset.slug}`).toContain(expectedAction.text) - const actionLink = actions[j].querySelector('a') - expect(actionLink.href).toBe(expectedAction.href) - }) }) }) it('Renders the correct status on each dataset card', () => { params.datasets.forEach((dataset, i) => { - const expectedHint = !dataset.endpoint ? 'Not provided' : dataset.error ? 'Error' : dataset.issue ? 'Issues' : 'No issues' + const expectedHint = !dataset.endpoint ? 'Not submitted' : dataset.error ? 'Error' : dataset.issue ? 'Needs fixing' : 'Live' const statusIndicator = datasetCards[i].querySelector('.govuk-task-list__status') expect(statusIndicator.textContent).toContain(expectedHint) From d402aef40dcd414792f667e7adea413823f61e9a Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Fri, 9 Aug 2024 16:32:40 +0100 Subject: [PATCH 05/34] wording ('endpoint provided' -> 'data URL submitted' etc) --- src/views/organisations/overview.html | 6 +++--- test/unit/lpaOverviewPage.test.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index d0251bcb..72aacbfd 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -73,15 +73,15 @@

{% if dataset.endpoint is null %} -

Endpoint not provided

+

Data URL not submitted

{% elif dataset.error %}

{{dataset.error}}

{% elif dataset.issue %}

{{dataset.issue}}

{% else %} -

Endpoint provided

+

Data URL submitted

{% endif %} - +

diff --git a/test/unit/lpaOverviewPage.test.js b/test/unit/lpaOverviewPage.test.js index 73fff999..e092967f 100644 --- a/test/unit/lpaOverviewPage.test.js +++ b/test/unit/lpaOverviewPage.test.js @@ -117,7 +117,7 @@ describe('LPA Overview Page', () => { it('The dataset cards are rendered with the correct hints', () => { params.datasets.forEach((dataset, i) => { - const expectedHint = !dataset.endpoint ? 'Endpoint not provided' : dataset.error ? dataset.error : dataset.issue ? dataset.issue : 'Endpoint provided' + const expectedHint = !dataset.endpoint ? 'Data URL not submitted' : dataset.error ? dataset.error : dataset.issue ? dataset.issue : 'Data URL submitted' expect(datasetCards[i].querySelector('.govuk-task-list__hint').textContent).toContain(expectedHint) }) }) From f9d142d7874e7509ffe0a4a7c4c5435f8590f6df Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Fri, 9 Aug 2024 16:39:15 +0100 Subject: [PATCH 06/34] change wording of HTTP error status message --- src/services/performanceDbApi.js | 2 +- test/unit/performanceDbApi.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 830d3606..3e7f1020 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -132,7 +132,7 @@ ORDER BY const datasets = result.formattedData.reduce((accumulator, row) => { let error if (row.http_status !== '200' || row.exception) { - error = row.exception ? row.exception : `endpoint returned with a status of ${row.http_status}` + error = row.exception ? row.exception : `There was a ${row.http_status} error accessing the data URL` } let issue diff --git a/test/unit/performanceDbApi.test.js b/test/unit/performanceDbApi.test.js index 71740cd7..eb4df7e6 100644 --- a/test/unit/performanceDbApi.test.js +++ b/test/unit/performanceDbApi.test.js @@ -45,7 +45,7 @@ describe('performanceDbApi', () => { name: 'Some Organisation', organisation: 'some-organisation', datasets: { - 'dataset-slug-1': { endpoint: 'https://example.com/endpoint-1', error: 'endpoint returned with a status of 404', issue: undefined }, + 'dataset-slug-1': { endpoint: 'https://example.com/endpoint-1', error: 'There was a 404 error accessing the data URL', issue: undefined }, 'dataset-slug-2': { endpoint: 'https://example.com/endpoint-2', error: 'resource not found', issue: undefined }, 'dataset-slug-3': { endpoint: 'https://example.com/endpoint-3', error: undefined, issue: 'There are 4 issues in this dataset' } } From 126299208dd44304f9009f59c6d555fc03a32e75 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Fri, 9 Aug 2024 16:39:42 +0100 Subject: [PATCH 07/34] remove CTA from overview page --- src/views/organisations/overview.html | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index 72aacbfd..250304ed 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -115,10 +115,4 @@

-
-
-

View your task list to fix and improve your datasets

-
-
- {% endblock %} \ No newline at end of file From 497303f88276ea3e4b05d3f88d3aaa6a71c17070 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Fri, 9 Aug 2024 16:41:04 +0100 Subject: [PATCH 08/34] only provide a link to dataset when it submission was successful --- src/views/organisations/overview.html | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index 250304ed..98796567 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -66,10 +66,14 @@

Datasets

  • - + {% if dataset.endpoint %} + + {{dataset.slug | datasetSlugToReadableName}} + + {% else %} {{dataset.slug | datasetSlugToReadableName}} - + {% endif %}

    {% if dataset.endpoint is null %} From 884ede941bbdfcd6ce2c300d56793fa7d940e004 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Mon, 12 Aug 2024 14:53:30 +0100 Subject: [PATCH 09/34] content changes related to dataset 'status' - arbitrary strings that were sometimes used, sometimes used in combination with other columns, even when it looked like the same information was needed (e.g. `status`, which had values inconsistent with the ones we actually want to surface to the users, I can't find any reason why we would want them to be different) - some post-processing of the data was done (e.g. adding `error` and `issue` columns in code) - I removed that and now the query returns the `error` column, the issue column was removed. - remove needless checks from `perfomanceDbApi.test.js` - another issue 'testing mock data' - no real benefit of doing extensive tests here, better taken care of in other tests (e.g. in `lpaOverviewPage.test.js`) --- src/services/performanceDbApi.js | 44 ++++++++----------- src/views/organisations/overview.html | 41 ++++++++--------- test/unit/lpaOverviewPage.test.js | 63 +++++++++++++++++++-------- test/unit/performanceDbApi.test.js | 31 ++----------- 4 files changed, 85 insertions(+), 94 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 3e7f1020..dca46cb7 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -80,12 +80,17 @@ export default { rle.resource, rle.exception, rle.status as http_status, - case - when (rle.status != '200') then 'Error' - when (it.severity = 'error') then 'Issue' - when (it.severity = 'warning') then 'Warning' - else 'No issues' - end as status, + case + when (rle.status is null) then 'Not Submitted' + when (rle.status != '200') then 'Error' + when (it.severity = 'error') then 'Need fixing' + when (it.severity = 'warning') then 'Warning' + else 'Live' + end as status, + case + when ((cast(rle.status as integer) > 200)) then format('There was a %s error accessing the data URL', rle.status) + else null + end as error, case when (it.severity = 'info') then '' else i.issue_type @@ -130,24 +135,16 @@ ORDER BY const result = await datasette.runQuery(query) const datasets = result.formattedData.reduce((accumulator, row) => { - let error - if (row.http_status !== '200' || row.exception) { - error = row.exception ? row.exception : `There was a ${row.http_status} error accessing the data URL` - } - - let issue - if (row.issue_count > 0) { - issue = `There are ${row.issue_count} issues in this dataset` - } - accumulator[row.dataset] = { - endpoint: row.endpoint, - error, - issue + endpoint: row.endpoint } return accumulator }, {}) + if (result.formattedData.length === 0) { + throw new Error(`No records found for LPA=${lpa}`) + } + return { name: result.formattedData[0].name, organisation: result.formattedData[0].organisation, @@ -206,13 +203,8 @@ ORDER BY ORDER BY it.severity` const result = await datasette.runQuery(sql) - return result.formattedData.map((row) => { - return { - num_issues: row.num_issues, - issue_type: row.issue_type, - resource: row.resource, - status: row.status - } + return result.formattedData.map(({num_issues, issue_type, resource, status}) => { + return { num_issues, issue_type, resource, status } }) }, diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index 98796567..bf0a4e51 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -6,6 +6,18 @@ {% set pageName = organisation.name + " overview" %} {% set serviceType = 'Manage' %} +{% macro tagCardClass(status) %} + {% if status == 'Not submitted' %} + {% set statusCardClasses = 'govuk-tag--grey' %} + {% elif status == 'Error' %} + {% set statusCardClasses = 'govuk-tag--red' %} + {% elif status == 'Need fixing' %} + {% set statusCardClasses = 'govuk-tag--yellow' %} + {% else %} + {% set statusCardClasses = 'govuk-tag--green' %} + {% endif %} +{% endmacro %} + {% block beforeContent %} {{ super() }} @@ -80,8 +92,8 @@

    Data URL not submitted

    {% elif dataset.error %}

    {{dataset.error}}

    - {% elif dataset.issue %} -

    {{dataset.issue}}

    + {% elif dataset.issue_count > 0 %} +

    There are {{ dataset.issue_count }} issues in this dataset

    {% else %}

    Data URL submitted

    {% endif %} @@ -90,27 +102,10 @@

    - {% if dataset.endpoint is null %} - {{govukTag({ - text: "Not submitted", - classes: "govuk-tag--grey" - })}} - {% elif dataset.error %} - {{govukTag({ - text: "Error", - classes: "govuk-tag--red" - })}} - {% elif dataset.issue %} - {{govukTag({ - text: "Needs fixing", - classes: "govuk-tag--yellow" - })}} - {% else %} - {{govukTag({ - text: "Live", - classes: "govuk-tag--green" - })}} - {% endif %} + {{govukTag({ + text: dataset.status, + classes: tagCardClass(dataset.status) + })}}
    • {% endfor %} diff --git a/test/unit/lpaOverviewPage.test.js b/test/unit/lpaOverviewPage.test.js index e092967f..76e02bca 100644 --- a/test/unit/lpaOverviewPage.test.js +++ b/test/unit/lpaOverviewPage.test.js @@ -35,47 +35,59 @@ describe('LPA Overview Page', () => { datasets: [ { slug: 'article-4-direction', - endpoint: null + endpoint: null, + status: 'Not submitted', + issue_count: 0 }, { slug: 'article-4-direction-area', - endpoint: null + endpoint: null, + status: 'Not submitted' }, { slug: 'conservation-area', endpoint: 'http://conservation-area.json', + status: 'Need fixing', error: null, - issue: 'Endpoint has not been updated since 21 May 2023' + issue: 'Endpoint has not been updated since 21 May 2023', + issue_count: 1 }, { slug: 'conservation-area-document', endpoint: 'http://conservation-area-document.json', + status: 'Live', error: null, - issue: null + issue_count: 0 }, { slug: 'listed-building-outline', endpoint: 'http://listed-building-outline.json', + status: 'Live', error: null, - issue: null + issue_count: 0 }, { slug: 'tree', endpoint: 'http://tree.json', error: null, - issue: 'There are 20 issues in this dataset' + status: 'Need fixing', + issue: 'There are 20 issues in this dataset', + issue_count: 1 }, { slug: 'tree-preservation-order', endpoint: 'http://tree-preservation-order.json', - error: 'Error connecting to endpoint', - issue: null + http_error: '404', + error: 'There was 404 error accessing the data URL', + status: 'Error', + issue_count: 0 }, { slug: 'tree-preservation-zone', endpoint: 'http://tree-preservation-zone.json', - error: 'Error connecting to endpoint', - issue: null + status: 'Error', + error: '400', + issue_count: 0 } ] } @@ -115,10 +127,17 @@ describe('LPA Overview Page', () => { }) }) - it('The dataset cards are rendered with the correct hints', () => { - params.datasets.forEach((dataset, i) => { - const expectedHint = !dataset.endpoint ? 'Data URL not submitted' : dataset.error ? dataset.error : dataset.issue ? dataset.issue : 'Data URL submitted' - expect(datasetCards[i].querySelector('.govuk-task-list__hint').textContent).toContain(expectedHint) + params.datasets.forEach((dataset, i) => { + it(`dataset cards are rendered with correct hints for dataset='${dataset.slug}'`, () => { + let expectedHint = 'Data URL submitted' + if (dataset.status === 'Not submitted') { + expectedHint = 'Data URL not submitted' + } else if (dataset.error) { + expectedHint = dataset.error + } else if (dataset.issue_count > 0) { + expectedHint = `There are ${dataset.issue_count} issues in this dataset` + } + expect(datasetCards[i].querySelector('.govuk-task-list__hint').textContent.trim()).toContain(expectedHint) }) }) @@ -139,12 +158,20 @@ describe('LPA Overview Page', () => { }) }) - it('Renders the correct status on each dataset card', () => { - params.datasets.forEach((dataset, i) => { - const expectedHint = !dataset.endpoint ? 'Not submitted' : dataset.error ? 'Error' : dataset.issue ? 'Needs fixing' : 'Live' + params.datasets.forEach((dataset, i) => { + it(`Renders the correct status on each dataset card for dataset='${dataset.slug}'`, () => { + let expectedHint = 'Live' + if (dataset.status === 'Not submitted') { + expectedHint = 'Not submitted' + } else if (dataset.status === 'Error') { + expectedHint = dataset.status + } else if (dataset.status === 'Need fixing') { + expectedHint = 'Need fixing' + } + const statusIndicator = datasetCards[i].querySelector('.govuk-task-list__status') - expect(statusIndicator.textContent).toContain(expectedHint) + expect(statusIndicator.textContent.trim()).toContain(expectedHint) }) }) }) diff --git a/test/unit/performanceDbApi.test.js b/test/unit/performanceDbApi.test.js index eb4df7e6..82dd6fd2 100644 --- a/test/unit/performanceDbApi.test.js +++ b/test/unit/performanceDbApi.test.js @@ -14,42 +14,19 @@ describe('performanceDbApi', () => { dataset: 'dataset-slug-1', endpoint: 'https://example.com/endpoint-1', exception: null, - http_status: '404' - }, - { - organisation: 'some-organisation', - name: 'Some Organisation', - dataset: 'dataset-slug-2', - endpoint: 'https://example.com/endpoint-2', - exception: 'resource not found', - http_status: '404' - }, - { - organisation: 'some-organisation', - name: 'Some Organisation', - dataset: 'dataset-slug-3', - endpoint: 'https://example.com/endpoint-3', - http_status: '200', - issue_count: 4 + http_status: undefined, + error: undefined, + status: 'Live' } ] } vi.spyOn(datasette, 'runQuery').mockResolvedValue(mockResponse) - const result = await performanceDbApi.getLpaOverview(lpa) + await performanceDbApi.getLpaOverview(lpa) expect(datasette.runQuery).toHaveBeenCalledTimes(1) expect(datasette.runQuery).toHaveBeenCalledWith(expect.stringContaining(lpa)) - expect(result).toEqual({ - name: 'Some Organisation', - organisation: 'some-organisation', - datasets: { - 'dataset-slug-1': { endpoint: 'https://example.com/endpoint-1', error: 'There was a 404 error accessing the data URL', issue: undefined }, - 'dataset-slug-2': { endpoint: 'https://example.com/endpoint-2', error: 'resource not found', issue: undefined }, - 'dataset-slug-3': { endpoint: 'https://example.com/endpoint-3', error: undefined, issue: 'There are 4 issues in this dataset' } - } - }) }) it('adds the filter if a dataset list is passed into the params', async () => { From bb283cbd163ca71bda93ef25acf23a0d561b05d8 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Mon, 12 Aug 2024 14:59:08 +0100 Subject: [PATCH 10/34] fix linter issues --- src/services/performanceDbApi.js | 3 ++- test/unit/lpaOverviewPage.test.js | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index dca46cb7..243574bb 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -203,7 +203,8 @@ ORDER BY ORDER BY it.severity` const result = await datasette.runQuery(sql) - return result.formattedData.map(({num_issues, issue_type, resource, status}) => { + /* eslint camelcase: "off" */ + return result.formattedData.map(({ num_issues, issue_type, resource, status }) => { return { num_issues, issue_type, resource, status } }) }, diff --git a/test/unit/lpaOverviewPage.test.js b/test/unit/lpaOverviewPage.test.js index 76e02bca..09216406 100644 --- a/test/unit/lpaOverviewPage.test.js +++ b/test/unit/lpaOverviewPage.test.js @@ -158,7 +158,6 @@ describe('LPA Overview Page', () => { }) }) - params.datasets.forEach((dataset, i) => { it(`Renders the correct status on each dataset card for dataset='${dataset.slug}'`, () => { let expectedHint = 'Live' @@ -169,7 +168,7 @@ describe('LPA Overview Page', () => { } else if (dataset.status === 'Need fixing') { expectedHint = 'Need fixing' } - + const statusIndicator = datasetCards[i].querySelector('.govuk-task-list__status') expect(statusIndicator.textContent.trim()).toContain(expectedHint) }) From 735b73f86fee98cca4cfcd47dc0466836e3cf9f1 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Mon, 12 Aug 2024 15:14:20 +0100 Subject: [PATCH 11/34] fix wording --- src/services/performanceDbApi.js | 2 +- src/views/organisations/overview.html | 2 +- test/unit/lpaOverviewPage.test.js | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 243574bb..1a46d3ea 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -83,7 +83,7 @@ export default { case when (rle.status is null) then 'Not Submitted' when (rle.status != '200') then 'Error' - when (it.severity = 'error') then 'Need fixing' + when (it.severity = 'error') then 'Needs fixing' when (it.severity = 'warning') then 'Warning' else 'Live' end as status, diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index bf0a4e51..9b5462ba 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -11,7 +11,7 @@ {% set statusCardClasses = 'govuk-tag--grey' %} {% elif status == 'Error' %} {% set statusCardClasses = 'govuk-tag--red' %} - {% elif status == 'Need fixing' %} + {% elif status == 'Needs fixing' %} {% set statusCardClasses = 'govuk-tag--yellow' %} {% else %} {% set statusCardClasses = 'govuk-tag--green' %} diff --git a/test/unit/lpaOverviewPage.test.js b/test/unit/lpaOverviewPage.test.js index 09216406..a504695a 100644 --- a/test/unit/lpaOverviewPage.test.js +++ b/test/unit/lpaOverviewPage.test.js @@ -47,7 +47,7 @@ describe('LPA Overview Page', () => { { slug: 'conservation-area', endpoint: 'http://conservation-area.json', - status: 'Need fixing', + status: 'Needs fixing', error: null, issue: 'Endpoint has not been updated since 21 May 2023', issue_count: 1 @@ -70,7 +70,7 @@ describe('LPA Overview Page', () => { slug: 'tree', endpoint: 'http://tree.json', error: null, - status: 'Need fixing', + status: 'Needs fixing', issue: 'There are 20 issues in this dataset', issue_count: 1 }, @@ -165,8 +165,8 @@ describe('LPA Overview Page', () => { expectedHint = 'Not submitted' } else if (dataset.status === 'Error') { expectedHint = dataset.status - } else if (dataset.status === 'Need fixing') { - expectedHint = 'Need fixing' + } else if (dataset.status === 'Needs fixing') { + expectedHint = 'Needs fixing' } const statusIndicator = datasetCards[i].querySelector('.govuk-task-list__status') From 5d29d240189e7cabcadaed254ef4bbeb8fca81c6 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Tue, 13 Aug 2024 13:04:26 +0100 Subject: [PATCH 12/34] fix wrong breadcrumb url --- src/views/organisations/overview.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index 9b5462ba..59d011e8 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -29,7 +29,7 @@ }, { text: "Organisations", - href: "/overview/organisations" + href: "/organisations" } ] }) }} From afd7f951036f78ae7c62fb52076f7306fd93f576 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Tue, 13 Aug 2024 13:05:17 +0100 Subject: [PATCH 13/34] fix: more useful log messages --- src/controllers/OrganisationsController.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 8c254f27..86ead25c 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -100,7 +100,7 @@ const organisationsController = { res.render('organisations/overview.html', params) } catch (error) { - logger.error(error) + logger.warn('organisationsController.getOverview(): ' + error.message ?? error.errorMessage, {type: types.App}) next(error) } }, @@ -130,7 +130,7 @@ const organisationsController = { res.render('organisations/find.html', { alphabetisedOrgs }) } catch (err) { - logger.warn(err) + logger.warn('organisationsController.getOrganisations(): ' + err.message ?? err.errorMessage, {type: types.App}) next(err) } }, From 294ae9666f0d7f2caf6b8d4c7650a505984f286e Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Tue, 13 Aug 2024 13:06:00 +0100 Subject: [PATCH 14/34] more useful log messages --- src/controllers/OrganisationsController.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 86ead25c..5d32f943 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -100,7 +100,7 @@ const organisationsController = { res.render('organisations/overview.html', params) } catch (error) { - logger.warn('organisationsController.getOverview(): ' + error.message ?? error.errorMessage, {type: types.App}) + logger.warn('organisationsController.getOverview(): ' + error.message ?? error.errorMessage, { type: types.App }) next(error) } }, @@ -130,7 +130,7 @@ const organisationsController = { res.render('organisations/find.html', { alphabetisedOrgs }) } catch (err) { - logger.warn('organisationsController.getOrganisations(): ' + err.message ?? err.errorMessage, {type: types.App}) + logger.warn('organisationsController.getOrganisations(): ' + err.message ?? err.errorMessage, { type: types.App }) next(err) } }, From 4985517fb4839a39bac035c143ace306e4f33ae6 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 14:04:27 +0100 Subject: [PATCH 15/34] add more breadcrumbs --- src/controllers/OrganisationsController.js | 13 ++++---- src/services/performanceDbApi.js | 2 -- src/views/organisations/datasetTaskList.html | 22 +++++++++++++ src/views/organisations/issueDetails.html | 33 ++++++++++++++++++++ 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 8c254f27..34ef8eff 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -46,6 +46,9 @@ const organisationsController = { try { const lpa = req.params.lpa + const organisationResult = await datasette.runQuery(`SELECT name, organisation FROM organisation WHERE organisation = '${lpa}'`) + const organisation = organisationResult.formattedData[0] + const datasetsFilter = ['article-4-direction', 'article-4-direction-area', 'conservation-area', @@ -87,10 +90,7 @@ const organisationsController = { }, [0, 0, 0]) const params = { - organisation: { - name: lpaOverview.name, - organisation: lpaOverview.organisation - }, + organisation, datasets, totalDatasets, datasetsWithEndpoints, @@ -183,7 +183,7 @@ const organisationsController = { const datasetId = req.params.dataset try { - const organisationResult = await datasette.runQuery(`SELECT name FROM organisation WHERE organisation = '${lpa}'`) + const organisationResult = await datasette.runQuery(`SELECT name, organisation FROM organisation WHERE organisation = '${lpa}'`) const organisation = organisationResult.formattedData[0] const datasetResult = await datasette.runQuery(`SELECT name FROM dataset WHERE dataset = '${datasetId}'`) @@ -377,7 +377,8 @@ const organisationsController = { dataset, errorHeading, issueItems, - entry + entry, + issueType } res.render('organisations/issueDetails.html', params) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 830d3606..4352b3ed 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -149,8 +149,6 @@ ORDER BY }, {}) return { - name: result.formattedData[0].name, - organisation: result.formattedData[0].organisation, datasets } }, diff --git a/src/views/organisations/datasetTaskList.html b/src/views/organisations/datasetTaskList.html index 26d30fb9..d549829e 100644 --- a/src/views/organisations/datasetTaskList.html +++ b/src/views/organisations/datasetTaskList.html @@ -1,3 +1,5 @@ +{% from "govuk/components/breadcrumbs/macro.njk" import govukBreadcrumbs %} + {% extends "layouts/main.html" %} {% from 'govuk/components/task-list/macro.njk' import govukTaskList %} @@ -10,6 +12,26 @@ {{ super() }} +{{ govukBreadcrumbs({ + items: [ + { + text: "Home", + href: "/manage" + }, + { + text: "Organisations", + href: '/organisations' + }, + { + text: organisation.name, + href: '/organisations/' + organisation.organisation + }, + { + text: dataset.name + } + ] +}) }} + {% endblock %} {% block content %} diff --git a/src/views/organisations/issueDetails.html b/src/views/organisations/issueDetails.html index 571096ef..087a0c93 100644 --- a/src/views/organisations/issueDetails.html +++ b/src/views/organisations/issueDetails.html @@ -3,6 +3,7 @@ {% from 'govuk/components/error-summary/macro.njk' import govukErrorSummary %} {% from "govuk/components/summary-list/macro.njk" import govukSummaryList %} {% from "govuk/components/pagination/macro.njk" import govukPagination %} +{% from "govuk/components/breadcrumbs/macro.njk" import govukBreadcrumbs %} {% set serviceType = 'Submit'%} @@ -12,7 +13,39 @@ {% set pageName %}{{organisation.name}} - {{dataset.name}} - Issues{% endset %} {%endif%} +{% block beforeContent %} + +{{ super() }} + +{{ govukBreadcrumbs({ + items: [ + { + text: "Home", + href: "/manage" + }, + { + text: "Organisations", + href: '/organisations' + }, + { + text: organisation.name, + href: '/organisations/' + organisation.organisation + }, + { + text: dataset.name, + href: '/organisations/' + organisation.organisation + '/' + dataset.dataset + }, + { + text: issueType + } + ] +}) }} + +{% endblock %} + {% block content %} + +
    {% include "includes/_dataset-page-header.html" %}
    From 4bf5e0f4483953e16a8a593f42b8782e8dbbea3a Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 14:39:12 +0100 Subject: [PATCH 16/34] add breadcrumb tests to generic page tests --- test/unit/findPage.test.js | 3 ++- test/unit/generic-page.js | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/test/unit/findPage.test.js b/test/unit/findPage.test.js index 3bb18cc2..a6392d8a 100644 --- a/test/unit/findPage.test.js +++ b/test/unit/findPage.test.js @@ -53,7 +53,8 @@ describe('Organisations Find Page', () => { const document = dom.window.document runGenericPageTests(html, { - pageTitle: `Find your organisation - ${config.serviceNames.submit}` + pageTitle: `Find your organisation - ${config.serviceNames.submit}`, + breadcrumbs: [{ text: 'Home', href: '/manage' }, { text: 'Organisations' }] }) it('correct has a form element with the correct data-filter attribute', () => { diff --git a/test/unit/generic-page.js b/test/unit/generic-page.js index c2a901ae..37fa349a 100644 --- a/test/unit/generic-page.js +++ b/test/unit/generic-page.js @@ -30,4 +30,21 @@ export const runGenericPageTests = (html, options) => { expect(document.title).toBe(options.pageTitle) }) } + + if(options.breadcrumbs) { + it('has the correct breadcrumbs', () => { + const breadcrumbs = document.querySelector('.govuk-breadcrumbs__list') + const breadcrumbsChildren = breadcrumbs.children + + options.breadcrumbs.forEach((breadcrumb, i) => { + expect(breadcrumbsChildren[i].textContent).toContain(breadcrumb.text) + + if(breadcrumb.href){ + const breadcrumbLink = breadcrumbsChildren[i].children[0] + expect(breadcrumbLink.getAttribute('href')).toEqual(breadcrumb.href) + } + + }); + }) + } } From a7996f0f8a1fa6fc7084c743c1ef8fda20f9d81f Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 14:39:20 +0100 Subject: [PATCH 17/34] linting --- test/unit/generic-page.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/unit/generic-page.js b/test/unit/generic-page.js index 37fa349a..053ca638 100644 --- a/test/unit/generic-page.js +++ b/test/unit/generic-page.js @@ -31,20 +31,19 @@ export const runGenericPageTests = (html, options) => { }) } - if(options.breadcrumbs) { + if (options.breadcrumbs) { it('has the correct breadcrumbs', () => { const breadcrumbs = document.querySelector('.govuk-breadcrumbs__list') const breadcrumbsChildren = breadcrumbs.children - + options.breadcrumbs.forEach((breadcrumb, i) => { expect(breadcrumbsChildren[i].textContent).toContain(breadcrumb.text) - if(breadcrumb.href){ + if (breadcrumb.href) { const breadcrumbLink = breadcrumbsChildren[i].children[0] expect(breadcrumbLink.getAttribute('href')).toEqual(breadcrumb.href) } - - }); + }) }) } } From 1fcc7860ddf7141384cadd3eea50c284eb35b6f7 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 14:54:30 +0100 Subject: [PATCH 18/34] added breadcrumb tests --- test/unit/dataset-details.test.js | 13 +++++++++--- test/unit/datasetTaskListPage.test.js | 6 ++++-- test/unit/issueDetailsPage.test.js | 29 ++++++++++++++++++++++----- test/unit/lpaOverviewPage.test.js | 6 ++++-- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/test/unit/dataset-details.test.js b/test/unit/dataset-details.test.js index 7c0315a7..054e48a5 100644 --- a/test/unit/dataset-details.test.js +++ b/test/unit/dataset-details.test.js @@ -3,7 +3,6 @@ import { describe, expect, it } from 'vitest' import { setupNunjucks } from '../../src/serverSetup/nunjucks.js' import { runGenericPageTests } from './generic-page.js' -import config from '../../config/index.js' import { stripWhitespace } from '../utils/stripWhiteSpace.js' import { testValidationErrorMessage } from './validation-tests.js' import { mockDataSubjects } from './data.js' @@ -35,6 +34,14 @@ function errorTestFn ({ describe('dataset details View', () => { const params = { + organisation: { + name: 'mock org', + organisation: 'mock-org' + }, + dataset: { + name: 'mock dataset', + dataset: 'mock-dataset' + }, values: { dataset: 'mockDataset' }, @@ -42,9 +49,9 @@ describe('dataset details View', () => { } const html = stripWhitespace(nunjucks.render('dataset-details.html', params)) const datasetName = mockDataSubjects.mockDataset.dataSets[0].value + runGenericPageTests(html, { - pageTitle: `Enter ${datasetName.toLowerCase()} details - Submit and update your planning data`, - serviceName: config.serviceNames.submit + pageTitle: `Enter ${datasetName.toLowerCase()} details - Submit and update your planning data` }) it('should render the correct header', () => { diff --git a/test/unit/datasetTaskListPage.test.js b/test/unit/datasetTaskListPage.test.js index 9f5fbfee..459ce7e0 100644 --- a/test/unit/datasetTaskListPage.test.js +++ b/test/unit/datasetTaskListPage.test.js @@ -23,7 +23,8 @@ addFilters(nunjucksEnv, { datasetNameMapping }) describe('Dataset Task List Page', () => { const params = { organisation: { - name: 'fake organisation' + name: 'mock org', + organisation: 'mock-org' }, dataset: { name: 'Article 4 direction area' @@ -73,7 +74,8 @@ describe('Dataset Task List Page', () => { const document = dom.window.document runGenericPageTests(html, { - pageTitle: 'fake organisation - Article 4 direction area - Task list - Submit and update your planning data' + pageTitle: 'mock org - Article 4 direction area - Task list - Submit and update your planning data', + breadcrumbs: [{ text: 'Home', href: '/manage' }, { text: 'Organisations', href: '/organisations' }, { text: 'mock org', href: '/organisations/mock-org' }, { text: 'Article 4 direction area' }] }) it('Renders the correct headings', () => { diff --git a/test/unit/issueDetailsPage.test.js b/test/unit/issueDetailsPage.test.js index 1ae625be..6b97d68f 100644 --- a/test/unit/issueDetailsPage.test.js +++ b/test/unit/issueDetailsPage.test.js @@ -22,11 +22,13 @@ addFilters(nunjucksEnv, { datasetNameMapping }) describe('issueDetails.html', () => { const organisation = { - name: 'Test Organisation' + name: 'mock org', + organisation: 'mock-org' } const dataset = { - name: 'Test Dataset' + name: 'mock Dataset', + dataset: 'mock-dataset' } const errorHeading = 'Example error heading' @@ -74,12 +76,15 @@ describe('issueDetails.html', () => { ] } + const issueType = 'mock issue' + const params = { organisation, dataset, errorHeading, issueItems, - entry + entry, + issueType } const html = nunjucks.render('organisations/issueDetails.html', params) @@ -87,7 +92,14 @@ describe('issueDetails.html', () => { const document = dom.window.document runGenericPageTests(html, { - pageTitle: `Test Organisation - Test Dataset - Issues - ${config.serviceNames.submit}` + pageTitle: `mock org - mock Dataset - Issues - ${config.serviceNames.submit}`, + breadcrumbs: [ + { text: 'Home', href: '/manage' }, + { text: 'Organisations', href: '/organisations' }, + { text: 'mock org', href: '/organisations/mock-org' }, + { text: 'mock dataset', href: 'mock-dataset' }, + { text: 'mock issue' } + ] }) it('Renders the correct headings', () => { @@ -127,7 +139,14 @@ describe('issueDetails.html', () => { // const multiPageDom = new JSDOM(multiPageHtml) // const multiPageDocument = multiPageDom.window.document runGenericPageTests(multiPageHtml, { - pageTitle: `Test Organisation - Test Dataset - Issues (Page 2 of 3) - ${config.serviceNames.submit}` + pageTitle: `mock org - mock Dataset - Issues (Page 2 of 3) - ${config.serviceNames.submit}`, + breadcrumbs: [ + { text: 'Home', href: '/manage' }, + { text: 'Organisations', href: '/organisations' }, + { text: 'mock org', href: '/organisations/mock-org' }, + { text: 'mock dataset', href: 'mock-dataset' }, + { text: 'mock issue' } + ] }) it.todo('correctly renders the pagination', () => { diff --git a/test/unit/lpaOverviewPage.test.js b/test/unit/lpaOverviewPage.test.js index 0f1518ce..8b1cbaa2 100644 --- a/test/unit/lpaOverviewPage.test.js +++ b/test/unit/lpaOverviewPage.test.js @@ -26,7 +26,8 @@ addFilters(nunjucksEnv, { datasetNameMapping }) describe('LPA Overview Page', () => { const params = { organisation: { - name: 'mock org' + name: 'mock org', + organisation: 'mock-org' }, datasetsWithEndpoints: 2, totalDatasets: 8, @@ -85,7 +86,8 @@ describe('LPA Overview Page', () => { const document = dom.window.document runGenericPageTests(html, { - pageTitle: 'mock org overview - Submit and update your planning data' + pageTitle: 'mock org overview - Submit and update your planning data', + breadcrumbs: [{ text: 'Home', href: '/manage' }, { text: 'Organisations', href: '/organisations' }, { text: 'mock org' }] }) const statsBoxes = document.querySelector('.dataset-status').children From 171720389cb3fa603f59f529acbd0a0017656bab Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 15:10:24 +0100 Subject: [PATCH 19/34] fix tests --- test/unit/issueDetailsPage.test.js | 4 ++-- test/unit/organisationsController.test.js | 10 +++++++--- test/unit/performanceDbApi.test.js | 2 -- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/test/unit/issueDetailsPage.test.js b/test/unit/issueDetailsPage.test.js index 6b97d68f..7e19ed72 100644 --- a/test/unit/issueDetailsPage.test.js +++ b/test/unit/issueDetailsPage.test.js @@ -97,7 +97,7 @@ describe('issueDetails.html', () => { { text: 'Home', href: '/manage' }, { text: 'Organisations', href: '/organisations' }, { text: 'mock org', href: '/organisations/mock-org' }, - { text: 'mock dataset', href: 'mock-dataset' }, + { text: 'mock Dataset', href: '/organisations/mock-org/mock-dataset' }, { text: 'mock issue' } ] }) @@ -144,7 +144,7 @@ describe('issueDetails.html', () => { { text: 'Home', href: '/manage' }, { text: 'Organisations', href: '/organisations' }, { text: 'mock org', href: '/organisations/mock-org' }, - { text: 'mock dataset', href: 'mock-dataset' }, + { text: 'mock Dataset', href: '/organisations/mock-org/mock-dataset' }, { text: 'mock issue' } ] }) diff --git a/test/unit/organisationsController.test.js b/test/unit/organisationsController.test.js index a2fca79e..8eaeff3f 100644 --- a/test/unit/organisationsController.test.js +++ b/test/unit/organisationsController.test.js @@ -29,13 +29,15 @@ describe('OrganisationsController.js', () => { const next = vi.fn() const expectedResponse = { - name: 'Test LPA', + name: 'test LPA', datasets: { dataset1: { endpoint: 'https://example.com', issue: false, error: false }, dataset2: { endpoint: null, issue: true, error: false }, dataset3: { endpoint: 'https://example.com', issue: false, error: true } } } + + vi.mocked(datasette.runQuery).mockResolvedValue({ formattedData: [{name: 'Test lpa', organisation: 'test-lpa'}] }) performanceDbApi.getLpaOverview = vi.fn().mockResolvedValue(expectedResponse) @@ -43,7 +45,7 @@ describe('OrganisationsController.js', () => { expect(res.render).toHaveBeenCalledTimes(1) expect(res.render).toHaveBeenCalledWith('organisations/overview.html', expect.objectContaining({ - organisation: { name: 'Test LPA' }, + organisation: { name: 'Test lpa', organisation: 'test-lpa' }, datasets: expect.arrayContaining([ { endpoint: 'https://example.com', issue: false, error: false, slug: 'dataset1' }, { endpoint: null, issue: true, error: false, slug: 'dataset2' }, @@ -63,6 +65,7 @@ describe('OrganisationsController.js', () => { const error = new Error('Test error') + vi.mocked(datasette.runQuery).mockResolvedValue({ formattedData: [{name: 'Test lpa', organisation: 'test-lpa'}] }) vi.mocked(performanceDbApi.getLpaOverview).mockRejectedValue(error) await organisationsController.getOverview(req, res, next) @@ -388,7 +391,8 @@ describe('OrganisationsController.js', () => { classes: '' } ] - } + }, + issueType: "test-issue-type", }) }) diff --git a/test/unit/performanceDbApi.test.js b/test/unit/performanceDbApi.test.js index 71740cd7..51e0a63a 100644 --- a/test/unit/performanceDbApi.test.js +++ b/test/unit/performanceDbApi.test.js @@ -42,8 +42,6 @@ describe('performanceDbApi', () => { expect(datasette.runQuery).toHaveBeenCalledTimes(1) expect(datasette.runQuery).toHaveBeenCalledWith(expect.stringContaining(lpa)) expect(result).toEqual({ - name: 'Some Organisation', - organisation: 'some-organisation', datasets: { 'dataset-slug-1': { endpoint: 'https://example.com/endpoint-1', error: 'endpoint returned with a status of 404', issue: undefined }, 'dataset-slug-2': { endpoint: 'https://example.com/endpoint-2', error: 'resource not found', issue: undefined }, From d92bcb4f9fa53b89fd06e9a3cea085dff6158282 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 15:10:34 +0100 Subject: [PATCH 20/34] linting --- test/unit/organisationsController.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/organisationsController.test.js b/test/unit/organisationsController.test.js index 8eaeff3f..525f77dc 100644 --- a/test/unit/organisationsController.test.js +++ b/test/unit/organisationsController.test.js @@ -36,8 +36,8 @@ describe('OrganisationsController.js', () => { dataset3: { endpoint: 'https://example.com', issue: false, error: true } } } - - vi.mocked(datasette.runQuery).mockResolvedValue({ formattedData: [{name: 'Test lpa', organisation: 'test-lpa'}] }) + + vi.mocked(datasette.runQuery).mockResolvedValue({ formattedData: [{ name: 'Test lpa', organisation: 'test-lpa' }] }) performanceDbApi.getLpaOverview = vi.fn().mockResolvedValue(expectedResponse) @@ -65,7 +65,7 @@ describe('OrganisationsController.js', () => { const error = new Error('Test error') - vi.mocked(datasette.runQuery).mockResolvedValue({ formattedData: [{name: 'Test lpa', organisation: 'test-lpa'}] }) + vi.mocked(datasette.runQuery).mockResolvedValue({ formattedData: [{ name: 'Test lpa', organisation: 'test-lpa' }] }) vi.mocked(performanceDbApi.getLpaOverview).mockRejectedValue(error) await organisationsController.getOverview(req, res, next) @@ -392,7 +392,7 @@ describe('OrganisationsController.js', () => { } ] }, - issueType: "test-issue-type", + issueType: 'test-issue-type' }) }) From 18aab0032460c6f455a028856a23a90f78822401 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 15:14:49 +0100 Subject: [PATCH 21/34] make sure link points to check service --- src/views/organisations/datasetTaskList.html | 2 +- src/views/organisations/issueDetails.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/views/organisations/datasetTaskList.html b/src/views/organisations/datasetTaskList.html index 26d30fb9..6bfdfa74 100644 --- a/src/views/organisations/datasetTaskList.html +++ b/src/views/organisations/datasetTaskList.html @@ -45,7 +45,7 @@

    1. Fix the errors indicated
      2. -
    2. Use the check service to make sure the data meets +
    3. Use the check service to make sure the data meets the standard
    4. Publish the updated data on the data URL
    diff --git a/src/views/organisations/issueDetails.html b/src/views/organisations/issueDetails.html index 571096ef..7a0d95ec 100644 --- a/src/views/organisations/issueDetails.html +++ b/src/views/organisations/issueDetails.html @@ -51,7 +51,7 @@

    1. Fix the errors indicated
      2. -
    2. Use the check service to make sure the data meets +
    3. Use the check service to make sure the data meets the standard
    4. Publish the updated data on the data URL
    From 2a265bd4171936379d30c043f0ac8b350c033cfc Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 15:17:26 +0100 Subject: [PATCH 22/34] make sure links are relative --- src/views/organisations/get-started.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/views/organisations/get-started.html b/src/views/organisations/get-started.html index b9678e16..0434060f 100644 --- a/src/views/organisations/get-started.html +++ b/src/views/organisations/get-started.html @@ -79,7 +79,7 @@

    1. - The check service can help you understand + The check service can help you understand if your data is ready to submit or if you need to change anything before you publish it on your website.

      @@ -99,7 +99,7 @@

      1. Check your data + href="/">Check your data

      2. @@ -161,7 +161,7 @@

      -

      Use the submit +

      Use the submit data service to submit your data URL, your documentation URL and that the data is provided under the Open Government Licence.

      @@ -172,7 +172,7 @@

      1. Submit + href="/submit">Submit your data
      From 180e5f4d04edd8fa574b00d514f8f5ebd1d9b0da Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 13 Aug 2024 15:18:13 +0100 Subject: [PATCH 23/34] update content --- src/views/organisations/get-started.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/views/organisations/get-started.html b/src/views/organisations/get-started.html index 0434060f..3c607134 100644 --- a/src/views/organisations/get-started.html +++ b/src/views/organisations/get-started.html @@ -200,12 +200,12 @@

      - Whenever the data changes, update it on your endpoint URL. We will collect data from your endpoint URL + Whenever the data changes, update it on your data URL. We will collect data from your data URL every day.

      - Your endpoint URL needs to remain the same, don’t change it when you make updates. + Your data URL needs to remain the same, don’t change it when you make updates.

      From b0cc7393d3a22bf755b9a2c68ea31f4282703743 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Tue, 13 Aug 2024 15:45:16 +0100 Subject: [PATCH 24/34] fix: tag class lookup, missing 'status', incorrect status counts --- src/controllers/OrganisationsController.js | 19 ++++------- src/filters/filters.js | 20 ++++++++++- src/services/datasette.js | 2 +- src/services/performanceDbApi.js | 39 +++++++++++----------- src/views/organisations/overview.html | 14 +------- 5 files changed, 48 insertions(+), 46 deletions(-) diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 5d32f943..9085b4b8 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -3,6 +3,7 @@ import performanceDbApi from '../services/performanceDbApi.js' // Assume you hav import logger from '../utils/logger.js' import { types } from '../utils/logging.js' import { dataSubjects } from '../utils/utils.js' +import { statusToTagClass } from '../filters/filters.js' // get a list of available datasets const availableDatasets = Object.values(dataSubjects) @@ -19,17 +20,10 @@ const availableDatasets = Object.values(dataSubjects) * @returns {object} - An object with a `tag` property containing the text label and CSS class. */ function getStatusTag (status) { - const statusToTagClass = { - Error: 'govuk-tag--red', - 'Needs fixing': 'govuk-tag--yellow', - Warning: 'govuk-tag--blue', - Issue: 'govuk-tag--blue' - } - return { tag: { text: status, - classes: statusToTagClass[status] + classes: statusToTagClass(status) } } } @@ -73,16 +67,17 @@ const organisationsController = { if (!keys.includes(dataset)) { datasets.push({ slug: dataset, - endpoint: null + endpoint: null, + status: 'Not submitted' }) } }) const totalDatasets = datasets.length const [datasetsWithEndpoints, datasetsWithIssues, datasetsWithErrors] = datasets.reduce((accumulator, dataset) => { - if (dataset.endpoint !== null) accumulator[0]++ - if (dataset.issue) accumulator[1]++ - if (dataset.error) accumulator[2]++ + if (dataset.status === 'Live' || dataset.status === 'Warning') accumulator[0]++ + if (dataset.status === 'Needs fixing') accumulator[1]++ + if (dataset.status === 'Error') accumulator[2]++ return accumulator }, [0, 0, 0]) diff --git a/src/filters/filters.js b/src/filters/filters.js index 2ce7bdb5..514caedf 100644 --- a/src/filters/filters.js +++ b/src/filters/filters.js @@ -6,6 +6,23 @@ import prettifyColumnName from './prettifyColumnName.js' import getFullServiceName from './getFullServiceName.js' import { makeDatasetSlugToReadableNameFilter } from './makeDatasetSlugToReadableNameFilter.js' +/** maps dataset status (as returned by performanceDbApi/getLpaOverview()) to a + * CSS class used by the govuk-tag component + */ +const statusToTagClassMapping = { + Error: 'govuk-tag--red', + 'Not submitted': 'govuk-tag--red', + 'Needs fixing': 'govuk-tag--yellow', + Warning: 'govuk-tag--blue', + Issue: 'govuk-tag--blue', // deprecated + Live: 'govuk-tag--green' +} + +export function statusToTagClass(status) { + console.assert(status in statusToTagClassMapping, `statusToTagClass: unknown status ${status}`) + return statusToTagClassMapping[status] +} + const { govukMarkdown, govukDateTime } = xGovFilters const addFilters = (nunjucksEnv, { datasetNameMapping }) => { @@ -19,7 +36,8 @@ const addFilters = (nunjucksEnv, { datasetNameMapping }) => { nunjucksEnv.addFilter('validationMessageLookup', validationMessageLookup) nunjucksEnv.addFilter('toErrorList', toErrorList) nunjucksEnv.addFilter('prettifyColumnName', prettifyColumnName) - nunjucksEnv.addFilter('getFullServiceName', getFullServiceName) + nunjucksEnv.addFilter('getFullServiceName', getFullServiceName), + nunjucksEnv.addFilter('statusToTagClass', statusToTagClass) } export default addFilters diff --git a/src/services/datasette.js b/src/services/datasette.js index 0048269b..976b1bb9 100644 --- a/src/services/datasette.js +++ b/src/services/datasette.js @@ -8,7 +8,7 @@ export default { * Executes a SQL query on the Datasette instance and returns the results. * * @param {string} query - The SQL query to execute. - * @returns {Promise<{data: object, formattedData: object}>} - A promise that resolves to an object with the following properties: + * @returns {Promise<{data: object, formattedData: object[]}>} - A promise that resolves to an object with the following properties: * - `data`: The raw data returned by Datasette. * - `formattedData`: The formatted data, with columns and rows parsed into a usable format. * @throws {Error} If the query fails or there is an error communicating with Datasette. diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 1a46d3ea..19fb6700 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -41,9 +41,9 @@ fs.createReadStream('src/content/entityIssueMessages.csv') /** * @typedef {object} Dataset + * @property {'Not submitted' | 'Error' | 'Needs fixing' | 'Warning' | 'Live' } status * @property {string} endpoint * @property {?string} error - * @property {?string} issue */ /** @@ -86,26 +86,26 @@ export default { when (it.severity = 'error') then 'Needs fixing' when (it.severity = 'warning') then 'Warning' else 'Live' - end as status, + end as status, case when ((cast(rle.status as integer) > 200)) then format('There was a %s error accessing the data URL', rle.status) else null - end as error, - case - when (it.severity = 'info') then '' - else i.issue_type - end as issue_type, - case - when (it.severity = 'info') then '' - else it.severity - end as severity, - it.responsibility, - COUNT( - case - when it.severity != 'info' then 1 - else null - end - ) as issue_count + end as error, + case + when (it.severity = 'info') then '' + else i.issue_type + end as issue_type, + case + when (it.severity = 'info') then '' + else it.severity + end as severity, + it.responsibility, + COUNT( + case + when it.severity != 'info' then 1 + else null + end + ) as issue_count FROM provision p LEFT JOIN @@ -136,7 +136,8 @@ ORDER BY const datasets = result.formattedData.reduce((accumulator, row) => { accumulator[row.dataset] = { - endpoint: row.endpoint + endpoint: row.endpoint, + status: row.status } return accumulator }, {}) diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index 59d011e8..dcd7a6da 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -6,18 +6,6 @@ {% set pageName = organisation.name + " overview" %} {% set serviceType = 'Manage' %} -{% macro tagCardClass(status) %} - {% if status == 'Not submitted' %} - {% set statusCardClasses = 'govuk-tag--grey' %} - {% elif status == 'Error' %} - {% set statusCardClasses = 'govuk-tag--red' %} - {% elif status == 'Needs fixing' %} - {% set statusCardClasses = 'govuk-tag--yellow' %} - {% else %} - {% set statusCardClasses = 'govuk-tag--green' %} - {% endif %} -{% endmacro %} - {% block beforeContent %} {{ super() }} @@ -104,7 +92,7 @@

      {{govukTag({ text: dataset.status, - classes: tagCardClass(dataset.status) + classes: dataset.status | statusToTagClass })}}
      From 62dbb8ed741ed949deb256f6b770fe920d912e9d Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Tue, 13 Aug 2024 15:47:05 +0100 Subject: [PATCH 25/34] fix linter issues --- src/filters/filters.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/filters/filters.js b/src/filters/filters.js index 514caedf..0dcdb5cb 100644 --- a/src/filters/filters.js +++ b/src/filters/filters.js @@ -6,8 +6,8 @@ import prettifyColumnName from './prettifyColumnName.js' import getFullServiceName from './getFullServiceName.js' import { makeDatasetSlugToReadableNameFilter } from './makeDatasetSlugToReadableNameFilter.js' -/** maps dataset status (as returned by performanceDbApi/getLpaOverview()) to a - * CSS class used by the govuk-tag component +/** maps dataset status (as returned by performanceDbApi/getLpaOverview()) to a + * CSS class used by the govuk-tag component */ const statusToTagClassMapping = { Error: 'govuk-tag--red', @@ -18,7 +18,7 @@ const statusToTagClassMapping = { Live: 'govuk-tag--green' } -export function statusToTagClass(status) { +export function statusToTagClass (status) { console.assert(status in statusToTagClassMapping, `statusToTagClass: unknown status ${status}`) return statusToTagClassMapping[status] } @@ -36,7 +36,7 @@ const addFilters = (nunjucksEnv, { datasetNameMapping }) => { nunjucksEnv.addFilter('validationMessageLookup', validationMessageLookup) nunjucksEnv.addFilter('toErrorList', toErrorList) nunjucksEnv.addFilter('prettifyColumnName', prettifyColumnName) - nunjucksEnv.addFilter('getFullServiceName', getFullServiceName), + nunjucksEnv.addFilter('getFullServiceName', getFullServiceName) nunjucksEnv.addFilter('statusToTagClass', statusToTagClass) } From e08c3847f3693f4ff706bcfdad51a63884990c3e Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Tue, 13 Aug 2024 15:59:47 +0100 Subject: [PATCH 26/34] count all datasets with set endpoints, ignoring status --- src/controllers/OrganisationsController.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 9085b4b8..6bed4b65 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -75,7 +75,7 @@ const organisationsController = { const totalDatasets = datasets.length const [datasetsWithEndpoints, datasetsWithIssues, datasetsWithErrors] = datasets.reduce((accumulator, dataset) => { - if (dataset.status === 'Live' || dataset.status === 'Warning') accumulator[0]++ + if (dataset.endpoint) accumulator[0]++ if (dataset.status === 'Needs fixing') accumulator[1]++ if (dataset.status === 'Error') accumulator[2]++ return accumulator From c5a29ca6e607842416a86a89f47064d8c6c5c64a Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Tue, 13 Aug 2024 15:59:58 +0100 Subject: [PATCH 27/34] fix test data --- test/unit/organisationsController.test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/unit/organisationsController.test.js b/test/unit/organisationsController.test.js index a2fca79e..14d0af06 100644 --- a/test/unit/organisationsController.test.js +++ b/test/unit/organisationsController.test.js @@ -31,9 +31,9 @@ describe('OrganisationsController.js', () => { const expectedResponse = { name: 'Test LPA', datasets: { - dataset1: { endpoint: 'https://example.com', issue: false, error: false }, - dataset2: { endpoint: null, issue: true, error: false }, - dataset3: { endpoint: 'https://example.com', issue: false, error: true } + dataset1: { endpoint: 'https://example.com', status: 'Live' }, + dataset2: { endpoint: null, status: 'Needs fixing' }, + dataset3: { endpoint: 'https://example.com', status: 'Error' } } } @@ -45,9 +45,9 @@ describe('OrganisationsController.js', () => { expect(res.render).toHaveBeenCalledWith('organisations/overview.html', expect.objectContaining({ organisation: { name: 'Test LPA' }, datasets: expect.arrayContaining([ - { endpoint: 'https://example.com', issue: false, error: false, slug: 'dataset1' }, - { endpoint: null, issue: true, error: false, slug: 'dataset2' }, - { endpoint: 'https://example.com', issue: false, error: true, slug: 'dataset3' } + { endpoint: 'https://example.com', status: 'Live', slug: 'dataset1' }, + { endpoint: null, status: 'Needs fixing', slug: 'dataset2' }, + { endpoint: 'https://example.com', status: 'Error', slug: 'dataset3' } ]), totalDatasets: 3, datasetsWithEndpoints: 2, From 813034f3a6544bb1ada7faf56394cb597d9a54bb Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 14 Aug 2024 08:59:32 +0100 Subject: [PATCH 28/34] ensure issue_count is passed to the templates --- src/controllers/OrganisationsController.js | 1 + src/services/performanceDbApi.js | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 7674525f..7e60719e 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -71,6 +71,7 @@ const organisationsController = { datasets.push({ slug: dataset, endpoint: null, + issue_count: 0, status: 'Not submitted' }) } diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index e5785dd7..8c251a09 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -137,7 +137,8 @@ ORDER BY const datasets = result.formattedData.reduce((accumulator, row) => { accumulator[row.dataset] = { endpoint: row.endpoint, - status: row.status + status: row.status, + issue_count: row.issue_count } return accumulator }, {}) From fd1ec495d69bafe4ad46dba8deb11911e9795435 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 14 Aug 2024 09:10:02 +0100 Subject: [PATCH 29/34] update jsdoc --- src/services/performanceDbApi.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index e5785dd7..6732807c 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -43,6 +43,7 @@ fs.createReadStream('src/content/entityIssueMessages.csv') * @typedef {object} Dataset * @property {'Not submitted' | 'Error' | 'Needs fixing' | 'Warning' | 'Live' } status * @property {string} endpoint + * @property {number} issue_count * @property {?string} error */ From 2c45ff5c25909bb1dd94c6cd41b93d1d8e7e20dd Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 14 Aug 2024 09:19:48 +0100 Subject: [PATCH 30/34] also pass errors through to template --- src/services/performanceDbApi.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 8c251a09..97bdd7ab 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -138,7 +138,8 @@ ORDER BY accumulator[row.dataset] = { endpoint: row.endpoint, status: row.status, - issue_count: row.issue_count + issue_count: row.issue_count, + error: row.http_status !== '200' ? (row.exception || `The endpoint returned a status of: ${row.http_status}`) : undefined } return accumulator }, {}) From f8b4d2d810a86fe47b10e26559b1e9e3d790a925 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 14 Aug 2024 09:29:27 +0100 Subject: [PATCH 31/34] allow empty datasets from performanceDbApi.getLpaOverview() --- src/services/performanceDbApi.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 6732807c..31f44f85 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -49,7 +49,6 @@ fs.createReadStream('src/content/entityIssueMessages.csv') /** * @typedef {object} LpaOverview - * @property {string} name * @property {{ [dataset: string]: Dataset }} datasets */ @@ -132,10 +131,12 @@ ORDER BY p.organisation, o.name; ` - const result = await datasette.runQuery(query) + if (result.formattedData.length === 0) { + logger.info(`No records found for LPA=${lpa}`) + } - const datasets = result.formattedData.reduce((accumulator, row) => { + let datasets = result.formattedData.reduce((accumulator, row) => { accumulator[row.dataset] = { endpoint: row.endpoint, status: row.status @@ -143,13 +144,7 @@ ORDER BY return accumulator }, {}) - if (result.formattedData.length === 0) { - throw new Error(`No records found for LPA=${lpa}`) - } - - return { - datasets - } + return { datasets } }, getResourceStatus: async (lpa, datasetId) => { From c391ab2cbf366a77c05cd68a3c1422c6b8e80cb2 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 14 Aug 2024 09:31:28 +0100 Subject: [PATCH 32/34] change to just use error column --- src/services/performanceDbApi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 97bdd7ab..4a10a4a3 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -139,7 +139,7 @@ ORDER BY endpoint: row.endpoint, status: row.status, issue_count: row.issue_count, - error: row.http_status !== '200' ? (row.exception || `The endpoint returned a status of: ${row.http_status}`) : undefined + error: row.error } return accumulator }, {}) From 8210c0ef585c6aea1036b46992ed1a3742ca0807 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 14 Aug 2024 09:34:44 +0100 Subject: [PATCH 33/34] revert to const --- src/services/performanceDbApi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 31f44f85..0fc008b1 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -136,7 +136,7 @@ ORDER BY logger.info(`No records found for LPA=${lpa}`) } - let datasets = result.formattedData.reduce((accumulator, row) => { + const datasets = result.formattedData.reduce((accumulator, row) => { accumulator[row.dataset] = { endpoint: row.endpoint, status: row.status From f0e87a9bf4e72136efbb98031e7b4e956a89f92a Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 14 Aug 2024 09:41:03 +0100 Subject: [PATCH 34/34] missing import --- src/services/performanceDbApi.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 0fc008b1..289c1d3b 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -2,6 +2,7 @@ * Performance DB API service */ import datasette from './datasette.js' +import logger from '../utils/logger.js' // ===========================================