paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-09 16:37:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

move_topic_to_stream: Delete UserMessage for new stream unsubs.

Browse Source 
For users who are unsubscribed from the new stream but are in
the old stream, we delete the UserMessage.

We send the delete_message event only to guest users,
who have completely lost asses to the moved messages, for other
users we send the normal update_message event which moves
the messages to the new unsubed stream which
otherwise would look broken to the
user without reloading to the webpage.
This commit is contained in:
Aman Agrawal
2020-06-29 20:10:51 +05:30
committed by Tim Abbott
parent 8a54dc43ce
commit 3f42d15168
2 changed files with 24 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
zerver/lib/actions.py
View File

@@ -4556,14 +4556,20 @@ def do_update_message(user_profile: UserProfile, message: Message,
new_stream_sub_ids = [user.user_profile_id for user in subs_to_new_stream] new_stream_sub_ids = [user.user_profile_id for user in subs_to_new_stream]
# Get guest users who aren't subscribed to the new_stream. # Get users who aren't subscribed to the new_stream.
guest_subs_losing_access = [ subs_losing_usermessages = [
sub for sub in subscribers sub for sub in subscribers
if sub.user_profile_id not in new_stream_sub_ids
]
# Users who can longer access the message without some action
# from admins.
# TODO: Extend for case when new stream is private.
subs_losing_access = [
sub for sub in subs_losing_usermessages
if sub.user_profile.is_guest if sub.user_profile.is_guest
and sub.user_profile_id not in new_stream_sub_ids
] ]
ums = ums.exclude(user_profile_id__in=[ ums = ums.exclude(user_profile_id__in=[
sub.user_profile_id for sub in guest_subs_losing_access]) sub.user_profile_id for sub in subs_losing_usermessages])
if topic_name is not None: if topic_name is not None:
topic_name = truncate_topic(topic_name) topic_name = truncate_topic(topic_name)
@@ -4590,12 +4596,12 @@ def do_update_message(user_profile: UserProfile, message: Message,
assert stream_being_edited is not None assert stream_being_edited is not None
message_ids = [msg.id for msg in changed_messages] message_ids = [msg.id for msg in changed_messages]
# Delete UserMessage objects from guest users who will no # Delete UserMessage objects for users who will no
# longer have access to these messages. Note: This could be # longer have access to these messages. Note: This could be
# very expensive, since it's N guest users x M messages. # very expensive, since it's N guest users x M messages.
UserMessage.objects.filter( UserMessage.objects.filter(
user_profile_id__in=[sub.user_profile_id for sub in user_profile_id__in=[sub.user_profile_id for sub in
guest_subs_losing_access], subs_losing_usermessages],
message_id__in=message_ids, message_id__in=message_ids,
).delete() ).delete()
@@ -4606,7 +4612,7 @@ def do_update_message(user_profile: UserProfile, message: Message,
'stream_id': stream_being_edited.id, 'stream_id': stream_being_edited.id,
'topic': orig_topic_name, 'topic': orig_topic_name,
} }
delete_event_notify_user_ids = [sub.user_profile_id for sub in guest_subs_losing_access] delete_event_notify_user_ids = [sub.user_profile_id for sub in subs_losing_access]
send_event(user_profile.realm, delete_event, delete_event_notify_user_ids) send_event(user_profile.realm, delete_event, delete_event_notify_user_ids)
if message.edit_history is not None: if message.edit_history is not None:
@@ -4655,9 +4661,8 @@ def do_update_message(user_profile: UserProfile, message: Message,
subscribers = subscribers.exclude(user_profile_id__in=[um.user_profile_id for um in ums]) subscribers = subscribers.exclude(user_profile_id__in=[um.user_profile_id for um in ums])
if new_stream is not None: if new_stream is not None:
assert guest_subs_losing_access is not None assert subs_losing_access is not None
# Exclude guest users who are not subscribed to the new stream from receing this event. subscribers = subscribers.exclude(user_profile_id__in=[sub.user_profile_id for sub in subs_losing_access])
subscribers = subscribers.exclude(user_profile_id__in=[sub.user_profile_id for sub in guest_subs_losing_access])
# All users that are subscribed to the stream must be notified when a message is edited # All users that are subscribed to the stream must be notified when a message is edited
subscribers_ids = [user.user_profile_id for user in subscribers] subscribers_ids = [user.user_profile_id for user in subscribers]

9
zerver/tests/test_messages.py
View File

@@ -3572,11 +3572,15 @@ class EditMessageTest(ZulipTestCase):
"iago", "test move stream", "new stream", "test") "iago", "test move stream", "new stream", "test")
guest_user = self.example_user('polonius') guest_user = self.example_user('polonius')
non_guest_user = self.example_user('hamlet')
self.subscribe(guest_user, old_stream.name) self.subscribe(guest_user, old_stream.name)
self.subscribe(non_guest_user, old_stream.name)
msg_id_to_test_acesss = self.send_stream_message(user_profile, old_stream.name, msg_id_to_test_acesss = self.send_stream_message(user_profile, old_stream.name,
topic_name='test', content="fourth") topic_name='test', content="fourth")
self.assertEqual(has_message_access(guest_user, Message.objects.get(id=msg_id_to_test_acesss), None), True) self.assertEqual(has_message_access(guest_user, Message.objects.get(id=msg_id_to_test_acesss), None), True)
self.assertEqual(has_message_access(non_guest_user, Message.objects.get(id=msg_id_to_test_acesss), None), True)
result = self.client_patch("/json/messages/" + str(msg_id), { result = self.client_patch("/json/messages/" + str(msg_id), {
'message_id': msg_id, 'message_id': msg_id,
@@ -3587,6 +3591,11 @@ class EditMessageTest(ZulipTestCase):
self.assert_json_success(result) self.assert_json_success(result)
self.assertEqual(has_message_access(guest_user, Message.objects.get(id=msg_id_to_test_acesss), None), False) self.assertEqual(has_message_access(guest_user, Message.objects.get(id=msg_id_to_test_acesss), None), False)
self.assertEqual(has_message_access(non_guest_user, Message.objects.get(id=msg_id_to_test_acesss), None), True)
self.assertEqual(UserMessage.objects.filter(
user_profile_id=non_guest_user.id,
message_id=msg_id_to_test_acesss,
).count(), 0)
self.assertEqual(has_message_access(self.example_user('iago'), Message.objects.get(id=msg_id_to_test_acesss), None), True) self.assertEqual(has_message_access(self.example_user('iago'), Message.objects.get(id=msg_id_to_test_acesss), None), True)
def test_no_notify_move_message_to_stream(self) -> None: def test_no_notify_move_message_to_stream(self) -> None: