Merge pull request #1154 from digital-land/fix/check-show-all

check page now shows all rows found by async

Author: pooleycodes

src/controllers/resultsController.js

@@ -15,7 +15,7 @@ const failedUrlRequestTemplate = 'results/failedUrlRequest'
 const resultsTemplate = 'results/results'
 
 class ResultsController extends PageController {
-  /* Custom middleware */
+  /* Custom middleware, currently the controller can load results/results, results/failedFileRequest and results/failedUrlRequest templates */
   middlewareSetup () {
     super.middlewareSetup()
     this.use(validateParams)
@@ -129,10 +129,8 @@ export async function fetchResponseDetails (req, res, next) {
   try {
     if (req.locals.template !== failedFileRequestTemplate && req.locals.template !== failedUrlRequestTemplate) {
       const detailsOpts = req.locals.detailsOptions ?? {}
-      const responseDetails = req.locals.template === resultsTemplate
-        // pageNumber starts with: 1, fetchResponseDetails parameter `pageOffset` starts with 0
-        ? await req.locals.requestData.fetchResponseDetails(pageNumber - 1, 50, { severity: 'error', ...detailsOpts })
-        : await req.locals.requestData.fetchResponseDetails(pageNumber - 1, 50, { ...detailsOpts })
+      // Original code used a if statement to check template and filter by error severity accordingly, but move to always showing all details in results/results template
+      const responseDetails = await req.locals.requestData.fetchResponseDetails(pageNumber - 1, 50, { ...detailsOpts })
       req.locals.responseDetails = responseDetails
     }
   } catch (e) {
@@ -161,7 +159,8 @@ export const fieldToColumnMapping = ({ columns }) => {
 export function setupTableParams (req, res, next) {
   if (req.locals.template !== failedFileRequestTemplate && req.locals.template !== failedUrlRequestTemplate) {
     const responseDetails = req.locals.responseDetails
-    let rows = responseDetails.getRowsWithVerboseColumns(req.locals.requestData.hasErrors())
+    // Optionally filter out all non - error rows from dataset
+    let rows = responseDetails.getRowsWithVerboseColumns(false)
     // remove any issues that aren't of severity error
     rows = rows.map((row) => {
       const { columns, ...rest } = row
@@ -368,6 +367,7 @@ export function getTotalRows (req, res, next) {
   const totalRows = Number.parseInt(responseDetails.pagination.totalResults)
   // NOTE: the fallback number may not be accurate, but it's better than just giving up and throwing
   req.totalRows = Number.isInteger(totalRows) ? totalRows : responseDetails.getRows().length
+  req.locals.totalRows = req.totalRows
   next()
 }
 
@@ -471,13 +471,8 @@ export function getPassedChecks (req, res, next) {
     }
   }
 
-  // add task complete for how many rows are in the table
-  if (totalRows > 0) {
-    passedChecks.unshift(makePassedCheck(`Found ${totalRows} rows`))
-
-    if (tasks.length === 0 && missingColumnTasks.length === 0) {
-      passedChecks.push(makePassedCheck('All data is valid'))
-    }
+  if (totalRows > 0 && tasks.length === 0 && missingColumnTasks.length === 0) {
+    passedChecks.push(makePassedCheck('All data is valid'))
   }
 
   req.locals.passedChecks = passedChecks

src/views/check/results/results.html

@@ -105,6 +105,14 @@ <h1 class="govuk-heading-l">
         value: {
           text: options.uploadInfo.checkedTime | govukDateTime
         }
+      },
+      {
+        key: {
+          text: "Found"
+        },
+        value: {
+          text: options.totalRows + (" row" if options.totalRows === 1 else " rows")
+        }
       }
     ] %}
 

test/integration/test_recieving_results.playwright.test.js

@@ -55,7 +55,8 @@ test('receiving a result with errors', async ({ page }) => {
 
   expect(tableValues[0]).toEqual(expectedTableValues[0])
   expect(tableValues[1]).toEqual(expectedTableValues[1])
-  expect(tableValues[2]).toEqual(expectedTableValues[3])
+  expect(tableValues[2]).toEqual(expectedTableValues[2])
+  expect(tableValues[3]).toEqual(expectedTableValues[3])
 
   const issues = errorResponseDetails.map(detail => detail.issue_logs.filter(issue => issue.severity === 'error').map(issue => issue.message)).filter(issue => issue.length > 0)
 

test/unit/resultsController.test.js

@@ -299,15 +299,11 @@ describe('getPassedChecks()', () => {
     totalRows: 99,
     missingColumnTasks: []
   }
-  it('displays correct number of rows', () => {
+  it('shows all data is valid when no errors', () => {
     const req = structuredClone(reqTemplate)
     getPassedChecks(req, {}, vi.fn())
 
     expect(req.locals.passedChecks).toMatchObject([
-      {
-        status: { tag: { text: 'Passed' } },
-        title: { text: 'Found 99 rows' }
-      },
       {
         status: { tag: { text: 'Passed' } },
         title: { text: 'All data is valid' }