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.
This commit is contained in:
Prakhar Pratyush 2023-12-01 16:22:41 +05:30 committed by Tim Abbott
parent 62bfc20ebc
commit ac8af3d6de
12 changed files with 66 additions and 44 deletions

View File

@ -20,6 +20,16 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 8.0 ## 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** **Feature level 232**
* [`POST /register`](/api/register-queue): Added a new * [`POST /register`](/api/register-queue): Added a new

View File

@ -154,6 +154,7 @@ EXEMPT_FILES = make_set(
"web/src/navbar_help_menu.js", "web/src/navbar_help_menu.js",
"web/src/navbar_menus.js", "web/src/navbar_menus.js",
"web/src/navigate.js", "web/src/navigate.js",
"web/src/onboarding_steps.js",
"web/src/overlay_util.ts", "web/src/overlay_util.ts",
"web/src/overlays.ts", "web/src/overlays.ts",
"web/src/padded_widget.ts", "web/src/padded_widget.ts",

View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
# Changes should be accompanied by documentation explaining what the # Changes should be accompanied by documentation explaining what the
# new level means in api_docs/changelog.md, as well as "**Changes**" # new level means in api_docs/changelog.md, as well as "**Changes**"
# entries in the endpoint's documentation in `zulip.yaml`. # 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 # 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 # only when going from an old version of the code to a newer version. Bump

View File

@ -6,8 +6,8 @@ import render_hotspot_icon from "../templates/hotspot_icon.hbs";
import render_hotspot_overlay from "../templates/hotspot_overlay.hbs"; import render_hotspot_overlay from "../templates/hotspot_overlay.hbs";
import * as blueslip from "./blueslip"; import * as blueslip from "./blueslip";
import * as channel from "./channel";
import * as message_viewport from "./message_viewport"; import * as message_viewport from "./message_viewport";
import * as onboarding_steps from "./onboarding_steps";
import * as overlays from "./overlays"; import * as overlays from "./overlays";
import {page_params} from "./page_params"; 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) { export function post_hotspot_as_read(hotspot_name) {
channel.post({ onboarding_steps.post_onboarding_step_as_read(hotspot_name);
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,
});
}
},
});
} }
function place_icon(hotspot) { function place_icon(hotspot) {

View File

@ -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,
});
}
},
});
}

View File

@ -3,7 +3,7 @@ from zerver.models import OnboardingStep, UserProfile
from zerver.tornado.django_api import send_event from zerver.tornado.django_api import send_event
def do_mark_hotspot_as_read(user: UserProfile, hotspot: str) -> None: def do_mark_onboarding_step_as_read(user: UserProfile, onboarding_step: str) -> None:
OnboardingStep.objects.get_or_create(user=user, onboarding_step=hotspot) OnboardingStep.objects.get_or_create(user=user, onboarding_step=onboarding_step)
event = dict(type="hotspots", hotspots=get_next_hotspots(user)) event = dict(type="hotspots", hotspots=get_next_hotspots(user))
send_event(user.realm, event, [user.id]) send_event(user.realm, event, [user.id])

View File

@ -70,6 +70,7 @@ NON_INTRO_HOTSPOTS: List[Hotspot] = []
# a part of the initial tutorial. To that end, classifying them into # a part of the initial tutorial. To that end, classifying them into
# categories which are aggregated in ALL_HOTSPOTS, seems like a good start. # categories which are aggregated in ALL_HOTSPOTS, seems like a good start.
ALL_HOTSPOTS = [*INTRO_HOTSPOTS, *NON_INTRO_HOTSPOTS] 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]]]: def get_next_hotspots(user: UserProfile) -> List[Dict[str, Union[str, float, bool]]]:

View File

@ -1083,7 +1083,7 @@ class TestHumanUsersOnlyDecorator(ZulipTestCase):
post_endpoints = [ post_endpoints = [
"/api/v1/users/me/apns_device_token", "/api/v1/users/me/apns_device_token",
"/api/v1/users/me/android_gcm_reg_id", "/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/presence",
"/api/v1/users/me/tutorial_status", "/api/v1/users/me/tutorial_status",
] ]

View File

