Browse Source

feat(release-comparison): Add issue filter tabs to release details (#26826)

* feat(release-comparison): Add issue filter tabs to release details

Add issue filter tabs to release details for Release Comparison feature.

FIXES WOR-968

* refactor, use different endpoints, fix styles

* fix query for count for unhandled

* refactor

* refactor

* fix: double refetch, correct counts, race condition

* style(lint): Auto commit lint changes

* update tests

* refactor, add new color aliases for light/dark mode

* refactor, include 0 in button count

* null count init

* fix ts count type

* nullable queryCount

Co-authored-by: Matej Minar <matej.minar@sentry.io>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Kelly Carino 3 years ago
parent
commit
c15ab4f248

+ 1 - 0
static/app/components/buttonBar.tsx

@@ -125,4 +125,5 @@ const ButtonGrid = styled('div')<{gap: ValidSize; merged: boolean}>`
   ${p => p.merged && MergedStyles}
 `;
 
+export {ButtonGrid, MergedStyles};
 export default ButtonBar;

+ 8 - 1
static/app/components/issues/groupList.tsx

@@ -2,6 +2,7 @@ import * as React from 'react';
 import {browserHistory, withRouter, WithRouterProps} from 'react-router';
 import * as Sentry from '@sentry/react';
 import isEqual from 'lodash/isEqual';
+import omit from 'lodash/omit';
 import * as qs from 'query-string';
 
 import {fetchOrgMembers, indexMembersByProject} from 'app/actionCreators/members';
@@ -85,11 +86,16 @@ class GroupList extends React.Component<Props, State> {
   }
 
   componentDidUpdate(prevProps: Props) {
+    const ignoredQueryParams = ['end'];
+
     if (
       prevProps.orgId !== this.props.orgId ||
       prevProps.endpointPath !== this.props.endpointPath ||
       prevProps.query !== this.props.query ||
-      !isEqual(prevProps.queryParams, this.props.queryParams)
+      !isEqual(
+        omit(prevProps.queryParams, ignoredQueryParams),
+        omit(this.props.queryParams, ignoredQueryParams)
+      )
     ) {
       this.fetchData();
     }
@@ -106,6 +112,7 @@ class GroupList extends React.Component<Props, State> {
   fetchData = () => {
     GroupStore.loadInitialData([]);
     const {api, orgId} = this.props;
+    api.clear();
 
     this.setState({loading: true, error: false});
 

+ 1 - 1
static/app/components/queryCount.tsx

@@ -1,7 +1,7 @@
 import {defined} from 'app/utils';
 
 type Props = {
-  count?: number;
+  count?: number | null;
   max?: number;
   hideIfEmpty?: boolean;
   hideParens?: boolean;

+ 12 - 0
static/app/utils/theme.tsx

@@ -235,6 +235,16 @@ const lightAliases = {
    * Search filter "token" border
    */
   searchTokenBorder: '#B5DAFF',
+
+  /**
+   * Count on button when active
+   */
+  buttonCountActive: colors.gray100,
+
+  /**
+   * Count on button
+   */
+  buttonCount: colors.gray400,
 };
 
 const dataCategory = {
@@ -639,6 +649,8 @@ const darkAliases = {
   badgeText: colors.black,
   searchTokenBackground: '#1F1A3D',
   searchTokenBorder: '#554E80',
+  buttonCountActive: colors.gray100,
+  buttonCount: colors.gray400,
 };
 
 export const lightTheme = {

+ 246 - 49
static/app/views/releases/detail/overview/issues.tsx

@@ -1,20 +1,27 @@
 import {Component, Fragment} from 'react';
+import {browserHistory} from 'react-router';
 import styled from '@emotion/styled';
 import {Location} from 'history';
+import isEqual from 'lodash/isEqual';
+import * as qs from 'query-string';
 
+import {Client} from 'app/api';
 import GuideAnchor from 'app/components/assistant/guideAnchor';
-import Button from 'app/components/button';
-import ButtonBar from 'app/components/buttonBar';
+import Button, {ButtonLabel} from 'app/components/button';
+import ButtonBar, {ButtonGrid} from 'app/components/buttonBar';
 import DiscoverButton from 'app/components/discoverButton';
 import DropdownButton from 'app/components/dropdownButton';
 import DropdownControl, {DropdownItem} from 'app/components/dropdownControl';
 import GroupList from 'app/components/issues/groupList';
 import Pagination from 'app/components/pagination';
+import QueryCount from 'app/components/queryCount';
 import {DEFAULT_RELATIVE_PERIODS} from 'app/constants';
 import {t, tct} from 'app/locale';
 import space from 'app/styles/space';
 import {GlobalSelection, Organization} from 'app/types';
 import {QueryResults} from 'app/utils/tokenizeSearch';
+import withApi from 'app/utils/withApi';
+import withOrganization from 'app/utils/withOrganization';
 import {IssueSortOptions} from 'app/views/issueList/utils';
 
 import {getReleaseParams, ReleaseBounds} from '../../utils';
@@ -29,6 +36,13 @@ enum IssuesType {
   ALL = 'all',
 }
 
+enum IssuesQuery {
+  NEW = 'first-release',
+  UNHANDLED = 'error.handled:0',
+  RESOLVED = 'is:resolved',
+  ALL = 'release',
+}
+
 type IssuesQueryParams = {
   limit: number;
   sort: string;
@@ -40,6 +54,7 @@ const defaultProps = {
 };
 
 type Props = {
+  api: Client;
   organization: Organization;
   version: string;
   selection: GlobalSelection;
@@ -51,16 +66,72 @@ type Props = {
 
 type State = {
   issuesType: IssuesType;
+  count: {
+    new: number | null;
+    unhandled: number | null;
+    resolved: number | null;
+    all: number | null;
+  };
   pageLinks?: string;
   onCursor?: () => void;
 };
 
 class Issues extends Component<Props, State> {
   static defaultProps = defaultProps;
+  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.UNHANDLED
+      : query.includes(IssuesType.RESOLVED)
+      ? IssuesType.RESOLVED
+      : query.includes(IssuesType.ALL)
+      ? IssuesType.ALL
+      : IssuesType.ALL;
 
-  state: State = {
-    issuesType: IssuesType.NEW,
-  };
+    return {
+      issuesType: issuesTypeState,
+      count: {
+        new: null,
+        all: null,
+        resolved: null,
+        unhandled: null,
+      },
+    };
+  }
+
+  componentDidMount() {
+    this.fetchIssuesCount();
+  }
+
+  componentDidUpdate(prevProps: Props) {
+    if (
+      !isEqual(
+        getReleaseParams({
+          location: this.props.location,
+          releaseBounds: this.props.releaseBounds,
+          defaultStatsPeriod: this.props.defaultStatsPeriod,
+          allowEmptyPeriod:
+            this.props.organization.features.includes('release-comparison'),
+        }),
+        getReleaseParams({
+          location: prevProps.location,
+          releaseBounds: prevProps.releaseBounds,
+          defaultStatsPeriod: prevProps.defaultStatsPeriod,
+          allowEmptyPeriod:
+            prevProps.organization.features.includes('release-comparison'),
+        })
+      )
+    ) {
+      this.fetchIssuesCount();
+    }
+  }
 
   getDiscoverUrl() {
     const {version, organization, selection} = this.props;
@@ -123,7 +194,7 @@ class Issues extends Component<Props, State> {
           path: `/organizations/${organization.slug}/issues/`,
           queryParams: {
             ...queryParams,
-            query: new QueryResults([`release:${version}`]).formatString(),
+            query: new QueryResults([`${IssuesQuery.ALL}:${version}`]).formatString(),
           },
         };
       case IssuesType.RESOLVED:
@@ -137,8 +208,8 @@ class Issues extends Component<Props, State> {
           queryParams: {
             ...queryParams,
             query: new QueryResults([
-              `release:${version}`,
-              'error.handled:0',
+              `${IssuesQuery.ALL}:${version}`,
+              IssuesQuery.UNHANDLED,
             ]).formatString(),
           },
         };
@@ -148,13 +219,84 @@ class Issues extends Component<Props, State> {
           path: `/organizations/${organization.slug}/issues/`,
           queryParams: {
             ...queryParams,
-            query: new QueryResults([`first-release:${version}`]).formatString(),
+            query: new QueryResults([`${IssuesQuery.NEW}:${version}`]).formatString(),
           },
         };
     }
   }
 
+  async fetchIssuesCount() {
+    const {api, organization, version} = this.props;
+    const issueCountEndpoint = this.getIssueCountEndpoint();
+    const resolvedEndpoint = `/organizations/${organization.slug}/releases/${version}/resolved/`;
+
+    try {
+      await Promise.all([
+        api.requestPromise(issueCountEndpoint),
+        api.requestPromise(resolvedEndpoint),
+      ]).then(([issueResponse, resolvedResponse]) => {
+        this.setState({
+          count: {
+            all: issueResponse[`${IssuesQuery.ALL}:${version}`] || 0,
+            new: issueResponse[`${IssuesQuery.NEW}:${version}`] || 0,
+            resolved: resolvedResponse.length,
+            unhandled:
+              issueResponse[`${IssuesQuery.UNHANDLED} ${IssuesQuery.ALL}:${version}`] ||
+              0,
+          },
+        });
+      });
+    } catch {
+      // do nothing
+    }
+  }
+
+  getIssueCountEndpoint() {
+    const {organization, version, location, releaseBounds, defaultStatsPeriod} =
+      this.props;
+    const issuesCountPath = `/organizations/${organization.slug}/issues-count/`;
+
+    const params = [
+      `${IssuesQuery.NEW}:${version}`,
+      `${IssuesQuery.ALL}:${version}`,
+      `${IssuesQuery.UNHANDLED} ${IssuesQuery.ALL}:${version}`,
+    ];
+    const queryParams = params.map(param => param);
+    const queryParameters = {
+      ...getReleaseParams({
+        location,
+        releaseBounds,
+        defaultStatsPeriod,
+        allowEmptyPeriod: organization.features.includes('release-comparison'),
+      }),
+      query: queryParams,
+    };
+
+    return `${issuesCountPath}?${qs.stringify(queryParameters)}`;
+  }
+
   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
+        : '';
+
+    const to = {
+      ...location,
+      query: {
+        ...location.query,
+        issuesType: issuesTypeQuery,
+      },
+    };
+
+    browserHistory.replace(to);
     this.setState({issuesType});
   };
 
@@ -193,59 +335,93 @@ class Issues extends Component<Props, State> {
   };
 
   render() {
-    const {issuesType, pageLinks, onCursor} = this.state;
+    const {issuesType, count, pageLinks, onCursor} = this.state;
     const {organization, queryFilterDescription, withChart} = this.props;
     const {path, queryParams} = this.getIssuesEndpoint();
+    const hasReleaseComparison = organization.features.includes('release-comparison');
     const issuesTypes = [
-      {value: IssuesType.NEW, label: t('New Issues')},
-      {value: IssuesType.RESOLVED, label: t('Resolved Issues')},
-      {value: IssuesType.UNHANDLED, label: t('Unhandled Issues')},
-      {value: IssuesType.ALL, label: t('All Issues')},
+      {value: IssuesType.NEW, label: t('New Issues'), issueCount: count.new},
+      {
+        value: IssuesType.RESOLVED,
+        label: t('Resolved Issues'),
+        issueCount: count.resolved,
+      },
+      {
+        value: IssuesType.UNHANDLED,
+        label: t('Unhandled Issues'),
+        issueCount: count.unhandled,
+      },
+      {value: IssuesType.ALL, label: t('All Issues'), issueCount: count.all},
     ];
 
     return (
       <Fragment>
         <ControlsWrapper>
-          <DropdownControl
-            button={({isOpen, getActorProps}) => (
-              <StyledDropdownButton
-                {...getActorProps()}
-                isOpen={isOpen}
-                prefix={t('Filter')}
-                size="small"
-              >
-                {issuesTypes.find(i => i.value === issuesType)?.label}
-              </StyledDropdownButton>
-            )}
-          >
-            {issuesTypes.map(({value, label}) => (
-              <StyledDropdownItem
-                key={value}
-                onSelect={this.handleIssuesTypeSelection}
-                data-test-id={`filter-${value}`}
-                eventKey={value}
-                isActive={value === issuesType}
-              >
-                {label}
-              </StyledDropdownItem>
-            ))}
-          </DropdownControl>
+          {hasReleaseComparison ? (
+            <StyledButtonBar active={issuesType} merged>
+              {issuesTypes.map(({value, label, issueCount}) => (
+                <Button
+                  key={value}
+                  barId={value}
+                  size="small"
+                  onClick={() => this.handleIssuesTypeSelection(value)}
+                >
+                  {label}
+                  <QueryCount
+                    count={issueCount}
+                    max={99}
+                    hideParens
+                    hideIfEmpty={false}
+                  />
+                </Button>
+              ))}
+            </StyledButtonBar>
+          ) : (
+            <DropdownControl
+              button={({isOpen, getActorProps}) => (
+                <StyledDropdownButton
+                  {...getActorProps()}
+                  isOpen={isOpen}
+                  prefix={t('Filter')}
+                  size="small"
+                >
+                  {issuesTypes.find(i => i.value === issuesType)?.label}
+                </StyledDropdownButton>
+              )}
+            >
+              {issuesTypes.map(({value, label}) => (
+                <StyledDropdownItem
+                  key={value}
+                  onSelect={this.handleIssuesTypeSelection}
+                  data-test-id={`filter-${value}`}
+                  eventKey={value}
+                  isActive={value === issuesType}
+                >
+                  {label}
+                </StyledDropdownItem>
+              ))}
+            </DropdownControl>
+          )}
 
           <OpenInButtonBar gap={1}>
             <Button to={this.getIssuesUrl()} size="small" data-test-id="issues-button">
               {t('Open in Issues')}
             </Button>
 
-            <GuideAnchor target="release_issues_open_in_discover">
-              <DiscoverButton
-                to={this.getDiscoverUrl()}
-                size="small"
-                data-test-id="discover-button"
-              >
-                {t('Open in Discover')}
-              </DiscoverButton>
-            </GuideAnchor>
-            <StyledPagination pageLinks={pageLinks} onCursor={onCursor} />
+            {!hasReleaseComparison && (
+              <GuideAnchor target="release_issues_open_in_discover">
+                <DiscoverButton
+                  to={this.getDiscoverUrl()}
+                  size="small"
+                  data-test-id="discover-button"
+                >
+                  {t('Open in Discover')}
+                </DiscoverButton>
+              </GuideAnchor>
+            )}
+            {!hasReleaseComparison && (
+              <StyledPagination pageLinks={pageLinks} onCursor={onCursor} />
+            )}
           </OpenInButtonBar>
         </ControlsWrapper>
         <div data-test-id="release-wrapper">
@@ -275,6 +451,9 @@ const ControlsWrapper = styled('div')`
   margin-bottom: ${space(1)};
   @media (max-width: ${p => p.theme.breakpoints[0]}) {
     display: block;
+    ${ButtonGrid} {
+      overflow: auto;
+    }
   }
 `;
 
@@ -284,6 +463,24 @@ const OpenInButtonBar = styled(ButtonBar)`
   }
 `;
 
+const StyledButtonBar = styled(ButtonBar)`
+  grid-template-columns: repeat(4, 1fr);
+  ${ButtonLabel} {
+    white-space: nowrap;
+    grid-gap: ${space(0.5)};
+    span:last-child {
+      color: ${p => p.theme.buttonCount};
+    }
+  }
+  .active {
+    ${ButtonLabel} {
+      span:last-child {
+        color: ${p => p.theme.buttonCountActive};
+      }
+    }
+  }
+`;
+
 const StyledDropdownButton = styled(DropdownButton)`
   min-width: 145px;
 `;
@@ -296,4 +493,4 @@ const StyledPagination = styled(Pagination)`
   margin: 0;
 `;
 
-export default Issues;
+export default withApi(withOrganization(Issues));

+ 8 - 0
tests/js/spec/views/releases/detail/overview/issues.spec.jsx

@@ -10,6 +10,7 @@ describe('Release Issues', function () {
     allIssuesEndpoint;
 
   const props = {
+    orgId: 'org',
     organization: TestStubs.Organization(),
     version: '1.0.0',
     selection: {projects: [], environments: [], datetime: {period: '14d'}},
@@ -24,6 +25,13 @@ describe('Release Issues', function () {
       body: [],
     });
 
+    MockApiClient.addMockResponse({
+      url: `/organizations/${props.organization.slug}/issues-count/?query=first-release%3A1.0.0&query=release%3A1.0.0&query=error.handled%3A0%20release%3A1.0.0&statsPeriod=14d`,
+    });
+    MockApiClient.addMockResponse({
+      url: `/organizations/${props.organization.slug}/releases/1.0.0/resolved/`,
+    });
+
     newIssuesEndpoint = MockApiClient.addMockResponse({
       url: `/organizations/${props.organization.slug}/issues/?groupStatsPeriod=auto&limit=10&query=first-release%3A1.0.0&sort=freq&statsPeriod=14d`,
       body: [],