mirror of
https://github.com/zulip/zulip.git
synced 2025-10-24 08:33:43 +00:00
onboarding: Mark a few onboarding messages as starred.
To improve onboarding experience following onboarding messages are marked as starred: * First message in each onboarding topic. * Initial DM sent by Welcome bot Note: The onboarding topic messages needs to be tracked in 'OnboardingUserMessage' model to get starred. Fixes #29298.
This commit is contained in:
committed by
Tim Abbott
parent
3c5dc73f50
commit
ed605328fb
@@ -4,6 +4,7 @@ from typing import Any, Dict, Iterable, List, Optional, Sequence, Set
|
||||
|
||||
from django.conf import settings
|
||||
from django.db import transaction
|
||||
from django.db.models import F
|
||||
from django.utils.timezone import now as timezone_now
|
||||
from django.utils.translation import gettext as _
|
||||
from django.utils.translation import override as override_language
|
||||
@@ -209,9 +210,13 @@ def add_new_user_history(
|
||||
)
|
||||
|
||||
tracked_onboarding_message_ids = set()
|
||||
message_id_to_onboarding_user_message = {}
|
||||
onboarding_user_messages_queryset = OnboardingUserMessage.objects.filter(realm_id=realm.id)
|
||||
for onboarding_user_message in onboarding_user_messages_queryset:
|
||||
tracked_onboarding_message_ids.add(onboarding_user_message.message_id)
|
||||
message_id_to_onboarding_user_message[onboarding_user_message.message_id] = (
|
||||
onboarding_user_message
|
||||
)
|
||||
tracked_onboarding_messages_exist = len(tracked_onboarding_message_ids) > 0
|
||||
|
||||
message_history_ids = recent_message_ids.union(tracked_onboarding_message_ids)
|
||||
@@ -242,12 +247,12 @@ def add_new_user_history(
|
||||
# They are not marked as historical.
|
||||
if not realm_creation:
|
||||
um.flags = UserMessage.flags.historical
|
||||
if (
|
||||
tracked_onboarding_messages_exist
|
||||
and message_id not in tracked_onboarding_message_ids
|
||||
):
|
||||
um.flags |= UserMessage.flags.read
|
||||
if not tracked_onboarding_messages_exist and message_id in older_message_ids:
|
||||
if tracked_onboarding_messages_exist:
|
||||
if message_id not in tracked_onboarding_message_ids:
|
||||
um.flags |= UserMessage.flags.read
|
||||
elif message_id_to_onboarding_user_message[message_id].flags.starred.is_set:
|
||||
um.flags |= UserMessage.flags.starred
|
||||
elif message_id in older_message_ids:
|
||||
um.flags |= UserMessage.flags.read
|
||||
ums_to_create.append(um)
|
||||
|
||||
@@ -335,7 +340,10 @@ def process_new_human_user(
|
||||
# to keep all the onboarding code in zerver/lib/onboarding.py.
|
||||
from zerver.lib.onboarding import send_initial_direct_message
|
||||
|
||||
send_initial_direct_message(user_profile)
|
||||
message_id = send_initial_direct_message(user_profile)
|
||||
UserMessage.objects.filter(user_profile=user_profile, message_id=message_id).update(
|
||||
flags=F("flags").bitor(UserMessage.flags.starred)
|
||||
)
|
||||
|
||||
# The 'visibility_policy_banner' is only displayed to existing users.
|
||||
# Mark it as read for a new user.
|
||||
|
||||
@@ -40,7 +40,7 @@ def create_if_missing_realm_internal_bots() -> None:
|
||||
setup_realm_internal_bots(realm)
|
||||
|
||||
|
||||
def send_initial_direct_message(user: UserProfile) -> None:
|
||||
def send_initial_direct_message(user: UserProfile) -> int:
|
||||
# We adjust the initial Welcome Bot direct message for education organizations.
|
||||
education_organization = user.realm.org_type in (
|
||||
Realm.ORG_TYPES["education_nonprofit"]["id"],
|
||||
@@ -97,7 +97,7 @@ Here are a few messages I understand: {bot_commands}
|
||||
bot_commands=bot_commands(),
|
||||
)
|
||||
|
||||
internal_send_private_message(
|
||||
message_id = internal_send_private_message(
|
||||
get_system_bot(settings.WELCOME_BOT, user.realm_id),
|
||||
user,
|
||||
remove_single_newlines(content),
|
||||
@@ -105,6 +105,8 @@ Here are a few messages I understand: {bot_commands}
|
||||
# as this is intended to be seen contextually in the application.
|
||||
disable_external_notifications=True,
|
||||
)
|
||||
assert message_id is not None
|
||||
return message_id
|
||||
|
||||
|
||||
def bot_commands(no_help_command: bool = False) -> str:
|
||||
@@ -388,12 +390,23 @@ This **greetings** topic is a great place to say “hi” :wave: to your teammat
|
||||
sent_message_result.message_id for sent_message_result in do_send_messages(messages)
|
||||
]
|
||||
|
||||
onboarding_user_messages = [
|
||||
OnboardingUserMessage(
|
||||
realm=realm, message_id=message_id, flags=OnboardingUserMessage.flags.historical
|
||||
seen_topics = set()
|
||||
onboarding_topics_first_message_ids = set()
|
||||
for index, message in enumerate(welcome_messages):
|
||||
topic_name = message["topic_name"]
|
||||
if topic_name not in seen_topics:
|
||||
onboarding_topics_first_message_ids.add(message_ids[index])
|
||||
seen_topics.add(topic_name)
|
||||
|
||||
onboarding_user_messages = []
|
||||
for message_id in message_ids:
|
||||
flags = OnboardingUserMessage.flags.historical
|
||||
if message_id in onboarding_topics_first_message_ids:
|
||||
flags |= OnboardingUserMessage.flags.starred
|
||||
onboarding_user_messages.append(
|
||||
OnboardingUserMessage(realm=realm, message_id=message_id, flags=flags)
|
||||
)
|
||||
for message_id in message_ids
|
||||
]
|
||||
|
||||
OnboardingUserMessage.objects.bulk_create(onboarding_user_messages)
|
||||
|
||||
# We find the one of our just-sent greetings messages, and react to it.
|
||||
|
||||
@@ -364,6 +364,56 @@ class AddNewUserHistoryTest(ZulipTestCase):
|
||||
self.assertFalse(user_message.flags.read.is_set)
|
||||
self.assertFalse(user_message.flags.historical.is_set)
|
||||
|
||||
def test_tracked_onboarding_topics_first_messages_marked_starred(self) -> None:
|
||||
"""
|
||||
Realms with tracked onboarding messages have only
|
||||
first message in each onboarding topic marked as starred.
|
||||
"""
|
||||
realm = do_create_realm("realm_string_id", "realm name")
|
||||
hamlet = do_create_user(
|
||||
"hamlet", "password", realm, "hamlet", realm_creation=True, acting_user=None
|
||||
)
|
||||
|
||||
# Onboarding messages sent during realm creation are tracked.
|
||||
self.assertTrue(OnboardingUserMessage.objects.filter(realm=realm).exists())
|
||||
|
||||
seen_topics = set()
|
||||
onboarding_topics_first_message_ids = set()
|
||||
onboarding_messages = Message.objects.filter(
|
||||
realm=realm, recipient__type=Recipient.STREAM
|
||||
).order_by("id")
|
||||
for message in onboarding_messages:
|
||||
topic_name = message.topic_name()
|
||||
if topic_name not in seen_topics:
|
||||
onboarding_topics_first_message_ids.add(message.id)
|
||||
seen_topics.add(topic_name)
|
||||
|
||||
# The first onboarding message in each topic are in the
|
||||
# user's history and marked starred.
|
||||
onboarding_user_messages = UserMessage.objects.filter(
|
||||
user_profile=hamlet, message_id__in=onboarding_topics_first_message_ids
|
||||
)
|
||||
for user_message in onboarding_user_messages:
|
||||
self.assertTrue(user_message.flags.starred.is_set)
|
||||
self.assertFalse(user_message.flags.read.is_set)
|
||||
self.assertFalse(user_message.flags.historical.is_set)
|
||||
|
||||
# Other messages are in user's history but not marked starred.
|
||||
other_user_messages = UserMessage.objects.filter(
|
||||
user_profile=hamlet, message__recipient__type=Recipient.STREAM
|
||||
).exclude(message_id__in=onboarding_topics_first_message_ids)
|
||||
self.assertTrue(other_user_messages.exists())
|
||||
for user_message in other_user_messages:
|
||||
self.assertFalse(user_message.flags.starred.is_set)
|
||||
self.assertFalse(user_message.flags.read.is_set)
|
||||
self.assertFalse(user_message.flags.historical.is_set)
|
||||
|
||||
# Initial DM sent by welcome bot is also starred.
|
||||
initial_direct_user_message = UserMessage.objects.get(
|
||||
user_profile=hamlet, message__recipient__type=Recipient.PERSONAL
|
||||
)
|
||||
self.assertTrue(initial_direct_user_message.flags.starred.is_set)
|
||||
|
||||
def test_auto_subbed_to_personals(self) -> None:
|
||||
"""
|
||||
Newly created users are auto-subbed to the ability to receive
|
||||
@@ -996,7 +1046,7 @@ class LoginTest(ZulipTestCase):
|
||||
# seem to be any O(N) behavior. Some of the cache hits are related
|
||||
# to sending messages, such as getting the welcome bot, looking up
|
||||
# the alert words for a realm, etc.
|
||||
with self.assert_database_query_count(92), self.assert_memcached_count(14):
|
||||
with self.assert_database_query_count(93), self.assert_memcached_count(14):
|
||||
with self.captureOnCommitCallbacks(execute=True):
|
||||
self.register(self.nonreg_email("test"), "test")
|
||||
|
||||
|
||||
@@ -908,7 +908,7 @@ class QueryCountTest(ZulipTestCase):
|
||||
|
||||
prereg_user = PreregistrationUser.objects.get(email="fred@zulip.com")
|
||||
|
||||
with self.assert_database_query_count(82):
|
||||
with self.assert_database_query_count(83):
|
||||
with self.assert_memcached_count(19):
|
||||
with self.capture_send_event_calls(expected_num_events=10) as events:
|
||||
fred = do_create_user(
|
||||
|
||||
Reference in New Issue
Block a user