message_edit: Don't rely on .recipient_id change not affecting recipient.

The codepath for moving a topic changes the message.recipient_id to the
id of the new recipient, but later, in update_messages_for_topic_edit,
it uses message.recipient when querying for messages with the matching
topic in the *old* stream (because those are the other messages that
need to be moved). This is a bug which happens to work fine, because in
Django 2, if message.recipient gets fetched first and then
message.recipient_id is mutated, message.recipient will not be altered
and thus will retain the outdated, previously fetched value.

In Django 3 changing .recipient_id causes .recipient to be updated to
the new Recipient objects, which is the Recipient of the *new* stream.
That will cause the bug to manifest.

This is a bugfix preparing for the upgrade to Django 3.
This commit is contained in:
Mateusz Mandera
2021-01-09 19:56:12 +01:00
committed by Tim Abbott
parent f76202dd59
commit 3623681d30
2 changed files with 11 additions and 1 deletions

View File

@@ -4680,6 +4680,7 @@ def do_update_message(user_profile: UserProfile, message: Message,
event["propagate_mode"] = propagate_mode
event["stream_id"] = message.recipient.type_id
old_recipient_id = None
if new_stream is not None:
assert content is None
assert message.is_stream_message()
@@ -4687,6 +4688,7 @@ def do_update_message(user_profile: UserProfile, message: Message,
edit_history_event['prev_stream'] = stream_being_edited.id
event[ORIG_TOPIC] = orig_topic_name
old_recipient_id = message.recipient_id
message.recipient_id = new_stream.recipient_id
event["new_stream_id"] = new_stream.id
@@ -4751,6 +4753,7 @@ def do_update_message(user_profile: UserProfile, message: Message,
orig_topic_name=orig_topic_name,
topic_name=topic_name,
new_stream=new_stream,
old_recipient_id=old_recipient_id,
edit_history_event=edit_history_event,
last_edit_time=timestamp
)

View File

@@ -111,10 +111,17 @@ def update_messages_for_topic_edit(message: Message,
orig_topic_name: str,
topic_name: Optional[str],
new_stream: Optional[Stream],
old_recipient_id: Optional[int],
edit_history_event: Dict[str, Any],
last_edit_time: datetime) -> List[Message]:
assert (new_stream and old_recipient_id) or (not new_stream and not old_recipient_id)
propagate_query = Q(recipient = message.recipient, subject__iexact = orig_topic_name)
if old_recipient_id is not None:
recipient_id = old_recipient_id
else:
recipient_id = message.recipient_id
propagate_query = Q(recipient_id = recipient_id, subject__iexact = orig_topic_name)
if propagate_mode == 'change_all':
propagate_query = propagate_query & ~Q(id = message.id)
if propagate_mode == 'change_later':