Browse Source

feat(ui): Move issue subscribe button to action bar (#19327)

Scott Cooper 4 years ago
parent
commit
39ab7e3d90

+ 3 - 52
src/sentry/static/sentry/app/components/group/sidebar.jsx

@@ -6,7 +6,7 @@ import keyBy from 'lodash/keyBy';
 import pickBy from 'lodash/pickBy';
 
 import {addLoadingMessage, clearIndicators} from 'app/actionCreators/indicator';
-import {t, tct} from 'app/locale';
+import {t} from 'app/locale';
 import ErrorBoundary from 'app/components/errorBoundary';
 import ExternalIssueList from 'app/components/group/externalIssuesList';
 import GroupParticipants from 'app/components/group/participants';
@@ -15,19 +15,9 @@ import GroupTagDistributionMeter from 'app/components/group/tagDistributionMeter
 import GuideAnchor from 'app/components/assistant/guideAnchor';
 import LoadingError from 'app/components/loadingError';
 import SentryTypes from 'app/sentryTypes';
-import SubscribeButton from 'app/components/subscribeButton';
 import SuggestedOwners from 'app/components/group/suggestedOwners/suggestedOwners';
 import withApi from 'app/utils/withApi';
-
-const SUBSCRIPTION_REASONS = {
-  commented: t("You're receiving updates because you have commented on this issue."),
-  assigned: t("You're receiving updates because you were assigned to this issue."),
-  bookmarked: t("You're receiving updates because you have bookmarked this issue."),
-  changed_status: t(
-    "You're receiving updates because you have changed the status of this issue."
-  ),
-  mentioned: t("You're receiving updates because you have been mentioned in this issue."),
-};
+import {getSubscriptionReason} from 'app/views/organizationGroupDetails/utils';
 
 class GroupSidebar extends React.Component {
   static propTypes = {
@@ -175,40 +165,8 @@ class GroupSidebar extends React.Component {
     return null;
   }
 
-  canChangeSubscriptionState() {
-    return !(this.props.group.subscriptionDetails || {disabled: false}).disabled;
-  }
-
   getNotificationText() {
-    const {group} = this.props;
-
-    if (group.isSubscribed) {
-      let result = t(
-        "You're receiving updates because you are subscribed to this issue."
-      );
-      if (group.subscriptionDetails) {
-        const reason = group.subscriptionDetails.reason;
-        if (SUBSCRIPTION_REASONS.hasOwnProperty(reason)) {
-          result = SUBSCRIPTION_REASONS[reason];
-        }
-      } else {
-        result = tct(
-          "You're receiving updates because you are [link:subscribed to workflow notifications] for this project.",
-          {
-            link: <a href="/account/settings/notifications/" />,
-          }
-        );
-      }
-      return result;
-    } else {
-      if (group.subscriptionDetails && group.subscriptionDetails.disabled) {
-        return tct('You have [link:disabled workflow notifications] for this project.', {
-          link: <a href="/account/settings/notifications/" />,
-        });
-      } else {
-        return t("You're not subscribed to this issue.");
-      }
-    }
+    return getSubscriptionReason(this.props.group);
   }
 
   renderParticipantData() {
@@ -289,17 +247,10 @@ class GroupSidebar extends React.Component {
         )}
 
         {this.renderParticipantData()}
-
         <h6>
           <span>{t('Notifications')}</span>
         </h6>
         <p className="help-block">{this.getNotificationText()}</p>
-        {this.canChangeSubscriptionState() && (
-          <SubscribeButton
-            isSubscribed={group.isSubscribed}
-            onClick={() => this.toggleSubscription()}
-          />
-        )}
       </div>
     );
   }

+ 1 - 1
src/sentry/static/sentry/app/components/subscribeButton.tsx

@@ -3,7 +3,7 @@ import React from 'react';
 
 import Button from 'app/components/button';
 import {t} from 'app/locale';
-import {IconBell} from 'app/icons/iconBell';
+import {IconBell} from 'app/icons';
 
 type Props = {
   onClick: (e: React.MouseEvent) => void;

+ 1 - 0
src/sentry/static/sentry/app/icons/index.tsx

@@ -96,3 +96,4 @@ export {IconWindow} from './iconWindow';
 export {IconTerminal} from './iconTerminal';
 export {IconSwitch} from './iconSwitch';
 export {IconMobile} from './iconMobile';
+export {IconBell} from './iconBell';

+ 1 - 0
src/sentry/static/sentry/app/types/index.tsx

@@ -612,6 +612,7 @@ export type Group = {
   type: EventOrGroupType;
   userCount: number;
   userReportCount: number;
+  subscriptionDetails: {disabled?: boolean; reason?: string} | null;
 };
 
 /**

+ 18 - 16
src/sentry/static/sentry/app/views/organizationGroupDetails/actions.jsx

@@ -32,6 +32,8 @@ import space from 'app/styles/space';
 import withApi from 'app/utils/withApi';
 import withOrganization from 'app/utils/withOrganization';
 
+import SubscribeAction from './subscribeAction';
+
 class DeleteActions extends React.Component {
   static propTypes = {
     organization: SentryTypes.Organization.isRequired,
@@ -232,6 +234,10 @@ const GroupDetailsActions = createReactClass({
     this.onUpdate({isBookmarked: !this.props.group.isBookmarked});
   },
 
+  onToggleSubscribe() {
+    this.onUpdate({isSubscribed: !this.props.group.isSubscribed});
+  },
+
   onDiscard() {
     const {group, project, organization} = this.props;
     const id = uniqueId();
@@ -283,30 +289,15 @@ const GroupDetailsActions = createReactClass({
             isAutoResolved={isResolved && group.statusDetails.autoResolved}
           />
         </GuideAnchor>
-
         <GuideAnchor target="ignore_delete_discard" position="bottom" offset={space(3)}>
           <IgnoreActions isIgnored={isIgnored} onUpdate={this.onUpdate} />
         </GuideAnchor>
-
-        <div className="btn-group">
-          <div
-            className={bookmarkClassName}
-            title={t('Bookmark')}
-            onClick={this.onToggleBookmark}
-          >
-            <IconWrapper>
-              <IconStar isSolid size="xs" />
-            </IconWrapper>
-          </div>
-        </div>
-
         <DeleteActions
           organization={organization}
           project={project}
           onDelete={this.onDelete}
           onDiscard={this.onDiscard}
         />
-
         {orgFeatures.has('shared-issues') && (
           <div className="btn-group">
             <ShareIssue
@@ -319,7 +310,6 @@ const GroupDetailsActions = createReactClass({
             />
           </div>
         )}
-
         {orgFeatures.has('discover-basic') && (
           <div className="btn-group">
             <Link
@@ -331,6 +321,18 @@ const GroupDetailsActions = createReactClass({
             </Link>
           </div>
         )}
+        <div className="btn-group">
+          <div
+            className={bookmarkClassName}
+            title={t('Bookmark')}
+            onClick={this.onToggleBookmark}
+          >
+            <IconWrapper>
+              <IconStar isSolid size="xs" />
+            </IconWrapper>
+          </div>
+        </div>
+        <SubscribeAction group={group} onClick={this.onToggleSubscribe} />
       </div>
     );
   },

+ 52 - 0
src/sentry/static/sentry/app/views/organizationGroupDetails/subscribeAction.tsx

@@ -0,0 +1,52 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import styled from '@emotion/styled';
+
+import {Group} from 'app/types';
+import SentryTypes from 'app/sentryTypes';
+import {IconBell} from 'app/icons';
+import {t} from 'app/locale';
+import Tooltip from 'app/components/tooltip';
+
+import {getSubscriptionReason} from './utils';
+
+type Props = {
+  group: Group;
+  onClick: (event: React.MouseEvent<HTMLDivElement>) => void;
+};
+
+const SubscribeAction = ({group, onClick}: Props) => {
+  const canChangeSubscriptionState = (): boolean => {
+    return !(group.subscriptionDetails?.disabled ?? false);
+  };
+
+  const subscribedClassName = `group-subscribe btn btn-default btn-sm${
+    group.isSubscribed ? ' active' : ''
+  }`;
+
+  return (
+    canChangeSubscriptionState() && (
+      <div className="btn-group">
+        <Tooltip title={getSubscriptionReason(group, true)}>
+          <div className={subscribedClassName} title={t('Subscribe')} onClick={onClick}>
+            <IconWrapper>
+              <IconBell size="xs" />
+            </IconWrapper>
+          </div>
+        </Tooltip>
+      </div>
+    )
+  );
+};
+
+SubscribeAction.propTypes = {
+  group: SentryTypes.Group.isRequired,
+  onClick: PropTypes.func.isRequired,
+};
+
+export default SubscribeAction;
+
+const IconWrapper = styled('span')`
+  position: relative;
+  top: 1px;
+`;

+ 58 - 0
src/sentry/static/sentry/app/views/organizationGroupDetails/utils.jsx

@@ -1,3 +1,6 @@
+import React from 'react';
+
+import {t, tct} from 'app/locale';
 import {Client} from 'app/api';
 
 /**
@@ -62,3 +65,58 @@ export function getEventEnvironment(event) {
 
   return tag ? tag.value : null;
 }
+
+const SUBSCRIPTION_REASONS = {
+  commented: t(
+    "You're receiving workflow notifications because you have commented on this issue."
+  ),
+  assigned: t(
+    "You're receiving workflow notifications because you were assigned to this issue."
+  ),
+  bookmarked: t(
+    "You're receiving workflow notifications because you have bookmarked this issue."
+  ),
+  changed_status: t(
+    "You're receiving workflow notifications because you have changed the status of this issue."
+  ),
+  mentioned: t(
+    "You're receiving workflow notifications because you have been mentioned in this issue."
+  ),
+};
+
+/**
+ * @param {object} group
+ * @param {boolean} removeLinks add/remove links to subscription reasons text (default: false)
+ * @returns Reason for subscription
+ */
+export function getSubscriptionReason(group, removeLinks = false) {
+  if (group.subscriptionDetails && group.subscriptionDetails.disabled) {
+    return tct('You have [link:disabled workflow notifications] for this project.', {
+      link: removeLinks ? <span /> : <a href="/account/settings/notifications/" />,
+    });
+  }
+
+  if (!group.isSubscribed) {
+    return t("You're not subscribed to this issue.");
+  }
+
+  if (group.subscriptionDetails) {
+    const {reason} = group.subscriptionDetails;
+    if (reason === 'unknown') {
+      return t(
+        "You're receiving workflow notifications because you are subscribed to this issue."
+      );
+    }
+
+    if (SUBSCRIPTION_REASONS.hasOwnProperty(reason)) {
+      return SUBSCRIPTION_REASONS[reason];
+    }
+  }
+
+  return tct(
+    "You're receiving updates because you are [link:subscribed to workflow notifications] for this project.",
+    {
+      link: removeLinks ? <span /> : <a href="/account/settings/notifications/" />,
+    }
+  );
+}

+ 0 - 1
src/sentry/static/sentry/app/views/releasesV2/detail/overview/issues.tsx

@@ -179,7 +179,6 @@ class Issues extends React.Component<Props, State> {
             </Button>
           </OpenInButtonBar>
         </ControlsWrapper>
-
         <TableWrapper data-test-id="release-wrapper">
           <GroupList
             orgId={orgId}

+ 0 - 24
tests/js/spec/components/group/sidebar.spec.jsx

@@ -121,30 +121,6 @@ describe('GroupSidebar', function() {
     });
   });
 
-  describe('subscribing', function() {
-    let issuesApi;
-    beforeEach(function() {
-      issuesApi = MockApiClient.addMockResponse({
-        url: '/projects/org-slug/project-slug/issues/',
-        method: 'PUT',
-        body: TestStubs.Group({isSubscribed: false}),
-      });
-    });
-
-    it('can subscribe', function() {
-      const btn = wrapper.find('SubscribeButton');
-
-      btn.simulate('click');
-
-      expect(issuesApi).toHaveBeenCalledWith(
-        expect.anything(),
-        expect.objectContaining({
-          data: {isSubscribed: true},
-        })
-      );
-    });
-  });
-
   describe('environment toggle', function() {
     it('re-requests tags with correct environment', function() {
       const stagingEnv = {name: 'staging', displayName: 'Staging', id: '2'};

+ 83 - 1
tests/js/spec/views/organizationGroupDetails/actions.spec.jsx

@@ -1,6 +1,6 @@
 import React from 'react';
 
-import {shallow} from 'sentry-test/enzyme';
+import {shallow, mountWithTheme} from 'sentry-test/enzyme';
 
 import GroupActions from 'app/views/organizationGroupDetails/actions';
 import ConfigStore from 'app/stores/configStore';
@@ -35,4 +35,86 @@ describe('GroupActions', function() {
       expect(wrapper).toMatchSnapshot();
     });
   });
+
+  describe('subscribing', function() {
+    let issuesApi;
+    beforeEach(function() {
+      issuesApi = MockApiClient.addMockResponse({
+        url: '/projects/org/project/issues/',
+        method: 'PUT',
+        body: TestStubs.Group({isSubscribed: false}),
+      });
+    });
+
+    it('can subscribe', function() {
+      const wrapper = mountWithTheme(
+        <GroupActions
+          group={TestStubs.Group({
+            id: '1337',
+            pluginActions: [],
+            pluginIssues: [],
+          })}
+          project={TestStubs.ProjectDetails({
+            id: '2448',
+            name: 'project name',
+            slug: 'project',
+          })}
+          organization={TestStubs.Organization({
+            id: '4660',
+            slug: 'org',
+          })}
+        />
+      );
+      const btn = wrapper.find('.group-subscribe');
+      btn.simulate('click');
+
+      expect(issuesApi).toHaveBeenCalledWith(
+        expect.anything(),
+        expect.objectContaining({
+          data: {isSubscribed: true},
+        })
+      );
+    });
+  });
+
+  describe('bookmarking', function() {
+    let issuesApi;
+    beforeEach(function() {
+      issuesApi = MockApiClient.addMockResponse({
+        url: '/projects/org/project/issues/',
+        method: 'PUT',
+        body: TestStubs.Group({isBookmarked: false}),
+      });
+    });
+
+    it('can bookmark', function() {
+      const wrapper = mountWithTheme(
+        <GroupActions
+          group={TestStubs.Group({
+            id: '1337',
+            pluginActions: [],
+            pluginIssues: [],
+          })}
+          project={TestStubs.ProjectDetails({
+            id: '2448',
+            name: 'project name',
+            slug: 'project',
+          })}
+          organization={TestStubs.Organization({
+            id: '4660',
+            slug: 'org',
+          })}
+        />
+      );
+      const btn = wrapper.find('.group-bookmark');
+      btn.simulate('click');
+
+      expect(issuesApi).toHaveBeenCalledWith(
+        expect.anything(),
+        expect.objectContaining({
+          data: {isBookmarked: true},
+        })
+      );
+    });
+  });
 });

Some files were not shown because too many files changed in this diff