-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
Add gdoc tombstones #3979
Add gdoc tombstones #3979
Conversation
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 428 (89b864) ❌ Edited: 2024-10-03 16:16:57 UTC |
ce053c1
to
c6cdca9
Compare
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! I like it 👍
I wonder if we should have some admin UI to manage tombstones with, though. Obviously I couldn't resist trying to redelete a slug that already had a tombstone (which fails) plus I can imagine people potentially wanting to read/update/delete the tombstones. Just a simple table would work well, I think?
a825865
to
914fc8a
Compare
914fc8a
to
7302276
Compare
5649836
to
a6287f0
Compare
`SELECT * | ||
FROM images | ||
WHERE id IN (SELECT DISTINCT imageId FROM posts_gdocs_x_images) | ||
OR filename IN (SELECT DISTINCT relatedLinkThumbnail FROM posts_gdocs_tombstones)` |
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.
Does this make sense, @ikesau?
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.
BTW, is subquery faster than a join, or why are we doing that?
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.
Will finish the review tomorrow but one thing that jumps out here is that we could get duplicates in images
now, I think? Maybe SELECT DISTINCT *
could help.
I don't know of a particular reason to subqueries over joins in this case. Feel free to change it to whichever's more legible.
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.
I added DISTINCT
and index on relatedLinkThumbnail
.
@ikesau could you review the new comments since your last review, pls? |
cecbc74
to
a400bb1
Compare
- Copy fixes - Allow external related links - Include Wayback Machine link based on a checkbox
a400bb1
to
9f12eb6
Compare
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! Tested all the cases and it all seems good.
2 teeny tiny little comments 🤏
Closes #3902
Without reason or related link specified:
With reason and related link specified:
Generic 404:
Modified existing delete modal in the admin (remaining fields are visible only when the checkbox is checked):