Test result submission UX #1350

client/components/TestRun/index.jsx

@@ -1,3 +1,6 @@
+// Enabling GitHub actions on fork
+// Triggering GitHub Actions
+
 import React, { useCallback, useEffect, useRef, useState } from 'react';
 import { Helmet } from 'react-helmet';
 import { Link, useNavigate, useParams } from 'react-router-dom';
@@ -104,6 +107,7 @@ const TestRun = () => {
   const [showReviewConflictsModal, setShowReviewConflictsModal] =
     useState(false);
   const [showGetInvolvedModal, setShowGetInvolvedModal] = useState(false);
+  const [showConfirmNextModal, setShowConfirmNextModal] = useState(false);
 
   // Modal State Values
   const [isShowingAtBrowserModal, setIsShowingAtBrowserModal] = useState(true);
@@ -505,51 +509,52 @@ const TestRun = () => {
     return { ...testResult, scenarioResults };
   };
 
-  const performButtonAction = async (action, index) => {
-    // TODO: Revise function
-    const saveForm = async (
-      withResult = false,
-      forceSave = false,
-      forceEdit = false
-    ) => {
-      if (updateMessageComponent) {
-        setUpdateMessageComponent(null);
-      }
-      try {
-        if (forceEdit) setIsTestEditClicked(true);
-        else setIsTestEditClicked(false);
-
-        if (!isSignedIn) return true;
-        if (!forceEdit && currentTest.testResult?.completedAt) return true;
-
-        setIsSavingForm(true);
-        const scenarioResults = remapScenarioResults(
-          testRunStateRef.current || recentTestRunStateRef.current,
-          currentTest.testResult?.scenarioResults,
-          false
-        );
-
-        await handleSaveOrSubmitTestResultAction(
-          {
-            atVersionId: currentTestAtVersionId,
-            browserVersionId: currentTestBrowserVersionId,
-            scenarioResults
-          },
-          forceSave ? false : !!testRunResultRef.current
-        );
+  const saveForm = async (
+    withResult = false,
+    forceSave = false,
+    forceEdit = false
+  ) => {
+    if (updateMessageComponent) {
+      setUpdateMessageComponent(null);
+    }
+    try {
+      if (forceEdit) setIsTestEditClicked(true);
+      else setIsTestEditClicked(false);
+
+      if (!isSignedIn) return true;
+      if (!forceEdit && currentTest.testResult?.completedAt) return true;
+
+      setIsSavingForm(true);
+      const scenarioResults = remapScenarioResults(
+        testRunStateRef.current || recentTestRunStateRef.current,
+        currentTest.testResult?.scenarioResults,
+        false
+      );
 
-        if (withResult && !forceSave) {
-          setIsSavingForm(false);
-          return !!testRunResultRef.current;
-        }
+      await handleSaveOrSubmitTestResultAction(
+        {
+          atVersionId: currentTestAtVersionId,
+          browserVersionId: currentTestBrowserVersionId,
+          scenarioResults
+        },
+        forceSave ? false : !!testRunResultRef.current
+      );
 
+      if (withResult && !forceSave) {
         setIsSavingForm(false);
-        return true;
-      } catch (e) {
-        console.error('save.error', e);
-        setIsSavingForm(false);
+        return !!testRunResultRef.current;
       }
-    };
+
+      setIsSavingForm(false);
+      return true;
+    } catch (e) {
+      console.error('save.error', e);
+      setIsSavingForm(false);
+    }
+  };
+
+  const performButtonAction = async (action, index) => {
+    // TODO: Revise function
 
     switch (action) {
       case 'goToTestAtIndex': {
@@ -558,12 +563,6 @@ const TestRun = () => {
         setCurrentTestIndex(index);
         break;
       }
-      case 'goToNextTest': {
-        // Save renderer's form state
-        await saveForm(false, true);
-        navigateTests(false, currentTest, tests, setCurrentTestIndex);
-        break;
-      }
       case 'goToPreviousTest': {
         // Save renderer's form state
         await saveForm(false, true);
@@ -607,7 +606,10 @@ const TestRun = () => {
 
   const handleSaveClick = async () => performButtonAction('saveTest');
 
-  const handleNextTestClick = async () => performButtonAction('goToNextTest');
+  const handleNextTestClick = async () => {
+    setShowConfirmNextModal(false);
+    navigateTests(false, currentTest, tests, setCurrentTestIndex);
+  };
 
   const handlePreviousTestClick = async () =>
     performButtonAction('goToPreviousTest');
@@ -925,7 +927,13 @@ const TestRun = () => {
     let forwardButtons = []; // These are buttons that navigate to next tests and continue
 
     const nextButton = (
-      <Button variant="secondary" onClick={handleNextTestClick}>
+      <Button
+        variant="secondary"
+        onClick={async () => {
+          await saveForm(false, true); // Save before modal
+          setShowConfirmNextModal(true); // Then show modal
+        }}
+      >
         Next Test
       </Button>
     );
@@ -981,7 +989,7 @@ const TestRun = () => {
         </Button>
       );
       if (!isLastTest) forwardButtons = [nextButton];
-      primaryButtons = [previousButton, ...forwardButtons, saveResultsButton];
+      primaryButtons = [previousButton, saveResultsButton, ...forwardButtons];
     }
 
     const externalLogsUrl = testPlanRun?.collectionJob?.externalLogsUrl;
@@ -1337,6 +1345,24 @@ const TestRun = () => {
               handleClose={handleAtAndBrowserDetailsModalCloseAction}
             />
           )}
+
+          <BasicModal
+            show={showConfirmNextModal}
+            centered={true}
+            animation={false}
+            title="Proceed to Next Test?"
+            content="Are you sure you want to go to the next test? This action will NOT save the results of the current test before proceeding."
+            actions={[
+              {
+                label: 'Yes',
+                variant: 'primary',
+                onClick: () => handleNextTestClick()
+              }
+            ]}
+            closeLabel="No"
+            handleClose={() => setShowConfirmNextModal(false)}
+            closeButton={false}
+          />
         </Container>
       </CollectionJobContextProvider>
     )

client/tests/e2e/TestRun.e2e.test.js

@@ -1,3 +1,5 @@
+// Enabling GitHub actions on fork
+
 import getPage from '../util/getPage';
 import { text } from './util';
 
@@ -260,7 +262,6 @@ describe('Test Run when signed in as tester', () => {
 
       // Click each navigation item and confirm the h1 on page has changed
       for (let sequence = 1; sequence <= listItemsLength; sequence++) {
-        // const sequence = i + 1;
         const liSelector = `nav#test-navigator-nav ol li:nth-child(${sequence})`;
 
         // Select the next test to navigate to
@@ -360,8 +361,11 @@ describe('Test Run when signed in as tester', () => {
 
       // Navigate to test 3 with next button
       await page.click(nextTestButtonSelector);
+      await page.waitForSelector('button ::-p-text(Yes)');
+      await page.click('button ::-p-text(Yes)');
       await page.waitForNetworkIdle();
       await page.waitForSelector('h1 ::-p-text(Test 3:)');
+
       await page.waitForSelector('button ::-p-text(Next Test)');
       const test3CheckedCount = await page.$$eval(
         radioSelector,
@@ -372,7 +376,9 @@ describe('Test Run when signed in as tester', () => {
       await page.click(previousTestButtonSelector);
       await page.waitForNetworkIdle();
       await page.waitForSelector('h1 ::-p-text(Test 2:)');
+
       await page.waitForSelector('button ::-p-text(Next Test)');
+
       const test2CheckedCount = await page.$$eval(
         radioSelector,
         els => els.filter(radio => radio.checked).length
@@ -383,6 +389,7 @@ describe('Test Run when signed in as tester', () => {
       await page.waitForNetworkIdle();
       await page.waitForSelector('h1 ::-p-text(Test 1:)');
       await page.waitForSelector('button ::-p-text(Next Test)');
+
       const test1CheckedCount = await page.$$eval(
         radioSelector,
         els =>

client/tests/e2e/snapshots/saved/_test-plan-report_1.html

@@ -1495,13 +1495,13 @@ <h2 id="test-toolbar-heading" class="sr-only">
                             </button>
                           </li>
                           <li>
-                            <button type="button" class="btn btn-secondary">
-                              Next Test
+                            <button type="button" class="btn btn-primary">
+                              Submit Results
                             </button>
                           </li>
                           <li>
-                            <button type="button" class="btn btn-primary">
-                              Submit Results
+                            <button type="button" class="btn btn-secondary">
+                              Next Test
                             </button>
                           </li>
                         </ul>