Skip to content

Commit

Permalink
feat: Allows testers' results to be publicly viewable (#1209)
Browse files Browse the repository at this point in the history
* Add 'View Results for...' to Test Queue's Actions column

* Make elements of the TestRun + TestRenderer experience 'readonly' when a user is not signed in but viewing a user's run

* Update language when viewing while not admin

* Update props

* Update copy; update how read only mode is evaluated

* Update snapshots

* Fix whitespace

* Update snapshots

* Update tests

* Use handler for unexpected behaviors radio buttons

* Simplify the actions column dropdown for opening the results of a tester (as admin or otherwise)

* Additional differentiation for the status of the test results to a non admin / signed out user through the test navigator and heading message

* Update snapshots

* Remove unused attribute
  • Loading branch information
howard-e authored Sep 12, 2024
1 parent 08b2a91 commit afede8d
Show file tree
Hide file tree
Showing 15 changed files with 241 additions and 113 deletions.
44 changes: 22 additions & 22 deletions client/components/TestQueue/Actions.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ const Actions = ({
);
};

const openResultsLabel = isAdmin ? 'Open run as...' : 'View Results for...';

return (
<LoadingStatus message={loadingMessage}>
<ActionContainer>
Expand All @@ -248,28 +250,26 @@ const Actions = ({
: 'Start Testing'}
</Button>
)}
{isAdmin && (
<Dropdown focusFirstItemOnShow>
<Dropdown.Toggle
variant="secondary"
disabled={!nonSelfAssignedRuns.length}
>
<FontAwesomeIcon icon={faFileImport} />
Open run as...&nbsp;
</Dropdown.Toggle>
<Dropdown.Menu role="menu">
{nonSelfAssignedRuns.map(testPlanRun => (
<Dropdown.Item
key={testPlanRun.id}
role="menuitem"
href={`/run/${testPlanRun.id}?user=${testPlanRun.tester.id}`}
>
{testPlanRun.tester.username}
</Dropdown.Item>
))}
</Dropdown.Menu>
</Dropdown>
)}
<Dropdown focusFirstItemOnShow>
<Dropdown.Toggle
variant="secondary"
disabled={!nonSelfAssignedRuns.length}
>
<FontAwesomeIcon icon={faFileImport} />
{openResultsLabel}
</Dropdown.Toggle>
<Dropdown.Menu role="menu">
{nonSelfAssignedRuns.map(testPlanRun => (
<Dropdown.Item
key={testPlanRun.id}
role="menuitem"
href={`/run/${testPlanRun.id}?user=${testPlanRun.tester.id}`}
>
{testPlanRun.tester.username}
</Dropdown.Item>
))}
</Dropdown.Menu>
</Dropdown>
{isAdmin && assignedBotRun && (
<ManageBotRunDialogWithButton
testPlanRun={assignedBotRun}
Expand Down
14 changes: 10 additions & 4 deletions client/components/TestRenderer/AssertionsFieldset/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ const Label = styled.label`
}
`;

const AssertionsFieldset = ({ assertions, commandIndex, assertionsHeader }) => {
const AssertionsFieldset = ({
assertions,
commandIndex,
assertionsHeader,
readOnly = false
}) => {
// Handle case where build process didn't include assertionResponseQuestion
const normalizedHeader = useMemo(() => {
return assertionsHeader?.descriptionHeader?.replace(
Expand All @@ -39,7 +44,7 @@ const AssertionsFieldset = ({ assertions, commandIndex, assertionsHeader }) => {
id={`pass-${commandIndex}-${assertionIndex}-yes`}
name={`assertion-${commandIndex}-${assertionIndex}`}
checked={passed === true}
onChange={() => click(true)}
onChange={() => (!readOnly ? click(true) : false)}
data-testid={`radio-yes-${commandIndex}-${assertionIndex}`}
/>
Yes
Expand All @@ -50,7 +55,7 @@ const AssertionsFieldset = ({ assertions, commandIndex, assertionsHeader }) => {
id={`pass-${commandIndex}-${assertionIndex}-no`}
name={`assertion-${commandIndex}-${assertionIndex}`}
checked={passed === false}
onChange={() => click(false)}
onChange={() => (!readOnly ? click(false) : false)}
data-testid={`radio-no-${commandIndex}-${assertionIndex}`}
/>
No
Expand All @@ -65,7 +70,8 @@ const AssertionsFieldset = ({ assertions, commandIndex, assertionsHeader }) => {
AssertionsFieldset.propTypes = {
assertions: PropTypes.array.isRequired,
commandIndex: PropTypes.number.isRequired,
assertionsHeader: PropTypes.object.isRequired
assertionsHeader: PropTypes.object.isRequired,
readOnly: PropTypes.bool
};

export default AssertionsFieldset;
4 changes: 3 additions & 1 deletion client/components/TestRenderer/OutputTextArea/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ const OutputTextArea = ({
</label>
<NoOutputCheckbox
checked={noOutput}
disabled={atOutput.value && atOutput.value !== NO_OUTPUT_STRING}
disabled={
(atOutput.value && atOutput.value !== NO_OUTPUT_STRING) || readOnly
}
label="No output"
id={`no-output-checkbox-${commandIndex}`}
type="checkbox"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,20 @@ const Label = styled.label`
const UnexpectedBehaviorsFieldset = ({
commandIndex,
unexpectedBehaviors,
isSubmitted
isSubmitted,
readOnly = false
}) => {
const impactOptions = ['Moderate', 'Severe'];

const handleUnexpectedBehaviorsExistRadioClick = e => {
if (readOnly) e.preventDefault();
else {
const elId = e.target.id;
if (elId.includes('-true')) unexpectedBehaviors.passChoice.click();
else if (elId.includes('-false')) unexpectedBehaviors.failChoice.click();
}
};

return (
<Fieldset id={`cmd-${commandIndex}-problems`}>
<legend>{unexpectedBehaviors.description[0]}</legend>
Expand All @@ -67,7 +77,7 @@ const UnexpectedBehaviorsFieldset = ({
name={`problem-${commandIndex}`}
autoFocus={isSubmitted && unexpectedBehaviors.passChoice.focus}
defaultChecked={unexpectedBehaviors.passChoice.checked}
onClick={unexpectedBehaviors.passChoice.click}
onClick={handleUnexpectedBehaviorsExistRadioClick}
/>
<label
id={`problem-${commandIndex}-true-label`}
Expand All @@ -84,7 +94,7 @@ const UnexpectedBehaviorsFieldset = ({
name={`problem-${commandIndex}`}
autoFocus={isSubmitted && unexpectedBehaviors.failChoice.focus}
defaultChecked={unexpectedBehaviors.failChoice.checked}
onClick={unexpectedBehaviors.failChoice.click}
onClick={handleUnexpectedBehaviorsExistRadioClick}
/>
<label
id={`problem-${commandIndex}-false-label`}
Expand Down Expand Up @@ -129,7 +139,10 @@ const UnexpectedBehaviorsFieldset = ({
className={`undesirable-${commandIndex}`}
autoFocus={isSubmitted && focus}
checked={checked}
onChange={e => change(e.target.checked)}
onChange={e => {
if (readOnly) e.preventDefault();
else change(e.target.checked);
}}
/>
{description} behavior occurred
</Label>
Expand All @@ -150,6 +163,9 @@ const UnexpectedBehaviorsFieldset = ({
<option
key={`${descriptionId}-${commandIndex}-impact-${option}`}
value={option.toUpperCase()}
disabled={
readOnly ? option.toUpperCase() !== impact : false
}
>
{option}
</option>
Expand All @@ -173,6 +189,7 @@ const UnexpectedBehaviorsFieldset = ({
value={more.value}
onChange={e => more.change(e.target.value)}
disabled={!checked}
readOnly={readOnly}
/>
{isSubmitted && (
<Feedback
Expand Down Expand Up @@ -202,7 +219,8 @@ const UnexpectedBehaviorsFieldset = ({
UnexpectedBehaviorsFieldset.propTypes = {
commandIndex: PropTypes.number.isRequired,
unexpectedBehaviors: PropTypes.object.isRequired,
isSubmitted: PropTypes.bool
isSubmitted: PropTypes.bool,
readOnly: PropTypes.bool
};

export default UnexpectedBehaviorsFieldset;
6 changes: 5 additions & 1 deletion client/components/TestRenderer/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ const TestRenderer = ({
submitButtonRef,
isSubmitted = false,
isReviewingBot = false,
isReadOnly = false,
isEdit = false,
setIsRendererReady = false
}) => {
Expand Down Expand Up @@ -540,17 +541,19 @@ const TestRenderer = ({
commandIndex={commandIndex}
atOutput={atOutput}
isSubmitted={isSubmitted}
readOnly={isReviewingBot}
readOnly={isReviewingBot || isReadOnly}
/>
<AssertionsFieldset
assertions={assertions}
commandIndex={commandIndex}
assertionsHeader={assertionsHeader}
readOnly={isReadOnly}
/>
<UnexpectedBehaviorsFieldset
commandIndex={commandIndex}
unexpectedBehaviors={unexpectedBehaviors}
isSubmitted={isSubmitted}
readOnly={isReadOnly}
/>
</Fragment>
);
Expand Down Expand Up @@ -589,6 +592,7 @@ TestRenderer.propTypes = {
testRunResultRef: PropTypes.any,
submitButtonRef: PropTypes.any,
isSubmitted: PropTypes.bool,
isReadOnly: PropTypes.bool,
isEdit: PropTypes.bool,
isReviewingBot: PropTypes.bool,
setIsRendererReady: PropTypes.func
Expand Down
42 changes: 33 additions & 9 deletions client/components/TestRun/Heading.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
COLLECTION_JOB_STATUS,
isJobStatusFinal
} from '../../utils/collectionJobStatus';
import { TestResultPropType } from '../common/proptypes';

const TestRunHeading = ({
at,
Expand All @@ -24,7 +25,9 @@ const TestRunHeading = ({
showEditAtBrowser,
testPlanTitle,
testResults,
testCount
testIndex,
testCount,
isReadOnly
}) => {
const {
state: { collectionJob }
Expand Down Expand Up @@ -56,7 +59,7 @@ const TestRunHeading = ({
countTestResults,
countCompleteCollection
)} of ${testCount}`}</b>{' '}
responses collected.
responses collected
</p>
<p>
Collection Job Status: <b>{collectionJob.status}</b>
Expand All @@ -68,7 +71,6 @@ const TestRunHeading = ({
} else if (testCount) {
content = (
<>
{' '}
<b>{`${testResults.reduce(
(acc, { completedAt }) => acc + (completedAt ? 1 : 0),
0
Expand All @@ -95,22 +97,42 @@ const TestRunHeading = ({
if (openAsUser?.isBot) {
openAsUserHeading = (
<div className="test-info-entity reviewing-as bot">
Reviewing tests of <FontAwesomeIcon icon={faRobot} className="m-0" />{' '}
{isReadOnly ? 'Viewing' : 'Reviewing'} tests of{' '}
<FontAwesomeIcon icon={faRobot} className="m-0" />{' '}
<b>{`${openAsUser.username}`}.</b>
{!isJobStatusFinal(collectionJob.status) && (
<>
<br />
The collection bot is still updating information on this page.
Changes may be lost when updates arrive.
{isReadOnly ? '' : ' Changes may be lost when updates arrive.'}
</>
)}
</div>
);
} else if (openAsUser) {
let readOnlyStatus;
if (isReadOnly) {
const test = testResults[testIndex];
if (!test) readOnlyStatus = 'unopened';
else if (test.completedAt) readOnlyStatus = 'completed';
else if (test.startedAt) readOnlyStatus = 'in progress';
}

openAsUserHeading = (
<div className="test-info-entity reviewing-as">
Reviewing tests of <b>{`${openAsUser.username}`}.</b>
<p>{`All changes will be saved as performed by ${openAsUser.username}.`}</p>
{isReadOnly ? (
<>
Viewing {readOnlyStatus} tests of <b>{openAsUser.username}</b> in
read-only mode. <em>No changes can be made or saved.</em>
</>
) : (
<>
Reviewing tests of <b>{openAsUser.username}</b>.{' '}
<em>
All changes will be saved as performed by {openAsUser.username}.
</em>
</>
)}
</div>
);
}
Expand Down Expand Up @@ -164,9 +186,11 @@ TestRunHeading.propTypes = {
isBot: PropTypes.bool.isRequired,
username: PropTypes.string.isRequired
}),
testResults: PropTypes.arrayOf(PropTypes.shape({})),
testResults: PropTypes.arrayOf(TestResultPropType),
testIndex: PropTypes.number.isRequired,
testCount: PropTypes.number.isRequired,
handleEditAtBrowserDetailsClick: PropTypes.func.isRequired
handleEditAtBrowserDetailsClick: PropTypes.func.isRequired,
isReadOnly: PropTypes.bool
};

export default TestRunHeading;
15 changes: 10 additions & 5 deletions client/components/TestRun/TestNavigator.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ const TestNavigator = ({
currentTestIndex = 0,
toggleShowClick = () => {},
handleTestClick = () => {},
testPlanRun = null
testPlanRun = null,
isReadOnly = false
}) => {
const isBotCompletedTest = testPlanRun?.tester?.isBot;

Expand Down Expand Up @@ -61,8 +62,8 @@ const TestNavigator = ({
className="test-navigator-list"
>
{tests.map((test, index) => {
let resultClassName = 'not-started';
let resultStatus = 'Not Started';
let resultClassName = isReadOnly ? 'missing' : 'not-started';
let resultStatus = isReadOnly ? 'Missing' : 'Not Started';

const issuesExist = testPlanReport.issues?.filter(
issue =>
Expand Down Expand Up @@ -95,6 +96,9 @@ const TestNavigator = ({
if (test.hasConflicts) {
resultClassName = 'conflicts';
resultStatus = 'Has Conflicts';
} else if (!test.testResult && isReadOnly) {
resultClassName = 'missing';
resultStatus = 'Missing';
} else if (test.testResult) {
resultClassName = test.testResult.completedAt
? 'complete'
Expand All @@ -108,7 +112,7 @@ const TestNavigator = ({
test.index === currentTestIndex
) {
resultClassName = 'in-progress';
resultStatus = 'In Progress:';
resultStatus = 'In Progress';
} else if (isVendor) {
if (issuesExist) {
resultClassName = 'changes-requested';
Expand Down Expand Up @@ -161,7 +165,8 @@ TestNavigator.propTypes = {
viewedTests: PropTypes.array,
toggleShowClick: PropTypes.func,
handleTestClick: PropTypes.func,
testPlanRun: PropTypes.object
testPlanRun: PropTypes.object,
isReadOnly: PropTypes.bool
};

export default TestNavigator;
Loading

0 comments on commit afede8d

Please sign in to comment.