From 52a9846cdf4abfbe937a94559690d508e95f4065 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Thu, 25 Jul 2024 12:37:44 +0530 Subject: [PATCH] user_profile: Remove 'tutorial_status' field. The 'tutorial_status' field on 'UserProfile' model is no longer used to show onboarding tutorial. This commit removes the 'tutorial_status' field, 'POST users/me/tutorial_status' endpoint, and 'needs_tutorial' parameter in 'page_params'. Fixes part of zulip#30043. --- api_docs/changelog.md | 6 +++++ version.py | 2 +- web/src/base_page_params.ts | 1 - web/src/setup.ts | 15 +++---------- web/src/tutorial.js | 16 -------------- web/src/ui_init.js | 4 ---- zerver/lib/bulk_create.py | 1 - zerver/lib/create_user.py | 2 -- zerver/lib/home.py | 2 -- zerver/lib/onboarding_steps.py | 5 ----- ...0569_remove_userprofile_tutorial_status.py | 16 ++++++++++++++ zerver/models/users.py | 16 -------------- zerver/tests/test_decorators.py | 1 - zerver/tests/test_home.py | 2 -- zerver/tests/test_signup.py | 4 ---- zerver/tests/test_tutorial.py | 16 -------------- zerver/views/home.py | 8 ------- zerver/views/tutorial.py | 22 ------------------- zproject/urls.py | 11 ---------- 19 files changed, 26 insertions(+), 124 deletions(-) delete mode 100644 web/src/tutorial.js create mode 100644 zerver/migrations/0569_remove_userprofile_tutorial_status.py delete mode 100644 zerver/views/tutorial.py diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 27b7d5402c..dcdf801c6b 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 10.0 +**Feature level 282** + +* `POST users/me/tutorial_status`: Removed this undocumented endpoint, + as the state that it maintained has been replaced by a cleaner + `onboarding_steps` implementation. + **Feature level 281** * [`GET /events`](/api/get-events), [`POST /register`](/api/register-queue): diff --git a/version.py b/version.py index 01d2abce11..93f133e1a8 100644 --- a/version.py +++ b/version.py @@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 281 # Last bumped for realm_can_delete_any_message_group +API_FEATURE_LEVEL = 282 # Last bumped for removing "POST users/me/tutorial_status" # Bump the minor PROVISION_VERSION to indicate that folks should provision diff --git a/web/src/base_page_params.ts b/web/src/base_page_params.ts index bd163cd568..95932791b2 100644 --- a/web/src/base_page_params.ts +++ b/web/src/base_page_params.ts @@ -43,7 +43,6 @@ const home_params_schema = default_params_schema narrow: z.optional(z.array(narrow_term_schema)), narrow_stream: z.optional(z.string()), narrow_topic: z.optional(z.string()), - needs_tutorial: z.boolean(), promote_sponsoring_zulip: z.boolean(), // `realm_rendered_description` is only sent for spectators, because // it isn't displayed for logged-in users and requires markdown diff --git a/web/src/setup.ts b/web/src/setup.ts index 24ccf0d83a..6e9e69881d 100644 --- a/web/src/setup.ts +++ b/web/src/setup.ts @@ -2,27 +2,18 @@ import $ from "jquery"; import * as blueslip from "./blueslip"; import * as loading from "./loading"; -import {page_params} from "./page_params"; -import * as util from "./util"; export let page_load_time: number | undefined; // Miscellaneous early setup. $(() => { - if (util.is_mobile()) { - // Disable the tutorial; it's ugly on mobile. - page_params.needs_tutorial = false; - } - page_load_time = Date.now(); // Display loading indicator. This disappears after the first // get_events completes. - if (!page_params.needs_tutorial) { - loading.make_indicator($("#page_loading_indicator"), { - abs_positioned: true, - }); - } + loading.make_indicator($("#page_loading_indicator"), { + abs_positioned: true, + }); $.fn.get_offset_to_window = function () { return this[0]!.getBoundingClientRect(); diff --git a/web/src/tutorial.js b/web/src/tutorial.js deleted file mode 100644 index 5fc8fca9c8..0000000000 --- a/web/src/tutorial.js +++ /dev/null @@ -1,16 +0,0 @@ -import * as channel from "./channel"; -import {page_params} from "./page_params"; - -function set_tutorial_status(status, callback) { - return channel.post({ - url: "/json/users/me/tutorial_status", - data: {status}, - success: callback, - }); -} - -export function initialize() { - if (page_params.needs_tutorial) { - set_tutorial_status("started"); - } -} diff --git a/web/src/ui_init.js b/web/src/ui_init.js index 3366526df8..2dbeca0222 100644 --- a/web/src/ui_init.js +++ b/web/src/ui_init.js @@ -132,7 +132,6 @@ import * as tippyjs from "./tippyjs"; import * as topic_list from "./topic_list"; import * as topic_popover from "./topic_popover"; import * as transmit from "./transmit"; -import * as tutorial from "./tutorial"; import * as typeahead_helper from "./typeahead_helper"; import * as typing from "./typing"; import * as unread from "./unread"; @@ -642,9 +641,6 @@ export function initialize_everything(state_data) { ); }, }); - // This needs to happen after activity_ui.initialize, so that user_filter - // is defined. Also, must happen after people.initialize() - tutorial.initialize(); // All overlays, and also activity_ui, must be initialized before hashchange.js hashchange.initialize(); diff --git a/zerver/lib/bulk_create.py b/zerver/lib/bulk_create.py index 5ac0b471c4..d3eabcb8f2 100644 --- a/zerver/lib/bulk_create.py +++ b/zerver/lib/bulk_create.py @@ -61,7 +61,6 @@ def bulk_create_users( tos_version, timezone, default_language=realm.default_language, - tutorial_status=UserProfile.TUTORIAL_FINISHED, email_address_visibility=email_address_visibility, ) diff --git a/zerver/lib/create_user.py b/zerver/lib/create_user.py index f3ea7979b3..72fe62d811 100644 --- a/zerver/lib/create_user.py +++ b/zerver/lib/create_user.py @@ -92,7 +92,6 @@ def create_user_profile( tos_version: str | None, timezone: str, default_language: str, - tutorial_status: str = UserProfile.TUTORIAL_WAITING, force_id: int | None = None, force_date_joined: datetime | None = None, *, @@ -122,7 +121,6 @@ def create_user_profile( is_mirror_dummy=is_mirror_dummy, tos_version=tos_version, timezone=timezone, - tutorial_status=tutorial_status, default_language=default_language, delivery_email=email, email_address_visibility=email_address_visibility, diff --git a/zerver/lib/home.py b/zerver/lib/home.py index 47dcd5bc37..9ecfd07afc 100644 --- a/zerver/lib/home.py +++ b/zerver/lib/home.py @@ -140,7 +140,6 @@ def build_page_params_for_home_page_load( narrow: list[NarrowTerm], narrow_stream: Stream | None, narrow_topic_name: str | None, - needs_tutorial: bool, ) -> tuple[int, dict[str, object]]: """ This function computes page_params for when we load the home page. @@ -211,7 +210,6 @@ def build_page_params_for_home_page_load( corporate_enabled=settings.CORPORATE_ENABLED, ## Misc. extra data. language_list=get_language_list(), - needs_tutorial=needs_tutorial, furthest_read_time=furthest_read_time, bot_types=get_bot_types(user_profile), two_fa_enabled=two_fa_enabled, diff --git a/zerver/lib/onboarding_steps.py b/zerver/lib/onboarding_steps.py index e2f85c3ffb..2b2efa0987 100644 --- a/zerver/lib/onboarding_steps.py +++ b/zerver/lib/onboarding_steps.py @@ -85,8 +85,3 @@ def copy_onboarding_steps(source_profile: UserProfile, target_profile: UserProfi onboarding_step=onboarding_step.onboarding_step, timestamp=onboarding_step.timestamp, ) - - # TODO: The 'tutorial_status' field of 'UserProfile' model - # is no longer used. Remove it. - target_profile.tutorial_status = source_profile.tutorial_status - target_profile.save(update_fields=["tutorial_status"]) diff --git a/zerver/migrations/0569_remove_userprofile_tutorial_status.py b/zerver/migrations/0569_remove_userprofile_tutorial_status.py new file mode 100644 index 0000000000..d26ff47735 --- /dev/null +++ b/zerver/migrations/0569_remove_userprofile_tutorial_status.py @@ -0,0 +1,16 @@ +# Generated by Django 5.0.6 on 2024-07-25 06:59 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0568_mark_narrow_to_dm_with_welcome_bot_new_user_as_read"), + ] + + operations = [ + migrations.RemoveField( + model_name="userprofile", + name="tutorial_status", + ), + ] diff --git a/zerver/models/users.py b/zerver/models/users.py index 574bd35659..390000f39d 100644 --- a/zerver/models/users.py +++ b/zerver/models/users.py @@ -591,22 +591,6 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings): # us, pre-thumbnailing. avatar_hash = models.CharField(null=True, max_length=64) - # TODO: TUTORIAL_STATUS was originally an optimization designed to - # allow us to skip querying the OnboardingStep table when loading - # /. This optimization is no longer effective, so it's possible we - # should delete it. - TUTORIAL_WAITING = "W" - TUTORIAL_STARTED = "S" - TUTORIAL_FINISHED = "F" - TUTORIAL_STATES = ( - (TUTORIAL_WAITING, "Waiting"), - (TUTORIAL_STARTED, "Started"), - (TUTORIAL_FINISHED, "Finished"), - ) - tutorial_status = models.CharField( - default=TUTORIAL_WAITING, choices=TUTORIAL_STATES, max_length=1 - ) - zoom_token = models.JSONField(default=None, null=True) objects = UserManager() diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py index 73f6204d6d..3a8f55cf62 100644 --- a/zerver/tests/test_decorators.py +++ b/zerver/tests/test_decorators.py @@ -1116,7 +1116,6 @@ class TestHumanUsersOnlyDecorator(ZulipTestCase): "/api/v1/users/me/android_gcm_reg_id", "/api/v1/users/me/onboarding_steps", "/api/v1/users/me/presence", - "/api/v1/users/me/tutorial_status", ] for endpoint in post_endpoints: result = self.api_post(default_bot, endpoint) diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index 514027dd70..8d8b458779 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -55,7 +55,6 @@ class HomeTest(ZulipTestCase): "login_page", "narrow", "narrow_stream", - "needs_tutorial", "no_event_queue", "page_type", "promote_sponsoring_zulip", @@ -361,7 +360,6 @@ class HomeTest(ZulipTestCase): "language_cookie_name", "language_list", "login_page", - "needs_tutorial", "no_event_queue", "page_type", "promote_sponsoring_zulip", diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 43d5696ba2..6e10c3c85e 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -2823,7 +2823,6 @@ class UserSignUpTest(ZulipTestCase): hamlet_in_zulip.emojiset = "twitter" hamlet_in_zulip.high_contrast_mode = True hamlet_in_zulip.enter_sends = True - hamlet_in_zulip.tutorial_status = UserProfile.TUTORIAL_FINISHED hamlet_in_zulip.email_address_visibility = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE hamlet_in_zulip.save() @@ -2845,7 +2844,6 @@ class UserSignUpTest(ZulipTestCase): self.assertEqual(hamlet.high_contrast_mode, False) self.assertEqual(hamlet.enable_stream_audible_notifications, False) self.assertEqual(hamlet.enter_sends, False) - self.assertEqual(hamlet.tutorial_status, UserProfile.TUTORIAL_WAITING) def test_signup_with_user_settings_from_another_realm(self) -> None: hamlet_in_zulip = self.example_user("hamlet") @@ -2863,7 +2861,6 @@ class UserSignUpTest(ZulipTestCase): hamlet_in_zulip.emojiset = "twitter" hamlet_in_zulip.high_contrast_mode = True hamlet_in_zulip.enter_sends = True - hamlet_in_zulip.tutorial_status = UserProfile.TUTORIAL_FINISHED hamlet_in_zulip.email_address_visibility = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE hamlet_in_zulip.save() @@ -2909,7 +2906,6 @@ class UserSignUpTest(ZulipTestCase): self.assertEqual(hamlet_in_lear.high_contrast_mode, True) self.assertEqual(hamlet_in_lear.enter_sends, True) self.assertEqual(hamlet_in_lear.enable_stream_audible_notifications, False) - self.assertEqual(hamlet_in_lear.tutorial_status, UserProfile.TUTORIAL_FINISHED) self.assertEqual( hamlet_in_lear.email_address_visibility, UserProfile.EMAIL_ADDRESS_VISIBILITY_NOBODY ) diff --git a/zerver/tests/test_tutorial.py b/zerver/tests/test_tutorial.py index 017005557f..ebd53ed627 100644 --- a/zerver/tests/test_tutorial.py +++ b/zerver/tests/test_tutorial.py @@ -4,7 +4,6 @@ from typing_extensions import override from zerver.actions.message_send import internal_send_private_message from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import message_stream_count, most_recent_message -from zerver.models import UserProfile from zerver.models.users import get_system_bot @@ -28,21 +27,6 @@ class TutorialTests(ZulipTestCase): disable_external_notifications=True, ) - def test_tutorial_status(self) -> None: - user = self.example_user("hamlet") - self.login_user(user) - - cases = [ - ("started", UserProfile.TUTORIAL_STARTED), - ("finished", UserProfile.TUTORIAL_FINISHED), - ] - for incoming_status, expected_db_status in cases: - params = dict(status=incoming_status) - result = self.client_post("/json/users/me/tutorial_status", params) - self.assert_json_success(result) - user = self.example_user("hamlet") - self.assertEqual(user.tutorial_status, expected_db_status) - def test_response_to_pm_for_app(self) -> None: user = self.example_user("hamlet") bot = get_system_bot(settings.WELCOME_BOT, user.realm_id) diff --git a/zerver/views/home.py b/zerver/views/home.py index d0cc4b2e7c..043776559a 100644 --- a/zerver/views/home.py +++ b/zerver/views/home.py @@ -215,13 +215,6 @@ def home_real(request: HttpRequest) -> HttpResponse: narrow, narrow_stream, narrow_topic_name = detect_narrowed_window(request, user_profile) - if user_profile is not None: - needs_tutorial = user_profile.tutorial_status == UserProfile.TUTORIAL_WAITING - - else: - # The current tutorial doesn't super make sense for logged-out users. - needs_tutorial = False - queue_id, page_params = build_page_params_for_home_page_load( request=request, user_profile=user_profile, @@ -230,7 +223,6 @@ def home_real(request: HttpRequest) -> HttpResponse: narrow=narrow, narrow_stream=narrow_stream, narrow_topic_name=narrow_topic_name, - needs_tutorial=needs_tutorial, ) log_data = RequestNotes.get_notes(request).log_data diff --git a/zerver/views/tutorial.py b/zerver/views/tutorial.py deleted file mode 100644 index 70502f9d58..0000000000 --- a/zerver/views/tutorial.py +++ /dev/null @@ -1,22 +0,0 @@ -from typing import Literal - -from django.http import HttpRequest, HttpResponse - -from zerver.decorator import human_users_only -from zerver.lib.response import json_success -from zerver.lib.typed_endpoint import typed_endpoint -from zerver.models import UserProfile - - -@human_users_only -@typed_endpoint -def set_tutorial_status( - request: HttpRequest, user_profile: UserProfile, *, status: Literal["started", "finished"] -) -> HttpResponse: - if status == "started": - user_profile.tutorial_status = UserProfile.TUTORIAL_STARTED - elif status == "finished": - user_profile.tutorial_status = UserProfile.TUTORIAL_FINISHED - user_profile.save(update_fields=["tutorial_status"]) - - return json_success(request) diff --git a/zproject/urls.py b/zproject/urls.py index e24ec43036..222ebc5ba9 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -176,7 +176,6 @@ from zerver.views.streams import ( ) from zerver.views.submessage import process_submessage from zerver.views.thumbnail import backend_serve_thumbnail -from zerver.views.tutorial import set_tutorial_status from zerver.views.typing import send_notification_backend from zerver.views.unsubscribe import email_unsubscribe from zerver.views.upload import ( @@ -426,16 +425,6 @@ v1_api_and_json_patterns = [ {"intentionally_undocumented"}, ), ), - # users/me/tutorial_status -> zerver.views.tutorial - rest_path( - "users/me/tutorial_status", - POST=( - set_tutorial_status, - # This is a relic of an old Zulip tutorial model and - # should be deleted. - {"intentionally_undocumented"}, - ), - ), # settings -> zerver.views.user_settings rest_path("settings", PATCH=json_change_settings), # These next two are legacy aliases for /settings, from before