paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-23 04:52:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

message_edit: Fix code to set visibility policy on moving messages.

Browse Source 
The code for updating visibility policy values on moving messages
had two bugs.

- There was a typo in elif condition where "user_profile" was being
used instead of "user_profile_with_policy".

This commit fixes the typo.

- It was assumed that there would be no UserTopic rows for target
topic if the target topic didn't exist. But there can be such case
where some messages were sent to that topic and the user muted
the topic. But then the messages in that topic was deleted. In
such case there can be UserTopic rows for a stream-topic pair
that does not exist.

This commit fixes the code to handle such case as well and set
the visibility policy of new topic to what was set for the original
topic. This change simplifies the condition to just check whether
new_visibility_policy is equal to target_topic_visibility_policy
and skip if so, and update the visibility policy otherwise.

Due to this change, we now do not try to mute the already muted
topic if the topic is moved to a topic which didn't exist
previously and thus we modify the existing test to not expect
any INFO logs.
This commit is contained in:
Sahil Batra
2023-05-10 08:17:55 +05:30
committed by Tim Abbott
parent a2600a2b97
commit d645d5c0ec
2 changed files with 15 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
zerver/actions/message_edit.py
View File

@@ -837,7 +837,7 @@ def do_update_message(
target_topic_user_profile_to_visibility_policy[
user_profile_with_policy
] = UserTopic.VisibilityPolicy.INHERIT
elif user_profile not in orig_topic_user_profile_to_visibility_policy:
elif user_profile_with_policy not in orig_topic_user_profile_to_visibility_policy:
orig_topic_user_profile_to_visibility_policy[
user_profile_with_policy
] = UserTopic.VisibilityPolicy.INHERIT
@@ -917,11 +917,12 @@ def do_update_message(
)
else:
# This corresponds to the case when messages are moved
# to a stream-topic pair that didn't exist.
if new_visibility_policy == UserTopic.VisibilityPolicy.INHERIT:
# to a stream-topic pair that didn't exist. There can
# still be UserTopic rows for the stream-topic pair
# that didn't exist if the messages in that topic had
# been deleted.
if new_visibility_policy == target_topic_visibility_policy:
# This avoids unnecessary db operations and INFO logs.
# As INHERIT is the default visibility_policy in the new
# stream-topic pair for users.
continue
bulk_do_set_user_topic_visibility_policy(
user_profiles,

26
zerver/tests/test_message_edit.py
View File

@@ -1427,23 +1427,15 @@ class EditMessageTest(EditMessageTestCase):
assert_is_topic_muted(hamlet, stream.id, change_later_topic_name, muted=True)
# Make sure we safely handle the case of the new topic being already muted.
with self.assertLogs(level="INFO") as info_logs:
check_update_message(
user_profile=hamlet,
message_id=message_id,
stream_id=None,
topic_name=already_muted_topic,
propagate_mode="change_all",
send_notification_to_old_thread=False,
send_notification_to_new_thread=False,
content=None,
)
self.assertEqual(
set(info_logs.output),
{
f"INFO:root:User {hamlet.id} tried to set visibility_policy to its current value of {UserTopic.VisibilityPolicy.MUTED}",
f"INFO:root:User {cordelia.id} tried to set visibility_policy to its current value of {UserTopic.VisibilityPolicy.MUTED}",
},
check_update_message(
user_profile=hamlet,
message_id=message_id,
stream_id=None,
topic_name=already_muted_topic,
propagate_mode="change_all",
send_notification_to_old_thread=False,
send_notification_to_new_thread=False,
content=None,
)
assert_is_topic_muted(hamlet, stream.id, change_later_topic_name, muted=False)
assert_is_topic_muted(hamlet, stream.id, already_muted_topic, muted=True)