diff --git a/web/src/onboarding_steps.ts b/web/src/onboarding_steps.ts index 9ca8ced2b5..715e3301ec 100644 --- a/web/src/onboarding_steps.ts +++ b/web/src/onboarding_steps.ts @@ -2,7 +2,8 @@ import type {z} from "zod"; import * as blueslip from "./blueslip"; import * as channel from "./channel"; -import type {StateData, onboarding_step_schema} from "./state_data"; +import * as people from "./people"; +import type {NarrowTerm, StateData, onboarding_step_schema} from "./state_data"; export type OnboardingStep = z.output; @@ -34,6 +35,32 @@ export function update_onboarding_steps_to_display(onboarding_steps: OnboardingS } } -export function initialize(params: StateData["onboarding_steps"]): void { - update_onboarding_steps_to_display(params.onboarding_steps); +function narrow_to_dm_with_welcome_bot_new_user( + onboarding_steps: OnboardingStep[], + show_message_view: (raw_terms: NarrowTerm[], opts: {trigger: string}) => void, +): void { + if ( + onboarding_steps.some( + (onboarding_step) => onboarding_step.name === "narrow_to_dm_with_welcome_bot_new_user", + ) + ) { + show_message_view( + [ + { + operator: "dm", + operand: people.WELCOME_BOT.email, + }, + ], + {trigger: "sidebar"}, + ); + post_onboarding_step_as_read("narrow_to_dm_with_welcome_bot_new_user"); + } +} + +export function initialize( + params: StateData["onboarding_steps"], + show_message_view: (raw_terms: NarrowTerm[], opts: {trigger: string}) => void, +): void { + update_onboarding_steps_to_display(params.onboarding_steps); + narrow_to_dm_with_welcome_bot_new_user(params.onboarding_steps, show_message_view); } diff --git a/web/src/state_data.ts b/web/src/state_data.ts index 35e5a80e64..ac95c3c8b6 100644 --- a/web/src/state_data.ts +++ b/web/src/state_data.ts @@ -174,6 +174,11 @@ const one_time_notice_schema = z.object({ type: z.literal("one_time_notice"), }); +const one_time_action_schema = z.object({ + name: z.string(), + type: z.literal("one_time_action"), +}); + export const thumbnail_format_schema = z.object({ name: z.string(), max_width: z.number(), @@ -182,12 +187,7 @@ export const thumbnail_format_schema = z.object({ animated: z.boolean(), }); -/* We may introduce onboarding step of types other than 'one time notice' -in future. Earlier, we had 'hotspot' and 'one time notice' as the two -types. We can simply do: -const onboarding_step_schema = z.union([one_time_notice_schema, other_type_schema]); -to avoid major refactoring when new type is introduced in the future. */ -export const onboarding_step_schema = one_time_notice_schema; +export const onboarding_step_schema = z.union([one_time_notice_schema, one_time_action_schema]); // Sync this with zerver.lib.events.do_events_register. const current_user_schema = z.object({ diff --git a/web/src/tutorial.js b/web/src/tutorial.js index d13e5c403a..5fc8fca9c8 100644 --- a/web/src/tutorial.js +++ b/web/src/tutorial.js @@ -1,7 +1,5 @@ import * as channel from "./channel"; -import * as message_view from "./message_view"; import {page_params} from "./page_params"; -import * as people from "./people"; function set_tutorial_status(status, callback) { return channel.post({ @@ -14,14 +12,5 @@ function set_tutorial_status(status, callback) { export function initialize() { if (page_params.needs_tutorial) { set_tutorial_status("started"); - message_view.show( - [ - { - operator: "dm", - operand: people.WELCOME_BOT.email, - }, - ], - {trigger: "sidebar"}, - ); } } diff --git a/web/src/ui_init.js b/web/src/ui_init.js index cab35eb793..3366526df8 100644 --- a/web/src/ui_init.js +++ b/web/src/ui_init.js @@ -669,7 +669,9 @@ export function initialize_everything(state_data) { }); drafts.initialize_ui(); drafts_overlay_ui.initialize(); - onboarding_steps.initialize(state_data.onboarding_steps); + // This needs to happen after activity_ui.initialize, so that user_filter + // is defined. Also, must happen after people.initialize() + onboarding_steps.initialize(state_data.onboarding_steps, message_view.show); typing.initialize(); starred_messages_ui.initialize(); user_status_ui.initialize(); diff --git a/web/tests/dispatch.test.js b/web/tests/dispatch.test.js index 74a86f6692..ca8c380271 100644 --- a/web/tests/dispatch.test.js +++ b/web/tests/dispatch.test.js @@ -325,7 +325,7 @@ run_test("default_streams", ({override}) => { }); run_test("onboarding_steps", () => { - onboarding_steps.initialize({onboarding_steps: []}); + onboarding_steps.initialize({onboarding_steps: []}, () => {}); const event = event_fixtures.onboarding_steps; const one_time_notices = new Set(); for (const onboarding_step of event.onboarding_steps) { diff --git a/zerver/lib/onboarding_steps.py b/zerver/lib/onboarding_steps.py index 82c79fe80b..e2f85c3ffb 100644 --- a/zerver/lib/onboarding_steps.py +++ b/zerver/lib/onboarding_steps.py @@ -19,6 +19,17 @@ class OneTimeNotice: } +@dataclass +class OneTimeAction: + name: str + + def to_dict(self) -> dict[str, str]: + return { + "type": "one_time_action", + "name": self.name, + } + + ONE_TIME_NOTICES: list[OneTimeNotice] = [ OneTimeNotice( name="visibility_policy_banner", @@ -43,12 +54,9 @@ ONE_TIME_NOTICES: list[OneTimeNotice] = [ ), ] -# We may introduce onboarding step of types other than 'one time notice' -# in future. Earlier, we had 'hotspot' and 'one time notice' as the two -# types. We can simply do: -# ALL_ONBOARDING_STEPS: List[Union[OneTimeNotice, OtherType]] -# to avoid API changes when new type is introduced in the future. -ALL_ONBOARDING_STEPS: list[OneTimeNotice] = ONE_TIME_NOTICES +ONE_TIME_ACTIONS = [OneTimeAction(name="narrow_to_dm_with_welcome_bot_new_user")] + +ALL_ONBOARDING_STEPS: list[OneTimeNotice | OneTimeAction] = ONE_TIME_NOTICES + ONE_TIME_ACTIONS def get_next_onboarding_steps(user: UserProfile) -> list[dict[str, Any]]: @@ -62,10 +70,10 @@ def get_next_onboarding_steps(user: UserProfile) -> list[dict[str, Any]]: ) onboarding_steps: list[dict[str, Any]] = [] - for one_time_notice in ONE_TIME_NOTICES: - if one_time_notice.name in seen_onboarding_steps: + for onboarding_step in ALL_ONBOARDING_STEPS: + if onboarding_step.name in seen_onboarding_steps: continue - onboarding_steps.append(one_time_notice.to_dict()) + onboarding_steps.append(onboarding_step.to_dict()) return onboarding_steps diff --git a/zerver/migrations/0568_mark_narrow_to_dm_with_welcome_bot_new_user_as_read.py b/zerver/migrations/0568_mark_narrow_to_dm_with_welcome_bot_new_user_as_read.py new file mode 100644 index 0000000000..35055fce53 --- /dev/null +++ b/zerver/migrations/0568_mark_narrow_to_dm_with_welcome_bot_new_user_as_read.py @@ -0,0 +1,69 @@ +# Generated by Django 5.0.6 on 2024-07-25 13:24 + +from django.db import connection, migrations +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps +from django.utils.timezone import now as timezone_now +from psycopg2.sql import SQL + + +def mark_narrow_to_dm_with_welcome_bot_new_user_as_read( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + with connection.cursor() as cursor: + cursor.execute(SQL("SELECT MAX(id) FROM zerver_userprofile;")) + (max_id,) = cursor.fetchone() + + if max_id is None: + return + + BATCH_SIZE = 10000 + max_id += BATCH_SIZE / 2 + lower_id_bound = 0 + timestamp_value = timezone_now() + while lower_id_bound < max_id: + upper_id_bound = min(lower_id_bound + BATCH_SIZE, max_id) + with connection.cursor() as cursor: + query = SQL(""" + INSERT INTO zerver_onboardingstep (user_id, onboarding_step, timestamp) + SELECT id, 'narrow_to_dm_with_welcome_bot_new_user', %(timestamp_value)s + FROM zerver_userprofile + WHERE is_bot = False + AND is_mirror_dummy = False + AND tutorial_status != "W" + AND id > %(lower_id_bound)s AND id <= %(upper_id_bound)s; + """) + cursor.execute( + query, + { + "timestamp_value": timestamp_value, + "lower_id_bound": lower_id_bound, + "upper_id_bound": upper_id_bound, + }, + ) + + print(f"Processed {upper_id_bound} / {max_id}") + lower_id_bound += BATCH_SIZE + + +def mark_narrow_to_dm_with_welcome_bot_new_user_as_unread( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + OnboardingStep = apps.get_model("zerver", "OnboardingStep") + + OnboardingStep.objects.filter(onboarding_step="narrow_to_dm_with_welcome_bot_new_user").delete() + + +class Migration(migrations.Migration): + atomic = False + dependencies = [ + ("zerver", "0567_alter_realm_can_delete_any_message_group"), + ] + + operations = [ + migrations.RunPython( + mark_narrow_to_dm_with_welcome_bot_new_user_as_read, + reverse_code=mark_narrow_to_dm_with_welcome_bot_new_user_as_unread, + elidable=True, + ), + ] diff --git a/zerver/tests/test_onboarding_steps.py b/zerver/tests/test_onboarding_steps.py index 0f5d759326..183d255f08 100644 --- a/zerver/tests/test_onboarding_steps.py +++ b/zerver/tests/test_onboarding_steps.py @@ -2,7 +2,7 @@ from typing_extensions import override from zerver.actions.create_user import do_create_user from zerver.actions.onboarding_steps import do_mark_onboarding_step_as_read -from zerver.lib.onboarding_steps import ONE_TIME_NOTICES, get_next_onboarding_steps +from zerver.lib.onboarding_steps import ALL_ONBOARDING_STEPS, get_next_onboarding_steps from zerver.lib.test_classes import ZulipTestCase from zerver.models import OnboardingStep from zerver.models.realms import get_realm @@ -25,12 +25,13 @@ class TestGetNextOnboardingSteps(ZulipTestCase): do_mark_onboarding_step_as_read(self.user, "intro_inbox_view_modal") onboarding_steps = get_next_onboarding_steps(self.user) - self.assert_length(onboarding_steps, 5) + self.assert_length(onboarding_steps, 6) self.assertEqual(onboarding_steps[0]["name"], "intro_recent_view_modal") self.assertEqual(onboarding_steps[1]["name"], "first_stream_created_banner") self.assertEqual(onboarding_steps[2]["name"], "jump_to_conversation_banner") self.assertEqual(onboarding_steps[3]["name"], "non_interleaved_view_messages_fading") self.assertEqual(onboarding_steps[4]["name"], "interleaved_view_messages_fading") + self.assertEqual(onboarding_steps[5]["name"], "narrow_to_dm_with_welcome_bot_new_user") with self.settings(TUTORIAL_ENABLED=False): onboarding_steps = get_next_onboarding_steps(self.user) @@ -39,13 +40,18 @@ class TestGetNextOnboardingSteps(ZulipTestCase): def test_all_onboarding_steps_done(self) -> None: self.assertNotEqual(get_next_onboarding_steps(self.user), []) - for one_time_notice in ONE_TIME_NOTICES: # nocoverage - do_mark_onboarding_step_as_read(self.user, one_time_notice.name) + for onboarding_step in ALL_ONBOARDING_STEPS: # nocoverage + do_mark_onboarding_step_as_read(self.user, onboarding_step.name) self.assertEqual(get_next_onboarding_steps(self.user), []) class TestOnboardingSteps(ZulipTestCase): + @override + def setUp(self) -> None: + super().setUp() + OnboardingStep.objects.filter(user=self.example_user("hamlet")).delete() + def test_do_mark_onboarding_step_as_read(self) -> None: user = self.example_user("hamlet") do_mark_onboarding_step_as_read(user, "intro_inbox_view_modal") diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py index 759d72f510..535407595d 100644 --- a/zerver/tests/test_upload.py +++ b/zerver/tests/test_upload.py @@ -19,6 +19,7 @@ from urllib3.fields import RequestField import zerver.lib.upload from analytics.models import RealmCount from zerver.actions.create_realm import do_create_realm +from zerver.actions.create_user import do_create_user from zerver.actions.message_send import internal_send_private_message from zerver.actions.realm_icon import do_change_icon_source from zerver.actions.realm_logo import do_change_logo_source @@ -1305,7 +1306,9 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase): self.client_post("/json/users/me/avatar", {"file": image_file}) source_user_profile = self.example_user("hamlet") - target_user_profile = self.example_user("iago") + target_user_profile = do_create_user( + "user@zulip.com", "password", get_realm("zulip"), "user", acting_user=None + ) copy_default_settings(source_user_profile, target_user_profile) diff --git a/zerver/tests/test_upload_s3.py b/zerver/tests/test_upload_s3.py index 70c12da159..7498b75089 100644 --- a/zerver/tests/test_upload_s3.py +++ b/zerver/tests/test_upload_s3.py @@ -9,6 +9,7 @@ import pyvips from django.conf import settings import zerver.lib.upload +from zerver.actions.create_user import do_create_user from zerver.actions.user_settings import do_delete_avatar_image from zerver.lib.avatar_hash import user_avatar_path from zerver.lib.create_user import copy_default_settings @@ -321,7 +322,9 @@ class S3Test(ZulipTestCase): self.client_post("/json/users/me/avatar", {"file": image_file}) source_user_profile = self.example_user("hamlet") - target_user_profile = self.example_user("othello") + target_user_profile = do_create_user( + "user@zulip.com", "password", get_realm("zulip"), "user", acting_user=None + ) copy_default_settings(source_user_profile, target_user_profile) diff --git a/zilencer/management/commands/populate_db.py b/zilencer/management/commands/populate_db.py index 3166fa9c53..d53e5a753b 100644 --- a/zilencer/management/commands/populate_db.py +++ b/zilencer/management/commands/populate_db.py @@ -70,6 +70,7 @@ from zerver.models import ( ) from zerver.models.alert_words import flush_alert_word from zerver.models.clients import get_client +from zerver.models.onboarding_steps import OnboardingStep from zerver.models.realms import WildcardMentionPolicyEnum, get_realm from zerver.models.recipients import get_or_create_direct_message_group from zerver.models.streams import get_stream @@ -935,7 +936,18 @@ class Command(ZulipBaseCommand): defaults={"last_active_time": date, "last_connected_time": date}, ) - user_profiles_ids = [user_profile.id for user_profile in user_profiles] + user_profiles_ids = [] + onboarding_steps = [] + for user_profile in user_profiles: + user_profiles_ids.append(user_profile.id) + onboarding_steps.append( + OnboardingStep( + user=user_profile, onboarding_step="narrow_to_dm_with_welcome_bot_new_user" + ) + ) + + # Existing users shouldn't narrow to DM with welcome bot on first login. + OnboardingStep.objects.bulk_create(onboarding_steps) # Create several initial direct message groups for i in range(options["num_direct_message_groups"]):