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

refactor post delete #20

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alphatownsman
Copy link

@alphatownsman alphatownsman commented Jun 22, 2024

This PR moves most delete logic into Post.handle_deleted, and addresses the following issues of previous implementation:

  • mass deletion was done in API request handler immediately, it can be really slow for a popular post in a large instance
  • bookmark was still visible for 24 hr
  • if a local identity boost a remote post, remote followers of this local identity may still see the boost of this post in their timeline (unless they are on the same instance of the author, or anyone on that instance follows the author)

For the last issue, this PR only UNDO the boost, but not forward the DELETE activity to remote instances bc it's too much work. Although both were done in Mastodon.

@alphatownsman alphatownsman force-pushed the post-delete branch 2 times, most recently from c12aa62 to 06b4ddb Compare June 22, 2024 14:20
)
TimelineEvent.objects.filter(
identity=self.post.author,
type__in=[TimelineEvent.Types.post, TimelineEvent.Types.boost],
Copy link
Member

Choose a reason for hiding this comment

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

Why just these two types?

@dcwatson
Copy link
Member

  1. I'm on board with moving the deletes into a stator handler, but why the immediate partial delete vs. a full delete later? Some kind of client refresh timing thing?
  2. Good catch about the bookmarks.
  3. So this change does not fan out the delete to remote instances anymore? Was that intentional?

@alphatownsman
Copy link
Author

alphatownsman commented Jun 24, 2024

I'm on board with moving the deletes into a stator handler, but why the immediate partial delete vs. a full delete later? Some kind of client refresh timing thing?

yep, just to make sure the author sees change immediately in case stator does not catch up fast enough; I added a comment for this but I'm ok to remove this hackish delete if you prefer.

So this change does not fan out the delete to remote instances anymore? Was that intentional?

Sorry, accidentally dropped that when copy paste from my production branch, should be good now

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