From 70270e3d83bf1eca6ea8f74cef4fcba104b1e783 Mon Sep 17 00:00:00 2001 From: Stalgia Grigg Date: Thu, 20 Jun 2024 12:27:55 -0700 Subject: [PATCH] fix: use correct test number sequence for raising github issues (#1128) * Pass sequence number to createIssueLink * Calculate sequence number in test plan report * Use sequence number for issue search * Jsdoc comment for createIssueLink and getIssueLink utility methods * Update client/components/TestRun/index.jsx Co-authored-by: Howard Edwards --------- Co-authored-by: Howard Edwards --- .../CandidateTestPlanRun/index.jsx | 3 +- .../Reports/SummarizeTestPlanReport.jsx | 1 + client/components/TestRun/index.jsx | 1 + client/utils/createIssueLink.js | 50 +++++++++++++++++-- 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/client/components/CandidateReview/CandidateTestPlanRun/index.jsx b/client/components/CandidateReview/CandidateTestPlanRun/index.jsx index 85a1ae709..10be4e4bd 100644 --- a/client/components/CandidateReview/CandidateTestPlanRun/index.jsx +++ b/client/components/CandidateReview/CandidateTestPlanRun/index.jsx @@ -317,6 +317,7 @@ const CandidateTestPlanRun = () => { versionString: testPlanVersion.versionString, testTitle: currentTest.title, testRowNumber: currentTest.rowNumber, + testSequenceNumber: currentTest.seq, testRenderedUrl: currentTest.renderedUrl, atName: testPlanReport.at.name }; @@ -464,7 +465,7 @@ const CandidateTestPlanRun = () => { testPlanTitle: testPlanVersion.title, versionString: testPlanVersion.versionString, - testRowNumber: currentTest.rowNumber + testSequenceNumber: currentTest.seq })} /> ); diff --git a/client/components/Reports/SummarizeTestPlanReport.jsx b/client/components/Reports/SummarizeTestPlanReport.jsx index e67bcc326..c06e63a1d 100644 --- a/client/components/Reports/SummarizeTestPlanReport.jsx +++ b/client/components/Reports/SummarizeTestPlanReport.jsx @@ -186,6 +186,7 @@ const SummarizeTestPlanReport = ({ testPlanVersion, testPlanReports }) => { versionString: testPlanVersion.versionString, testTitle: test.title, testRowNumber: test.rowNumber, + testSequenceNumber: index + 1, testRenderedUrl: test.renderedUrl, atName: testPlanReport.at.name, atVersionName: testResult.atVersion.name, diff --git a/client/components/TestRun/index.jsx b/client/components/TestRun/index.jsx index 0f44ed085..f950d1b38 100644 --- a/client/components/TestRun/index.jsx +++ b/client/components/TestRun/index.jsx @@ -368,6 +368,7 @@ const TestRun = () => { )}`, testTitle: currentTest.title, testRowNumber: currentTest.rowNumber, + testSequenceNumber: currentTest.seq, testRenderedUrl: currentTest.renderedUrl, atName: testPlanReport.at.name, browserName: testPlanReport.browser.name, diff --git a/client/utils/createIssueLink.js b/client/utils/createIssueLink.js index 253d17517..4085b398f 100644 --- a/client/utils/createIssueLink.js +++ b/client/utils/createIssueLink.js @@ -9,6 +9,28 @@ const atLabelMap = { NVDA: 'nvda' }; +/** + * Creates a link to open a new issue on the GitHub repository. + * + * @param {Object} options - Options for creating the issue link + * @param {boolean} [options.isCandidateReview=false] - Whether this is a candidate review + * @param {boolean} [options.isCandidateReviewChangesRequested=false] - Whether changes are requested for a candidate review + * @param {string} [options.testPlanDirectory] - The directory of the test plan + * @param {string} [options.testPlanTitle] - The title of the test plan + * @param {string} [options.versionString] - The version string + * @param {string|null} [options.testTitle=null] - The title of the test + * @param {number|null} [options.testSequenceNumber=null] - The sequence number of the test. This is the number displayed to test runners + * @param {number|null} [options.testRowNumber=null] - The row number of the test in aria-at + * @param {string|null} [options.testRenderedUrl=null] - The rendered URL of the test + * @param {string} options.atName - The name of the assistive technology + * @param {string|null} [options.atVersionName=null] - The version name of the assistive technology + * @param {string|null} [options.browserName=null] - The name of the browser + * @param {string|null} [options.browserVersionName=null] - The version name of the browser + * @param {string|null} [options.conflictMarkdown=null] - The conflict markdown + * @param {string|null} [options.reportLink=null] - The link to the report + * @returns {string} The URL for creating a new issue on the GitHub repository + * @throws {Error} If required parameters are missing + */ const createIssueLink = ({ isCandidateReview = false, isCandidateReviewChangesRequested = false, @@ -16,6 +38,7 @@ const createIssueLink = ({ testPlanTitle, versionString, testTitle = null, + testSequenceNumber = null, testRowNumber = null, testRenderedUrl = null, atName, @@ -29,7 +52,12 @@ const createIssueLink = ({ throw new Error('Cannot create issue link due to missing parameters'); } - const hasTest = !!(testTitle && testRowNumber && testRenderedUrl); + const hasTest = !!( + testTitle && + testSequenceNumber && + testRowNumber && + testRenderedUrl + ); let title; if (hasTest) { @@ -44,7 +72,7 @@ const createIssueLink = ({ title = `${titleStart}: "${testTitle}" (${testPlanTitle}, ` + - `Test ${testRowNumber}, ${versionString})`; + `Test ${testSequenceNumber}, ${versionString})`; } else { title = `${atName} General Feedback: ${testPlanTitle} ${versionString}`; } @@ -99,6 +127,7 @@ const createIssueLink = ({ atName, browserName, testRowNumber, + testSequenceNumber, isCandidateReview, isCandidateReviewChangesRequested }); @@ -121,6 +150,19 @@ const createIssueLink = ({ ); }; +/** + * Returns a link to search for existing issues on the GitHub repository based on the provided parameters. + * + * @param {Object} options - Options for generating the issue search link + * @param {boolean} [options.isCandidateReview=false] - Whether this is a candidate review + * @param {boolean} [options.isCandidateReviewChangesRequested=false] - Whether changes are requested for a candidate review + * @param {string|null} [options.username=null] - The username of the author + * @param {string} options.atName - The name of the assistive technology + * @param {string} options.testPlanTitle - The title of the test plan + * @param {string} options.versionString - The version string + * @param {number|null} [options.testSequenceNumber=null] - The sequence number of the test, this is the test number displayed to test runners + * @returns {string} The URL for searching issues on the GitHub repository + */ export const getIssueSearchLink = ({ isCandidateReview = false, isCandidateReviewChangesRequested = false, @@ -128,7 +170,7 @@ export const getIssueSearchLink = ({ atName, testPlanTitle, versionString, - testRowNumber = null + testSequenceNumber = null }) => { let atKey; if (atName === 'JAWS' || atName === 'NVDA') { @@ -146,7 +188,7 @@ export const getIssueSearchLink = ({ username ? `author:${username}` : '', `label:${atKey}`, `"${testPlanTitle}"`, - testRowNumber ? `Test ${testRowNumber}` : '', + testSequenceNumber ? `Test ${testSequenceNumber}` : '', versionString ] .filter(str => str)