Просмотр исходного кода

fix(releases): Use query parameter when switching filters (#52937)

Scott Cooper 1 год назад
Родитель
Сommit
25c3cdd8c8

+ 0 - 1
static/app/views/releases/detail/overview/index.tsx

@@ -544,7 +544,6 @@ class ReleaseOverview extends DeprecatedAsyncView<Props> {
 
                           <ReleaseIssues
                             organization={organization}
-                            selection={selection}
                             version={version}
                             location={location}
                             releaseBounds={releaseBounds}

+ 50 - 27
static/app/views/releases/detail/overview/releaseIssues.spec.jsx → static/app/views/releases/detail/overview/releaseIssues.spec.tsx

@@ -15,8 +15,7 @@ describe('ReleaseIssues', function () {
     orgId: 'org',
     organization: TestStubs.Organization(),
     version: '1.0.0',
-    selection: {projects: [], environments: [], datetime: {}},
-    location: {href: '', query: {}},
+    location: TestStubs.location({query: {}}),
     releaseBounds: getReleaseBounds(TestStubs.Release({version: '1.0.0'})),
   };
 
@@ -65,18 +64,28 @@ describe('ReleaseIssues', function () {
   });
 
   it('shows an empty state', async function () {
-    render(<ReleaseIssues {...props} />);
+    const {rerender} = render(<ReleaseIssues {...props} />);
 
     expect(await screen.findByText('No new issues in this release.')).toBeInTheDocument();
 
     await userEvent.click(screen.getByRole('radio', {name: 'Resolved 0'}));
+    // Simulate query change
+    rerender(
+      <ReleaseIssues
+        {...props}
+        location={TestStubs.location({query: {issuesType: 'resolved'}})}
+      />
+    );
     expect(
       await screen.findByText('No resolved issues in this release.')
     ).toBeInTheDocument();
   });
 
   it('shows an empty sttate with stats period', async function () {
-    render(<ReleaseIssues {...props} location={{query: {pageStatsPeriod: '24h'}}} />);
+    const query = {pageStatsPeriod: '24h'};
+    const {rerender} = render(
+      <ReleaseIssues {...props} location={TestStubs.location({query})} />
+    );
 
     expect(
       await screen.findByText(
@@ -85,6 +94,13 @@ describe('ReleaseIssues', function () {
     ).toBeInTheDocument();
 
     await userEvent.click(screen.getByRole('radio', {name: 'Unhandled 0'}));
+    // Simulate query change
+    rerender(
+      <ReleaseIssues
+        {...props}
+        location={TestStubs.location({query: {...query, issuesType: 'unhandled'}})}
+      />
+    );
     expect(
       await screen.findByText(
         textWithMarkupMatcher('No unhandled issues for the last 24 hours.')
@@ -92,54 +108,61 @@ describe('ReleaseIssues', function () {
     ).toBeInTheDocument();
   });
 
-  it('filters the issues', async function () {
-    render(<ReleaseIssues {...props} />);
-
-    expect(screen.getAllByRole('radio')).toHaveLength(5);
-    await screen.findByRole('radio', {name: 'New Issues 0'});
-
-    await userEvent.click(screen.getByRole('radio', {name: 'New Issues 0'}));
-    expect(newIssuesEndpoint).toHaveBeenCalledTimes(1);
-
-    await userEvent.click(screen.getByRole('radio', {name: 'Resolved 0'}));
-    expect(resolvedIssuesEndpoint).toHaveBeenCalledTimes(1);
-
-    await userEvent.click(screen.getByRole('radio', {name: 'Unhandled 0'}));
-    expect(unhandledIssuesEndpoint).toHaveBeenCalledTimes(1);
-
-    await userEvent.click(screen.getByRole('radio', {name: 'All Issues 0'}));
-    expect(allIssuesEndpoint).toHaveBeenCalledTimes(1);
-  });
-
-  it('renders link to Issues', async function () {
+  it('can switch issue filters', async function () {
     const {routerContext} = initializeOrg();
 
-    render(<ReleaseIssues {...props} />, {context: routerContext});
+    const {rerender} = render(<ReleaseIssues {...props} />, {context: routerContext});
 
+    // New
+    expect(await screen.findByRole('radio', {name: 'New Issues 0'})).toBeChecked();
     expect(screen.getByRole('button', {name: 'Open in Issues'})).toHaveAttribute(
       'href',
       '/organizations/org-slug/issues/?end=2020-03-24T02%3A04%3A59Z&groupStatsPeriod=auto&query=firstRelease%3A1.0.0&sort=freq&start=2020-03-23T01%3A02%3A00Z'
     );
+    expect(newIssuesEndpoint).toHaveBeenCalledTimes(1);
 
-    await screen.findByRole('radio', {name: 'Resolved 0'});
-
+    // Resolved
     await userEvent.click(screen.getByRole('radio', {name: 'Resolved 0'}));
+    // Simulate query change
+    rerender(
+      <ReleaseIssues
+        {...props}
+        location={TestStubs.location({query: {issuesType: 'resolved'}})}
+      />
+    );
     expect(screen.getByRole('button', {name: 'Open in Issues'})).toHaveAttribute(
       'href',
       '/organizations/org-slug/issues/?end=2020-03-24T02%3A04%3A59Z&groupStatsPeriod=auto&query=release%3A1.0.0&sort=freq&start=2020-03-23T01%3A02%3A00Z'
     );
+    expect(resolvedIssuesEndpoint).toHaveBeenCalledTimes(1);
 
+    // Unhandled
     await userEvent.click(screen.getByRole('radio', {name: 'Unhandled 0'}));
+    rerender(
+      <ReleaseIssues
+        {...props}
+        location={TestStubs.location({query: {issuesType: 'unhandled'}})}
+      />
+    );
     expect(screen.getByRole('button', {name: 'Open in Issues'})).toHaveAttribute(
       'href',
       '/organizations/org-slug/issues/?end=2020-03-24T02%3A04%3A59Z&groupStatsPeriod=auto&query=release%3A1.0.0%20error.handled%3A0&sort=freq&start=2020-03-23T01%3A02%3A00Z'
     );
+    expect(unhandledIssuesEndpoint).toHaveBeenCalledTimes(1);
 
+    // All
     await userEvent.click(screen.getByRole('radio', {name: 'All Issues 0'}));
+    rerender(
+      <ReleaseIssues
+        {...props}
+        location={TestStubs.location({query: {issuesType: 'all'}})}
+      />
+    );
     expect(screen.getByRole('button', {name: 'Open in Issues'})).toHaveAttribute(
       'href',
       '/organizations/org-slug/issues/?end=2020-03-24T02%3A04%3A59Z&groupStatsPeriod=auto&query=release%3A1.0.0&sort=freq&start=2020-03-23T01%3A02%3A00Z'
     );
+    expect(allIssuesEndpoint).toHaveBeenCalledTimes(1);
   });
 
   it('includes release context when linking to issue', async function () {

+ 37 - 63
static/app/views/releases/detail/overview/releaseIssues.tsx

@@ -16,7 +16,7 @@ import {SegmentedControl} from 'sentry/components/segmentedControl';
 import {DEFAULT_RELATIVE_PERIODS} from 'sentry/constants';
 import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {Organization, PageFilters} from 'sentry/types';
+import {Organization} from 'sentry/types';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import withApi from 'sentry/utils/withApi';
 import withOrganization from 'sentry/utils/withOrganization';
@@ -33,13 +33,13 @@ enum IssuesType {
   ALL = 'all',
 }
 
-enum IssuesQuery {
-  NEW = 'first-release',
-  UNHANDLED = 'error.handled:0',
-  REGRESSED = 'regressed_in_release',
-  RESOLVED = 'is:resolved',
-  ALL = 'release',
-}
+const issuesQuery: Record<IssuesType, string> = {
+  [IssuesType.NEW]: 'first-release',
+  [IssuesType.UNHANDLED]: 'error.handled:0',
+  [IssuesType.REGRESSED]: 'regressed_in_release',
+  [IssuesType.RESOLVED]: 'is:resolved',
+  [IssuesType.ALL]: 'release',
+};
 
 type IssuesQueryParams = {
   limit: number;
@@ -56,7 +56,6 @@ type Props = {
   location: Location;
   organization: Organization;
   releaseBounds: ReleaseBounds;
-  selection: PageFilters;
   version: string;
   queryFilterDescription?: string;
 } & Partial<typeof defaultProps>;
@@ -69,7 +68,6 @@ type State = {
     resolved: number | null;
     unhandled: number | null;
   };
-  issuesType: IssuesType;
   onCursor?: () => void;
   pageLinks?: string;
 };
@@ -79,24 +77,7 @@ class ReleaseIssues extends Component<Props, State> {
   state: State = this.getInitialState();
 
   getInitialState() {
-    const {location} = this.props;
-    const query = location.query ? location.query.issuesType : null;
-    const issuesTypeState = !query
-      ? IssuesType.NEW
-      : query.includes(IssuesType.NEW)
-      ? IssuesType.NEW
-      : query.includes(IssuesType.UNHANDLED)
-      ? IssuesType.REGRESSED
-      : query.includes(IssuesType.REGRESSED)
-      ? IssuesType.UNHANDLED
-      : query.includes(IssuesType.RESOLVED)
-      ? IssuesType.RESOLVED
-      : query.includes(IssuesType.ALL)
-      ? IssuesType.ALL
-      : IssuesType.ALL;
-
     return {
-      issuesType: issuesTypeState,
       count: {
         new: null,
         all: null,
@@ -128,9 +109,16 @@ class ReleaseIssues extends Component<Props, State> {
     }
   }
 
+  getActiveIssuesType(): IssuesType {
+    const query = (this.props.location.query?.issuesType as string) ?? '';
+    return Object.values<string>(IssuesType).includes(query)
+      ? (query as IssuesType)
+      : IssuesType.NEW;
+  }
+
   getIssuesUrl() {
     const {version, organization} = this.props;
-    const {issuesType} = this.state;
+    const issuesType = this.getActiveIssuesType();
     const {queryParams} = this.getIssuesEndpoint();
     const query = new MutableSearch([]);
 
@@ -164,7 +152,7 @@ class ReleaseIssues extends Component<Props, State> {
 
   getIssuesEndpoint(): {path: string; queryParams: IssuesQueryParams} {
     const {version, organization, location, releaseBounds} = this.props;
-    const {issuesType} = this.state;
+    const issuesType = this.getActiveIssuesType();
 
     const queryParams = {
       ...getReleaseParams({
@@ -183,7 +171,7 @@ class ReleaseIssues extends Component<Props, State> {
           queryParams: {
             ...queryParams,
             query: new MutableSearch([
-              `${IssuesQuery.ALL}:${version}`,
+              `${issuesQuery.all}:${version}`,
               'is:unresolved',
             ]).formatString(),
           },
@@ -201,8 +189,8 @@ class ReleaseIssues extends Component<Props, State> {
           queryParams: {
             ...queryParams,
             query: new MutableSearch([
-              `${IssuesQuery.ALL}:${version}`,
-              IssuesQuery.UNHANDLED,
+              `${issuesQuery.all}:${version}`,
+              issuesQuery.unhandled,
               'is:unresolved',
             ]).formatString(),
           },
@@ -213,7 +201,7 @@ class ReleaseIssues extends Component<Props, State> {
           queryParams: {
             ...queryParams,
             query: new MutableSearch([
-              `${IssuesQuery.REGRESSED}:${version}`,
+              `${issuesQuery.regressed}:${version}`,
             ]).formatString(),
           },
         };
@@ -224,7 +212,7 @@ class ReleaseIssues extends Component<Props, State> {
           queryParams: {
             ...queryParams,
             query: new MutableSearch([
-              `${IssuesQuery.NEW}:${version}`,
+              `${issuesQuery.new}:${version}`,
               'is:unresolved',
             ]).formatString(),
           },
@@ -246,14 +234,14 @@ class ReleaseIssues extends Component<Props, State> {
       ]).then(([issueResponse, resolvedResponse]) => {
         this.setState({
           count: {
-            all: issueResponse[`${IssuesQuery.ALL}:"${version}" is:unresolved`] || 0,
-            new: issueResponse[`${IssuesQuery.NEW}:"${version}" is:unresolved`] || 0,
+            all: issueResponse[`${issuesQuery.all}:"${version}" is:unresolved`] || 0,
+            new: issueResponse[`${issuesQuery.new}:"${version}" is:unresolved`] || 0,
             resolved: resolvedResponse.length,
             unhandled:
               issueResponse[
-                `${IssuesQuery.UNHANDLED} ${IssuesQuery.ALL}:"${version}" is:unresolved`
+                `${issuesQuery.unhandled} ${issuesQuery.all}:"${version}" is:unresolved`
               ] || 0,
-            regressed: issueResponse[`${IssuesQuery.REGRESSED}:"${version}"`] || 0,
+            regressed: issueResponse[`${issuesQuery.regressed}:"${version}"`] || 0,
           },
         });
       });
@@ -267,10 +255,10 @@ class ReleaseIssues extends Component<Props, State> {
     const issuesCountPath = `/organizations/${organization.slug}/issues-count/`;
 
     const params = [
-      `${IssuesQuery.NEW}:"${version}" is:unresolved`,
-      `${IssuesQuery.ALL}:"${version}" is:unresolved`,
-      `${IssuesQuery.UNHANDLED} ${IssuesQuery.ALL}:"${version}" is:unresolved`,
-      `${IssuesQuery.REGRESSED}:"${version}"`,
+      `${issuesQuery.new}:"${version}" is:unresolved`,
+      `${issuesQuery.all}:"${version}" is:unresolved`,
+      `${issuesQuery.unhandled} ${issuesQuery.all}:"${version}" is:unresolved`,
+      `${issuesQuery.regressed}:"${version}"`,
     ];
     const queryParams = params.map(param => param);
     const queryParameters = {
@@ -286,29 +274,14 @@ class ReleaseIssues extends Component<Props, State> {
 
   handleIssuesTypeSelection = (issuesType: IssuesType) => {
     const {location} = this.props;
-    const issuesTypeQuery =
-      issuesType === IssuesType.ALL
-        ? IssuesType.ALL
-        : issuesType === IssuesType.NEW
-        ? IssuesType.NEW
-        : issuesType === IssuesType.RESOLVED
-        ? IssuesType.RESOLVED
-        : issuesType === IssuesType.UNHANDLED
-        ? IssuesType.UNHANDLED
-        : issuesType === IssuesType.REGRESSED
-        ? IssuesType.REGRESSED
-        : '';
-
-    const to = {
+
+    browserHistory.replace({
       ...location,
       query: {
         ...location.query,
-        issuesType: issuesTypeQuery,
+        issuesType,
       },
-    };
-
-    browserHistory.replace(to);
-    this.setState({issuesType});
+    });
   };
 
   handleFetchSuccess = (groupListState, onCursor) => {
@@ -317,7 +290,7 @@ class ReleaseIssues extends Component<Props, State> {
 
   renderEmptyMessage = () => {
     const {location, releaseBounds} = this.props;
-    const {issuesType} = this.state;
+    const issuesType = this.getActiveIssuesType();
     const isEntireReleasePeriod =
       !location.query.pageStatsPeriod && !location.query.pageStart;
 
@@ -367,7 +340,8 @@ class ReleaseIssues extends Component<Props, State> {
   };
 
   render() {
-    const {issuesType, count, pageLinks, onCursor} = this.state;
+    const {count, pageLinks, onCursor} = this.state;
+    const issuesType = this.getActiveIssuesType();
     const {organization, queryFilterDescription, withChart, version} = this.props;
     const {path, queryParams} = this.getIssuesEndpoint();
     const issuesTypes = [