@ -43,7 +43,7 @@ from zerver.actions.default_streams import (
do_remove_streams_from_default_stream_group, do_remove_streams_from_default_stream_group,
lookup_default_stream_groups, 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 ( from zerver.actions.invites import (
do_create_multiuse_invite_link, do_create_multiuse_invite_link,
do_invite_users, do_invite_users,
@ -3048,12 +3048,12 @@ class NormalActionsTest(BaseAction):
events = self.verify_action(action, state_change_expected=False) events = self.verify_action(action, state_change_expected=False)
check_realm_deactivated("events[0]", events[0]) 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.tutorial_status = UserProfile.TUTORIAL_WAITING
self.user_profile.save(update_fields=["tutorial_status"]) self.user_profile.save(update_fields=["tutorial_status"])
events = self.verify_action( 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]) check_hotspots("events[0]", events[0])

View File

@ -1,7 +1,7 @@
from typing_extensions import override from typing_extensions import override
from zerver.actions.create_user import do_create_user 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.hotspots import ALL_HOTSPOTS, INTRO_HOTSPOTS, NON_INTRO_HOTSPOTS, get_next_hotspots
from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_classes import ZulipTestCase
from zerver.models import OnboardingStep, UserProfile, get_realm from zerver.models import OnboardingStep, UserProfile, get_realm
@ -23,8 +23,8 @@ class TestGetNextHotspots(ZulipTestCase):
self.assertEqual(hotspots[0]["name"], "intro_streams") self.assertEqual(hotspots[0]["name"], "intro_streams")
def test_some_done_some_not(self) -> None: def test_some_done_some_not(self) -> None:
do_mark_hotspot_as_read(self.user, "intro_streams") do_mark_onboarding_step_as_read(self.user, "intro_streams")
do_mark_hotspot_as_read(self.user, "intro_compose") do_mark_onboarding_step_as_read(self.user, "intro_compose")
hotspots = get_next_hotspots(self.user) hotspots = get_next_hotspots(self.user)
self.assert_length(hotspots, 1) self.assert_length(hotspots, 1)
self.assertEqual(hotspots[0]["name"], "intro_topics") self.assertEqual(hotspots[0]["name"], "intro_topics")
@ -33,11 +33,11 @@ class TestGetNextHotspots(ZulipTestCase):
with self.settings(TUTORIAL_ENABLED=True): with self.settings(TUTORIAL_ENABLED=True):
self.assertNotEqual(self.user.tutorial_status, UserProfile.TUTORIAL_FINISHED) self.assertNotEqual(self.user.tutorial_status, UserProfile.TUTORIAL_FINISHED)
for hotspot in NON_INTRO_HOTSPOTS: # nocoverage 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) self.assertNotEqual(self.user.tutorial_status, UserProfile.TUTORIAL_FINISHED)
for hotspot in INTRO_HOTSPOTS: 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(self.user.tutorial_status, UserProfile.TUTORIAL_FINISHED)
self.assertEqual(get_next_hotspots(self.user), []) self.assertEqual(get_next_hotspots(self.user), [])
@ -51,10 +51,10 @@ class TestGetNextHotspots(ZulipTestCase):
self.assertEqual(get_next_hotspots(self.user), []) self.assertEqual(get_next_hotspots(self.user), [])
class TestHotspots(ZulipTestCase): class TestOnboardingSteps(ZulipTestCase):
def test_do_mark_hotspot_as_read(self) -> None: def test_do_mark_onboarding_step_as_read(self) -> None:
user = self.example_user("hamlet") user = self.example_user("hamlet")
do_mark_hotspot_as_read(user, "intro_compose") do_mark_onboarding_step_as_read(user, "intro_compose")
self.assertEqual( self.assertEqual(
list( list(
OnboardingStep.objects.filter(user=user).values_list("onboarding_step", flat=True) OnboardingStep.objects.filter(user=user).values_list("onboarding_step", flat=True)
@ -62,10 +62,12 @@ class TestHotspots(ZulipTestCase):
["intro_compose"], ["intro_compose"],
) )
def test_hotspots_url_endpoint(self) -> None: def test_onboarding_steps_url_endpoint(self) -> None:
user = self.example_user("hamlet") user = self.example_user("hamlet")
self.login_user(user) 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.assert_json_success(result)
self.assertEqual( self.assertEqual(
list( list(
@ -74,8 +76,8 @@ class TestHotspots(ZulipTestCase):
["intro_streams"], ["intro_streams"],
) )
result = self.client_post("/json/users/me/hotspots", {"hotspot": "invalid"}) result = self.client_post("/json/users/me/onboarding_steps", {"onboarding_step": "invalid"})
self.assert_json_error(result, "Unknown hotspot: invalid") self.assert_json_error(result, "Unknown onboarding_step: invalid")
self.assertEqual( self.assertEqual(
list( list(
OnboardingStep.objects.filter(user=user).values_list("onboarding_step", flat=True) OnboardingStep.objects.filter(user=user).values_list("onboarding_step", flat=True)

View File

@ -1,10 +1,10 @@
from django.http import HttpRequest, HttpResponse from django.http import HttpRequest, HttpResponse
from django.utils.translation import gettext as _ 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.decorator import human_users_only
from zerver.lib.exceptions import JsonableError 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.request import REQ, has_request_variables
from zerver.lib.response import json_success from zerver.lib.response import json_success
from zerver.models import UserProfile from zerver.models import UserProfile
@ -12,10 +12,12 @@ from zerver.models import UserProfile
@human_users_only @human_users_only
@has_request_variables @has_request_variables
def mark_hotspot_as_read( def mark_onboarding_step_as_read(
request: HttpRequest, user: UserProfile, hotspot: str = REQ() request: HttpRequest, user: UserProfile, onboarding_step: str = REQ()
) -> HttpResponse: ) -> HttpResponse:
if not any(h.name == hotspot for h in ALL_HOTSPOTS): if not any(step.name == onboarding_step for step in ALL_ONBOARDING_STEPS):
raise JsonableError(_("Unknown hotspot: {hotspot}").format(hotspot=hotspot)) raise JsonableError(
do_mark_hotspot_as_read(user, hotspot) _("Unknown onboarding_step: {onboarding_step}").format(onboarding_step=onboarding_step)
)
do_mark_onboarding_step_as_read(user, onboarding_step)
return json_success(request) return json_success(request)

View File

@ -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.events_register import events_register_backend
from zerver.views.health import health from zerver.views.health import health
from zerver.views.home import accounts_accept_terms, desktop_home, home 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 ( from zerver.views.invite import (
generate_multiuse_invite_backend, generate_multiuse_invite_backend,
get_user_invites, get_user_invites,
@ -411,11 +411,11 @@ v1_api_and_json_patterns = [
), ),
# users/me -> zerver.views.user_settings # users/me -> zerver.views.user_settings
rest_path("users/me/avatar", POST=set_avatar_backend, DELETE=delete_avatar_backend), 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( rest_path(
"users/me/hotspots", "users/me/onboarding_steps",
POST=( POST=(
mark_hotspot_as_read, mark_onboarding_step_as_read,
# This endpoint is low priority for documentation as # This endpoint is low priority for documentation as
# it is part of the web app-specific tutorial. # it is part of the web app-specific tutorial.
{"intentionally_undocumented"}, {"intentionally_undocumented"},