Skip to content

fix: Add Chat-Group-Name-Timestamp header and use it to update group names (#6412) #6414

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

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

iequidoo
Copy link
Collaborator

@iequidoo iequidoo commented Jan 7, 2025

Fix #6412

@iequidoo iequidoo marked this pull request as ready for review January 7, 2025 20:38
@link2xt
Copy link
Collaborator

link2xt commented Jan 12, 2025

This definitely needs a rebase now. Or rather a rewrite, seems all the actual logic has been changed.

And as mentioned in #6401 (comment) maybe introduce a new header for the change timestamp, then apply last-write-wins logic to the group name. For compatibility with older Delta Chat explict ChatGroupNameChanged can be applied locally with bumping the timestamp to the sender timestamp if no Chat-Group-Name-Timestamp is sent.

@iequidoo iequidoo marked this pull request as draft February 20, 2025 02:31
@iequidoo iequidoo force-pushed the iequidoo/update-grp-name branch from ec27e8c to 8ef3e7e Compare February 26, 2025 02:14
@iequidoo iequidoo changed the title fix: Update group name on self-additions and missing messages (#6412) fix: Add Chat-Group-Name-Timestamp header and use it to update group names (#6412) Feb 26, 2025
@iequidoo iequidoo force-pushed the iequidoo/update-grp-name branch 3 times, most recently from cc8f613 to 5630c6e Compare February 26, 2025 16:18
@iequidoo iequidoo marked this pull request as ready for review February 26, 2025 16:45
@iequidoo iequidoo force-pushed the iequidoo/update-grp-name branch from 5630c6e to 95776cc Compare February 26, 2025 17:15
@iequidoo iequidoo requested review from link2xt and Hocuri February 26, 2025 17:18
@iequidoo iequidoo force-pushed the iequidoo/update-grp-name branch from 95776cc to b501af0 Compare February 27, 2025 03:09
@iequidoo iequidoo requested a review from link2xt February 27, 2025 03:16
bob_chat_id.accept(&bob).await?;

// Bob changes the group name.
// TODO: If Bob does this too fast, it's not guaranteed that his group name wins because "Date"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just remove this TODO. If Bob changes group name shortly after receiving another name, it is basically a simultaneous group change, does not matter who wins.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, now Chat-Group-Name-Timestamp never decreases at least because create_send_msg_jobs() calls ChatId::update_timestamp(). This means it may be greater than Date, but let's not fix this.

Removed TODO, it's not that important indeed

…names (#6412)

Add "Chat-Group-Name-Timestamp" message header and use the last-write-wins logic when updating group
names (similar to group member timestamps). Note that if the "Chat-Group-Name-Changed" header is
absent though, we don't add a system message (`MsgGrpNameChangedBy`) because we don't want to blame
anyone.
@iequidoo iequidoo force-pushed the iequidoo/update-grp-name branch from b501af0 to bcea9c5 Compare February 27, 2025 18:21
@iequidoo iequidoo requested a review from link2xt February 27, 2025 18:27
@iequidoo iequidoo merged commit b5e9a5e into main Feb 27, 2025
30 checks passed
@iequidoo iequidoo deleted the iequidoo/update-grp-name branch February 27, 2025 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Altered group name not applied/updated for rejoined members
2 participants