Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

streams: Send message when a user is unsubscribed from a channel. #32149

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
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>
  • Loading branch information
Aditya8840 and tnmkr committed Nov 16, 2024
commit 3b5ea61e7b73c6d8ac83db4434ead94f193d6a85
2 changes: 2 additions & 0 deletions help/add-or-remove-users-from-a-channel.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ subscribe the user.

{end_tabs}

{!automated-dm-channel-remove.md!}

## Related articles

* [Introduction to channels](/help/introduction-to-channels)
Expand Down
2 changes: 1 addition & 1 deletion help/configure-automated-notices.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ automated notices to help others understand how content was moved.

## 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
changes your [group](/help/user-groups) membership.

Expand Down
6 changes: 6 additions & 0 deletions help/include/automated-dm-channel-remove.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion zerver/openapi/curl_param_value_generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def add_emoji_to_message() -> dict[str, object]:

# The message ID here is hardcoded based on the corresponding value
# for the example message IDs we use in zulip.yaml.
message_id = 48
message_id = 49
emoji_name = "octopus"
emoji_code = "1f419"
reaction_type = "unicode_emoji"
Expand Down
2 changes: 1 addition & 1 deletion zerver/openapi/zulip.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24691,7 +24691,7 @@ components:
The target message's ID.
schema:
type: integer
example: 43
example: 44
required: true
UserId:
name: user_id
Expand Down
2 changes: 1 addition & 1 deletion zerver/tests/test_openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def test_get_openapi_parameters(self) -> None:
description="The target message's ID.\n",
json_encoded=False,
value_schema={"type": "integer"},
example=43,
example=44,
required=True,
deprecated=False,
)
Expand Down
78 changes: 68 additions & 10 deletions zerver/tests/test_subs.py
Original file line number Diff line number Diff line change
Expand Up @@ -2552,6 +2552,20 @@ def test_archive_invite_only_stream_youre_not_on(self) -> None:
)
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(
self,
target_users: list[UserProfile],
Expand Down Expand Up @@ -2631,16 +2645,60 @@ def test_realm_admin_remove_others_from_public_stream(self) -> None:
those you aren't on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=17,
query_count=33,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=True,
invite_only=False,
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)
self.assert_length(json["removed"], 1)
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:
"""
Expand All @@ -2658,8 +2716,8 @@ def test_realm_admin_remove_multiple_users_from_stream(self) -> None:
for name in ["cordelia", "prospero", "iago", "hamlet", "outgoing_webhook_bot"]
]
result = self.attempt_unsubscribe_of_principal(
query_count=24,
cache_count=8,
query_count=56,
cache_count=27,
target_users=target_users,
is_realm_admin=True,
is_subbed=True,
Expand All @@ -2676,7 +2734,7 @@ def test_realm_admin_remove_others_from_subbed_private_stream(self) -> None:
are on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=17,
query_count=33,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=True,
Expand All @@ -2693,7 +2751,7 @@ def test_realm_admin_remove_others_from_unsubbed_private_stream(self) -> None:
streams you aren't on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=17,
query_count=33,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=False,
Expand All @@ -2719,7 +2777,7 @@ def test_cant_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(
query_count=17,
query_count=33,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=True,
Expand All @@ -2733,7 +2791,7 @@ def test_admin_remove_others_from_stream_legacy_emails(self) -> None:

def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None:
result = self.attempt_unsubscribe_of_principal(
query_count=19,
query_count=43,
target_users=[self.example_user("cordelia"), self.example_user("prospero")],
is_realm_admin=True,
is_subbed=True,
Expand All @@ -2747,7 +2805,7 @@ def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None:

def test_remove_unsubbed_user_along_with_subbed(self) -> None:
result = self.attempt_unsubscribe_of_principal(
query_count=16,
query_count=17,
target_users=[self.example_user("cordelia"), self.example_user("iago")],
is_realm_admin=True,
is_subbed=True,
Expand All @@ -2764,7 +2822,7 @@ def test_remove_already_not_subbed(self) -> None:
fails gracefully.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=9,
query_count=10,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=False,
Expand All @@ -2780,7 +2838,7 @@ def test_bot_owner_can_remove_bot_from_stream(self) -> None:
webhook_bot = self.example_user("webhook_bot")
do_change_bot_owner(webhook_bot, bot_owner=user_profile, acting_user=user_profile)
result = self.attempt_unsubscribe_of_principal(
query_count=14,
query_count=15,
target_users=[webhook_bot],
is_realm_admin=False,
is_subbed=True,
Expand Down
71 changes: 69 additions & 2 deletions zerver/views/streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,59 @@ def compose_views(thunks: list[Callable[[], HttpResponse]]) -> dict[str, Any]:
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
def remove_subscriptions_backend(
request: HttpRequest,
Expand Down Expand Up @@ -505,16 +558,30 @@ def remove_subscriptions_backend(
unsubscribing_others=unsubscribing_others,
)

id_to_user_profile: dict[str, UserProfile] = {}
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(
realm, people_to_unsub, streams, acting_user=user_profile
)

for subscriber, removed_stream in removed:
result["removed"].append(removed_stream.name)
for subscriber, removed_channel in removed:
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:
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)


Expand Down