Revert "ref(guides): Refactor assistant guides (#17353)"

This reverts commit 91c0af0cedef1901379ee36c75463aa1ab699a77.

src/sentry/api/endpoints/assistant.py

@@ -1,6 +1,8 @@
 from __future__ import absolute_import
 
-from django.db import IntegrityError
+from copy import deepcopy
+
+from django.db import IntegrityError, transaction
 from django.http import HttpResponse
 from django.utils import timezone
 from rest_framework import serializers
@@ -16,21 +18,18 @@ VALID_STATUSES = frozenset(("viewed", "dismissed"))
 
 
 class AssistantSerializer(serializers.Serializer):
-    guide = serializers.CharField(required=True)
+    guide_id = serializers.IntegerField(required=True)
     status = serializers.ChoiceField(choices=zip(VALID_STATUSES, VALID_STATUSES), required=True)
     useful = serializers.BooleanField()
 
-    def validate(self, attrs):
-        guide = attrs.get("guide")
-        if not guide:
-            raise serializers.ValidationError("Assistant guide is required")
-
-        guide_id = manager.get_id_by_name(guide)
-        if not guide_id:
-            raise serializers.ValidationError("Not a valid assistant guide")
+    def validate_guide_id(self, value):
+        valid_ids = manager.get_valid_ids()
 
-        attrs["guide_id"] = guide_id
-        return attrs
+        if not value:
+            raise serializers.ValidationError("Assistant guide id is required")
+        if value not in valid_ids:
+            raise serializers.ValidationError("Not a valid assistant guide id")
+        return value
 
 
 class AssistantEndpoint(Endpoint):
@@ -38,22 +37,19 @@ class AssistantEndpoint(Endpoint):
 
     def get(self, request):
         """Return all the guides with a 'seen' attribute if it has been 'viewed' or 'dismissed'."""
-        active_guides = manager.all()
+        guides = deepcopy(manager.all())
         seen_ids = set(
             AssistantActivity.objects.filter(user=request.user).values_list("guide_id", flat=True)
         )
-
-        guides = [
-            {"guide": guide.name.lower(), "seen": guide.value in seen_ids}
-            for guide in active_guides
-        ]
+        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.
 
         Request is of the form {
-            'guide': guide key (e.g. 'issue_details'),
+            'guide_id': <guide_id>,
             'status': 'viewed' / 'dismissed',
             'useful' (optional): true / false,
         }
@@ -62,11 +58,9 @@ class AssistantEndpoint(Endpoint):
         if not serializer.is_valid():
             return Response(serializer.errors, status=400)
 
-        data = serializer.validated_data
-
-        guide_id = data["guide_id"]
-        status = data["status"]
-        useful = data.get("useful")
+        guide_id = request.data["guide_id"]
+        status = request.data["status"]
+        useful = request.data.get("useful")
 
         fields = {}
         if useful is not None:
@@ -77,7 +71,8 @@ class AssistantEndpoint(Endpoint):
             fields["dismissed_ts"] = timezone.now()
 
         try:
-            AssistantActivity.objects.create(user=request.user, guide_id=guide_id, **fields)
+            with transaction.atomic():
+                AssistantActivity.objects.create(user=request.user, guide_id=guide_id, **fields)
         except IntegrityError:
             pass
 

src/sentry/assistant/__init__.py

@@ -1,10 +1,7 @@
 from __future__ import absolute_import
 
 from .manager import AssistantManager
-from .guides import AssistantGuide
+from .guides import GUIDES
 
 manager = AssistantManager()
-
-manager.add(AssistantGuide.ISSUE_DETAILS)
-manager.add(AssistantGuide.ISSUE_STREAM)
-manager.add(AssistantGuide.DISCOVER_SIDEBAR)
+manager.add(GUIDES)

src/sentry/assistant/guides.py

@@ -1,9 +1,127 @@
 from __future__ import absolute_import
 
