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

Unarchive thread to delete messages #520

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ckcr4lyf
Copy link
Contributor

@ckcr4lyf ckcr4lyf commented Jul 6, 2023

Currently, if a thread is archived, we just skip those messages. Instead, we can try and unarchive the thread, then delete messages (this is what discord does if you right-click & delete a message in an archived thread - first unarchive, then delete).

Since the while loop already attempts twice, if we fail due to archived, we can:

  1. Attempt to unarchive
  2. If unarchive successfully, return RETRY , so caller fn will try and delete again
  3. If fail to unarchive, skip (existing logic).

This can help people delete more messages.

Additionally, while implementing this I found a bug in system messages - Thread started messages (type 21) are not deletable (as per Discord, see: https://discord.com/developers/docs/resources/channel#message-object-message-types)

Updated the filter logic to be <=20 so 21 is not counted.

@ckcr4lyf
Copy link
Contributor Author

ckcr4lyf commented Jul 6, 2023

Before: Skip archived threads

fail_archived

After (v0): Unarchive (but fail on system message type 21)

thread_v0

After (v1): Unarchive, skip system message

thread_v1_skip

@ckcr4lyf
Copy link
Contributor Author

ckcr4lyf commented Jul 6, 2023

Actually, @victornpb , not sure if you are OK with this behavior (i.e auto-unarchive). Bug #516 seems to suggest a user would not expect this behavior, so maybe we need to either explicitly document it or offer it as an option?

(Personally: If I want to wipe my messages from a channel, I would want to wipe from threads under it as well.)

@RainerZufall187
Copy link

++++++++++++++++++++

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.

2 participants