Browse Source

feat(assistant): One-off tips in assistant (#9418)

- Create a one-off alert reminder tip (gated to superusers for testing)
- Allow starting a guide without cuing it
- Allow customizing the feedback in the last step. Now in addition to the was-this-useful yes/no pair we can also have a CTA/Dismiss pair
- Move "user is new" check from the experiment definition to the frontend because some tips will be applicable to all users. Corresponding experiment change is in getsentry in a different PR.
- Remove members guide (not useful)
adhiraj 6 years ago
parent
commit
7928928e56

+ 32 - 50
src/sentry/assistant/guides.py

@@ -20,6 +20,12 @@ from django.utils.translation import ugettext_lazy as _
 #                          otherwise the anchor will be pinged and scrolled to. If you'd like
 #                          your step to show always or have a step is not tied to a specific
 #                          element but you'd still like it to be shown, set this as None.
+# guide_type (text, optional): "guide" or "tip" (defaults to guide). If it's a tip, the cue won't
+#     be shown, and you should also specify the fields "cta_text" and "cta_link", which would
+#     replace the "Was this guide useful" message at the end with the CTA and a dismiss button.
+# cta_text (text, conditional): CTA button text on the last step of a tip. Must be present if
+#     guide_type = tip.
+# cta_link (text, conditional): Where the CTA button points to. Must be present if guide_type = tip.
 
 GUIDES = {
     'issue': {
@@ -151,34 +157,6 @@ GUIDES = {
             },
         ]
     },
-    'members': {
-        'id': 4,
-        'cue': _('Tips for inviting your team'),
-        'required_targets': ['member_add'],
-        'steps': [
-            {
-                'title': _('Fix issues faster, together'),
-                'message': _('Sentry isn\'t logs. It\'s about shipping faster by immediately '
-                             'alerting, triaging, and assigning issues to the right engineer.'),
-                'target': 'member_add',
-            },
-            {
-                'title': _('Status'),
-                'message': _('You can enforce <a href="/settings/${orgSlug}/#require2FA">2-factor auth</a> or '
-                             '<a href="/settings/${orgSlug}/auth/">SSO</a> across your organization. Status lets you see '
-                             'which members haven\'t configured them yet.'),
-                'target': 'member_status',
-            },
-            {
-                'title': _('Roles'),
-                'message': _('Consider having two owners, in case one person\'s out, and you '
-                             'need to adjust billing or a new hire.<br><br>'
-                             'Add finance as a billing member. They\'ll get access to '
-                             'invoices, so they won\'t email you for receipts.'),
-                'target': 'member_role',
-            },
-        ]
-    },
     # Ideally, this would only be sent if the organization has never
     # customized alert rules (as per FeatureAdoption)
     'alert_rules': {
@@ -187,40 +165,44 @@ GUIDES = {
         'required_targets': ['alert_conditions'],
         'steps': [
             {
-                'title': _('Reduce inbox noise'),
-                'message': _('Sentry, by default, alerts on every <i>new</i> issue via email. '
-                             'If that\'s too noisy, sending the new issue alerts to '
-                             'a service like Slack help reduce inbox noise.<br><br> Enabling '
-                             '<a href="https://sentry.io/settings/account/notifications/#weeklyReports" target="_blank">'
+                'title': _('Reduce Inbox Noise'),
+                'message': _('Sentry, by default, alerts on every new issue via email. '
+                             'If that\'s too noisy, send the alerts to a service like Slack to '
+                             'reduce inbox noise.<br><br> Enabling <a href="https://sentry.io/settings/account/notifications/#weeklyReports" target="_blank">'
                              'weekly reports</a> can also help you stay on top of issues without '
                              'getting overwhelmed.'),
                 'target': 'alert_conditions',
             },
             {
-                'title': _('Define priority alerts'),
-                'message': _('Not all alerts are created equal. Create rules for frequently occuring errors or specific '
-                             'tags to escalate critical issues by alerting you via email or PagerDuty.<br><br>'
-                             '<a href="https://blog.sentry.io/2017/10/12/proactive-alert-rules" target="_blank">Learn more</a> '
-                             'about wrangling alerts.'),
+                'title': _('Prioritize Alerts'),
+                'message': _('Not all alerts are equally important. Send the important ones to a '
+                             'service like PagerDuty. <a href="https://blog.sentry.io/2017/10/12/proactive-alert-rules" target="_blank">Learn more</a> '
+                             'about prioritizing alerts.'),
                 'target': 'alert_actions',
             },
             {
-                'title': _('Fine-tune your personal settings'),
-                'message': _('You can control alerts at the project <em>and</em> the user level. '
-                             'If you\'d like to customize your <i>personal</i> alert settings, '
-                             'go to <a href="/account/settings/notifications/" target="_blank">Account Notifications</a>. '
-                             'There, you can choose which project\'s alert or workflow notifications you\'d like to receive.'),
+                'title': _('Fine-tune notifications'),
+                'message': _('You can control alerts both at the project and the user level. '
+                             'Go to <a href="/account/settings/notifications/" target="_blank">Account Notifications</a> '
+                             'to choose which project\'s alert or workflow notifications you\'d like to receive.'),
                 'target': None,
             },
+        ],
+    },
+    'alert_reminder_1': {
+        'id': 6,
+        'guide_type': 'tip',
+        'required_targets': ['project_details'],
+        'steps': [
             {
-                'title': _('What are legacy integrations?'),
-                'message': _('You can see what integrations are legacy or not in '
-                             '<a href="/settings/${orgSlug}/${projectSlug}/plugins/" target="_blank">integration settings</a>. '
-                             'If you want an alert rule to trigger both legacy and global '
-                             'integrations, you need to add both as actions. '
-                             '<a href="https://help.sentry.io/hc/en-us/articles/360003063454" target="_blank">Learn more</a>.'),
-                'target': None,
+                'title': _('Alert Rules'),
+                'message': _('This project received ${numEvents} events in the last 30 days but doesn\'t have '
+                             'custom alerts. Customizing alerts gives you more control over how you get '
+                             'notified of issues. Learn more <a href="https://sentry.io/_/resources/customer-success/alert-rules/?referrer=assistant" target="_blank">here</a>.'),
+                'target': 'project_details',
             },
         ],
+        'cta_text': _('Customize Alerts'),
+        'cta_link': '/settings/${orgSlug}/${projectSlug}/alerts/rules/',
     },
 }

+ 40 - 32
src/sentry/static/sentry/app/components/assistant/guideDrawer.jsx

@@ -1,4 +1,6 @@
 import React from 'react';
+import PropTypes from 'prop-types';
+import {withRouter} from 'react-router';
 import Reflux from 'reflux';
 import createReactClass from 'create-react-class';
 import styled from 'react-emotion';
@@ -23,15 +25,16 @@ import {
 /* GuideDrawer is what slides up when the user clicks on a guide cue. */
 const GuideDrawer = createReactClass({
   displayName: 'GuideDrawer',
-
+  propTypes: {
+    router: PropTypes.object,
+  },
   mixins: [Reflux.listenTo(GuideStore, 'onGuideStateChange')],
 
   getInitialState() {
     return {
-      currentGuide: null,
-      currentStep: 0,
-      currentOrgSlug: null,
-      currentProjectSlug: null,
+      guide: null,
+      step: 0,
+      messageVariables: {},
     };
   },
 
@@ -41,26 +44,36 @@ const GuideDrawer = createReactClass({
 
   onGuideStateChange(data) {
     this.setState({
-      currentGuide: data.currentGuide,
-      currentStep: data.currentStep,
-      currentOrgSlug: data.currentOrg ? data.currentOrg.slug : null,
-      currentProjectSlug: data.currentProject ? data.currentProject.slug : null,
+      guide: data.currentGuide,
+      step: data.currentStep,
+      messageVariables: {
+        orgSlug: data.org && data.org.slug,
+        projectSlug: data.project && data.project.slug,
+        numEvents: data.project && data.projectStats[parseInt(data.project.id, 10)],
+      },
     });
   },
 
   /* Terminology:
-   - A guide can be FINISHED by answering whether or not is was useful in the last step.
-   - A guide can be DISMISSED by x-ing out of it at any time (including when it's cued).
+   - A guide can be FINISHED by clicking one of the buttons in the last step.
+   - A guide can be DISMISSED by x-ing out of it at any step except the last (where there is no x).
    - In both cases we consider it CLOSED.
   */
   handleFinish(useful) {
-    recordFinish(this.state.currentGuide.id, useful);
+    recordFinish(this.state.guide.id, useful);
     closeGuide();
+    // This is a bit racy. Technically the correct thing to do would be to wait until closeGuide
+    // has updated the guide store and triggered a component state change. But it doesn't seem
+    // to cause any issues in practice.
+    if (useful && this.state.guide.cta_link) {
+      let link = this.interpolate(this.state.guide.cta_link, this.state.messageVariables);
+      this.props.router.push(link);
+    }
   },
 
   handleDismiss(e) {
     e.stopPropagation();
-    recordDismiss(this.state.currentGuide.id, this.state.currentStep);
+    recordDismiss(this.state.guide.id, this.state.step);
     closeGuide();
   },
 
@@ -72,17 +85,17 @@ const GuideDrawer = createReactClass({
   },
 
   render() {
-    let {currentGuide, currentStep} = this.state;
+    let {guide, step, messageVariables} = this.state;
 
-    if (!currentGuide) {
+    if (!guide) {
       return null;
     }
 
-    if (currentStep === 0) {
+    if (step === 0) {
       return (
         <StyledCueContainer onClick={nextStep} className="assistant-cue">
           {<CueIcon hasGuide={true} />}
-          <StyledCueText>{currentGuide.cue}</StyledCueText>
+          <StyledCueText>{guide.cue}</StyledCueText>
           <div style={{display: 'flex'}} onClick={this.handleDismiss}>
             <CloseIcon />
           </div>
@@ -90,17 +103,14 @@ const GuideDrawer = createReactClass({
       );
     }
 
-    let messageVariables = {
-      orgSlug: this.state.currentOrgSlug,
-      projectSlug: this.state.currentProjectSlug,
-    };
+    let isTip = guide.guide_type === 'tip';
 
     return (
       <GuideContainer>
         <GuideInputRow>
           <CueIcon hasGuide={true} />
-          <StyledTitle>{currentGuide.steps[currentStep - 1].title}</StyledTitle>
-          {currentStep < currentGuide.steps.length && (
+          <StyledTitle>{guide.steps[step - 1].title}</StyledTitle>
+          {step < guide.steps.length && (
             <div
               className="close-button"
               style={{display: 'flex'}}
@@ -113,14 +123,11 @@ const GuideDrawer = createReactClass({
         <StyledContent>
           <div
             dangerouslySetInnerHTML={{
-              __html: this.interpolate(
-                currentGuide.steps[currentStep - 1].message,
-                messageVariables
-              ),
+              __html: this.interpolate(guide.steps[step - 1].message, messageVariables),
             }}
           />
           <div style={{marginTop: '1em'}}>
-            {currentStep < currentGuide.steps.length ? (
+            {step < guide.steps.length ? (
               <div>
                 <Button priority="success" size="small" onClick={nextStep}>
                   {t('Next')} &rarr;
@@ -128,13 +135,13 @@ const GuideDrawer = createReactClass({
               </div>
             ) : (
               <div style={{textAlign: 'center'}}>
-                <p>{t('Did you find this guide useful?')}</p>
+                {!isTip && <p>{t('Did you find this guide useful?')}</p>}
                 <Button
                   priority="success"
                   size="small"
                   onClick={() => this.handleFinish(true)}
                 >
-                  {t('Yes')} &nbsp; &#x2714;
+                  {isTip ? guide.cta_text : <span>{t('Yes')} &nbsp; &#x2714;</span>}
                 </Button>
                 <Button
                   priority="success"
@@ -142,7 +149,7 @@ const GuideDrawer = createReactClass({
                   style={{marginLeft: '0.25em'}}
                   onClick={() => this.handleFinish(false)}
                 >
-                  {t('No')} &nbsp; &#x2716;
+                  {isTip ? t('Dismiss') : <span>{t('No')} &nbsp; &#x2716;</span>}
                 </Button>
               </div>
             )}
@@ -196,4 +203,5 @@ const StyledContent = styled('div')`
   }
 `;
 
-export default GuideDrawer;
+export {GuideDrawer};
+export default withRouter(GuideDrawer);

+ 126 - 28
src/sentry/static/sentry/app/stores/guideStore.jsx

@@ -4,27 +4,37 @@ import GuideActions from 'app/actions/guideActions';
 import OrganizationsActions from 'app/actions/organizationsActions';
 import analytics from 'app/utils/analytics';
 import ProjectActions from 'app/actions/projectActions';
+import {Client} from 'app/api';
+import ConfigStore from 'app/stores/configStore';
+
+const ALERT_REMINDER_1 = 'alert_reminder_1';
 
 const GuideStore = Reflux.createStore({
   init() {
     this.state = {
       // All guides returned to us from the server.
       guides: {},
-      // All anchors that have been registered on this current view.
+      // All anchors that are currently mounted.
       anchors: new Set(),
       // The "on deck" guide.
       currentGuide: null,
-      // The current step of the current guide (1-indexed). 0 if there's no guide
+      // Current step of the current guide (1-indexed). 0 if there's no guide
       // or the guide is just cued but not opened.
       currentStep: 0,
-
-      currentOrg: null,
-
-      currentProject: null,
-
+      // Current organization.
+      org: null,
+      // Current project.
+      project: null,
+      // Total events received in the project in the last 30 days. id (int) -> int.
+      projectStats: {},
+      // Whether the project has customized alert rules. id (int) -> bool.
+      projectRules: {},
+      // We force show a guide if the URL contains #assistant.
       forceShow: false,
+      // The previously shown guide.
       prevGuide: null,
     };
+    this.api = new Client();
     this.listenTo(GuideActions.fetchSucceeded, this.onFetchSucceeded);
     this.listenTo(GuideActions.closeGuide, this.onCloseGuide);
     this.listenTo(GuideActions.nextStep, this.onNextStep);
@@ -32,7 +42,7 @@ const GuideStore = Reflux.createStore({
     this.listenTo(GuideActions.unregisterAnchor, this.onUnregisterAnchor);
     this.listenTo(OrganizationsActions.setActive, this.onSetActiveOrganization);
     this.listenTo(ProjectActions.setActive, this.onSetActiveProject);
-    this.listenTo(OrganizationsActions.changeSlug, this.onChangeSlug);
+    this.listenTo(OrganizationsActions.changeSlug, this.onChangeOrgSlug);
 
     window.addEventListener('hashchange', this.onURLChange, false);
     window.addEventListener('load', this.onURLChange, false);
@@ -44,17 +54,18 @@ const GuideStore = Reflux.createStore({
   },
 
   onSetActiveOrganization(data) {
-    this.state.currentOrg = data;
-    this.trigger(this.state);
+    this.state.org = data;
+    this.updateCurrentGuide();
   },
 
   onSetActiveProject(data) {
-    this.state.currentProject = data;
-    this.trigger(this.state);
+    this.state.project = data;
+    this.updateCurrentGuide();
   },
 
-  onChangeSlug(prev, next) {
-    this.state.currentOrg = next;
+  onChangeOrgSlug(prev, next) {
+    this.state.org = next;
+    this.updateCurrentGuide();
   },
 
   onFetchSucceeded(data) {
@@ -110,20 +121,107 @@ const GuideStore = Reflux.createStore({
     }
   },
 
+  isDefaultAlert(data) {
+    return (
+      data.length === 1 &&
+      data[0].actionMatch === 'all' &&
+      data[0].frequency === 30 &&
+      data[0].conditions.length === 1 &&
+      data[0].conditions[0].id ===
+        'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition' &&
+      data[0].actions.length === 1 &&
+      data[0].actions[0].id === 'sentry.rules.actions.notify_event.NotifyEventAction'
+    );
+  },
+
+  checkAlertTipData() {
+    // Check if we have the data needed to determine if the alert-reminder tip should be shown.
+    // If not, take the necessary actions to fetch the data.
+    let {org, project, projectStats, projectRules} = this.state;
+
+    if (!org || !project) {
+      return false;
+    }
+
+    let projectId = parseInt(project.id, 10);
+    let ready = true;
+
+    if (projectStats[projectId] === undefined) {
+      ready = false;
+      let path = `/projects/${org.slug}/${project.slug}/stats/`;
+      this.api.request(path, {
+        query: {
+          // Last 30 days.
+          since: new Date().getTime() / 1000 - 3600 * 24 * 30,
+        },
+        success: data => {
+          let eventsReceived = data.reduce((sum, point) => sum + point[1], 0);
+          projectStats[projectId] = eventsReceived;
+          this.updateCurrentGuide();
+        },
+      });
+    }
+
+    if (projectRules[projectId] === undefined) {
+      ready = false;
+      let path = `/projects/${org.slug}/${project.slug}/rules/`;
+      this.api.request(path, {
+        success: data => {
+          projectRules[projectId] = !this.isDefaultAlert(data);
+          this.updateCurrentGuide();
+        },
+      });
+    }
+
+    return ready;
+  },
+
   updateCurrentGuide() {
+    // Logic to determine if a guide is shown:
+    // 1. If any required target is missing, don't show the guide.
+    // 2. If the URL ends with #assistant, show the guide.
+    // 3. If the user has seen the guide, don't show it.
+    // 4. If the guide doesn't pass custom checks, don't show it.
+    // 5. Otherwise show the guide.
+
     let availableTargets = [...this.state.anchors].map(a => a.props.target);
+    // sort() so that we pick a guide deterministically every time this function is called.
+    let guideKeys = Object.keys(this.state.guides)
+      .sort()
+      .filter(key => {
+        return this.state.guides[key].required_targets.every(
+          t => availableTargets.indexOf(t) >= 0
+        );
+      });
 
-    // Select the first guide that hasn't been seen in this session and has all
-    // required anchors on the page.
-    // If url hash is #assistant, show the first guide regardless of seen and has
-    // all required anchors.
-    let bestGuideKey = Object.keys(this.state.guides).find(key => {
-      let guide = this.state.guides[key];
-      let allTargetsPresent = guide.required_targets.every(
-        t => availableTargets.indexOf(t) >= 0
-      );
-      return (this.state.forceShow || !guide.seen) && allTargetsPresent;
-    });
+    if (!this.state.forceShow) {
+      guideKeys = guideKeys.filter(key => !this.state.guides[key].seen);
+    }
+
+    // Pick the first guide that satisfies conditions.
+    let bestGuideKey = null;
+    let user = ConfigStore.get('user');
+    for (let key of guideKeys) {
+      if (key === ALERT_REMINDER_1) {
+        if (!this.checkAlertTipData()) {
+          // Wait for the required data.
+          break;
+        } else if (user.isSuperuser) {
+          // Only show this to superusers for now.
+          let projectId = parseInt(this.state.project.id, 10);
+          if (
+            this.state.projectStats[projectId] > 1000 &&
+            !this.state.projectRules[projectId]
+          ) {
+            bestGuideKey = key;
+            break;
+          }
+        }
+      } else if (user.isSuperuser || new Date(user.dateJoined) > new Date(2018, 4, 10)) {
+        bestGuideKey = key;
+        break;
+      }
+    }
 
     let bestGuide = null;
     if (bestGuideKey) {
@@ -135,11 +233,11 @@ const GuideStore = Reflux.createStore({
           (step.target && availableTargets.indexOf(step.target) >= 0)
       );
     }
+
     this.updatePrevGuide(bestGuide);
     this.state.currentGuide = bestGuide;
-
-    this.state.currentStep = this.state.forceShow ? 1 : 0;
-
+    this.state.currentStep =
+      bestGuide && (this.state.forceShow || bestGuide.guide_type === 'tip') ? 1 : 0;
     this.trigger(this.state);
   },
 });

+ 2 - 0
src/sentry/static/sentry/app/views/projectDetailsLayout.jsx

@@ -9,6 +9,7 @@ import EnvironmentStore from 'app/stores/environmentStore';
 import ProjectHeader from 'app/components/projectHeader';
 import ProjectState from 'app/mixins/projectState';
 import withEnvironment from 'app/utils/withEnvironment';
+import GuideAnchor from 'app/components/assistant/guideAnchor';
 
 const ProjectDetailsLayout = createReactClass({
   displayName: 'ProjectDetailsLayout',
@@ -49,6 +50,7 @@ const ProjectDetailsLayout = createReactClass({
 
     return (
       <React.Fragment>
+        <GuideAnchor target="project_details" type="invisible" />
         <ProjectHeader
           activeSection={this.state.projectNavSection}
           project={this.context.project}

+ 67 - 4
tests/js/spec/components/assistant/__snapshots__/guideDrawer.spec.jsx.snap

@@ -117,8 +117,10 @@ exports[`GuideDrawer renders next step 1`] = `
           priority="success"
           size="small"
         >
-          Yes
-             ✔
+          <span>
+            Yes
+               ✔
+          </span>
         </Button>
         <Button
           disabled={false}
@@ -131,8 +133,69 @@ exports[`GuideDrawer renders next step 1`] = `
             }
           }
         >
-          No
-             ✖
+          <span>
+            No
+               ✖
+          </span>
+        </Button>
+      </div>
+    </div>
+  </StyledContent>
+</GuideContainer>
+`;
+
+exports[`GuideDrawer renders tip 1`] = `
+<GuideContainer>
+  <GuideInputRow>
+    <CueIcon
+      hasGuide={true}
+    />
+    <StyledTitle>
+      1. Title 1
+    </StyledTitle>
+  </GuideInputRow>
+  <StyledContent>
+    <div
+      dangerouslySetInnerHTML={
+        Object {
+          "__html": "Message 1 56",
+        }
+      }
+    />
+    <div
+      style={
+        Object {
+          "marginTop": "1em",
+        }
+      }
+    >
+      <div
+        style={
+          Object {
+            "textAlign": "center",
+          }
+        }
+      >
+        <Button
+          disabled={false}
+          onClick={[Function]}
+          priority="success"
+          size="small"
+        >
+          cta_text
+        </Button>
+        <Button
+          disabled={false}
+          onClick={[Function]}
+          priority="success"
+          size="small"
+          style={
+            Object {
+              "marginLeft": "0.25em",
+            }
+          }
+        >
+          Dismiss
         </Button>
       </div>
     </div>

+ 69 - 33
tests/js/spec/components/assistant/guideDrawer.spec.jsx

@@ -1,35 +1,83 @@
 import React from 'react';
 import {shallow} from 'enzyme';
-import {Client} from 'app/api';
-import GuideDrawer from 'app/components/assistant/guideDrawer';
+import {GuideDrawer} from 'app/components/assistant/guideDrawer';
 
 describe('GuideDrawer', function() {
-  let data = {
-    cue: 'Click here for a tour of the issue page',
-    id: 1,
-    page: 'issue',
-    required_targets: ['target 1'],
-    steps: [
-      {message: 'Message 1 ${orgSlug}', target: 'target 1', title: '1. Title 1'},
-      {message: 'Message 2', target: 'target 2', title: '2. Title 2'},
-    ],
-  };
+  let guides = [
+    {
+      cue: 'Click here for a tour of the issue page',
+      id: 1,
+      required_targets: ['target 1'],
+      steps: [
+        {message: 'Message 1 ${orgSlug}', target: 'target 1', title: '1. Title 1'},
+        {message: 'Message 2', target: 'target 2', title: '2. Title 2'},
+      ],
+    },
+    {
+      id: 2,
+      guide_type: 'tip',
+      cta_text: 'cta_text',
+      cta_link: '/cta/link/${orgSlug}/${projectSlug}/',
+      required_targets: ['target 3'],
+      steps: [
+        {message: 'Message 1 ${numEvents}', target: 'target 3', title: '1. Title 1'},
+      ],
+    },
+  ];
+  let wrapper, component, closeMock, pushMock;
 
   beforeEach(function() {
     MockApiClient.addMockResponse({
       url: '/assistant/',
     });
-    MockApiClient.addMockResponse({
+    closeMock = MockApiClient.addMockResponse({
       method: 'PUT',
       url: '/assistant/',
     });
+    pushMock = jest.fn();
+    wrapper = shallow(
+      <GuideDrawer
+        router={{
+          push: pushMock,
+        }}
+      />,
+      {
+        context: {
+          router: TestStubs.router(),
+          organization: {
+            id: '100',
+          },
+        },
+      }
+    );
+    component = wrapper.instance();
+  });
+
+  afterEach(function() {
+    MockApiClient.clearMockResponses();
+  });
+
+  it('renders tip', function() {
+    component.onGuideStateChange({
+      currentGuide: guides[1],
+      currentStep: 1,
+      project: {id: '10', slug: 'testproj'},
+      projectStats: {10: 56},
+      org: {slug: 'testorg'},
+    });
+    wrapper.update();
+    expect(wrapper).toMatchSnapshot();
+    // Click on the CTA.
+    wrapper
+      .find('Button')
+      .first()
+      .simulate('click');
+    expect(pushMock).toHaveBeenCalledWith('/cta/link/testorg/testproj/');
   });
 
   it('renders drawer', function() {
-    const wrapper = shallow(<GuideDrawer />);
-    const component = wrapper.instance();
     component.onGuideStateChange({
-      currentGuide: data,
+      currentGuide: guides[0],
       currentStep: 0,
     });
     wrapper.update();
@@ -41,20 +89,14 @@ describe('GuideDrawer', function() {
   });
 
   it('gets dismissed', function() {
-    let wrapper = shallow(<GuideDrawer />);
-    const component = wrapper.instance();
     component.onGuideStateChange({
-      currentGuide: data,
+      currentGuide: guides[0],
       currentStep: 1,
-      currentOrg: {slug: 'testorg'},
+      org: {slug: 'testorg'},
     });
     wrapper.update();
     expect(wrapper).toMatchSnapshot();
 
-    let closeMock = Client.addMockResponse({
-      url: '/assistant/',
-      method: 'PUT',
-    });
     wrapper
       .find('.close-button')
       .last()
@@ -72,26 +114,20 @@ describe('GuideDrawer', function() {
   });
 
   it('renders next step', function() {
-    let wrapper = shallow(<GuideDrawer />);
-    const component = wrapper.instance();
     component.onGuideStateChange({
-      currentGuide: data,
+      currentGuide: guides[0],
       currentStep: 2,
-      currentOrg: {slug: 'testorg'},
+      org: {slug: 'testorg'},
     });
     wrapper.update();
     expect(wrapper).toMatchSnapshot();
 
     // Mark as useful.
-    let usefulMock = Client.addMockResponse({
-      url: '/assistant/',
-      method: 'PUT',
-    });
     wrapper
       .find('Button')
       .first()
       .simulate('click');
-    expect(usefulMock).toHaveBeenCalledWith(
+    expect(closeMock).toHaveBeenCalledWith(
       '/assistant/',
       expect.objectContaining({
         method: 'PUT',

+ 58 - 62
tests/js/spec/stores/guideStore.spec.jsx

@@ -1,33 +1,37 @@
 import React from 'react';
 import GuideStore from 'app/stores/guideStore';
 import GuideAnchor from 'app/components/assistant/guideAnchor';
+import ConfigStore from 'app/stores/configStore';
 
 describe('GuideStore', function() {
   let sandbox;
   let anchor1 = <GuideAnchor target="target 1" type="text" />;
   let anchor2 = <GuideAnchor target="target 2" type="text" />;
   let data;
+  ConfigStore.config = {
+    user: {
+      isSuperuser: true,
+    },
+  };
 
   beforeEach(function() {
     GuideStore.init();
     sandbox = sinon.sandbox.create();
     data = {
-      issue: {
+      Guide1: {
         cue: 'Click here for a tour of the issue page',
         id: 1,
-        page: 'issue',
         required_targets: ['target 1'],
         steps: [
           {message: 'Message 1', target: 'target 1', title: '1. Title 1'},
           {message: 'Message 2', target: 'target 2', title: '2. Title 2'},
           {message: 'Message 3', target: 'target 3', title: '3. Title 3'},
         ],
-        seen: false,
+        seen: true,
       },
-      other: {
+      Guide2: {
         cue: 'Some other guide here',
         id: 2,
-        page: 'random',
         required_targets: ['target 1'],
         steps: [
           {message: 'Message 1', target: 'target 1', title: '1. Title 1'},
@@ -35,93 +39,85 @@ describe('GuideStore', function() {
         ],
         seen: false,
       },
+      alert_reminder_1: {
+        id: 3,
+        guide_type: 'tip',
+        required_targets: ['target 1'],
+        steps: [{message: 'Message 1', target: 'target 1', title: '1. Title 1'}],
+        seen: false,
+      },
     };
+    GuideStore.onRegisterAnchor(anchor1);
+    GuideStore.onRegisterAnchor(anchor2);
+    MockApiClient.addMockResponse({
+      url: '/projects/org/proj/stats/',
+      body: [[1, 500], [2, 300], [3, 500]],
+    });
+    MockApiClient.addMockResponse({
+      url: '/projects/org/proj/rules/',
+      body: [],
+    });
   });
 
   afterEach(function() {
     sandbox.restore();
   });
 
-  it('should add guides to store', function() {
-    GuideStore.onFetchSucceeded(data);
-    expect(GuideStore.state.guides).toEqual(data);
-    expect(GuideStore.state.currentStep).toEqual(0);
-  });
-
-  it('should register anchors', function() {
-    GuideStore.onRegisterAnchor(anchor1);
-    GuideStore.onRegisterAnchor(anchor2);
-    expect(GuideStore.state.anchors).toEqual(new Set([anchor1, anchor2]));
-  });
-
   it('should move through the steps in the guide', function() {
-    GuideStore.onRegisterAnchor(anchor1);
-    GuideStore.onRegisterAnchor(anchor2);
     GuideStore.onFetchSucceeded(data);
-    // GuideStore should prune steps that don't have anchors.
-    expect(GuideStore.state.currentGuide.steps).toHaveLength(2);
-    expect(GuideStore.state.currentGuide.seen).toEqual(false);
+    let guide = GuideStore.state.currentGuide;
+    // Should pick the first non-seen guide in alphabetic order.
+    expect(guide.id).toEqual(2);
+    expect(guide.steps).toHaveLength(2);
     GuideStore.onNextStep();
     expect(GuideStore.state.currentStep).toEqual(1);
-    GuideStore.onNextStep();
-    expect(GuideStore.state.currentStep).toEqual(2);
     GuideStore.onCloseGuide();
-    expect(
-      Object.keys(GuideStore.state.guides).filter(
-        key => GuideStore.state.guides[key].seen == true
-      )
-    ).toEqual(['issue']);
-  });
-
-  it('should not show seen guides', function() {
-    data.issue.seen = true;
-    data.other.seen = true;
-    GuideStore.onRegisterAnchor(anchor1);
-    GuideStore.onRegisterAnchor(anchor2);
-    GuideStore.onFetchSucceeded(data);
-    expect(GuideStore.state.currentGuide).toEqual(null);
+    guide = GuideStore.state.currentGuide;
+    // We don't have the alert reminder guide's data yet, so we can't show it.
+    expect(guide).toEqual(null);
   });
 
   it('should force show a guide', function() {
-    data.issue.seen = true;
-    GuideStore.onRegisterAnchor(anchor1);
-    GuideStore.onRegisterAnchor(anchor2);
-    GuideStore.state.forceShow = true;
     GuideStore.onFetchSucceeded(data);
+    window.location.hash = '#assistant';
+    window.dispatchEvent(new Event('hashchange'));
     expect(GuideStore.state.currentGuide.id).toEqual(1);
+    // Should prune steps that don't have anchors.
+    expect(GuideStore.state.currentGuide.steps).toHaveLength(2);
     GuideStore.onCloseGuide();
     expect(GuideStore.state.currentGuide.id).toEqual(2);
-    GuideStore.onCloseGuide();
+    window.location.hash = '';
+  });
+
+  it('should render tip', async function() {
+    data.Guide2.seen = true;
+    GuideStore.onFetchSucceeded(data);
     expect(GuideStore.state.currentGuide).toEqual(null);
+    let spy = jest.spyOn(GuideStore, 'isDefaultAlert').mockImplementation(() => true);
+    GuideStore.onSetActiveOrganization({id: 1, slug: 'org'});
+    GuideStore.onSetActiveProject({id: 1, slug: 'proj'});
+    await tick();
+    expect(GuideStore.state.currentGuide.id).toEqual(3);
+    spy.mockRestore();
   });
 
   it('should record analytics events when guide is cued', function() {
-    let mockRecordCue = jest.fn();
-    GuideStore.recordCue = mockRecordCue;
+    let spy = jest.spyOn(GuideStore, 'recordCue');
 
-    GuideStore.onRegisterAnchor(anchor1);
-    GuideStore.onRegisterAnchor(anchor2);
     GuideStore.onFetchSucceeded(data);
-    expect(mockRecordCue).toHaveBeenCalledWith(data.issue.id, data.issue.cue);
-    expect(mockRecordCue).toHaveBeenCalledTimes(1);
-    GuideStore.onCloseGuide();
-
-    // Should trigger a record when a new guide is cued
-    expect(GuideStore.state.currentGuide).toEqual(data.other);
-    expect(mockRecordCue).toHaveBeenCalledWith(data.other.id, data.other.cue);
-    expect(mockRecordCue).toHaveBeenCalledTimes(2);
+    expect(spy).toHaveBeenCalledWith(data.Guide2.id, data.Guide2.cue);
+    expect(spy).toHaveBeenCalledTimes(1);
+    spy.mockRestore();
   });
 
   it('should not send multiple cue analytics events for same guide', function() {
-    let mockRecordCue = jest.fn();
-    GuideStore.recordCue = mockRecordCue;
+    let spy = jest.spyOn(GuideStore, 'recordCue');
 
-    GuideStore.onRegisterAnchor(anchor1);
-    GuideStore.onRegisterAnchor(anchor2);
     GuideStore.onFetchSucceeded(data);
-    expect(mockRecordCue).toHaveBeenCalledWith(data.issue.id, data.issue.cue);
-    expect(mockRecordCue).toHaveBeenCalledTimes(1);
+    expect(spy).toHaveBeenCalledWith(data.Guide2.id, data.Guide2.cue);
+    expect(spy).toHaveBeenCalledTimes(1);
     GuideStore.updateCurrentGuide();
-    expect(mockRecordCue).toHaveBeenCalledTimes(1);
+    expect(spy).toHaveBeenCalledTimes(1);
+    spy.mockRestore();
   });
 });