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

feat: delete messages on IMAP when deleting chat #6613

Merged
merged 3 commits into from
Mar 6, 2025

Conversation

r10s
Copy link
Contributor

@r10s r10s commented Mar 4, 2025

this PR deletes all known messages belonging to a chat when the chat is deleted.

this may not be an exhaustive list as a client might not know all message-ids (eg. when using different times for "delete from device"). in this case, other devices may know more IDs. otherwise, the chatmail server will eventually clean up at some point. for non-chatmail, this is up to the user then.

the deletion sql commands were inspired by delete_msgs_ex (in fact, a first try was adapting that part, however, that seems less performant as lots of sql commands are needed)

successor of #5007

@r10s r10s requested review from link2xt and iequidoo March 4, 2025 17:56
@r10s r10s changed the title delete messages on IMAP when deleting chat feat: delete messages on IMAP when deleting chat Mar 4, 2025
r10s added 2 commits March 4, 2025 19:04
this improves deletion in case different client know different messages to delete.

deletion is fast and it is not an issue to delete messages that are already deleted
(this is even the most common case for chatmail)
@iequidoo
Copy link
Collaborator

iequidoo commented Mar 6, 2025

Looks good, but if bcc_self is disabled, sync messages are not sent, so the result on other devices depends on whether they are online and have already received messages. I think that it's supposed that other devices are online and should still have messages deleted on the current device in this case

@link2xt
Copy link
Collaborator

link2xt commented Mar 6, 2025

bcc_self means "multi-device", so if you have a multi-device setup with bcc_self set to false, you have a broken multi-device setup.

(delete_msgs_target, self,),
)?;
transaction.execute(
"DELETE FROM smtp WHERE msg_id IN (SELECT id FROM msgs WHERE chat_id=?)",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought this might have weird consequences in large groups. If the message is split into multiple SMTP messages, then second device might receive a message and then delete the chat. Then first device will remove the message from SMTP queue and stop sending the rest of the copies.

But all is fine: we always place bcc-self copy as the last copy into smtp queue.

@r10s r10s merged commit 8e25639 into main Mar 6, 2025
29 checks passed
@r10s r10s deleted the r10s/clear-chat-on-delete2 branch March 6, 2025 21:43
@iequidoo
Copy link
Collaborator

iequidoo commented Mar 6, 2025

bcc_self means "multi-device", so if you have a multi-device setup with bcc_self set to false, you have a broken multi-device setup.

Generally, yes, but sometimes i disable bcc_self to make some actions that shouldn't be synced to other devices, e.g. i have different muted chats on devices because i use them for different purposes. Not that important though, and probably nobody deletes messages this way.

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.

3 participants