Bug 1609211 - Persist settings in URL (#6656)

SuyashSalampuria · web-flow · commit 030c67ac59ac · 2020-09-02T09:35:13.000-07:00
* persist group and order by changes in the URL
* Modify tests for modifying the way of changing orderBy or groupBy
* add a generic function to update state and params
* Persist changes in the metric in the URL
* persist the jobName in the URL
diff --git a/tests/ui/push-health/ClassificationGroup_test.jsx b/tests/ui/push-health/ClassificationGroup_test.jsx
@@ -1,5 +1,5 @@
 import React from 'react';
-import { render, waitFor, fireEvent } from '@testing-library/react';
+import { render, waitFor } from '@testing-library/react';
 
 import pushHealth from '../mock/push_health';
 import ClassificationGroup from '../../../ui/push-health/ClassificationGroup';
@@ -9,7 +9,7 @@ const tests = metrics.tests.details.needInvestigation;
 const repoName = 'autoland';
 
 describe('ClassificationGroup', () => {
-  const testClassificationGroup = (group) => (
+  const testClassificationGroup = (group, groupedBy) => (
     <ClassificationGroup
       jobs={jobs}
       tests={group}
@@ -20,26 +20,22 @@ describe('ClassificationGroup', () => {
       unfilteredLength={5}
       hasRetriggerAll
       notify={() => {}}
+      groupedBy={groupedBy}
     />
   );
 
   test('should group by test path', async () => {
-    const { getAllByTestId } = render(testClassificationGroup(tests));
+    const { getAllByTestId } = render(testClassificationGroup(tests, 'path'));
 
     expect(await waitFor(() => getAllByTestId('test-grouping'))).toHaveLength(
       3,
     );
   });
 
   test('should group by platform', async () => {
-    const { getAllByTestId, findByTestId, findByText } = render(
-      testClassificationGroup(tests),
+    const { getAllByTestId } = render(
+      testClassificationGroup(tests, 'platform'),
     );
-    const groupBy = await findByTestId('groupTestsDropdown');
-
-    fireEvent.click(groupBy);
-    const path = await findByText('Platform');
-    fireEvent.click(path);
 
     expect(await waitFor(() => getAllByTestId('test-grouping'))).toHaveLength(
       12,
diff --git a/tests/ui/push-health/PlatformConfig_test.jsx b/tests/ui/push-health/PlatformConfig_test.jsx
@@ -60,6 +60,7 @@ describe('PlatformConfig', () => {
       currentRepo={{ name: repoName, tc_root_url: 'http://foo.com' }}
       groupedBy="platform"
       notify={() => {}}
+      updateParamsAndState={() => {}}
     />
   );
 
diff --git a/tests/ui/push-health/TestMetric_test.jsx b/tests/ui/push-health/TestMetric_test.jsx
@@ -20,6 +20,7 @@ describe('TestMetric', () => {
       currentRepo={{ name: repoName, tc_root_url: 'http://foo.com' }}
       notify={() => {}}
       searchStr=""
+      updateParamsAndState={() => {}}
       showParentMatches={false}
     />
   );
diff --git a/ui/push-health/Action.jsx b/ui/push-health/Action.jsx
@@ -39,6 +39,11 @@ class Action extends PureComponent {
       currentRepo,
       notify,
       jobs,
+      testGroup,
+      selectedJobName,
+      selectedTest,
+      selectedTaskId,
+      updateParamsAndState,
     } = this.props;
     const groupedTests = this.getTestGroups(tests);
 
@@ -54,6 +59,11 @@ class Action extends PureComponent {
               currentRepo={currentRepo}
               notify={notify}
               jobs={jobs}
+              selectedJobName={selectedJobName}
+              selectedTest={selectedTest}
+              testGroup={testGroup}
+              selectedTaskId={selectedTaskId}
+              updateParamsAndState={updateParamsAndState}
             />
           </div>
         ))}
diff --git a/ui/push-health/ClassificationGroup.jsx b/ui/push-health/ClassificationGroup.jsx
@@ -33,12 +33,15 @@ class ClassificationGroup extends React.PureComponent {
     this.state = {
       detailsShowing: props.expanded,
       retriggerDropdownOpen: false,
-      groupedBy: 'path',
-      orderedBy: 'count',
     };
   }
 
   toggleDetails = () => {
+    const { updateParamsAndState, name } = this.props;
+
+    updateParamsAndState({
+      testGroup: name === 'Possible Regressions' ? 'pr' : 'ki',
+    });
     this.setState((prevState) => ({
       detailsShowing: !prevState.detailsShowing,
     }));
@@ -65,14 +68,6 @@ class ClassificationGroup extends React.PureComponent {
     JobModel.retrigger(uniqueJobs, currentRepo, notify, times);
   };
 
-  setGroupedBy = (groupedBy) => {
-    this.setState({ groupedBy });
-  };
-
-  setOrderedBy = (orderedBy) => {
-    this.setState({ orderedBy });
-  };
-
   getTestsByAction = (tests) => {
     const { log, crash, test } = groupBy(tests, 'action');
 
@@ -83,12 +78,7 @@ class ClassificationGroup extends React.PureComponent {
   };
 
   render() {
-    const {
-      detailsShowing,
-      retriggerDropdownOpen,
-      groupedBy,
-      orderedBy,
-    } = this.state;
+    const { detailsShowing, retriggerDropdownOpen } = this.state;
     const {
       jobs,
       tests,
@@ -100,6 +90,15 @@ class ClassificationGroup extends React.PureComponent {
       currentRepo,
       icon,
       iconColor,
+      groupedBy,
+      orderedBy,
+      testGroup,
+      selectedTest,
+      selectedJobName,
+      selectedTaskId,
+      setGroupedBy,
+      setOrderedBy,
+      updateParamsAndState,
     } = this.props;
     const expandIcon = detailsShowing ? faCaretDown : faCaretRight;
     const expandTitle = detailsShowing
@@ -187,21 +186,23 @@ class ClassificationGroup extends React.PureComponent {
                     <DropdownItem
                       className="pointable"
                       tag="a"
-                      onClick={() => this.setGroupedBy('none')}
+                      onClick={() => setGroupedBy('none')}
                     >
                       None
                     </DropdownItem>
                     <DropdownItem
                       className="pointable"
                       tag="a"
-                      onClick={() => this.setGroupedBy('path')}
+                      onClick={() => {
+                        setGroupedBy('path');
+                      }}
                     >
                       Path
                     </DropdownItem>
                     <DropdownItem
                       className="pointable"
                       tag="a"
-                      onClick={() => this.setGroupedBy('platform')}
+                      onClick={() => setGroupedBy('platform')}
                     >
                       Platform
                     </DropdownItem>
@@ -222,14 +223,18 @@ class ClassificationGroup extends React.PureComponent {
                     <DropdownItem
                       className="pointable"
                       tag="a"
-                      onClick={() => this.setOrderedBy('count')}
+                      onClick={() => {
+                        setOrderedBy('count');
+                      }}
                     >
                       Count
                     </DropdownItem>
                     <DropdownItem
                       className="pointable"
                       tag="a"
-                      onClick={() => this.setOrderedBy('text')}
+                      onClick={() => {
+                        setOrderedBy('text');
+                      }}
                     >
                       Text
                     </DropdownItem>
@@ -251,6 +256,11 @@ class ClassificationGroup extends React.PureComponent {
               notify={notify}
               key={key}
               jobs={jobs}
+              testGroup={testGroup}
+              selectedTest={selectedTest}
+              selectedJobName={selectedJobName}
+              selectedTaskId={selectedTaskId}
+              updateParamsAndState={updateParamsAndState}
             />
           ))}
         </Collapse>
@@ -269,13 +279,21 @@ ClassificationGroup.propTypes = {
   expanded: PropTypes.bool,
   className: PropTypes.string,
   iconColor: PropTypes.string,
+  orderedBy: PropTypes.string,
+  groupedBy: PropTypes.string,
+  setOrderedBy: PropTypes.func,
+  setGroupedBy: PropTypes.func,
 };
 
 ClassificationGroup.defaultProps = {
   expanded: true,
   className: '',
   iconColor: 'darker-info',
   hasRetriggerAll: false,
+  orderedBy: 'count',
+  groupedBy: 'path',
+  setOrderedBy: () => {},
+  setGroupedBy: () => {},
 };
 
 export default ClassificationGroup;
diff --git a/ui/push-health/Health.jsx b/ui/push-health/Health.jsx
@@ -54,18 +54,29 @@ export default class Health extends React.PureComponent {
       notifications: [],
       defaultTabIndex: 0,
       showParentMatches: false,
+      testGroup: params.get('testGroup') || '',
+      selectedTest: params.get('selectedTest') || '',
+      selectedTaskId: params.get('selectedTaskId') || '',
+      selectedJobName: params.get('selectedJobName') || '',
       searchStr: params.get('searchStr') || '',
+      regressionsOrderBy: params.get('regressionsOrderBy') || 'count',
+      regressionsGroupBy: params.get('regressionsGroupBy') || 'path',
+      knownIssuesOrderBy: params.get('knownIssuesOrderBy') || 'count',
+      knownIssuesGroupBy: params.get('knownIssuesGroupBy') || 'path',
     };
   }
 
   async componentDidMount() {
-    const { repo } = this.state;
+    const { repo, testGroup } = this.state;
     const {
       metrics: { linting, builds, tests },
     } = await this.updatePushHealth();
-    const defaultTabIndex = [linting, builds, tests].findIndex(
+    let defaultTabIndex = [linting, builds, tests].findIndex(
       (metric) => metric.result === 'fail',
     );
+    if (testGroup) {
+      defaultTabIndex = 2;
+    }
     const repos = await RepositoryModel.getList();
     const currentRepo = repos.find((repoObj) => repoObj.name === repo);
 
@@ -88,6 +99,18 @@ export default class Health extends React.PureComponent {
     this.setState({ user });
   };
 
+  updateParamsAndState = (stateObj) => {
+    const { location, history } = this.props;
+    const newParams = {
+      ...parseQueryParams(location.search),
+      ...stateObj,
+    };
+    const queryString = createQueryParams(newParams);
+
+    updateQueryParams(queryString, history, location);
+    this.setState(stateObj);
+  };
+
   updatePushHealth = async () => {
     const { repo, revision } = this.state;
     const { data, failureStatus } = await PushModel.getHealth(repo, revision);
@@ -174,7 +197,15 @@ export default class Health extends React.PureComponent {
       searchStr,
       currentRepo,
       showParentMatches,
+      testGroup,
+      selectedTest,
       defaultTabIndex,
+      selectedTaskId,
+      selectedJobName,
+      regressionsOrderBy,
+      regressionsGroupBy,
+      knownIssuesOrderBy,
+      knownIssuesGroupBy,
     } = this.state;
     const { tests, commitHistory, linting, builds } = metrics;
     const needInvestigationCount = tests
@@ -305,7 +336,16 @@ export default class Health extends React.PureComponent {
                       notify={this.notify}
                       setExpanded={this.setExpanded}
                       searchStr={searchStr}
+                      testGroup={testGroup}
+                      selectedTest={selectedTest}
                       showParentMatches={showParentMatches}
+                      regressionsOrderBy={regressionsOrderBy}
+                      regressionsGroupBy={regressionsGroupBy}
+                      knownIssuesOrderBy={knownIssuesOrderBy}
+                      knownIssuesGroupBy={knownIssuesGroupBy}
+                      selectedTaskId={selectedTaskId}
+                      selectedJobName={selectedJobName}
+                      updateParamsAndState={this.updateParamsAndState}
                     />
                   </TabPanel>
                 </div>
diff --git a/ui/push-health/PlatformConfig.jsx b/ui/push-health/PlatformConfig.jsx
@@ -22,12 +22,39 @@ class PlatformConfig extends React.PureComponent {
     };
   }
 
+  componentDidMount() {
+    const {
+      selectedJobName,
+      selectedTaskId,
+      jobs,
+      failure: { jobName, testName },
+    } = this.props;
+
+    this.setState({
+      detailsShowing: selectedJobName === `${testName} ${jobName}`,
+      selectedTask: selectedTaskId
+        ? jobs[jobName].filter((job) => job.id === parseInt(selectedTaskId, 10))
+            .length > 0 &&
+          jobs[jobName].filter(
+            (job) => job.id === parseInt(selectedTaskId, 10),
+          )[0]
+        : null,
+    });
+  }
+
   setSelectedTask = (task) => {
     const { selectedTask } = this.state;
+    const {
+      failure: { jobName, testName },
+    } = this.props;
 
     if (selectedTask === task || !task) {
       this.setState({ selectedTask: null, detailsShowing: false });
     } else {
+      this.props.updateParamsAndState({
+        selectedTaskId: task.id,
+        selectedJobName: `${testName} ${jobName}`,
+      });
       this.setState({ selectedTask: task, detailsShowing: true });
     }
   };
@@ -140,6 +167,7 @@ PlatformConfig.propTypes = {
   currentRepo: PropTypes.shape({}).isRequired,
   notify: PropTypes.func.isRequired,
   groupedBy: PropTypes.string.isRequired,
+  updateParamsAndState: PropTypes.func.isRequired,
 };
 
 export default PlatformConfig;
diff --git a/ui/push-health/Test.jsx b/ui/push-health/Test.jsx
diff --git a/ui/push-health/TestMetric.jsx b/ui/push-health/TestMetric.jsx
