Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

soft_reactivate: Soft reactivate if group mention has < 12 members. 

Earlier, we didn't soft-reactivate users for group mentions
at all because it wasn't easy to calculate group size.

Now, we will soft reactivate if the user group mentions has
less than 12 members.

We don't reactivate all users because a user group can have a
very large size, which can lead to large backlogs in the
deferred-work queue.

Fixes part of #27586.
This commit is contained in:
Prakhar Pratyush 2023-12-09 01:23:31 +05:30 committed by Tim Abbott
parent 43136abb70
commit c727b36e9c
6 changed files with 109 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
zerver/lib/email_notifications.py
View File

@ -414,9 +414,11 @@ def do_send_missedmessage_events_reply_in_zulip(
)
mentioned_user_group_name = None
mentioned_user_group_members_count = None
mentioned_user_group = get_mentioned_user_group(missed_messages, user_profile)
if mentioned_user_group is not None:
mentioned_user_group_name = mentioned_user_group.name
mentioned_user_group_members_count = mentioned_user_group.members_count
triggers = [message["trigger"] for message in missed_messages]
unique_triggers = set(triggers)
@ -564,7 +566,7 @@ def do_send_missedmessage_events_reply_in_zulip(
# Soft reactivate the long_term_idle user personally mentioned
soft_reactivate_if_personal_notification(
user_profile, unique_triggers, mentioned_user_group_name
user_profile, unique_triggers, mentioned_user_group_members_count
)
with override_language(user_profile.default_language):

6
zerver/lib/push_notifications.py
View File

@ -1356,13 +1356,17 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any
# message or not.
mentioned_user_group_id = None
mentioned_user_group_name = None
mentioned_user_group_members_count = None
mentioned_user_group = get_mentioned_user_group([missed_message], user_profile)
if mentioned_user_group is not None:
mentioned_user_group_id = mentioned_user_group.id
mentioned_user_group_name = mentioned_user_group.name
mentioned_user_group_members_count = mentioned_user_group.members_count
# Soft reactivate if pushing to a long_term_idle user that is personally mentioned
soft_reactivate_if_personal_notification(user_profile, {trigger}, mentioned_user_group_name)
soft_reactivate_if_personal_notification(
user_profile, {trigger}, mentioned_user_group_members_count
)
if message.is_stream_message():
# This will almost always be True. The corner case where you

35
zerver/lib/soft_deactivation.py
View File

@ -400,27 +400,35 @@ def queue_soft_reactivation(user_profile_id: int) -> None:
def soft_reactivate_if_personal_notification(
user_profile: UserProfile, unique_triggers: Set[str], mentioned_user_group_name: Optional[str]
user_profile: UserProfile,
unique_triggers: Set[str],
mentioned_user_group_members_count: Optional[int],
) -> None:
"""When we're about to send an email/push notification to a
long_term_idle user, it's very likely that the user will try to
return to Zulip. As a result, it makes sense to optimistically
soft-reactivate that user, to give them a good return experience.
It's important that we do nothing for stream wildcard or group mentions,
because soft-reactivating an entire realm would be very expensive
(and we can't easily check the group's size). The caller is
responsible for passing a mentioned_user_group_name that is None
for messages that contain both a personal mention and a group
mention.
It's important that we do nothing for stream wildcard or large
group mentions (size > 'settings.MAX_GROUP_SIZE_FOR_MENTION_REACTIVATION'),
because soft-reactivating an entire realm or a large group would be
very expensive. The caller is responsible for passing a
mentioned_user_group_members_count that is None for messages that
contain both a personal mention and a group mention.
"""
if not user_profile.long_term_idle:
return
direct_message = NotificationTriggers.DIRECT_MESSAGE in unique_triggers
personal_mention = (
NotificationTriggers.MENTION in unique_triggers and mentioned_user_group_name is None
)
personal_mention = False
small_group_mention = False
if NotificationTriggers.MENTION in unique_triggers:
if mentioned_user_group_members_count is None:
personal_mention = True
elif mentioned_user_group_members_count <= settings.MAX_GROUP_SIZE_FOR_MENTION_REACTIVATION:
small_group_mention = True
topic_wildcard_mention = any(
trigger in unique_triggers
for trigger in [
@ -428,7 +436,12 @@ def soft_reactivate_if_personal_notification(
NotificationTriggers.TOPIC_WILDCARD_MENTION_IN_FOLLOWED_TOPIC,
]
)
if not direct_message and not personal_mention and not topic_wildcard_mention:
if (
not direct_message
and not personal_mention
and not small_group_mention
and not topic_wildcard_mention
):
return
queue_soft_reactivation(user_profile.id)

42
zerver/tests/test_message_notification_emails.py
View File

@ -14,7 +14,7 @@ from django.test import override_settings
from django_stubs_ext import StrPromise
from zerver.actions.create_user import do_create_user
from zerver.actions.user_groups import check_add_user_group
from zerver.actions.user_groups import add_subgroups_to_user_group, check_add_user_group
from zerver.actions.user_settings import do_change_user_setting
from zerver.actions.user_topics import do_set_user_topic_visibility_policy
from zerver.lib.email_notifications import (
@ -1615,14 +1615,27 @@ class TestMessageNotificationEmails(ZulipTestCase):
email_subject = "DMs with Othello, the Moor of Venice"
self._test_cases(msg_id, verify_body_include, email_subject, verify_html_body=True)
@override_settings(MAX_GROUP_SIZE_FOR_MENTION_REACTIVATION=2)
def test_long_term_idle_user_missed_message(self) -> None:
hamlet = self.example_user("hamlet")
othello = self.example_user("othello")
cordelia = self.example_user("cordelia")
large_user_group = check_add_user_group(
get_realm("zulip"), "large_user_group", [hamlet, othello, cordelia], acting_user=None
zulip_realm = get_realm("zulip")
# user groups having upto 'MAX_GROUP_SIZE_FOR_MENTION_REACTIVATION'
# members are small user groups.
small_user_group = check_add_user_group(
zulip_realm, "small_user_group", [hamlet, othello], acting_user=None
)
large_user_group = check_add_user_group(
zulip_realm, "large_user_group", [hamlet], acting_user=None
)
subgroup = check_add_user_group(
zulip_realm, "subgroup", [othello, cordelia], acting_user=None
)
add_subgroups_to_user_group(large_user_group, [subgroup], acting_user=None)
def reset_hamlet_as_soft_deactivated_user() -> None:
nonlocal hamlet
hamlet = self.example_user("hamlet")
@ -1727,8 +1740,25 @@ class TestMessageNotificationEmails(ZulipTestCase):
reset_hamlet_as_soft_deactivated_user()
self.expect_to_stay_long_term_idle(hamlet, send_stream_wildcard_mention)
# Group mention should NOT soft reactivate the user
def send_group_mention() -> None:
# Small group mention should soft reactivate the user
def send_small_group_mention() -> None:
mention = "@*small_user_group*"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_missedmessage_emails(
hamlet.id,
{
stream_mentioned_message_id: MissedMessageData(
trigger=NotificationTriggers.MENTION,
mentioned_user_group_id=small_user_group.id,
),
},
)
reset_hamlet_as_soft_deactivated_user()
self.expect_soft_reactivation(hamlet, send_small_group_mention)
# Large group mention should NOT soft reactivate the user
def send_large_group_mention() -> None:
mention = "@*large_user_group*"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_missedmessage_emails(
@ -1742,7 +1772,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
)
reset_hamlet_as_soft_deactivated_user()
self.expect_to_stay_long_term_idle(hamlet, send_group_mention)
self.expect_to_stay_long_term_idle(hamlet, send_large_group_mention)
def test_followed_topic_missed_message(self) -> None:
hamlet = self.example_user("hamlet")

45
zerver/tests/test_push_notifications.py
View File

@ -36,7 +36,7 @@ from zerver.actions.realm_settings import (
do_deactivate_realm,
do_set_realm_authentication_methods,
)
from zerver.actions.user_groups import check_add_user_group
from zerver.actions.user_groups import add_subgroups_to_user_group, check_add_user_group
from zerver.actions.user_settings import do_change_user_setting, do_regenerate_api_key
from zerver.actions.user_topics import do_set_user_topic_visibility_policy
from zerver.lib import redis_utils
@ -3563,19 +3563,32 @@ class HandlePushNotificationTest(PushNotificationTest):
mock_send_android.assert_called_with(user_identity, android_devices, {"gcm": True}, {})
mock_push_notifications.assert_called_once()
@override_settings(MAX_GROUP_SIZE_FOR_MENTION_REACTIVATION=2)
@mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True)
def test_user_push_soft_reactivate_soft_deactivated_user(
self, mock_push_notifications: mock.MagicMock
) -> None:
othello = self.example_user("othello")
cordelia = self.example_user("cordelia")
large_user_group = check_add_user_group(
get_realm("zulip"),
"large_user_group",
[self.user_profile, othello, cordelia],
zulip_realm = get_realm("zulip")
# user groups having upto 'MAX_GROUP_SIZE_FOR_MENTION_REACTIVATION'
# members are small user groups.
small_user_group = check_add_user_group(
zulip_realm,
"small_user_group",
[self.user_profile, othello],
acting_user=None,
)
large_user_group = check_add_user_group(
zulip_realm, "large_user_group", [self.user_profile], acting_user=None
)
subgroup = check_add_user_group(
zulip_realm, "subgroup", [othello, cordelia], acting_user=None
)
add_subgroups_to_user_group(large_user_group, [subgroup], acting_user=None)
# Personal mention in a stream message should soft reactivate the user
def mention_in_stream() -> None:
mention = f"@**{self.user_profile.full_name}**"
@ -3670,8 +3683,24 @@ class HandlePushNotificationTest(PushNotificationTest):
self.soft_deactivate_main_user()
self.expect_to_stay_long_term_idle(self.user_profile, send_stream_wildcard_mention)
# Group mention should NOT soft reactivate the user
def send_group_mention() -> None:
# Small group mention should soft reactivate the user
def send_small_group_mention() -> None:
mention = "@*small_user_group*"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_push_notification(
self.user_profile.id,
{
"message_id": stream_mentioned_message_id,
"trigger": NotificationTriggers.MENTION,
"mentioned_user_group_id": small_user_group.id,
},
)
self.soft_deactivate_main_user()
self.expect_soft_reactivation(self.user_profile, send_small_group_mention)
# Large group mention should NOT soft reactivate the user
def send_large_group_mention() -> None:
mention = "@*large_user_group*"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_push_notification(
@ -3684,7 +3713,7 @@ class HandlePushNotificationTest(PushNotificationTest):
)
self.soft_deactivate_main_user()
self.expect_to_stay_long_term_idle(self.user_profile, send_group_mention)
self.expect_to_stay_long_term_idle(self.user_profile, send_large_group_mention)
@mock.patch("zerver.lib.push_notifications.logger.info")
@mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True)

4
zproject/default_settings.py
View File

@ -621,6 +621,10 @@ TYPING_STARTED_WAIT_PERIOD_MILLISECONDS = 30000
# load in large organizations.
MAX_STREAM_SIZE_FOR_TYPING_NOTIFICATIONS = 100
# The maximum user-group size value upto which members should
# be soft-reactivated in the case of user group mention.
MAX_GROUP_SIZE_FOR_MENTION_REACTIVATION = 11
# Limiting guest access to other users via the
# can_access_all_users_group setting makes presence queries much more
# expensive. This can be a significant performance problem for