From ac8af3d6debce3862f74b3bd394e2cfc12bb05b7 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Fri, 1 Dec 2023 16:22:41 +0530 Subject: [PATCH] urls: Add a new endpoint for hotspot and deprecate the old one. This commit adds a new endpoint 'users/me/onboarding_steps' deprecating the older 'users/me/hotspots' to mark hotspot as read. We also renamed the view `mark_hotspot_as_read` to `mark_onboarding_step_as_read`. Reason: Our plan is to make this endpoint flexible to support other types of UI elements not just restricted to hotspots. --- api_docs/changelog.md | 10 ++++++++++ tools/test-js-with-node | 1 + version.py | 2 +- web/src/hotspots.js | 16 ++-------------- web/src/onboarding_steps.js | 18 ++++++++++++++++++ zerver/actions/hotspots.py | 4 ++-- zerver/lib/hotspots.py | 1 + zerver/tests/test_decorators.py | 2 +- zerver/tests/test_events.py | 6 +++--- zerver/tests/test_hotspots.py | 26 ++++++++++++++------------ zerver/views/hotspots.py | 16 +++++++++------- zproject/urls.py | 8 ++++---- 12 files changed, 66 insertions(+), 44 deletions(-) create mode 100644 web/src/onboarding_steps.js diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 3336e9e979..14cb232560 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,16 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 8.0 +**Feature level 233** + +* `POST /users/me/onboarding_steps`: Added a new endpoint that + deprecates the `/users/me/hotspots` endpoint. Added support for + displaying one-time notices in addition to existing hotspots. + This is now used as a common endpoint to mark both types of + onboarding steps, i.e., 'hotspot' and 'one_time_notice'. + There is no compatibility support for `/users/me/hotspots` as + no client other than web app has this feature currently. + **Feature level 232** * [`POST /register`](/api/register-queue): Added a new diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 7e7b804f7f..1a4f5b22a4 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -154,6 +154,7 @@ EXEMPT_FILES = make_set( "web/src/navbar_help_menu.js", "web/src/navbar_menus.js", "web/src/navigate.js", + "web/src/onboarding_steps.js", "web/src/overlay_util.ts", "web/src/overlays.ts", "web/src/padded_widget.ts", diff --git a/version.py b/version.py index 63f62bafd2..0c9c9e7995 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # Changes should be accompanied by documentation explaining what the # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 232 +API_FEATURE_LEVEL = 233 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/web/src/hotspots.js b/web/src/hotspots.js index d37a7d5de9..3c723c905a 100644 --- a/web/src/hotspots.js +++ b/web/src/hotspots.js @@ -6,8 +6,8 @@ import render_hotspot_icon from "../templates/hotspot_icon.hbs"; import render_hotspot_overlay from "../templates/hotspot_overlay.hbs"; import * as blueslip from "./blueslip"; -import * as channel from "./channel"; import * as message_viewport from "./message_viewport"; +import * as onboarding_steps from "./onboarding_steps"; import * as overlays from "./overlays"; import {page_params} from "./page_params"; @@ -104,19 +104,7 @@ function compute_placement($elt, popover_height, popover_width, prefer_vertical_ } export function post_hotspot_as_read(hotspot_name) { - channel.post({ - url: "/json/users/me/hotspots", - data: {hotspot: hotspot_name}, - error(err) { - if (err.readyState !== 0) { - blueslip.error("Failed to fetch hotspots", { - readyState: err.readyState, - status: err.status, - body: err.responseText, - }); - } - }, - }); + onboarding_steps.post_onboarding_step_as_read(hotspot_name); } function place_icon(hotspot) { diff --git a/web/src/onboarding_steps.js b/web/src/onboarding_steps.js new file mode 100644 index 0000000000..844e4ffdc8 --- /dev/null +++ b/web/src/onboarding_steps.js @@ -0,0 +1,18 @@ +import * as blueslip from "./blueslip"; +import * as channel from "./channel"; + +export function post_onboarding_step_as_read(onboarding_step_name) { + channel.post({ + url: "/json/users/me/onboarding_steps", + data: {onboarding_step: onboarding_step_name}, + error(err) { + if (err.readyState !== 0) { + blueslip.error("Failed to fetch onboarding steps", { + readyState: err.readyState, + status: err.status, + body: err.responseText, + }); + } + }, + }); +} diff --git a/zerver/actions/hotspots.py b/zerver/actions/hotspots.py index 35daff66c5..33a3bbd27c 100644 --- a/zerver/actions/hotspots.py +++ b/zerver/actions/hotspots.py @@ -3,7 +3,7 @@ from zerver.models import OnboardingStep, UserProfile from zerver.tornado.django_api import send_event -def do_mark_hotspot_as_read(user: UserProfile, hotspot: str) -> None: - OnboardingStep.objects.get_or_create(user=user, onboarding_step=hotspot) +def do_mark_onboarding_step_as_read(user: UserProfile, onboarding_step: str) -> None: + OnboardingStep.objects.get_or_create(user=user, onboarding_step=onboarding_step) event = dict(type="hotspots", hotspots=get_next_hotspots(user)) send_event(user.realm, event, [user.id]) diff --git a/zerver/lib/hotspots.py b/zerver/lib/hotspots.py index 6b104fba44..cf1ad7539e 100644 --- a/zerver/lib/hotspots.py +++ b/zerver/lib/hotspots.py @@ -70,6 +70,7 @@ NON_INTRO_HOTSPOTS: List[Hotspot] = [] # a part of the initial tutorial. To that end, classifying them into # categories which are aggregated in ALL_HOTSPOTS, seems like a good start. ALL_HOTSPOTS = [*INTRO_HOTSPOTS, *NON_INTRO_HOTSPOTS] +ALL_ONBOARDING_STEPS = ALL_HOTSPOTS def get_next_hotspots(user: UserProfile) -> List[Dict[str, Union[str, float, bool]]]: diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py index a9ecd9c1ff..af666bed0b 100644 --- a/zerver/tests/test_decorators.py +++ b/zerver/tests/test_decorators.py @@ -1083,7 +1083,7 @@ class TestHumanUsersOnlyDecorator(ZulipTestCase): post_endpoints = [ "/api/v1/users/me/apns_device_token", "/api/v1/users/me/android_gcm_reg_id", - "/api/v1/users/me/hotspots", + "/api/v1/users/me/onboarding_steps", "/api/v1/users/me/presence", "/api/v1/users/me/tutorial_status", ] diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 81fa8c8ab1..efbe534014 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -43,7 +43,7 @@ from zerver.actions.default_streams import ( do_remove_streams_from_default_stream_group, lookup_default_stream_groups, ) -from zerver.actions.hotspots import do_mark_hotspot_as_read +from zerver.actions.hotspots import do_mark_onboarding_step_as_read from zerver.actions.invites import ( do_create_multiuse_invite_link, do_invite_users, @@ -3048,12 +3048,12 @@ class NormalActionsTest(BaseAction): events = self.verify_action(action, state_change_expected=False) check_realm_deactivated("events[0]", events[0]) - def test_do_mark_hotspot_as_read(self) -> None: + def test_do_mark_onboarding_step_as_read(self) -> None: self.user_profile.tutorial_status = UserProfile.TUTORIAL_WAITING self.user_profile.save(update_fields=["tutorial_status"]) events = self.verify_action( - lambda: do_mark_hotspot_as_read(self.user_profile, "intro_streams") + lambda: do_mark_onboarding_step_as_read(self.user_profile, "intro_streams") ) check_hotspots("events[0]", events[0]) diff --git a/zerver/tests/test_hotspots.py b/zerver/tests/test_hotspots.py index 3c598da799..69ac490531 100644 --- a/zerver/tests/test_hotspots.py +++ b/zerver/tests/test_hotspots.py @@ -1,7 +1,7 @@ from typing_extensions import override from zerver.actions.create_user import do_create_user -from zerver.actions.hotspots import do_mark_hotspot_as_read +from zerver.actions.hotspots import do_mark_onboarding_step_as_read from zerver.lib.hotspots import ALL_HOTSPOTS, INTRO_HOTSPOTS, NON_INTRO_HOTSPOTS, get_next_hotspots from zerver.lib.test_classes import ZulipTestCase from zerver.models import OnboardingStep, UserProfile, get_realm @@ -23,8 +23,8 @@ class TestGetNextHotspots(ZulipTestCase): self.assertEqual(hotspots[0]["name"], "intro_streams") def test_some_done_some_not(self) -> None: - do_mark_hotspot_as_read(self.user, "intro_streams") - do_mark_hotspot_as_read(self.user, "intro_compose") + do_mark_onboarding_step_as_read(self.user, "intro_streams") + do_mark_onboarding_step_as_read(self.user, "intro_compose") hotspots = get_next_hotspots(self.user) self.assert_length(hotspots, 1) self.assertEqual(hotspots[0]["name"], "intro_topics") @@ -33,11 +33,11 @@ class TestGetNextHotspots(ZulipTestCase): with self.settings(TUTORIAL_ENABLED=True): self.assertNotEqual(self.user.tutorial_status, UserProfile.TUTORIAL_FINISHED) for hotspot in NON_INTRO_HOTSPOTS: # nocoverage - do_mark_hotspot_as_read(self.user, hotspot.name) + do_mark_onboarding_step_as_read(self.user, hotspot.name) self.assertNotEqual(self.user.tutorial_status, UserProfile.TUTORIAL_FINISHED) for hotspot in INTRO_HOTSPOTS: - do_mark_hotspot_as_read(self.user, hotspot.name) + do_mark_onboarding_step_as_read(self.user, hotspot.name) self.assertEqual(self.user.tutorial_status, UserProfile.TUTORIAL_FINISHED) self.assertEqual(get_next_hotspots(self.user), []) @@ -51,10 +51,10 @@ class TestGetNextHotspots(ZulipTestCase): self.assertEqual(get_next_hotspots(self.user), []) -class TestHotspots(ZulipTestCase): - def test_do_mark_hotspot_as_read(self) -> None: +class TestOnboardingSteps(ZulipTestCase): + def test_do_mark_onboarding_step_as_read(self) -> None: user = self.example_user("hamlet") - do_mark_hotspot_as_read(user, "intro_compose") + do_mark_onboarding_step_as_read(user, "intro_compose") self.assertEqual( list( OnboardingStep.objects.filter(user=user).values_list("onboarding_step", flat=True) @@ -62,10 +62,12 @@ class TestHotspots(ZulipTestCase): ["intro_compose"], ) - def test_hotspots_url_endpoint(self) -> None: + def test_onboarding_steps_url_endpoint(self) -> None: user = self.example_user("hamlet") self.login_user(user) - result = self.client_post("/json/users/me/hotspots", {"hotspot": "intro_streams"}) + result = self.client_post( + "/json/users/me/onboarding_steps", {"onboarding_step": "intro_streams"} + ) self.assert_json_success(result) self.assertEqual( list( @@ -74,8 +76,8 @@ class TestHotspots(ZulipTestCase): ["intro_streams"], ) - result = self.client_post("/json/users/me/hotspots", {"hotspot": "invalid"}) - self.assert_json_error(result, "Unknown hotspot: invalid") + result = self.client_post("/json/users/me/onboarding_steps", {"onboarding_step": "invalid"}) + self.assert_json_error(result, "Unknown onboarding_step: invalid") self.assertEqual( list( OnboardingStep.objects.filter(user=user).values_list("onboarding_step", flat=True) diff --git a/zerver/views/hotspots.py b/zerver/views/hotspots.py index dbc7d7b237..9cafd56dfe 100644 --- a/zerver/views/hotspots.py +++ b/zerver/views/hotspots.py @@ -1,10 +1,10 @@ from django.http import HttpRequest, HttpResponse from django.utils.translation import gettext as _ -from zerver.actions.hotspots import do_mark_hotspot_as_read +from zerver.actions.hotspots import do_mark_onboarding_step_as_read from zerver.decorator import human_users_only from zerver.lib.exceptions import JsonableError -from zerver.lib.hotspots import ALL_HOTSPOTS +from zerver.lib.hotspots import ALL_ONBOARDING_STEPS from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success from zerver.models import UserProfile @@ -12,10 +12,12 @@ from zerver.models import UserProfile @human_users_only @has_request_variables -def mark_hotspot_as_read( - request: HttpRequest, user: UserProfile, hotspot: str = REQ() +def mark_onboarding_step_as_read( + request: HttpRequest, user: UserProfile, onboarding_step: str = REQ() ) -> HttpResponse: - if not any(h.name == hotspot for h in ALL_HOTSPOTS): - raise JsonableError(_("Unknown hotspot: {hotspot}").format(hotspot=hotspot)) - do_mark_hotspot_as_read(user, hotspot) + if not any(step.name == onboarding_step for step in ALL_ONBOARDING_STEPS): + raise JsonableError( + _("Unknown onboarding_step: {onboarding_step}").format(onboarding_step=onboarding_step) + ) + do_mark_onboarding_step_as_read(user, onboarding_step) return json_success(request) diff --git a/zproject/urls.py b/zproject/urls.py index 167acb1d4d..8384480e39 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -56,7 +56,7 @@ from zerver.views.email_mirror import email_mirror_message from zerver.views.events_register import events_register_backend from zerver.views.health import health from zerver.views.home import accounts_accept_terms, desktop_home, home -from zerver.views.hotspots import mark_hotspot_as_read +from zerver.views.hotspots import mark_onboarding_step_as_read from zerver.views.invite import ( generate_multiuse_invite_backend, get_user_invites, @@ -411,11 +411,11 @@ v1_api_and_json_patterns = [ ), # users/me -> zerver.views.user_settings rest_path("users/me/avatar", POST=set_avatar_backend, DELETE=delete_avatar_backend), - # users/me/hotspots -> zerver.views.hotspots + # users/me/onboarding_steps -> zerver.views.hotspots rest_path( - "users/me/hotspots", + "users/me/onboarding_steps", POST=( - mark_hotspot_as_read, + mark_onboarding_step_as_read, # This endpoint is low priority for documentation as # it is part of the web app-specific tutorial. {"intentionally_undocumented"},