-from enum import Enum
+from django.utils.translation import ugettext_lazy as _
 
 
-class AssistantGuide(Enum):
-    ISSUE_DETAILS = 1
-    ISSUE_STREAM = 3
-    DISCOVER_SIDEBAR = 4
+# Guide Schema
+# id (text, required): unique id
+# required_targets (list): An empty list will cause the guide to be shown regardless
+#                          of page/targets presence.
+# steps (list): List of steps
+
+# Step Schema
+# title (text, required): Title text. Tone should be active.
+# message (text, optional): Message text. Should help illustrate how to do a task, not
+#                           just literally what the button does.
+# target (text, optional): step is tied to an anchor target. If the anchor doesn't exist,
+#                          the step will not be shown. if the anchor exists but is of type
+#                         "invisible", it will not be pinged but will be scrolled to.
+#                          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.
+
+GUIDES = {
+    "issue": {
+        "id": 1,
+        "required_targets": ["issue_title", "exception"],
+        "steps": [
+            {
+                "title": _("Issue Details"),
+                "message": _(
+                    "The issue page contains all the details about an issue. Let's get started."
+                ),
+                "target": "issue_title",
+            },
+            {
+                "title": _("Stacktrace"),
+                "message": _(
+                    "See the sequence of function calls that led to the error, and "
+                    "global/local variables for each stack frame."
+                ),
+                "target": "exception",
+            },
+            {
+                "title": _("Breadcrumbs"),
+                "message": _(
+                    "Breadcrumbs are a trail of events that happened prior to the error. They're "
+                    "similar to traditional logs but can record more rich structured data. "
+                    "When Sentry is used with web frameworks, breadcrumbs are automatically "
+                    "captured for events like database calls and network requests."
+                ),
+                "target": "breadcrumbs",
+            },
+            {
+                "title": _("Tags"),
+                "message": _(
+                    "Attach arbitrary key-value pairs to each event which you can search and filter on. "
+                    "View a heatmap of all tags for an issue on the right panel. "
+                ),
+                "target": "tags",
+            },
+            {
+                "title": _("Resolve"),
+                "message": _(
+                    "Resolve an issue to remove it from your issue list. "
+                    'Sentry can also <a href="/settings/account/notifications/" target="_blank"> '
+                    "alert you</a> when a new issue occurs or a resolved issue re-occurs."
+                ),
+                "target": "resolve",
+            },
+            {
+                "title": _("Delete and Ignore"),
+                "message": _(
+                    "Delete an issue to remove it from your issue list until it happens again. "
+                    "Ignore an issue to remove it permanently or until certain conditions are met."
+                ),
+                "target": "ignore_delete_discard",
+            },
+            {
+                "title": _("Issue Number"),
+                "message": _(
+                    "Include this unique identifier in your commit message to have Sentry automatically "
+                    "resolve the issue when your code is deployed. "
+                    '<a href="https://docs.sentry.io/learn/releases/" target="_blank">Learn more</a>.'
+                ),
+                "target": "issue_number",
+            },
+            {
+                "title": _("Ownership Rules"),
+                "message": _(
+                    "Define users or teams responsible for specific file paths or URLs so "
+                    "that alerts can be routed to the right person. "
+                    '<a href="https://docs.sentry.io/learn/issue-owners/" target="_blank">Learn more</a>.'
+                ),
+                "target": "owners",
+            },
+        ],
+    },
+    "issue_stream": {
+        "id": 3,
+        "required_targets": ["issue_stream"],
+        "steps": [
+            {
+                "title": _("Issues"),
+                "message": _(
+                    "Sentry automatically groups similar events together into an issue. Similarity "
+                    "is determined by stacktrace and other factors. "
+                    '<a href="https://docs.sentry.io/data-management/rollups/" target="_blank">Learn more</a>. '
+                ),
+                "target": "issue_stream",
+            }
+        ],
+    },
+    "discover_sidebar": {
+        "id": 4,
+        "required_targets": ["discover_sidebar"],
+        "steps": [
+            {
+                "title": _("Event Pages have moved"),
+                "message": _(
+                    "These are now in our powerful new query builder, Discover "
+                    '<a href="https://docs.sentry.io/workflow/discover2/" target="_blank">Learn more about its advanced features</a>. '
+                ),
+                "target": "discover_sidebar",
+            }
+        ],
+    },
+}

src/sentry/assistant/manager.py

@@ -1,20 +1,17 @@
 from __future__ import absolute_import
-
-from enum import Enum
+import six
 
 
 class AssistantManager(object):
     def __init__(self):
-        self._guides = []
+        self._guides = {}
 
-    def add(self, guide):
-        if isinstance(guide, Enum):
-            self._guides.append(guide)
+    def add(self, guides):
+        for k, v in six.iteritems(guides):
+            self._guides[k] = v
 
-    def get_id_by_name(self, name):
-        for guide in self._guides:
-            if name == guide.name.lower():
-                return guide.value
+    def get_valid_ids(self):
+        return list(v["id"] for k, v in six.iteritems(self._guides))
 
     def all(self):
         return self._guides

src/sentry/static/sentry/app/actionCreators/guides.tsx → src/sentry/static/sentry/app/actionCreators/guides.jsx

@@ -13,12 +13,12 @@ export function fetchGuides() {
   });
 }
 
