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

patch: skip archived messages after one attempt #518

Merged
merged 7 commits into from
Jul 4, 2023

Conversation

ckcr4lyf
Copy link
Contributor

@ckcr4lyf ckcr4lyf commented Jul 4, 2023

Currently, archived threads are a big problem: We fail to delete messages in an archived thread. This itself could be seen as a problem, but the bigger problem (IMO), is that these messages keep coming up when searching new pages, since they are still returned by the search call! This is a major problem when the number of archived messages hits 25, since a new page will return all archived messages, and no progress can be made.

This MR implements a kind of patch - if we fail to delete a message, and the reason is that the thread is archived , then we increment the offset.

This allows the fetch of the next page to skip these archived messages from the search results, allowing the overall deletion to make further progress.

Other MRs

#457 -> This hack fix doesn't help much, since the next search will yield these messages again.

#393 -> This is arguable more efficient, but has a lot of changes, so may take more time to review merge. In the meantime a patch to at least allow further progress on non-archived messages could be useful to some.

@ckcr4lyf
Copy link
Contributor Author

ckcr4lyf commented Jul 4, 2023

Before: Notice how the next page gets the archived messages again...

before_undiscord

After: Next page continues off of last deletion (thanks to increment on offset)

after_undiscord

@ckcr4lyf
Copy link
Contributor Author

ckcr4lyf commented Jul 4, 2023

Seems there are conflicts, let me rebase

@ckcr4lyf
Copy link
Contributor Author

ckcr4lyf commented Jul 4, 2023

FYI: RE: 2k changed lines on the compiled script, I just ran npm run lint:fix after merging latest master, I think #439 erroneously introduced those different line break stuffs

@victornpb
Copy link
Owner

thank you for your contribution, looks good to me. A more long term solution would be trying to filter out threads entirely, idk if thats possible tho

@victornpb
Copy link
Owner

FYI: RE: 2k changed lines on the compiled script, I just ran npm run lint:fix after merging latest master, I think #439 erroneously introduced those different line break stuffs

you are right, thank you

@victornpb victornpb merged commit f92ada0 into victornpb:master Jul 4, 2023
3 checks passed
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.

None yet

2 participants