mirror of https://github.com/zulip/zulip.git
streams: Send message when a user is unsubscribed from a channel.
Previously, Notification Bot only sent messages to notify users when they were added to channels by others. However, there was no corresponding notification when they were removed from channels. This commit ensures that when a user is unsubscribed from a channel by another user, Notification Bot sends a message to inform them, using a silent mention of the acting user. The message follows this format: "@_**username** unsubscribed you from the channel #channel_name." Fixes: #29272 Co-authored-by: Tanmay Kumar <tnmkr@users.noreply.github.com>
This commit is contained in:
parent
0a8723b08b
commit
3b5ea61e7b
|
@ -146,6 +146,8 @@ subscribe the user.
|
||||||
|
|
||||||
{end_tabs}
|
{end_tabs}
|
||||||
|
|
||||||
|
{!automated-dm-channel-remove.md!}
|
||||||
|
|
||||||
## Related articles
|
## Related articles
|
||||||
|
|
||||||
* [Introduction to channels](/help/introduction-to-channels)
|
* [Introduction to channels](/help/introduction-to-channels)
|
||||||
|
|
|
@ -54,7 +54,7 @@ automated notices to help others understand how content was moved.
|
||||||
|
|
||||||
## Notices about users
|
## Notices about users
|
||||||
|
|
||||||
You will be notified if someone [subscribes you to a
|
You will be notified if someone [subscribes you to or removes you from a
|
||||||
channel](/help/add-or-remove-users-from-a-channel#add-users-to-a-channel), or
|
channel](/help/add-or-remove-users-from-a-channel#add-users-to-a-channel), or
|
||||||
changes your [group](/help/user-groups) membership.
|
changes your [group](/help/user-groups) membership.
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
!!! warn ""
|
||||||
|
|
||||||
|
**Note**: Removing someone else from a channel sends them an
|
||||||
|
automated direct message from the [Notification Bot][notification-bot].
|
||||||
|
|
||||||
|
[notification-bot]: /help/configure-automated-notices
|
|
@ -126,7 +126,7 @@ def add_emoji_to_message() -> dict[str, object]:
|
||||||
|
|
||||||
# The message ID here is hardcoded based on the corresponding value
|
# The message ID here is hardcoded based on the corresponding value
|
||||||
# for the example message IDs we use in zulip.yaml.
|
# for the example message IDs we use in zulip.yaml.
|
||||||
message_id = 48
|
message_id = 49
|
||||||
emoji_name = "octopus"
|
emoji_name = "octopus"
|
||||||
emoji_code = "1f419"
|
emoji_code = "1f419"
|
||||||
reaction_type = "unicode_emoji"
|
reaction_type = "unicode_emoji"
|
||||||
|
|
|
@ -24691,7 +24691,7 @@ components:
|
||||||
The target message's ID.
|
The target message's ID.
|
||||||
schema:
|
schema:
|
||||||
type: integer
|
type: integer
|
||||||
example: 43
|
example: 44
|
||||||
required: true
|
required: true
|
||||||
UserId:
|
UserId:
|
||||||
name: user_id
|
name: user_id
|
||||||
|
|
|
@ -88,7 +88,7 @@ class OpenAPIToolsTest(ZulipTestCase):
|
||||||
description="The target message's ID.\n",
|
description="The target message's ID.\n",
|
||||||
json_encoded=False,
|
json_encoded=False,
|
||||||
value_schema={"type": "integer"},
|
value_schema={"type": "integer"},
|
||||||
example=43,
|
example=44,
|
||||||
required=True,
|
required=True,
|
||||||
deprecated=False,
|
deprecated=False,
|
||||||
)
|
)
|
||||||
|
|
|
@ -2552,6 +2552,20 @@ class StreamAdminTest(ZulipTestCase):
|
||||||
)
|
)
|
||||||
self.archive_stream(priv_stream)
|
self.archive_stream(priv_stream)
|
||||||
|
|
||||||
|
def assert_user_got_unsubscription_notification(
|
||||||
|
self, user: UserProfile, expected_msg: str
|
||||||
|
) -> None:
|
||||||
|
# verify that the user was sent a message informing them about the subscription
|
||||||
|
realm = user.realm
|
||||||
|
msg = most_recent_message(user)
|
||||||
|
self.assertEqual(msg.recipient.type, msg.recipient.PERSONAL)
|
||||||
|
self.assertEqual(msg.sender_id, self.notification_bot(realm).id)
|
||||||
|
|
||||||
|
def non_ws(s: str) -> str:
|
||||||
|
return s.replace("\n", "").replace(" ", "")
|
||||||
|
|
||||||
|
self.assertEqual(non_ws(msg.content), non_ws(expected_msg))
|
||||||
|
|
||||||
def attempt_unsubscribe_of_principal(
|
def attempt_unsubscribe_of_principal(
|
||||||
self,
|
self,
|
||||||
target_users: list[UserProfile],
|
target_users: list[UserProfile],
|
||||||
|
@ -2631,16 +2645,60 @@ class StreamAdminTest(ZulipTestCase):
|
||||||
those you aren't on.
|
those you aren't on.
|
||||||
"""
|
"""
|
||||||
result = self.attempt_unsubscribe_of_principal(
|
result = self.attempt_unsubscribe_of_principal(
|
||||||
query_count=17,
|
query_count=33,
|
||||||
target_users=[self.example_user("cordelia")],
|
target_users=[self.example_user("cordelia")],
|
||||||
is_realm_admin=True,
|
is_realm_admin=True,
|
||||||
is_subbed=True,
|
is_subbed=True,
|
||||||
invite_only=False,
|
invite_only=False,
|
||||||
target_users_subbed=True,
|
target_users_subbed=True,
|
||||||
)
|
)
|
||||||
|
user_profile = self.example_user("iago")
|
||||||
|
unsubscription_notification_message = f"""
|
||||||
|
@_**{user_profile.full_name}|{user_profile.id}** unsubscribed you from the channel #**hümbüǵ**.
|
||||||
|
"""
|
||||||
json = self.assert_json_success(result)
|
json = self.assert_json_success(result)
|
||||||
self.assert_length(json["removed"], 1)
|
self.assert_length(json["removed"], 1)
|
||||||
self.assert_length(json["not_removed"], 0)
|
self.assert_length(json["not_removed"], 0)
|
||||||
|
self.assert_user_got_unsubscription_notification(
|
||||||
|
self.example_user("cordelia"), unsubscription_notification_message
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_realm_admin_remove_others_from_multiple_public_streams(self) -> None:
|
||||||
|
user_profile = self.example_user("iago")
|
||||||
|
self.login_user(user_profile)
|
||||||
|
|
||||||
|
target_users = [self.example_user(name) for name in ["cordelia", "prospero", "hamlet"]]
|
||||||
|
principals = [user.id for user in target_users]
|
||||||
|
public_channels_to_unsubscribe = []
|
||||||
|
|
||||||
|
for i in range(1, 5):
|
||||||
|
channel_name = f"channel_{i}"
|
||||||
|
self.make_stream(channel_name, invite_only=False)
|
||||||
|
public_channels_to_unsubscribe.append(channel_name)
|
||||||
|
self.subscribe(user_profile, channel_name)
|
||||||
|
for user in target_users:
|
||||||
|
self.subscribe(user, channel_name)
|
||||||
|
|
||||||
|
result = self.client_delete(
|
||||||
|
"/json/users/me/subscriptions",
|
||||||
|
{
|
||||||
|
"subscriptions": orjson.dumps(public_channels_to_unsubscribe).decode(),
|
||||||
|
"principals": orjson.dumps(principals).decode(),
|
||||||
|
},
|
||||||
|
)
|
||||||
|
self.assert_json_success(result)
|
||||||
|
|
||||||
|
unsubscription_notification_message = f"""
|
||||||
|
@_**{user_profile.full_name}|{user_profile.id}** unsubscribed you from the following channels:
|
||||||
|
"""
|
||||||
|
unsubscription_notification_message += "\n\n"
|
||||||
|
for channel_name in public_channels_to_unsubscribe:
|
||||||
|
unsubscription_notification_message += f"* #**{channel_name}**\n"
|
||||||
|
|
||||||
|
for target_user in target_users:
|
||||||
|
self.assert_user_got_unsubscription_notification(
|
||||||
|
target_user, unsubscription_notification_message
|
||||||
|
)
|
||||||
|
|
||||||
def test_realm_admin_remove_multiple_users_from_stream(self) -> None:
|
def test_realm_admin_remove_multiple_users_from_stream(self) -> None:
|
||||||
"""
|
"""
|
||||||
|
@ -2658,8 +2716,8 @@ class StreamAdminTest(ZulipTestCase):
|
||||||
for name in ["cordelia", "prospero", "iago", "hamlet", "outgoing_webhook_bot"]
|
for name in ["cordelia", "prospero", "iago", "hamlet", "outgoing_webhook_bot"]
|
||||||
]
|
]
|
||||||
result = self.attempt_unsubscribe_of_principal(
|
result = self.attempt_unsubscribe_of_principal(
|
||||||
query_count=24,
|
query_count=56,
|
||||||
cache_count=8,
|
cache_count=27,
|
||||||
target_users=target_users,
|
target_users=target_users,
|
||||||
is_realm_admin=True,
|
is_realm_admin=True,
|
||||||
is_subbed=True,
|
is_subbed=True,
|
||||||
|
@ -2676,7 +2734,7 @@ class StreamAdminTest(ZulipTestCase):
|
||||||
are on.
|
are on.
|
||||||
"""
|
"""
|
||||||
result = self.attempt_unsubscribe_of_principal(
|
result = self.attempt_unsubscribe_of_principal(
|
||||||
query_count=17,
|
query_count=33,
|
||||||
target_users=[self.example_user("cordelia")],
|
target_users=[self.example_user("cordelia")],
|
||||||
is_realm_admin=True,
|
is_realm_admin=True,
|
||||||
is_subbed=True,
|
is_subbed=True,
|
||||||
|
@ -2693,7 +2751,7 @@ class StreamAdminTest(ZulipTestCase):
|
||||||
streams you aren't on.
|
streams you aren't on.
|
||||||
"""
|
"""
|
||||||
result = self.attempt_unsubscribe_of_principal(
|
result = self.attempt_unsubscribe_of_principal(
|
||||||
query_count=17,
|
query_count=33,
|
||||||
target_users=[self.example_user("cordelia")],
|
target_users=[self.example_user("cordelia")],
|
||||||
is_realm_admin=True,
|
is_realm_admin=True,
|
||||||
is_subbed=False,
|
is_subbed=False,
|
||||||
|
@ -2719,7 +2777,7 @@ class StreamAdminTest(ZulipTestCase):
|
||||||
|
|
||||||
def test_admin_remove_others_from_stream_legacy_emails(self) -> None:
|
def test_admin_remove_others_from_stream_legacy_emails(self) -> None:
|
||||||
result = self.attempt_unsubscribe_of_principal(
|
result = self.attempt_unsubscribe_of_principal(
|
||||||
query_count=17,
|
query_count=33,
|
||||||
target_users=[self.example_user("cordelia")],
|
target_users=[self.example_user("cordelia")],
|
||||||
is_realm_admin=True,
|
is_realm_admin=True,
|
||||||
is_subbed=True,
|
is_subbed=True,
|
||||||
|
@ -2733,7 +2791,7 @@ class StreamAdminTest(ZulipTestCase):
|
||||||
|
|
||||||
def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None:
|
def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None:
|
||||||
result = self.attempt_unsubscribe_of_principal(
|
result = self.attempt_unsubscribe_of_principal(
|
||||||
query_count=19,
|
query_count=43,
|
||||||
target_users=[self.example_user("cordelia"), self.example_user("prospero")],
|
target_users=[self.example_user("cordelia"), self.example_user("prospero")],
|
||||||
is_realm_admin=True,
|
is_realm_admin=True,
|
||||||
is_subbed=True,
|
is_subbed=True,
|
||||||
|
@ -2747,7 +2805,7 @@ class StreamAdminTest(ZulipTestCase):
|
||||||
|
|
||||||
def test_remove_unsubbed_user_along_with_subbed(self) -> None:
|
def test_remove_unsubbed_user_along_with_subbed(self) -> None:
|
||||||
result = self.attempt_unsubscribe_of_principal(
|
result = self.attempt_unsubscribe_of_principal(
|
||||||
query_count=16,
|
query_count=17,
|
||||||
target_users=[self.example_user("cordelia"), self.example_user("iago")],
|
target_users=[self.example_user("cordelia"), self.example_user("iago")],
|
||||||
is_realm_admin=True,
|
is_realm_admin=True,
|
||||||
is_subbed=True,
|
is_subbed=True,
|
||||||
|
@ -2764,7 +2822,7 @@ class StreamAdminTest(ZulipTestCase):
|
||||||
fails gracefully.
|
fails gracefully.
|
||||||
"""
|
"""
|
||||||
result = self.attempt_unsubscribe_of_principal(
|
result = self.attempt_unsubscribe_of_principal(
|
||||||
query_count=9,
|
query_count=10,
|
||||||
target_users=[self.example_user("cordelia")],
|
target_users=[self.example_user("cordelia")],
|
||||||
is_realm_admin=True,
|
is_realm_admin=True,
|
||||||
is_subbed=False,
|
is_subbed=False,
|
||||||
|
@ -2780,7 +2838,7 @@ class StreamAdminTest(ZulipTestCase):
|
||||||
webhook_bot = self.example_user("webhook_bot")
|
webhook_bot = self.example_user("webhook_bot")
|
||||||
do_change_bot_owner(webhook_bot, bot_owner=user_profile, acting_user=user_profile)
|
do_change_bot_owner(webhook_bot, bot_owner=user_profile, acting_user=user_profile)
|
||||||
result = self.attempt_unsubscribe_of_principal(
|
result = self.attempt_unsubscribe_of_principal(
|
||||||
query_count=14,
|
query_count=15,
|
||||||
target_users=[webhook_bot],
|
target_users=[webhook_bot],
|
||||||
is_realm_admin=False,
|
is_realm_admin=False,
|
||||||
is_subbed=True,
|
is_subbed=True,
|
||||||
|
|
|
@ -475,6 +475,59 @@ def compose_views(thunks: list[Callable[[], HttpResponse]]) -> dict[str, Any]:
|
||||||
return json_dict
|
return json_dict
|
||||||
|
|
||||||
|
|
||||||
|
def get_just_unsubscribed_message_content(acting_user: UserProfile, channel_names: set[str]) -> str:
|
||||||
|
subscriptions = sorted(channel_names)
|
||||||
|
if len(subscriptions) == 1:
|
||||||
|
return _("{user_full_name} unsubscribed you from the channel {channel_name}.").format(
|
||||||
|
user_full_name=f"@_**{acting_user.full_name}|{acting_user.id}**",
|
||||||
|
channel_name=f"#**{subscriptions[0]}**",
|
||||||
|
)
|
||||||
|
|
||||||
|
message = _("{user_full_name} unsubscribed you from the following channels:").format(
|
||||||
|
user_full_name=f"@_**{acting_user.full_name}|{acting_user.id}**",
|
||||||
|
)
|
||||||
|
message += "\n\n"
|
||||||
|
for channel_name in subscriptions:
|
||||||
|
message += f"* #**{channel_name}**\n"
|
||||||
|
return message
|
||||||
|
|
||||||
|
|
||||||
|
def send_messages_for_unsubscribers(
|
||||||
|
acting_user: UserProfile,
|
||||||
|
unsubscribed_users: set[UserProfile],
|
||||||
|
unsubscribed_channels_by_user: dict[str, list[str]],
|
||||||
|
id_to_user_profile: dict[str, UserProfile],
|
||||||
|
) -> None:
|
||||||
|
"""
|
||||||
|
Send notifications to users when they are unsubscribed from channels.
|
||||||
|
"""
|
||||||
|
bot_ids = {str(user.id) for user in unsubscribed_users if user.is_bot}
|
||||||
|
notifications = []
|
||||||
|
|
||||||
|
sender = get_system_bot(settings.NOTIFICATION_BOT, acting_user.realm_id)
|
||||||
|
if unsubscribed_channels_by_user:
|
||||||
|
for user_id, unsubscribed_channel_names in unsubscribed_channels_by_user.items():
|
||||||
|
if user_id == str(acting_user.id) or user_id in bot_ids:
|
||||||
|
continue
|
||||||
|
|
||||||
|
recipient_user = id_to_user_profile[user_id]
|
||||||
|
msg = get_just_unsubscribed_message_content(
|
||||||
|
acting_user=acting_user,
|
||||||
|
channel_names=set(unsubscribed_channel_names),
|
||||||
|
)
|
||||||
|
|
||||||
|
notifications.append(
|
||||||
|
internal_prep_private_message(
|
||||||
|
sender=sender,
|
||||||
|
recipient_user=recipient_user,
|
||||||
|
content=msg,
|
||||||
|
mention_backend=MentionBackend(acting_user.realm.id),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
if notifications:
|
||||||
|
do_send_messages(notifications, mark_as_read=[acting_user.id])
|
||||||
|
|
||||||
|
|
||||||
@typed_endpoint
|
@typed_endpoint
|
||||||
def remove_subscriptions_backend(
|
def remove_subscriptions_backend(
|
||||||
request: HttpRequest,
|
request: HttpRequest,
|
||||||
|
@ -505,16 +558,30 @@ def remove_subscriptions_backend(
|
||||||
unsubscribing_others=unsubscribing_others,
|
unsubscribing_others=unsubscribing_others,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
id_to_user_profile: dict[str, UserProfile] = {}
|
||||||
result: dict[str, list[str]] = dict(removed=[], not_removed=[])
|
result: dict[str, list[str]] = dict(removed=[], not_removed=[])
|
||||||
|
unsubscribed_channels_by_user_dict: dict[str, list[str]] = defaultdict(list)
|
||||||
(removed, not_subscribed) = bulk_remove_subscriptions(
|
(removed, not_subscribed) = bulk_remove_subscriptions(
|
||||||
realm, people_to_unsub, streams, acting_user=user_profile
|
realm, people_to_unsub, streams, acting_user=user_profile
|
||||||
)
|
)
|
||||||
|
|
||||||
for subscriber, removed_stream in removed:
|
for subscriber, removed_channel in removed:
|
||||||
result["removed"].append(removed_stream.name)
|
user_id = str(subscriber.id)
|
||||||
|
result["removed"].append(removed_channel.name)
|
||||||
|
unsubscribed_channels_by_user_dict[user_id].append(removed_channel.name)
|
||||||
|
id_to_user_profile[user_id] = subscriber
|
||||||
for subscriber, not_subscribed_stream in not_subscribed:
|
for subscriber, not_subscribed_stream in not_subscribed:
|
||||||
result["not_removed"].append(not_subscribed_stream.name)
|
result["not_removed"].append(not_subscribed_stream.name)
|
||||||
|
|
||||||
|
for user_id in unsubscribed_channels_by_user_dict:
|
||||||
|
unsubscribed_channels_by_user_dict[user_id].sort()
|
||||||
|
|
||||||
|
send_messages_for_unsubscribers(
|
||||||
|
acting_user=user_profile,
|
||||||
|
unsubscribed_users=people_to_unsub,
|
||||||
|
unsubscribed_channels_by_user=unsubscribed_channels_by_user_dict,
|
||||||
|
id_to_user_profile=id_to_user_profile,
|
||||||
|
)
|
||||||
return json_success(request, data=result)
|
return json_success(request, data=result)
|
||||||
|
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue