improv: Report Search UI refactor and Needs Review category (#274)

GovTechSG · Feb 5, 2024 · bea2bfb · bea2bfb
1 parent 2596181
commit bea2bfb
Show file tree

Hide file tree

Showing 25 changed files with 222 additions and 199 deletions.
diff --git a/INTEGRATION.md b/INTEGRATION.md
@@ -30,7 +30,7 @@ In order to use this functionality, the testing framework must support:
 
 ### API Reference
 
-#### `async purpleA11yInit(entryUrl, testLabel, name, email, needsReview, includeScreenshots, viewportSettings, thresholds, scanAboutMetadata)`
+#### `async purpleA11yInit(entryUrl, testLabel, name, email, includeScreenshots, viewportSettings, thresholds, scanAboutMetadata)`
 
 Returns an instance of Purple A11y
 
@@ -44,8 +44,6 @@ Returns an instance of Purple A11y
   - For Purple A11y data collection purposes
 - `email`
   - For Purple A11y data collection purposes
-- `needsReview` (optional)
-  - Show potential false positive issues in the report. Defaults to false.
 - `includeScreenshots` (optional)
   - Include screenshots of affected elements in the report. Defaults to false.
 - `viewportSettings` (optional)
@@ -178,7 +176,6 @@ Create <code>cypress.config.js</code> with the following contents, and change yo
         "Demo Cypress Scan", // label for test
         "Your Name",
         "[email protected]",
-        false, // whether to show false positive issues in the report
         true, // include screenshots of affected elements in the report
         viewportSettings,
         thresholds,
@@ -292,7 +289,6 @@ On your project's root folder, create a Playwright test file <code>purpleA11y-pl
         "Demo Playwright Scan", // label for test
         "Your Name",
         "[email protected]",
-        false, // whether to show false positive issues in the report
         true, // include screenshots of affected elements in the report
         viewportSettings,
         thresholds,

diff --git a/README.md b/README.md
@@ -267,9 +267,6 @@ Options:
   -t, --specifiedMaxConcurrency  Maximum number of pages to scan concurrently.
                                  Use for sites with throttling. Defaults to 25.
                                                                         [number]
-  -f, --needsReviewItems  Whether to display rule items requiring manual review 
-                          in report. Defaults to false.
-                                                      [choices: "true", "false"]
   -i, --fileTypes                    File types to include in the scan. Defaults
                                       to html-only.
        [string] [choices: "all", "pdf-only", "html-only"] [default: "html-only"]

diff --git a/cli.js b/cli.js
@@ -162,16 +162,6 @@ Usage: node cli.js -c <crawler> -d <device> -w <viewport> -u <url> OPTIONS`,
     }
     return nameEmail;
   })
-  .coerce('f', option => {
-    if (!cliOptions.f.choices.includes(option)) {
-      printMessage(
-        [`Invalid value for needsReviewItems. Please provide boolean value(true/false).`],
-        messageOptions,
-      );
-      process.exit(1);
-    }
-    return option;
-  })
   .coerce('e', option => {
     const validationErrors = validateDirPath(option);
     if (validationErrors) {

diff --git a/combine.js b/combine.js
@@ -27,7 +27,6 @@ const combineRun = async (details, deviceToScan) => {
     userDataDirectory,
     strategy,
     specifiedMaxConcurrency,
-    needsReviewItems,
     fileTypes,
     blacklistedPatternsFilename,
     includeScreenshots,
@@ -77,7 +76,6 @@ const combineRun = async (details, deviceToScan) => {
         url,
         randomToken,
         viewportSettings,
-        needsReviewItems,
         blacklistedPatterns,
         includeScreenshots,
       );
@@ -93,7 +91,6 @@ const combineRun = async (details, deviceToScan) => {
         browser,
         userDataDirectory,
         specifiedMaxConcurrency,
-        needsReviewItems,
         fileTypes,
         blacklistedPatterns,
         includeScreenshots,
@@ -112,7 +109,6 @@ const combineRun = async (details, deviceToScan) => {
         userDataDirectory,
         strategy,
         specifiedMaxConcurrency,
-        needsReviewItems,
         fileTypes,
         blacklistedPatterns,
         includeScreenshots,

diff --git a/constants/cliFunctions.js b/constants/cliFunctions.js
@@ -101,13 +101,7 @@ export const cliOptions = {
     type: 'number',
     demandOption: false,
   },
-  f: {
-    alias: 'needsReviewItems',
-    describe: 'Whether to display items requiring manual review in report. Defaults to false.',
-    type: 'string',
-    choices: ['true', 'false'],
-    demandOption: false,
-  },
+
   i: {
     alias: 'fileTypes',
     describe: 'File types to include in the scan. Defaults to html-only.',

diff --git a/constants/common.js b/constants/common.js
@@ -522,7 +522,6 @@ export const prepareData = async argv => {
     nameEmail,
     customFlowLabel,
     specifiedMaxConcurrency,
-    needsReviewItems,
     fileTypes,
     blacklistedPatternsFilename,
     additional,
@@ -558,7 +557,6 @@ export const prepareData = async argv => {
     nameEmail,
     customFlowLabel,
     specifiedMaxConcurrency,
-    needsReviewItems,
     randomToken: resultFilename,
     fileTypes,
     blacklistedPatternsFilename,

diff --git a/constants/itemTypeDescription.js b/constants/itemTypeDescription.js
@@ -3,6 +3,8 @@ const itemTypeDescription = {
     'Issues that need to be addressed promptly, as they create significant barriers for persons with disabilities and can prevent them from accessing essential content or features.',
   goodToFix:
     'Issues that could pose certain challenges for persons with disabilities (PWDs), but are unlikely to completely hinder their access to essential content or features.',
+  needsReview:
+    'Occurrences could potentially be false positives, requiring human validation for accuracy.',
   passed: 'Occurrences that passed the automated checks.',
 };
 

diff --git a/crawlers/commonCrawlerFunc.js b/crawlers/commonCrawlerFunc.js
@@ -6,13 +6,14 @@ import { axeScript, guiInfoStatusTypes, saflyIconSelector } from '../constants/c
 import { guiInfoLog } from '../logs.js';
 import { takeScreenshotForHTMLElements } from '../screenshotFunc/htmlScreenshotFunc.js';
 
-export const filterAxeResults = (needsReview, results, pageTitle, customFlowDetails) => {
+export const filterAxeResults = (results, pageTitle, customFlowDetails) => {
   const { violations, passes, incomplete, url } = results;
 
   let totalItems = 0;
   const mustFix = { totalItems: 0, rules: {} };
   const goodToFix = { totalItems: 0, rules: {} };
   const passed = { totalItems: 0, rules: {} };
+  const needsReview = { totalItems: 0, rules: {} };
 
   const process = (item, displayNeedsReview) => {
     const { id: rule, help: description, helpUrl, tags, nodes } = item;
@@ -66,7 +67,9 @@ export const filterAxeResults = (needsReview, results, pageTitle, customFlowDeta
 
     nodes.forEach(node => {
       const { impact } = node;
-      if (impact === 'critical' || impact === 'serious') {
+      if (displayNeedsReview) {
+        addTo(needsReview, node);
+      } else if (impact === 'critical' || impact === 'serious') {
         addTo(mustFix, node);
       } else {
         addTo(goodToFix, node);
@@ -75,9 +78,8 @@ export const filterAxeResults = (needsReview, results, pageTitle, customFlowDeta
   };
 
   violations.forEach(item => process(item, false));
-  if (needsReview) {
-    incomplete.forEach(item => process(item, true));
-  }
+  incomplete.forEach(item => process(item, true));
+
 
   passes.forEach(item => {
     const { id: rule, help: description, axeImpact, helpUrl, tags, nodes } = item;
@@ -107,12 +109,12 @@ export const filterAxeResults = (needsReview, results, pageTitle, customFlowDeta
     totalItems,
     mustFix,
     goodToFix,
+    needsReview,
     passed,
   };
 };
 
 export const runAxeScript = async (
-  needsReview,
   includeScreenshots,
   page,
   randomToken,
@@ -122,7 +124,7 @@ export const runAxeScript = async (
   await crawlee.playwrightUtils.injectFile(page, axeScript);
 
   const results = await page.evaluate(
-    async ({ selectors, saflyIconSelector, needsReview }) => {
+    async ({ selectors, saflyIconSelector}) => {
       // remove so that axe does not scan
       document.querySelector(saflyIconSelector)?.remove();
 
@@ -132,15 +134,15 @@ export const runAxeScript = async (
         },
       });
 
-      isReturnReviewItems = needsReview
-        ? ['violations', 'passes', 'incomplete']
-        : ['violations', 'passes'];
+      //removed needsReview condition
+      defaultResultTypes = ['violations', 'passes', 'incomplete']
+
 
       return axe.run(selectors, {
-        resultTypes: isReturnReviewItems,
+        resultTypes: defaultResultTypes,
       });
     },
-    { selectors, saflyIconSelector, needsReview },
+    { selectors, saflyIconSelector},
   );
 
   if (includeScreenshots) {
@@ -149,7 +151,7 @@ export const runAxeScript = async (
   }
 
   const pageTitle = await page.evaluate(() => document.title);
-  return filterAxeResults(needsReview, results, pageTitle, customFlowDetails);
+  return filterAxeResults(results, pageTitle, customFlowDetails);
 };
 
 export const createCrawleeSubFolders = async randomToken => {

diff --git a/crawlers/crawlDomain.js b/crawlers/crawlDomain.js
@@ -34,14 +34,12 @@ const crawlDomain = async (
   userDataDirectory,
   strategy,
   specifiedMaxConcurrency,
-  needsReviewItems,
   fileTypes,
   blacklistedPatterns,
   includeScreenshots,
   followRobots,
   extraHTTPHeaders
 ) => {
-  let needsReview = needsReviewItems;
   const isScanHtml = ['all', 'html-only'].includes(fileTypes);
   const isScanPdfs = ['all', 'pdf-only'].includes(fileTypes);
   const urlsCrawled = { ...constants.urlsCrawledObj };
@@ -312,7 +310,7 @@ const crawlDomain = async (
         } else {
           if (isScanHtml) {
 
-            const results = await runAxeScript(needsReview, includeScreenshots, page, randomToken);
+            const results = await runAxeScript(includeScreenshots, page, randomToken);
             guiInfoLog(guiInfoStatusTypes.SCANNED, {
               numScanned: urlsCrawled.scanned.length,
               urlScanned: request.url,

diff --git a/crawlers/crawlSitemap.js b/crawlers/crawlSitemap.js
@@ -29,7 +29,6 @@ const crawlSitemap = async (
   browser,
   userDataDirectory,
   specifiedMaxConcurrency,
-  needsReviewItems,
   fileTypes,
   blacklistedPatterns,
   includeScreenshots,
@@ -60,7 +59,7 @@ const crawlSitemap = async (
       basicAuthPage = -2;
    } 
 
-  let needsReview = needsReviewItems;
+
   const isScanHtml = ['all', 'html-only'].includes(fileTypes);
   const isScanPdfs = ['all', 'pdf-only'].includes(fileTypes);
 
@@ -167,7 +166,7 @@ const crawlSitemap = async (
         basicAuthPage++;
       } else {
         if (isScanHtml && status === 200 && isWhitelistedContentType(contentType)) {
-          const results = await runAxeScript(needsReview, includeScreenshots, page, randomToken);
+          const results = await runAxeScript(includeScreenshots, page, randomToken);
           guiInfoLog(guiInfoStatusTypes.SCANNED, {
             numScanned: urlsCrawled.scanned.length,
             urlScanned: request.url,

diff --git a/crawlers/custom/utils.js b/crawlers/custom/utils.js
@@ -95,15 +95,13 @@ export const screenshotFullPage = async (page, screenshotsDir, screenshotIdx) =>
 
 export const runAxeScan = async (
   page,
-  needsReviewItems,
   includeScreenshots,
   randomToken,
   customFlowDetails,
   dataset,
   urlsCrawled,
 ) => {
   const result = await runAxeScript(
-    needsReviewItems,
     includeScreenshots,
     page,
     randomToken,
@@ -124,7 +122,6 @@ export const processPage = async (page, processPageParams) => {
   processPageParams.scannedIdx += 1;
   const {
     scannedIdx,
-    needsReviewItems,
     blacklistedPatterns,
     includeScreenshots,
     dataset,
@@ -177,7 +174,6 @@ export const processPage = async (page, processPageParams) => {
 
   await runAxeScan(
     page,
-    needsReviewItems,
     includeScreenshots,
     randomToken,
     {

diff --git a/crawlers/runCustom.js b/crawlers/runCustom.js
@@ -13,7 +13,6 @@ const runCustom = async (
   url,
   randomToken,
   viewportSettings,
-  needsReviewItems,
   blacklistedPatterns,
   includeScreenshots,
 ) => {
@@ -26,7 +25,6 @@ const runCustom = async (
   const intermediateScreenshotsPath = getIntermediateScreenshotsPath(randomToken);
   const processPageParams = {
     scannedIdx: 0,
-    needsReviewItems,
     blacklistedPatterns,
     includeScreenshots,
     dataset,

diff --git a/mergeAxeResults.js b/mergeAxeResults.js
@@ -238,13 +238,16 @@ const pushResults = async (pageResults, allIssues, isCustomFlow) => {
   const totalIssuesInPage = new Set();
   Object.keys(pageResults.mustFix.rules).forEach(k => totalIssuesInPage.add(k));
   Object.keys(pageResults.goodToFix.rules).forEach(k => totalIssuesInPage.add(k));
+  Object.keys(pageResults.needsReview.rules).forEach(k => totalIssuesInPage.add(k));
+
   allIssues.topFiveMostIssues.push({ url, pageTitle, totalIssues: totalIssuesInPage.size });
 
-  ['mustFix', 'goodToFix', 'passed'].forEach(category => {
+  ['mustFix', 'goodToFix', 'needsReview', 'passed'].forEach(category => {
     if (!pageResults[category]) return;
+
     const { totalItems, rules } = pageResults[category];
     const currCategoryFromAllIssues = allIssues.items[category];
-
+    
     currCategoryFromAllIssues.totalItems += totalItems;
 
     Object.keys(rules).forEach(rule => {
@@ -309,7 +312,7 @@ const pushResults = async (pageResults, allIssues, isCustomFlow) => {
 };
 
 const flattenAndSortResults = (allIssues, isCustomFlow) => {
-  ['mustFix', 'goodToFix', 'passed'].forEach(category => {
+  ['mustFix', 'goodToFix', 'needsReview', 'passed'].forEach(category => {
     allIssues.totalItems += allIssues.items[category].totalItems;
     allIssues.items[category].rules = Object.entries(allIssues.items[category].rules)
       .map(ruleEntry => {
@@ -359,6 +362,7 @@ const createRuleIdJson = allIssues => {
 
   allIssues.items.mustFix.rules.forEach(ruleIterator);
   allIssues.items.goodToFix.rules.forEach(ruleIterator);
+  allIssues.items.needsReview.rules.forEach(ruleIterator);
   return compiledRuleJson;
 };
 
@@ -407,6 +411,7 @@ export const generateArtifacts = async (
     items: {
       mustFix: { description: itemTypeDescription.mustFix, totalItems: 0, rules: {} },
       goodToFix: { description: itemTypeDescription.goodToFix, totalItems: 0, rules: {} },
+      needsReview: { description: itemTypeDescription.needsReview, totalItems: 0, rules: {} },
       passed: { description: itemTypeDescription.passed, totalItems: 0, rules: {} },
     },
     cypressScanAboutMetadata
@@ -430,11 +435,14 @@ export const generateArtifacts = async (
 
   // allIssues.totalPages = allIssues.totalPagesScanned;
 
+  // console.log(allIssues.items.mustFix);
+
   printMessage([
     'Scan Summary',
     '',
     `Must Fix: ${allIssues.items.mustFix.rules.length} issues / ${allIssues.items.mustFix.totalItems} occurrences`,
     `Good to Fix: ${allIssues.items.goodToFix.rules.length} issues / ${allIssues.items.goodToFix.totalItems} occurrences`,
+    `Needs Review: ${allIssues.items.needsReview.rules.length} issues / ${allIssues.items.needsReview.totalItems} occurrences`,
     `Passed: ${allIssues.items.passed.totalItems} occurrences`,
   ]);