feat(assistant): Guides triggerable with #assistant (#7861)

* feat(assistant): Guides triggerable with #assistant

* lynnagara's comment

* adding seen attribute to /assistant/ api. fixing tests

* adding seen attribute to /assistant/ api. fixing tests

* simplifying url-triggered assistant open flow, leveraging GuideStore's existing update paths

* fixing adhiraj's comments

* fix tests

* fixing adhiraj's comments

* fixing more comments

* improving quality

* fixes

* fixes tests

* fixing after rebase

src/sentry/api/endpoints/assistant.py

@@ -1,5 +1,7 @@
 from __future__ import absolute_import
 
+from copy import deepcopy
+
 from django.db import IntegrityError, transaction
 from django.http import HttpResponse
 from django.utils import timezone
@@ -37,14 +39,14 @@ class AssistantEndpoint(Endpoint):
     permission_classes = (IsAuthenticated, )
 
     def get(self, request):
-        """Return all the guides the user has not viewed or dismissed."""
-        guides = manager.all()
-        exclude_ids = set(AssistantActivity.objects.filter(
+        """Return all the guides with a 'seen' attribute if it has been 'viewed' or 'dismissed'."""
+        guides = deepcopy(manager.all())
+        seen_ids = set(AssistantActivity.objects.filter(
             user=request.user,
         ).values_list('guide_id', flat=True))
-        result = {k: v for k, v in guides.items() if v['id'] not in exclude_ids}
-
-        return Response(result)
+        for k, v in guides.items():
+            v['seen'] = v['id'] in seen_ids
+        return Response(guides)
 
     def put(self, request):
         """Mark a guide as viewed or dismissed.

src/sentry/static/sentry/app/stores/guideStore.jsx

@@ -9,9 +9,6 @@ const GuideStore = Reflux.createStore({
     this.state = {
       // All guides returned to us from the server.
       guides: {},
-      // We record guides seen on the server, but immediately after a user dismisses a guide
-      // it may not have been synced yet, so the local copy helps in filtering correctly.
-      guidesSeen: new Set(),
       // All anchors that have been registered on this current view.
       anchors: new Set(),
       // The "on deck" guide.
@@ -21,6 +18,8 @@ const GuideStore = Reflux.createStore({
       currentStep: 0,
 
       currentOrg: null,
+
+      forceShow: false,
     };
     this.listenTo(GuideActions.fetchSucceeded, this.onFetchSucceeded);
     this.listenTo(GuideActions.closeGuideOrSupport, this.onCloseGuideOrSupport);
@@ -29,6 +28,13 @@ const GuideStore = Reflux.createStore({
     this.listenTo(GuideActions.unregisterAnchor, this.onUnregisterAnchor);
     this.listenTo(OrganizationsActions.setActive, this.onSetActiveOrganization);
     this.listenTo(OrganizationsActions.changeSlug, this.onChangeSlug);
+
+    window.addEventListener('hashchange', this.onHashChange, false);
+  },
+
+  onHashChange() {
+    this.state.forceShow = window.location.hash === '#assistant';
+    this.updateCurrentGuide();
   },
 
   onSetActiveOrganization(data) {
@@ -48,7 +54,11 @@ const GuideStore = Reflux.createStore({
   onCloseGuideOrSupport() {
     let {currentGuide} = this.state;
     if (currentGuide) {
-      this.state.guidesSeen.add(currentGuide.id);
+      this.state.guides[
+        Object.keys(this.state.guides).find(key => {
+          return this.state.guides[key].id == currentGuide.id;
+        })
+      ].seen = true;
     }
     this.updateCurrentGuide();
   },
@@ -80,13 +90,14 @@ const GuideStore = Reflux.createStore({
 
     // 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 seen = this.state.guidesSeen.has(guide.id);
       let allTargetsPresent = guide.required_targets.every(
         t => availableTargets.indexOf(t) >= 0
       );
-      return !seen && allTargetsPresent;
+      return (this.state.forceShow || !guide.seen) && allTargetsPresent;
     });
 
     let bestGuide = null;
@@ -99,7 +110,9 @@ const GuideStore = Reflux.createStore({
     }
 
     this.state.currentGuide = bestGuide;
-    this.state.currentStep = 0;
+
+    this.state.currentStep = this.state.forceShow ? 1 : 0;
+
     this.trigger(this.state);
   },
 });

tests/js/spec/stores/guideStore.spec.jsx

@@ -4,26 +4,27 @@ import GuideAnchor from 'app/components/assistant/guideAnchor';
 
 describe('GuideStore', function() {
   let sandbox;
-  let data = {
-    issue: {
-      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'},
-      ],
-    },
-  };
-
   let anchor1 = <GuideAnchor target="target 1" type="text" />;
   let anchor2 = <GuideAnchor target="target 2" type="text" />;
+  let data;
 
   beforeEach(function() {
     GuideStore.init();
     sandbox = sinon.sandbox.create();
+    data = {
+      issue: {
+        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,
+      },
+    };
   });
 
   afterEach(function() {
@@ -48,11 +49,33 @@ describe('GuideStore', function() {
     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);
     GuideStore.onNextStep();
     expect(GuideStore.state.currentStep).toEqual(1);
     GuideStore.onNextStep();
     expect(GuideStore.state.currentStep).toEqual(2);
     GuideStore.onCloseGuideOrSupport();
-    expect(GuideStore.state.guidesSeen).toEqual(new Set([1]));
+    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;
+    GuideStore.onRegisterAnchor(anchor1);
+    GuideStore.onRegisterAnchor(anchor2);
+    GuideStore.onFetchSucceeded(data);
+    expect(GuideStore.state.currentGuide).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);
+    expect(GuideStore.state.currentGuide).not.toEqual(null);
   });
 });

tests/sentry/api/endpoints/test_assistant.py

@@ -1,5 +1,7 @@
 from __future__ import absolute_import
 
+from copy import deepcopy
+
 from django.core.urlresolvers import reverse
 
 from sentry.assistant import manager
@@ -28,9 +30,13 @@ class AssistantActivity(APITestCase):
         assert resp.status_code == 400
 
     def test_activity(self):
+        guides_with_seen = deepcopy(manager.all())
+        for g in guides_with_seen:
+            guides_with_seen[g]['seen'] = False
+
         resp = self.client.get(self.path)
         assert resp.status_code == 200
-        assert resp.data == self.guides
+        assert resp.data == guides_with_seen
 
         # Dismiss the guide and make sure it is not returned again.
         resp = self.client.put(self.path, {
@@ -39,8 +45,9 @@ class AssistantActivity(APITestCase):
         })
         assert resp.status_code == 201
         resp = self.client.get(self.path)
+        guides_with_seen['releases']['seen'] = True
         assert resp.status_code == 200
-        assert resp.data == {k: v for k, v in self.guides.items() if v['id'] != 2}
+        assert resp.data == guides_with_seen
 
     def test_validate_guides(self):
         # Steps in different guides should not have the same target.