-
Notifications
You must be signed in to change notification settings - Fork 96
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
fix: shared post empty state #2615
Conversation
@@ -21,6 +22,10 @@ const worker: Worker = { | |||
typeof flags === 'string' ? JSON.parse(flags as string) : flags; | |||
const { deletedBy } = parsedFlags; | |||
|
|||
if (deletedBy === DELETED_BY_WORKER) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
.createQueryBuilder() | ||
.update() | ||
.where({ | ||
sharedPostId: post.id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure we have index for sharedPostId
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have one:
IDX_sharedPostId (it's just singular, but for this query probably ok)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes should be ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks amazing. Just the order of the migration queries 🚀
Couple things that happen here:
a. If has commentary we set it to not show on feed
b. If no commentary we simply set as soft deleted, but add the new DELETED_BY_WORKER flag, this way no reputation is lost.