-export function registerAnchor(target: string) {
-  GuideActions.registerAnchor(target);
+export function registerAnchor(anchor) {
+  GuideActions.registerAnchor(anchor);
 }
 
-export function unregisterAnchor(target: string) {
-  GuideActions.unregisterAnchor(target);
+export function unregisterAnchor(anchor) {
+  GuideActions.unregisterAnchor(anchor);
 }
 
 export function nextStep() {
@@ -29,42 +29,46 @@ export function closeGuide() {
   GuideActions.closeGuide();
 }
 
-export function dismissGuide(guide: string, step: number, orgId: string) {
-  recordDismiss(guide, step, orgId);
+export function dismissGuide(guideId, step, org) {
+  recordDismiss(guideId, step, org);
   closeGuide();
 }
 
-export function recordFinish(guide: string, orgId: string) {
+export function recordFinish(guideId, org) {
   api.request('/assistant/', {
     method: 'PUT',
     data: {
-      guide,
+      guide_id: guideId,
       status: 'viewed',
     },
   });
   const data = {
     eventKey: 'assistant.guide_finished',
     eventName: 'Assistant Guide Finished',
-    guide,
-    organization_id: orgId,
+    guide: guideId,
   };
+  if (org) {
+    data.organization_id = org.id;
+  }
   trackAnalyticsEvent(data);
 }
 
-export function recordDismiss(guide: string, step: number, orgId: string) {
+export function recordDismiss(guideId, step, org) {
   api.request('/assistant/', {
     method: 'PUT',
     data: {
-      guide,
+      guide_id: guideId,
       status: 'dismissed',
     },
   });
   const data = {
     eventKey: 'assistant.guide_dismissed',
     eventName: 'Assistant Guide Dismissed',
-    guide,
+    guide: guideId,
     step,
-    organization_id: orgId,
   };
+  if (org) {
+    data.organization_id = org.id;
+  }
   trackAnalyticsEvent(data);
 }

src/sentry/static/sentry/app/components/assistant/getGuidesContent.tsx

@@ -1,122 +0,0 @@
-import React from 'react';
-
-import {t, tct} from 'app/locale';
-import {GuidesContent} from 'app/components/assistant/types';
-import ExternalLink from 'app/components/links/externalLink';
-
-export default function getGuidesContent(): GuidesContent {
-  return [
-    {
-      guide: 'issue_details',
-      requiredTargets: ['issue-title', 'exception'],
-      steps: [
-        {
-          title: t('Issue Details'),
-          target: 'issue-title',
-          description: t(
-            "The issue page contains all the details about an issue. Let's get started."
-          ),
-        },
-        {
-          title: t('Stacktrace'),
-          target: 'exception',
-          description: t(
-            `See the sequence of function calls that led to the error, and global/local variables
-            for each stack frame.`
-          ),
-        },
-        {
-          title: t('Breadcrumbs'),
-          target: 'breadcrumbs',
-          description: t(
-            `Breadcrumbs are a trail of events that happened prior to the error. They're similar
-            to traditional logs but can record more rich structured data. When Sentry is used with
-            web frameworks, breadcrumbs are automatically captured for events like database calls and
-            network requests.`
-          ),
-        },
-        {
-          title: t('Tags'),
-          target: 'tags',
-          description: t(
-            `Attach arbitrary key-value pairs to each event which you can search and filter on.
-            View a heatmap of all tags for an issue on the right panel.`
-          ),
-        },
-        {
-          title: t('Resolve'),
-          target: 'resolve',
-          description: tct(
-            `Resolve an issue to remove it from your issue list. Sentry can also [link:alert you]
-            when a new issue occurs or a resolved issue re-occurs.`,
-            {link: <ExternalLink href="/settings/account/notifications/" />}
-          ),
-        },
-        {
-          title: t('Delete and Ignore'),
-          target: 'ignore-delete-discard',
-          description: t(
-            `Delete an issue to remove it from your issue list until it happens again.
-            Ignore an issue to remove it permanently or until certain conditions are met.`
-          ),
-        },
-        {
-          title: t('Issue Number'),
-          target: 'issue-number',
-          description: tct(
-            `Include this unique identifier in your commit message to have Sentry automatically
-            resolve the issue when your code is deployed. [link:Learn more].`,
-            {link: <ExternalLink href="https://docs.sentry.io/learn/releases/" />}
-          ),
-        },
-        {
-          title: t('Ownership Rules'),
-          target: 'owners',
-          description: tct(
-            `Define users or teams responsible for specific file paths or URLs so that alerts can
-            be routed to the right person. [link:Learn more]`,
-            {
-              link: <ExternalLink href="https://docs.sentry.io/learn/issue-owners/" />,
-            }
-          ),
-        },
-      ],
-    },
-    {
-      guide: 'issue_stream',
-      requiredTargets: ['issue-stream'],
-      steps: [
-        {
-          title: t('Issues'),
-          target: 'issue-stream',
-          description: tct(
-            `Sentry automatically groups similar events together into an issue. Similarity is
-            determined by stacktrace and other factors. [link:Learn more].`,
-            {
-              link: (
-                <ExternalLink href="https://docs.sentry.io/data-management/rollups/" />
-              ),
-            }
-          ),
-        },
-      ],
-    },
-    {
-      guide: 'discover_sidebar',
-      requiredTargets: ['discover-sidebar'],
-      steps: [
-        {
-          title: t('Event Pages have moved'),
-          target: 'discover-sidebar',
-          description: tct(
-            `These are now in our powerful new query builder, Discover.
-            [link:Learn more about its advanced features].`,
-            {
-              link: <ExternalLink href="https://docs.sentry.io/workflow/discover2/" />,
-            }
-          ),
-        },
-      ],
-    },
-  ];
-}

src/sentry/static/sentry/app/components/assistant/guideAnchor.tsx → src/sentry/static/sentry/app/components/assistant/guideAnchor.jsx

@@ -2,9 +2,9 @@ import {ClassNames} from '@emotion/core';
 import PropTypes from 'prop-types';
 import React from 'react';
 import styled from '@emotion/styled';
+import $ from 'jquery';
 import createReactClass from 'create-react-class';
 import Reflux from 'reflux';
-import * as Sentry from '@sentry/browser';
 
 import theme from 'app/utils/theme';
 import {
@@ -15,94 +15,78 @@ import {
   recordFinish,
   dismissGuide,
 } from 'app/actionCreators/guides';
-import {CloseIcon} from 'app/components/assistant/styles';
-import {Guide} from 'app/components/assistant/types';
-import {t} from 'app/locale';
 import GuideStore from 'app/stores/guideStore';
 import Hovercard from 'app/components/hovercard';
 import Button from 'app/components/button';
 import space from 'app/styles/space';
+import {t} from 'app/locale';
+import {CloseIcon} from 'app/components/assistant/styles';
 
-type Props = {
-  target: string;
-  position?: string;
-};
-
-type State = {
-  active: boolean;
-  orgId: string | null;
-  currentGuide?: Guide;
-  step?: number;
-};
-
-/**
- * A GuideAnchor puts an informative hovercard around an element.
- * Guide anchors register with the GuideStore, which uses registrations
- * from one or more anchors on the page to determine which guides can
- * be shown on the page.
- */
-const GuideAnchor = createReactClass<Props, State>({
+// A GuideAnchor puts an informative hovercard around an element.
+// Guide anchors register with the GuideStore, which uses registrations
+// from one or more anchors on the page to determine which guides can
+// be shown on the page.
+const GuideAnchor = createReactClass({
   propTypes: {
     target: PropTypes.string.isRequired,
     position: PropTypes.string,
   },
 
-  mixins: [Reflux.listenTo(GuideStore, 'onGuideStateChange') as any],
+  mixins: [Reflux.listenTo(GuideStore, 'onGuideStateChange')],
 
   getInitialState() {
     return {
       active: false,
-      orgId: null,
     };
   },
 
   componentDidMount() {
-    const {target} = this.props;
-    target && registerAnchor(target);
+    registerAnchor(this);
   },
 
   componentDidUpdate(_prevProps, prevState) {
     if (this.containerElement && !prevState.active && this.state.active) {
-      try {
-        const {top} = this.containerElement.getBoundingClientRect();
-        const scrollTop = window.pageYOffset;
-        const centerElement = top + scrollTop - window.innerHeight / 2;
-        window.scrollTo({top: centerElement});
-      } catch (err) {
-        Sentry.captureException(err);
-      }
+      const windowHeight = $(window).height();
+      $('html,body').animate({
+        scrollTop: $(this.containerElement).offset().top - windowHeight / 2,
+      });
     }
   },
 
   componentWillUnmount() {
-    const {target} = this.props;
-    target && unregisterAnchor(target);
+    unregisterAnchor(this);
   },
 
   onGuideStateChange(data) {
     const active =
       data.currentGuide &&
       data.currentGuide.steps[data.currentStep].target === this.props.target;
-
     this.setState({
       active,
-      currentGuide: data.currentGuide,
+      guide: data.currentGuide,
       step: data.currentStep,
-      orgId: data.orgId,
+      org: data.org,
+      messageVariables: {
+        orgSlug: data.org && data.org.slug,
+        projectSlug: data.project && data.project.slug,
+      },
     });
   },
 
-  /**
-   * Terminology:
-   *
-   *  - 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
-   */
+  interpolate(template, variables) {
+    const regex = /\${([^{]+)}/g;
+    return template.replace(regex, (_match, g1) => variables[g1.trim()]);
+  },
+
+  /* Terminology:
+   - 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(e) {
     e.stopPropagation();
-    const {currentGuide, orgId} = this.state;
-    recordFinish(currentGuide.guide, orgId);
+    const {guide, org} = this.state;
+    recordFinish(guide.id, org);
     closeGuide();
   },
 
@@ -113,12 +97,12 @@ const GuideAnchor = createReactClass<Props, State>({
 
   handleDismiss(e) {
     e.stopPropagation();
-    const {currentGuide, step, orgId} = this.state;
-    dismissGuide(currentGuide.guide, step, orgId);
+    const {guide, step, org} = this.state;
+    dismissGuide(guide.id, step, org);
   },
 
   render() {
-    const {active, currentGuide, step} = this.state;
+    const {active, guide, step, messageVariables} = this.state;
     if (!active) {
       return this.props.children ? this.props.children : null;
     }
@@ -126,24 +110,28 @@ const GuideAnchor = createReactClass<Props, State>({
     const body = (
       <GuideContainer>
         <GuideInputRow>
-          <StyledTitle>{currentGuide.steps[step].title}</StyledTitle>
-          {step < currentGuide.steps.length - 1 && (
+          <StyledTitle>{guide.steps[step].title}</StyledTitle>
+          {step < guide.steps.length - 1 && (
             <CloseLink onClick={this.handleDismiss} href="#" data-test-id="close-button">
               <CloseIcon />
             </CloseLink>
           )}
         </GuideInputRow>
         <StyledContent>
-          <div>{currentGuide.steps[step].description}</div>
+          <div
+            dangerouslySetInnerHTML={{
+              __html: this.interpolate(guide.steps[step].message, messageVariables),
+            }}
+          />
           <Actions>
             <div>
-              {step < currentGuide.steps.length - 1 ? (
+              {step < guide.steps.length - 1 ? (
                 <Button priority="success" size="small" onClick={this.handleNextStep}>
-                  {t('Next')}
+                  {t('Next')} &rarr;
                 </Button>
               ) : (
                 <Button priority="success" size="small" onClick={this.handleFinish}>
-                  {t(currentGuide.steps.length === 1 ? 'Got It' : 'Done')}
+                  {t(guide.steps.length === 1 ? 'Got It' : 'Done')}
                 </Button>
               )}
             </div>

src/sentry/static/sentry/app/components/assistant/types.tsx

@@ -1,33 +0,0 @@
-export type GuideStep = {
-  title: string;
-  /**
-   * Step is tied to an anchor target. If the anchor doesn't exist,
-   * the step will not be shown. If the anchor exists but is of type
-   * "invisible", it will not be pinged but will be scrolled to.
-   * Otherwise the anchor will be pinged and scrolled to.
-   */
-  target: string;
-  description: React.ReactNode;
-};
-
-export type Guide = {
-  guide: string;
-  requiredTargets: string[];
-  steps: GuideStep[];
-  seen?: boolean;
-};
-
-export type GuidesContent = {
-  guide: string;
-  /**
-   * Anchor targets required on the page. An empty list will cause the
-   * guide to be shown regardless.
-   */
-  requiredTargets: string[];
-  steps: GuideStep[];
-}[];
-
-export type GuidesServerData = {
-  guide: string;
-  seen: boolean;
-}[];

src/sentry/static/sentry/app/components/sidebar/index.jsx

@@ -365,7 +365,7 @@ class Sidebar extends React.Component {
                       features={['discover-basic']}
                       organization={organization}
                     >
-                      <GuideAnchor position="right" target="discover-sidebar">
+                      <GuideAnchor position="right" target="discover_sidebar">
                         <SidebarItem
                           {...sidebarItemProps}
                           onClick={(_id, evt) =>

src/sentry/static/sentry/app/components/stream/group.jsx

@@ -110,7 +110,7 @@ const StreamGroup = createReactClass({
           <EventOrGroupHeader data={data} query={query} />
           <EventOrGroupExtraDetails {...data} />
         </GroupSummary>
-        {hasGuideAnchor && <GuideAnchor target="issue-stream" />}
+        {hasGuideAnchor && <GuideAnchor target="issue_stream" />}
         <Box width={160} mx={2} className="hidden-xs hidden-sm">
           <GroupChart id={data.id} statsPeriod={this.props.statsPeriod} data={data} />
         </Box>