Skip to content

Commit

Permalink
Bug 1609211 - Persist settings in URL (#6656)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
SuyashSalampuria authored Sep 2, 2020
1 parent 5e6bc09 commit 030c67a
Show file tree
Hide file tree
Showing 9 changed files with 214 additions and 37 deletions.
16 changes: 6 additions & 10 deletions tests/ui/push-health/ClassificationGroup_test.jsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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}
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/push-health/PlatformConfig_test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ describe('PlatformConfig', () => {
currentRepo={{ name: repoName, tc_root_url: 'http://foo.com' }}
groupedBy="platform"
notify={() => {}}
updateParamsAndState={() => {}}
/>
);

Expand Down
1 change: 1 addition & 0 deletions tests/ui/push-health/TestMetric_test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('TestMetric', () => {
currentRepo={{ name: repoName, tc_root_url: 'http://foo.com' }}
notify={() => {}}
searchStr=""
updateParamsAndState={() => {}}
showParentMatches={false}
/>
);
Expand Down
10 changes: 10 additions & 0 deletions ui/push-health/Action.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ class Action extends PureComponent {
currentRepo,
notify,
jobs,
testGroup,
selectedJobName,
selectedTest,
selectedTaskId,
updateParamsAndState,
} = this.props;
const groupedTests = this.getTestGroups(tests);

Expand All @@ -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>
))}
Expand Down
60 changes: 39 additions & 21 deletions ui/push-health/ClassificationGroup.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}));
Expand All @@ -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');

Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand All @@ -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;
44 changes: 42 additions & 2 deletions ui/push-health/Health.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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>
Expand Down
28 changes: 28 additions & 0 deletions ui/push-health/PlatformConfig.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
};
Expand Down Expand Up @@ -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;
Loading

0 comments on commit 030c67a

Please sign in to comment.