Merge pull request #1129 from digital-land/efficient-fetchEntityIssueCounts

Efficient fetch entity issue counts

Author: pooleycodes

package.json

@@ -32,7 +32,7 @@
     "test:integration": "NODE_ENV=test playwright test --config ./test/integration/playwright.config.js",
     "test:acceptance": "NODE_ENV=development playwright test --config ./test/acceptance/playwright.config.js",
     "test:acceptance:ci": "NODE_ENV=ci playwright test --config ./test/acceptance/playwright.config.js",
-    "test:acceptance:local": "NODE_ENV=local playwright test --config ./test/acceptance/playwright.config.js",
+    "test:acceptance:local": "NODE_ENV=local playwright test --config ./test/acceptance/playwright.config.js --ui",
     "playwright-codegen": "playwright codegen http://localhost:5000",
     "lint": "standard",
     "lint:fix": "standard --fix",

src/middleware/common.middleware.js

@@ -618,7 +618,6 @@ export const fetchEntryIssues = fetchMany({
 export const fetchEntityIssueCounts = fetchMany({
   query: ({ req }) => {
     const datasetClause = req.params.dataset ? `AND i.dataset = '${req.params.dataset}'` : ''
-
     return `
       WITH unique_issues AS (
         SELECT DISTINCT

src/middleware/lpa-overview.middleware.js

@@ -5,32 +5,30 @@
  */
 
 import performanceDbApi from '../services/performanceDbApi.js'
-import { expectationFetcher, expectations, fetchEndpointSummary, fetchEntityIssueCounts, fetchEntryIssueCounts, fetchOrgInfo, fetchResources, logPageError, noop, setAvailableDatasets } from './common.middleware.js'
-import { fetchMany, FetchOptions, renderTemplate, fetchOneFromAllDatasets } from './middleware.builders.js'
+import { expectationFetcher, expectations, fetchEndpointSummary, fetchEntryIssueCounts, fetchOrgInfo, fetchResources, logPageError, noop, setAvailableDatasets } from './common.middleware.js'
+import { fetchMany, FetchOptions, renderTemplate, fetchOneFromAllDatasets, parallel } from './middleware.builders.js'
 import { getDeadlineHistory, requiredDatasets } from '../utils/utils.js'
-import config from '../../config/index.js'
 import _ from 'lodash'
 import logger from '../utils/logger.js'
 import { isFeatureEnabled } from '../utils/features.js'
 
 /**
- * Middleware. Updates req with 'datasetErrorStatus'.
+ * Middleware. Updates req with 'entityIssueCounts' same as fetchEntityIssueCounts so not to be used together!
  *
- * Fetches datasets which have active endpoints in error state.
+ * Functionally equivalent (for the utilization of the LPA Dashboard) to fetchEntityIssueCounts but using performanceDb
  */
-const fetchDatasetErrorStatus = fetchMany({
+const fetchEntityIssueCountsPerformanceDb = fetchMany({
   query: ({ params }) => {
-    return performanceDbApi.datasetErrorStatusQuery(params.lpa, { datasetsFilter: Object.keys(config.datasetsConfig) })
+    return performanceDbApi.fetchEntityIssueCounts(params.lpa)
   },
-  result: 'datasetErrorStatus',
+  result: 'entityIssueCounts',
   dataset: FetchOptions.performanceDb
 })
 
 const fetchProvisions = fetchMany({
   query: ({ params }) => {
-    const excludeDatasets = Object.keys(config.datasetsConfig).map(dataset => `'${dataset}'`).join(',')
     return /* sql */ `select dataset, project, provision_reason
-       from provision where organisation = '${params.lpa}' and dataset in (${excludeDatasets})`
+       from provision where organisation = '${params.lpa}'`
   },
   result: 'provisions'
 })
@@ -285,6 +283,10 @@ export function prepareOverviewTemplateParams (req, res, next) {
     switch (reason) {
       case 'statutory':
         return 'statutory'
+      case 'expected':
+        return 'expected'
+      case 'prospective':
+        return 'prospective'
       default:
         return 'other'
     }
@@ -355,13 +357,17 @@ const fetchOutOfBoundsExpectations = expectationFetcher({
  * Organisation (LPA) overview page middleware chain.
  */
 export default [
-  fetchOrgInfo,
-  fetchResources,
-  fetchDatasetErrorStatus,
-  fetchEndpointSummary,
-  fetchEntityIssueCounts,
-  fetchEntryIssueCounts,
-  fetchEntityCounts,
+  parallel([
+    fetchOrgInfo,
+    fetchResources,
+    fetchEndpointSummary,
+    fetchEntityIssueCountsPerformanceDb,
+    fetchProvisions
+  ]),
+  parallel([
+    fetchEntryIssueCounts, // needs fetchResources to complete
+    fetchEntityCounts // needs fetchOrgInfo to complete
+  ]),
   setAvailableDatasets,
   isFeatureEnabled('expectationOutOfBoundsTask') ? fetchOutOfBoundsExpectations : noop,
   groupResourcesByDataset,
@@ -372,7 +378,6 @@ export default [
 
   // datasetSubmissionDeadlineCheck,  // commented out as the logic is currently incorrect (https://github.com/digital-land/submit/issues/824)
   // addNoticesToDatasets,            // commented out as the logic is currently incorrect (https://github.com/digital-land/submit/issues/824)
-  fetchProvisions,
   prepareOverviewTemplateParams,
   getOverview,
   logPageError

src/routes/schemas.js

@@ -136,6 +136,8 @@ export const OrgOverviewPage = v.strictObject({
   organisation: OrgField,
   datasets: v.object({
     statutory: v.optional(v.array(DatasetItem)),
+    expected: v.optional(v.array(DatasetItem)),
+    prospective: v.optional(v.array(DatasetItem)),
     other: v.optional(v.array(DatasetItem))
   }),
   totalDatasets: v.integer(),

src/services/performanceDbApi.js

@@ -250,6 +250,22 @@ export default {
     AND pipeline = '${datasetId}'`
   },
 
+  // Query to simulate fetchEntityIssueCounts in common.middleware.js but against performanceDb
+  fetchEntityIssueCounts (datasetId) {
+    return /* sql */ `
+    SELECT 
+    dataset,
+    issue_type,
+    field,
+    COUNT(*) AS count 
+    FROM endpoint_dataset_issue_type_summary 
+    WHERE organisation = '${datasetId}'
+        AND severity = 'error' 
+        AND responsibility = 'external' 
+        AND (resource_end_date = '' OR resource_end_date IS NULL)
+    GROUP BY dataset, issue_type, field`
+  },
+
   datasetIssuesQuery,
 
   /**

src/views/organisations/overview.html

@@ -91,11 +91,17 @@ <h1 class="govuk-heading-xl">
       {% endif %}
     {% endfor %}
 
-    {% for dataset in datasets.other %}
-    {% if dataset.notice %}
-      {{ deadlineNotice(dataset.dataset, dataset.notice, organisation) }}
-    {% endif %}
-  {% endfor %}
+    {% for dataset in datasets.expected %}
+      {% if dataset.notice %}
+        {{ deadlineNotice(dataset.dataset, dataset.notice, organisation) }}
+      {% endif %}
+    {% endfor %}
+
+    {% for dataset in datasets.prospective %}
+      {% if dataset.notice %}
+        {{ deadlineNotice(dataset.dataset, dataset.notice, organisation) }}
+      {% endif %}
+    {% endfor %}
 
   </div>
 </div>
@@ -107,17 +113,17 @@ <h1 class="govuk-heading-xl">
     <div class="dataset-status">
       <div class="dataset-status--item">
         <span class="big-number">{{datasetsWithEndpoints}}/{{totalDatasets}}</span>
-        datasets submitted
+        authoritative dataset{{ "s" if (datasetsWithEndpoints != 1) }} provided
       </div>
 
       <div class="dataset-status--item">
         <span class="big-number">{{datasetsWithErrors}}</span>
-        dataset{{ "s" if (datasetsWithErrors != 1) }} with endpoint errors
+        error{{ "s" if (datasetsWithErrors != 1) }} accessing URLs
       </div>
 
       <div class="dataset-status--item">
         <span class="big-number">{{datasetsWithIssues}}</span>
-        {{ "dataset" | pluralise(datasetsWithIssues) }} need{{ "s" if (datasetsWithIssues == 1) }} fixing
+        {{ "dataset" | pluralise(datasetsWithIssues) }} can be improved
       </div>
     </div>
   </div>
@@ -126,10 +132,10 @@ <h1 class="govuk-heading-xl">
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
 
-
   {% if datasets.statutory.length > 0 %}
     <div data-testid="datasetsMandatory">
       <h2 class="govuk-heading-m">Datasets {{ organisation.name }} must provide</h2>
+      <p class="org-membership-info">You are required to make these datasets available under legislation. Using the Planning Data Platform to do this can help you to create and maintain authoritative, trustworthy data.</p>
       <ul class="govuk-task-list govuk-!-margin-bottom-8" data-reason="statutory">
         {% for dataset in datasets.statutory %}
           {{ datasetItem(dataset) }}
@@ -138,22 +144,29 @@ <h2 class="govuk-heading-m">Datasets {{ organisation.name }} must provide</h2>
     </div>
   {% endif %}
 
-  {% if datasets.other.length > 0 %}
-    <div data-testid="datasetsOdpMandatory">
-      <h2 class="govuk-heading-m">Datasets organisations in Open Digital Planning programme must provide</h2>
-      {% if isODPMember %}
-        <p class="org-membership-info">{{ organisation.name}} is a member of the Open Digital Planning programme.</p>
-      {% else %}
-        <p class="org-membership-info">{{ organisation.name}} is not a member of the Open Digital Planning programme.</p>
-      {% endif %}
-      <ul class="govuk-task-list" data-reason="other">
-        {% for dataset in datasets.other %}
+  {% if datasets.expected.length > 0 and isODPMember %}
+    <div data-testid="datasetsExpected">
+      <h2 class="govuk-heading-m">Datasets {{ organisation.name}} are expected to provide</h2>
+      <p class="org-membership-info">We expect you to provide these datasets as a condition of your funding with the Open Digital Planning community.</p>
+      <ul class="govuk-task-list govuk-!-margin-bottom-8" data-reason="expected">
+        {% for dataset in datasets.expected %}
           {{ datasetItem(dataset) }}
         {% endfor %}
       </ul>
     </div>
   {% endif %}
 
+  {% if datasets.prospective.length > 0 %}
+    <div data-testid="datasetsProspective">
+      <h2 class="govuk-heading-m">Datasets {{ organisation.name}} can provide</h2>
+      <p class="org-membership-info">Your organisation is the authoritative source of this data. Providing this data to the platform helps ensure it is accurate and up to date.</p>
+      <ul class="govuk-task-list" data-reason="prospective">
+        {% for dataset in datasets.prospective %}
+          {{ datasetItem(dataset) }}
+        {% endfor %}
+      </ul>
+    </div>
+  {% endif %}
 
   </div>
 </div>

test/acceptance/dataset_overview.test.js

@@ -19,10 +19,10 @@ test.describe('Dataset overview', () => {
     expect(await page.locator('h1').innerText()).toEqual('London Borough of Lambeth overview')
     expect(await page.locator('[data-testid=datasetsMandatory] h2.govuk-heading-m').innerText())
       .toEqual('Datasets London Borough of Lambeth must provide')
-    expect(await page.locator('[data-testid=datasetsMandatory] .govuk-task-list li').count()).toBeGreaterThanOrEqual(1)
-    expect(await page.locator('[data-testid=datasetsOdpMandatory] h2.govuk-heading-m').innerText())
-      .toEqual('Datasets organisations in Open Digital Planning programme must provide')
-    expect(await page.locator('[data-testid=datasetsOdpMandatory] .govuk-task-list li').count()).toBeGreaterThanOrEqual(8)
+    expect(await page.locator('[data-testid=datasetsExpected] .govuk-task-list li').count()).toBeGreaterThanOrEqual(1)
+    expect(await page.locator('[data-testid=datasetsExpected] h2.govuk-heading-m').innerText())
+      .toEqual('Datasets London Borough of Lambeth are expected to provide')
+    expect(await page.locator('[data-testid=datasetsExpected] .govuk-task-list li').count()).toBeGreaterThanOrEqual(8)
   })
 
   test('Can view dataset overview', async ({ page, context }) => {
@@ -99,7 +99,7 @@ test.describe('Dataset overview', () => {
     await organisationOverviewPage.navigateHere()
 
     expect(await page.locator('h1').innerText()).toEqual('London Borough of Lambeth overview')
-    const datasetElements = await page.locator('[data-testid=datasetsOdpMandatory] .govuk-task-list li').allInnerTexts()
+    const datasetElements = await page.locator('[data-testid=datasetsExpected] .govuk-task-list li').allInnerTexts()
     const displayedNames = datasetElements
       .map(text => text.split('\n')[0].trim())
 

test/unit/middleware/lpa-overview.middleware.test.js

@@ -96,7 +96,7 @@ describe('lpa-overview.middleware', () => {
             { endpointCount: 1, status: 'Live', dataset: 'dataset1', error: undefined, issueCount: 0, endpointErrorCount: 0 },
             { endpointCount: 1, status: 'Error', dataset: 'dataset3', error: undefined, issueCount: 0, endpointErrorCount: 1 }
           ]),
-          other: expect.arrayContaining([
+          expected: expect.arrayContaining([
             { endpointCount: 0, status: 'Needs fixing', dataset: 'dataset2', error: undefined, issueCount: 1, endpointErrorCount: 0 },
             { endpointCount: 0, status: 'Error', dataset: 'dataset4', error: 'There was a 404 error', issueCount: 0, endpointErrorCount: 1 }
           ])
@@ -114,17 +114,23 @@ describe('lpa-overview.middleware', () => {
       expect(errorCardNodes[0].querySelector('.govuk-task-list__hint').textContent.trim()).toBe('There was an error accessing the endpoint URL')
       expect(errorCardNodes[1].querySelector('.govuk-task-list__hint').textContent.trim()).toBe('There was a 404 error')
 
-      const orgMemebershipInfo = doc.querySelector('.org-membership-info').textContent.trim()
-      expect(orgMemebershipInfo).toMatch('is a member of the Open Digital Planning programme')
+      // Check for ODP membership info in the expected datasets section
+      const expectedSection = doc.querySelector('[data-testid="datasetsExpected"]')
+      if (expectedSection) {
+        const orgMembershipInfo = expectedSection.querySelector('.org-membership-info')
+        expect(orgMembershipInfo.textContent.trim()).toMatch(/Open Digital Planning/)
+      }
 
-      // verify proper label for non-OPD memebers gets rendered
+      // verify proper label for non-ODP members gets rendered
       const reqNotMember = structuredClone(reqTemplate)
       reqNotMember.provisions.forEach((provision) => {
         provision.project = ''
       })
       prepareOverviewTemplateParams(reqNotMember, res, () => { })
       const { doc: docNotMember } = getRenderedErrorCards(reqNotMember.templateParams)
-      expect(docNotMember.querySelector('.org-membership-info').textContent.trim()).toMatch('is not a member of the Open Digital Planning programme')
+      // When not an ODP member, expected datasets won't render (requires isODPMember), so datasetsExpected won't be present
+      const expectedSectionNotMember = docNotMember.querySelector('[data-testid="datasetsExpected"]')
+      expect(expectedSectionNotMember).toBeNull()
     })
 
     it('should patch dataset status based on the provision_summary info', () => {
@@ -139,7 +145,7 @@ describe('lpa-overview.middleware', () => {
       expect(ds1.status).toBe('Live')
       expect(ds1.error).toBeUndefined()
 
-      const ds4 = req.templateParams.datasets.other[1]
+      const ds4 = req.templateParams.datasets.expected[1]
       expect(ds4.status).toBe('Error')
       expect(ds4.error).toBe(req.datasets[3].error) // Error message should be left untouched
     })

test/unit/views/organisations/lpaOverviewPage.test.js

@@ -38,7 +38,7 @@ const datasetGroup = ({ expect }, key, datasets, document) => {
 
 describe(`LPA Overview Page (seed: ${seed})`, () => {
   const params = mocker(OrgOverviewPage, seed)
-  console.debug(`mocked datasets: statutory = ${params.datasets.statutory?.length ?? 'none'}, other = ${params.datasets.other?.length ?? 'none'}`)
+  console.debug(`mocked datasets: statutory = ${params.datasets.statutory?.length ?? 'none'}, expected = ${params.datasets.expected?.length ?? 'none'}, prospective = ${params.datasets.prospective?.length ?? 'none'}`)
 
   const html = nunjucks.render('organisations/overview.html', params)
 
@@ -53,17 +53,18 @@ describe(`LPA Overview Page (seed: ${seed})`, () => {
   const statsBoxes = document.querySelector('.dataset-status').children
   it('Datasets provided gives the correct value', () => {
     expect(statsBoxes[0].textContent).toContain(`${params.datasetsWithEndpoints}/${params.totalDatasets}`)
-    expect(statsBoxes[0].textContent).toContain('datasets submitted')
+    expect(statsBoxes[0].textContent).toContain('authoritative dataset')
+    expect(statsBoxes[0].textContent).toContain('provided')
   })
 
   it('Datasets with errors gives the correct value', () => {
     expect(statsBoxes[1].textContent).toContain(params.datasetsWithErrors)
-    expect(statsBoxes[1].textContent).toMatch(/datasets? with endpoint errors/)
+    expect(statsBoxes[1].textContent).toContain('accessing URLs')
   })
 
   it('Datasets with issues gives the correct value', () => {
     expect(statsBoxes[2].textContent).toContain(params.datasetsWithIssues)
-    expect(statsBoxes[2].textContent).toMatch(/datasets? needs? fixing/)
+    expect(statsBoxes[2].textContent).toContain('can be improved')
   })
 
   it('The correct number of dataset cards are rendered with the correct titles in group "statutory"', () => {
@@ -72,16 +73,23 @@ describe(`LPA Overview Page (seed: ${seed})`, () => {
     }
   })
 
-  it('The correct number of dataset cards are rendered with the correct titles in group "other"', () => {
-    if (params.datasets.other && params.datasets.other.length > 0) {
-      datasetGroup({ expect }, 'other', params.datasets.other, document)
+  it('The correct number of dataset cards are rendered with the correct titles in group "expected"', () => {
+    if (params.datasets.expected && params.datasets.expected.length > 0) {
+      datasetGroup({ expect }, 'expected', params.datasets.expected, document)
+    }
+  })
 
-      const infoText = document.querySelector('.org-membership-info').textContent
-      expect(infoText).match(new RegExp(`${params.organisation.name} is (a|not a) member of .*`))
+  it('The correct number of dataset cards are rendered with the correct titles in group "prospective"', () => {
+    if (params.datasets.prospective && params.datasets.prospective.length > 0) {
+      datasetGroup({ expect }, 'prospective', params.datasets.prospective, document)
     }
   })
 
-  const allDatasets = [].concat(...Object.values(params.datasets.statutory ?? []), ...Object.values(params.datasets.other ?? []))
+  const allDatasets = [
+    ...(params.datasets.statutory ?? []),
+    ...(params.datasets.expected ?? []),
+    ...(params.datasets.prospective ?? [])
+  ]
   allDatasets.forEach((dataset, i) => {
     it(`dataset cards are rendered with correct hints for dataset='${dataset.dataset}'`, () => {
       let expectedHint = 'Endpoint URL submitted'