Browse Source

Onboarding redirect (#33904)

* onboarding redirect

* fix

* added set states everywhere else except on skip

* send skipped states

* redirect on login

* added unit tests

* fix tests

* wrap logExperiment in useEffect

* add comment

* use convert args

* changes

* add test for home page redirect

* test for onlogin redirect

* add url validation

* update acceptance test
Zhixing Zhang 2 years ago
parent
commit
66b5c27a09

+ 15 - 25
src/sentry/api/endpoints/client_state.py

@@ -8,24 +8,7 @@ from rest_framework.response import Response
 
 from sentry.api.bases.organization import OrganizationEndpoint
 from sentry.utils import json, redis
-
-STATE_CATEGORIES = {
-    "onboarding": {
-        "ttl": 30 * 24 * 60 * 60,  # the time in seconds that the state will be persisted
-        "scope": "org",  # Can be "org" or "member"
-        "max_payload_size": 1024,
-    }
-}
-
-
-def get_client_state_key(organization, category, user):
-    if category not in STATE_CATEGORIES:
-        raise NotFound(detail="Category not found")
-    scope = STATE_CATEGORIES[category]["scope"]
-    if scope == "member":
-        return f"client-state:{category}:{organization}:{user.id}"
-    elif scope == "org":
-        return f"client-state:{category}:{organization}"
+from sentry.utils.client_state import STATE_CATEGORIES, get_client_state_key, get_redis_client
 
 
 class ClientStateListEndpoint(OrganizationEndpoint):
@@ -50,12 +33,21 @@ class ClientStateEndpoint(OrganizationEndpoint):
     private = True
 
     def __init__(self, **options) -> None:
-        cluster_key = getattr(settings, "SENTRY_CLIENT_STATE_REDIS_CLUSTER", "default")
-        self.client = redis.redis_clusters.get(cluster_key)
+        self.client = get_redis_client()
         super().__init__(**options)
 
-    def get(self, request: Request, organization, category) -> Response:
+    def convert_args(self, request: Request, organization_slug, *args, **kwargs):
+        (args, kwargs) = super().convert_args(request, organization_slug, *args, **kwargs)
+        organization = kwargs["organization"]
+        category = kwargs["category"]
         key = get_client_state_key(organization.slug, category, request.user)
+        if not key:
+            raise NotFound(detail="Category not found")
+
+        kwargs["key"] = key
+        return (args, kwargs)
+
+    def get(self, request: Request, organization, category, key) -> Response:
         value = self.client.get(key)
         if value:
             response = HttpResponse(value)
@@ -64,15 +56,13 @@ class ClientStateEndpoint(OrganizationEndpoint):
         else:
             return Response({})
 
-    def put(self, request: Request, organization, category):
-        key = get_client_state_key(organization.slug, category, request.user)
+    def put(self, request: Request, organization, category, key):
         data_to_write = json.dumps(request.data)
         if len(data_to_write) > STATE_CATEGORIES[category]["max_payload_size"]:
             return Response(status=413)
         self.client.setex(key, STATE_CATEGORIES[category]["ttl"], data_to_write)
         return Response(status=201)
 
-    def delete(self, request: Request, organization, category):
-        key = get_client_state_key(organization.slug, category, request.user)
+    def delete(self, request: Request, organization, category, key):
         self.client.delete(key)
         return Response(status=204)

+ 52 - 0
src/sentry/utils/client_state.py

@@ -0,0 +1,52 @@
+from django.conf import settings
+
+from sentry.utils import json, redis
+
+STATE_CATEGORIES = {
+    "onboarding": {
+        "ttl": 30 * 24 * 60 * 60,  # the time in seconds that the state will be persisted
+        "scope": "org",  # Can be "org" or "member"
+        "max_payload_size": 1024,
+    }
+}
+
+
+def get_redis_client():
+    cluster_key = getattr(settings, "SENTRY_CLIENT_STATE_REDIS_CLUSTER", "default")
+    return redis.redis_clusters.get(cluster_key)
+
+
+def get_client_state_key(organization, category, user):
+    if category not in STATE_CATEGORIES:
+        return None
+    scope = STATE_CATEGORIES[category]["scope"]
+    if scope == "member":
+        return f"client-state:{category}:{organization}:{user.id}"
+    elif scope == "org":
+        return f"client-state:{category}:{organization}"
+    return None
+
+
+def get_client_state(category, organization, user, client=None):
+    key = get_client_state_key(organization, category, user)
+    if not key:
+        return None
+    if not client:
+        client = get_redis_client()
+    value = client.get(key)
+    if not value:
+        return None
+    return json.loads(value)
+
+
+def get_client_state_redirect_uri(organization_slug, fallback_url):
+    if organization_slug:
+        state = get_client_state("onboarding", organization_slug, None)
+        if (
+            state
+            and state.get("state") != "finished"
+            and state.get("state") != "skipped"
+            and state.get("url")
+        ):
+            return f'/onboarding/{organization_slug}/{state["url"]}'
+    return fallback_url

+ 8 - 0
src/sentry/web/frontend/auth_login.py

@@ -23,6 +23,7 @@ from sentry.utils.auth import (
     is_valid_redirect,
     login,
 )
+from sentry.utils.client_state import get_client_state_redirect_uri
 from sentry.utils.sdk import capture_exception
 from sentry.utils.urls import add_params_to_url
 from sentry.web.forms.accounts import AuthenticationForm, RegistrationForm
@@ -230,6 +231,13 @@ class AuthLoginView(BaseView):
                             if om.user is None:
                                 request.session.pop("_next", None)
 
+                # On login, redirect to onboarding
+                active_org = self.get_active_organization(request)
+                if active_org:
+                    onboarding_redirect = get_client_state_redirect_uri(active_org.slug, None)
+                    if onboarding_redirect:
+                        request.session["_next"] = onboarding_redirect
+
                 return self.redirect(get_login_redirect(request))
             else:
                 metrics.incr(

+ 12 - 0
src/sentry/web/frontend/home.py

@@ -1,9 +1,21 @@
+from django.http import HttpResponseRedirect
 from rest_framework.request import Request
 from rest_framework.response import Response
 
+from sentry.utils.auth import is_valid_redirect
+from sentry.utils.client_state import get_client_state_redirect_uri
 from sentry.web.frontend.base import BaseView
 
 
 class HomeView(BaseView):
     def get(self, request: Request) -> Response:
+        # If the active organization has an ongoing onboarding session, redirect to onboarding
+        organization = self.get_active_organization(request)
+        if organization:
+            redirect_uri = get_client_state_redirect_uri(organization.slug, None)
+            if redirect_uri and is_valid_redirect(
+                redirect_uri, allowed_hosts=(request.get_host(),)
+            ):
+                return HttpResponseRedirect(redirect_uri)
+
         return self.redirect_to_org(request)

+ 7 - 5
static/app/views/onboarding/onboardingController.tsx

@@ -1,4 +1,4 @@
-import {ComponentPropsWithoutRef} from 'react';
+import {ComponentPropsWithoutRef, useEffect} from 'react';
 
 import {logExperiment} from 'sentry/utils/analytics';
 import withOrganization from 'sentry/utils/withOrganization';
@@ -9,10 +9,12 @@ import Onboarding from './onboarding';
 type Props = Omit<ComponentPropsWithoutRef<typeof Onboarding>, 'projects'>;
 
 function OnboardingController({...rest}: Props) {
-  logExperiment({
-    key: 'TargetedOnboardingMultiSelectExperiment',
-    organization: rest.organization,
-  });
+  useEffect(() => {
+    logExperiment({
+      key: 'TargetedOnboardingMultiSelectExperiment',
+      organization: rest.organization,
+    });
+  }, [rest.organization]);
   if (rest.organization?.experiments.TargetedOnboardingMultiSelectExperiment) {
     return <TargetedOnboarding {...rest} />;
   }

+ 40 - 0
tests/acceptance/test_onboarding.py

@@ -54,3 +54,43 @@ class OrganizationOnboardingTest(AcceptanceTestCase):
         self.browser.click('[data-test-id="onboarding-getting-started-invite-members"]')
         self.browser.wait_until("[role='dialog']")
         self.browser.snapshot(name="onboarding - invite members")
+
+    @mock.patch("sentry.models.ProjectKey.generate_api_key", return_value="test-dsn")
+    @mock.patch(
+        "sentry.experiments.all", return_value={"TargetedOnboardingMultiSelectExperiment": 1}
+    )
+    def test_onboarding_new(self, generate_api_key, all_experiments):
+        self.browser.get("/onboarding/%s/" % self.org.slug)
+
+        # Welcome step
+        self.browser.wait_until('[data-test-id="onboarding-step-welcome"]')
+        self.browser.snapshot(name="onboarding - new - welcome")
+
+        # Platform selection step
+        self.browser.click('[aria-label="Start"]')
+        self.browser.wait_until('[data-test-id="onboarding-step-select-platform"]')
+
+        self.browser.snapshot(name="onboarding - new - select platform")
+
+        # Select and create node JS project
+        self.browser.click('[data-test-id="platform-node"]')
+        self.browser.wait_until_not('[data-test-id="platform-select-next"][aria-disabled="true"]')
+        self.browser.wait_until('[data-test-id="platform-select-next"][aria-disabled="false"]')
+
+        @TimedRetryPolicy.wrap(timeout=5, exceptions=((TimeoutException,)))
+        def click_platform_select_name(browser):
+            browser.click('[data-test-id="platform-select-next"]')
+            # Project getting started
+            browser.wait_until('[data-test-id="onboarding-step-setup-docs"]')
+
+        click_platform_select_name(self.browser)
+        self.browser.snapshot(name="onboarding - new - setup docs")
+
+        # Verify project was created for org
+        project = Project.objects.get(organization=self.org)
+        assert project.name == "node"
+        assert project.platform == "node"
+
+        # The homepage should redirect to onboarding
+        self.browser.get("/")
+        self.browser.wait_until('[data-test-id="onboarding-step-setup-docs"]')

+ 15 - 0
tests/sentry/web/frontend/test_auth_login.py

@@ -13,6 +13,7 @@ from sentry.auth.authenticators import RecoveryCodeInterface, TotpInterface
 from sentry.models import OrganizationMember, User
 from sentry.testutils import TestCase
 from sentry.utils import json
+from sentry.utils.client_state import get_client_state_key, get_redis_client
 
 
 # TODO(dcramer): need tests for SSO behavior and single org behavior
@@ -269,6 +270,20 @@ class AuthLoginTest(TestCase):
             resp = self.client.get(self.path)
             self.assertRedirects(resp, "/organizations/new/")
 
+    def test_redirect_onboarding(self):
+        org = self.create_organization(owner=self.user)
+        key = get_client_state_key(org.slug, "onboarding", None)
+        get_redis_client().set(key, json.dumps({"state": "started", "url": "select-platform/"}))
+
+        self.client.get(self.path)
+
+        resp = self.client.post(
+            self.path, {"username": self.user.username, "password": "admin", "op": "login"}
+        )
+
+        assert resp.status_code == 302
+        assert resp.get("Location", "").endswith(f"/onboarding/{org.slug}/select-platform/")
+
 
 @pytest.mark.skipif(
     settings.SENTRY_NEWSLETTER != "sentry.newsletter.dummy.DummyNewsletter",

+ 11 - 0
tests/sentry/web/frontend/test_home.py

@@ -2,6 +2,8 @@ from django.urls import reverse
 from exam import fixture
 
 from sentry.testutils import TestCase
+from sentry.utils import json
+from sentry.utils.client_state import get_client_state_key, get_redis_client
 
 
 class HomeTest(TestCase):
@@ -39,3 +41,12 @@ class HomeTest(TestCase):
             resp = self.client.get(self.path)
 
         self.assertRedirects(resp, f"/organizations/{org.slug}/issues/")
+
+    def test_redirect_to_onboarding(self):
+        self.login_as(self.user)
+        org = self.create_organization(owner=self.user)
+
+        key = get_client_state_key(org.slug, "onboarding", None)
+        get_redis_client().set(key, json.dumps({"state": "started", "url": "select-platform/"}))
+        resp = self.client.get(self.path)
+        self.assertRedirects(resp, f"/onboarding/{org.slug}/select-platform/")