feat: Enhanced conflict review (#1195)

Addresses #975. Introduces a new dedicated "Conflicts" page that allows a user to compare the changes between all testers while going through the review process.
w3c · Sep 17, 2024 · fbae626 · fbae626
1 parent afede8d
commit fbae626
Show file tree

Hide file tree

Showing 17 changed files with 1,513 additions and 16 deletions.
diff --git a/client/components/TestPlanReportStatusDialog/index.jsx b/client/components/TestPlanReportStatusDialog/index.jsx
@@ -60,6 +60,7 @@ const TestPlanReportStatusDialog = ({
           <ReportStatusSummary
             testPlanVersion={testPlanVersion}
             testPlanReport={testPlanReport}
+            me={me}
           />
           {isSignedIn && isAdmin && !testPlanReport ? (
             <AddTestToQueueWithConfirmation

diff --git a/client/components/TestQueue/Conflicts/AssertionConflictsTable.jsx b/client/components/TestQueue/Conflicts/AssertionConflictsTable.jsx
@@ -0,0 +1,62 @@
+import React from 'react';
+import { ConflictTable } from './ConflictSummaryTable';
+import { UserPropType } from '../../common/proptypes';
+import PropTypes from 'prop-types';
+
+const AssertionConflictsTable = ({ conflictingResults, testers }) => {
+  const commandString = scenario => {
+    return `Assertions for 'After ${scenario.commands
+      .map(command => command.text)
+      .join(' then ')}'`;
+  };
+
+  const allAssertions =
+    conflictingResults[0].scenarioResult.assertionResults.map(
+      ar => ar.assertion.text
+    );
+
+  return (
+    <>
+      <h3 style={{ marginBottom: '1rem' }}>
+        {commandString(conflictingResults[0].scenario)}
+      </h3>
+
+      <ConflictTable bordered responsive>
+        <thead>
+          <tr>
+            <th>Assertion</th>
+            {testers.map(tester => (
+              <th key={tester.username}>{tester.username}</th>
+            ))}
+          </tr>
+        </thead>
+        <tbody>
+          {allAssertions.map((assertion, index) => {
+            const results = conflictingResults.map(
+              cr => cr.scenarioResult.assertionResults[index].passed
+            );
+            const hasConflict = results.some(r => r !== results[0]);
+            if (!hasConflict) {
+              return null;
+            }
+            return (
+              <tr key={index}>
+                <td>{assertion}</td>
+                {results.map((result, i) => (
+                  <td key={i}>{result ? 'Passed' : 'Failed'}</td>
+                ))}
+              </tr>
+            );
+          })}
+        </tbody>
+      </ConflictTable>
+    </>
+  );
+};
+
+AssertionConflictsTable.propTypes = {
+  conflictingResults: PropTypes.arrayOf(PropTypes.object).isRequired,
+  testers: PropTypes.arrayOf(UserPropType).isRequired
+};
+
+export default AssertionConflictsTable;
diff --git a/client/components/TestQueue/Conflicts/ConflictIssueDetails.jsx b/client/components/TestQueue/Conflicts/ConflictIssueDetails.jsx
@@ -0,0 +1,118 @@
+import React from 'react';
+import styled from '@emotion/styled';
+import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
+import {
+  faExclamationCircle,
+  faExternalLinkAlt
+} from '@fortawesome/free-solid-svg-icons';
+import PropTypes from 'prop-types';
+import { IssuePropType } from '../../common/proptypes';
+import { dates } from 'shared';
+
+const IssuesContainer = styled.div`
+  margin-bottom: 2rem;
+`;
+
+const IssueContainer = styled.div`
+  background-color: #f8f9fa;
+  border: 1px solid #e9ecef;
+  border-radius: 4px;
+  padding: 1rem;
+  margin: 1rem 0;
+`;
+
+const IssueHeader = styled.div`
+  display: flex;
+  align-items: center;
+  margin-bottom: 0.5rem;
+`;
+
+const IssueTitle = styled.h4`
+  margin: 0;
+  margin-left: 0.5rem;
+  flex-grow: 1;
+`;
+
+const IssueGrid = styled.div`
+  display: grid;
+  grid-template-columns: auto 1fr;
+  gap: 0.5rem 1rem;
+`;
+
+const IssueLabel = styled.span`
+  font-weight: bold;
+`;
+
+const IssueValue = styled.span`
+  word-break: break-word;
+`;
+
+const IssueLink = styled.a`
+  color: #0366d6;
+  text-decoration: none;
+  display: inline-flex;
+  align-items: center;
+  margin-top: 0.5rem;
+  &:hover {
+    text-decoration: underline;
+  }
+`;
+
+const ConflictIssueDetails = ({ issues }) => {
+  if (!issues || issues.length === 0) return null;
+
+  return (
+    <IssuesContainer>
+      <h3>Related GitHub Issues</h3>
+      {issues.map((issue, index) => (
+        <IssueContainer key={index}>
+          <IssueHeader>
+            <FontAwesomeIcon
+              icon={faExclamationCircle}
+              style={{ color: issue.isOpen ? '#28a745' : '#6a737d' }}
+            />
+            <IssueTitle>{issue.title}</IssueTitle>
+          </IssueHeader>
+          <IssueGrid>
+            <IssueLabel>Status:</IssueLabel>
+            <IssueValue>{issue.isOpen ? 'Open' : 'Closed'}</IssueValue>
+            <IssueLabel>Author:</IssueLabel>
+            <IssueValue>{issue.author}</IssueValue>
+            <IssueLabel>Type:</IssueLabel>
+            <IssueValue>{issue.feedbackType}</IssueValue>
+            <IssueLabel>Created:</IssueLabel>
+            <IssueValue>
+              {dates.convertDateToString(issue.createdAt)}
+            </IssueValue>
+            {issue.closedAt && (
+              <>
+                <IssueLabel>Closed:</IssueLabel>
+                <IssueValue>
+                  {dates.convertDateToString(issue.closedAt)}
+                </IssueValue>
+              </>
+            )}
+          </IssueGrid>
+          <IssueLink
+            href={issue.link}
+            target="_blank"
+            rel="noopener noreferrer"
+          >
+            View on GitHub&nbsp;
+            <FontAwesomeIcon
+              icon={faExternalLinkAlt}
+              size="sm"
+              style={{ marginLeft: '0.25rem' }}
+            />
+          </IssueLink>
+        </IssueContainer>
+      ))}
+    </IssuesContainer>
+  );
+};
+
+ConflictIssueDetails.propTypes = {
+  issues: PropTypes.arrayOf(IssuePropType).isRequired
+};
+
+export default ConflictIssueDetails;
diff --git a/client/components/TestQueue/Conflicts/ConflictSummaryTable.jsx b/client/components/TestQueue/Conflicts/ConflictSummaryTable.jsx
@@ -0,0 +1,101 @@
+import React, { useMemo } from 'react';
+import PropTypes from 'prop-types';
+import styled from '@emotion/styled';
+import { ThemeTable } from '../../common/ThemeTable';
+import { IssuePropType } from '../../common/proptypes';
+import AssertionConflictsTable from './AssertionConflictsTable';
+import UnexpectedBehaviorsConflictsTable from './UnexpectedBehaviorsConflictsTable';
+
+export const ConflictTable = styled(ThemeTable)`
+  th,
+  td {
+    text-align: left;
+    padding: 0.75rem;
+  }
+  margin-bottom: 2rem;
+`;
+
+const ConflictSummaryTable = ({ conflictingResults }) => {
+  const commandString = scenario => {
+    return `Output for 'After ${scenario.commands
+      .map(command => command.text)
+      .join(' then ')}'`;
+  };
+
+  const testers = useMemo(
+    () => conflictingResults.map(result => result.testPlanRun.tester),
+    [conflictingResults]
+  );
+
+  const hasAssertionConflicts = useMemo(
+    () =>
+      conflictingResults[0].scenarioResult.assertionResults.some((ar, index) =>
+        conflictingResults.some(
+          cr => cr.scenarioResult.assertionResults[index].passed !== ar.passed
+        )
+      ),
+    [conflictingResults]
+  );
+
+  const hasUnexpectedBehaviorConflicts = useMemo(
+    () =>
+      conflictingResults.some(
+        result => result.scenarioResult.unexpectedBehaviors.length > 0
+      ),
+    [conflictingResults]
+  );
+
+  return (
+    <>
+      <h3 style={{ marginBottom: '1rem' }}>
+        {commandString(conflictingResults[0].scenario)}
+      </h3>
+
+      <ConflictTable bordered responsive>
+        <thead>
+          <tr>
+            <th>Tester</th>
+            <th>Output</th>
+          </tr>
+        </thead>
+        <tbody>
+          {testers.map(tester => (
+            <tr key={tester.username}>
+              <td>{tester.username}</td>
+              <td>
+                {
+                  conflictingResults.find(
+                    cr => cr.testPlanRun.tester.id === tester.id
+                  ).scenarioResult.output
+                }
+              </td>
+            </tr>
+          ))}
+        </tbody>
+      </ConflictTable>
+
+      {hasAssertionConflicts && (
+        <AssertionConflictsTable
+          conflictingResults={conflictingResults}
+          testers={testers}
+        />
+      )}
+      {hasUnexpectedBehaviorConflicts && (
+        <UnexpectedBehaviorsConflictsTable
+          conflictingResults={conflictingResults}
+          testers={testers}
+        />
+      )}
+    </>
+  );
+};
+
+ConflictSummaryTable.propTypes = {
+  conflictingResults: PropTypes.arrayOf(PropTypes.object).isRequired,
+  issues: PropTypes.arrayOf(IssuePropType),
+  issueLink: PropTypes.string.isRequired,
+  isAdmin: PropTypes.bool.isRequired,
+  testIndex: PropTypes.number.isRequired
+};
+
+export default ConflictSummaryTable;
diff --git a/client/components/TestQueue/Conflicts/TestConflictsActions.jsx b/client/components/TestQueue/Conflicts/TestConflictsActions.jsx
@@ -0,0 +1,68 @@
+import React from 'react';
+import styled from '@emotion/styled';
+import { Button, Dropdown } from 'react-bootstrap';
+import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
+import {
+  faExclamationCircle,
+  faFileImport
+} from '@fortawesome/free-solid-svg-icons';
+import PropTypes from 'prop-types';
+import { TestPlanRunPropType } from '../../common/proptypes';
+
+const ActionContainer = styled.div`
+  display: flex;
+  gap: 1rem;
+  margin-top: 2rem;
+  max-width: 500px;
+  & > * {
+    flex-grow: 1;
+    flex-basis: 0;
+    min-width: 0;
+  }
+`;
+
+const ActionButton = styled(Button)`
+  flex-grow: 1;
+  flex-basis: 0;
+  min-width: 0;
+  width: 100%;
+  margin: 0;
+`;
+
+const TestConflictsActions = ({ issueLink, isAdmin, testPlanRuns }) => {
+  return (
+    <ActionContainer>
+      <Button variant="secondary" target="_blank" href={issueLink}>
+        <FontAwesomeIcon icon={faExclamationCircle} />
+        Raise an Issue for Conflict
+      </Button>
+      {isAdmin && (
+        <Dropdown>
+          <Dropdown.Toggle variant="secondary" as={ActionButton}>
+            <FontAwesomeIcon icon={faFileImport} />
+            Open run as...
+          </Dropdown.Toggle>
+          <Dropdown.Menu>
+            {testPlanRuns.map(testPlanRun => (
+              <Dropdown.Item
+                key={testPlanRun.id}
+                href={`/run/${testPlanRun.id}?user=${testPlanRun.tester.id}`}
+              >
+                {testPlanRun.tester.username}
+              </Dropdown.Item>
+            ))}
+          </Dropdown.Menu>
+        </Dropdown>
+      )}
+    </ActionContainer>
+  );
+};
+
+TestConflictsActions.propTypes = {
+  issueLink: PropTypes.string.isRequired,
+  isAdmin: PropTypes.bool.isRequired,
+  testPlanRuns: PropTypes.arrayOf(TestPlanRunPropType).isRequired,
+  testIndex: PropTypes.number.isRequired
+};
+
+export default TestConflictsActions;