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

Deduplicate new puzzles by URL. #2313

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jpd236
Copy link
Contributor

@jpd236 jpd236 commented Oct 24, 2024

When adding a puzzle, we start a transaction to atomically check if a puzzle already exists with that URL and only add it if either no such puzzle exists or if the client indicates that duplicate URLs are allowed. In the event of a transaction failure, we delete the Google document that was created optimistically before saving the puzzle.

On the client side, we add a new checkbox to allow duplicate URLs, unchecked by default. If we encounter a duplicate URL error, we show an error indicating that the puzzle may be a dupe and telling the user what to check if the duplicate URLs are intentional.

See #2275

@jpd236
Copy link
Contributor Author

jpd236 commented Oct 24, 2024

This should be functional, but there are some less-polished (IMO) elements as I wanted to get feedback before moving forward. At least the following:

  • The UX. Do we always want to show this checkbox as is done here? Or should it be a more interactive flow, e.g. a (nested) dialog indicating that the puzzle is potential duplicate and offering the user a simple Continue/Abort option? Should we provide more details about the duplicate? I kept it simple for now.
  • The error passing. I just throw a Meteor.Error with an unusual (but reasonable) HTTP error code and detect it client side. This should probably be something a bit more semantically meaningful. What's the canonical way of doing this?
  • The transaction. This seems like the first use of a transaction in the codebase. It seems reasonable at the face of it, but I'm pretty unfamiliar with MongoDB and am not sure about the semantics. In other places, we just use locks - that could also be reasonable here, e.g. with a hunt-specific puzzle creation (and updating?) lock, if that's preferable.

When adding a puzzle, we start a transaction to atomically check if a
puzzle already exists with that URL and only add it if either no such
puzzle exists or if the client indicates that duplicate URLs are
allowed. In the event of a transaction failure, we delete the Google
document that was created optimistically before saving the puzzle.

On the client side, we add a new checkbox to allow duplicate URLs,
unchecked by default. If we encounter a duplicate URL error, we show an
error indicating that the puzzle may be a dupe and telling the user what
to check if the duplicate URLs are intentional.
@jpd236 jpd236 force-pushed the dedupe-add-puzzle branch from c988d1e to 4f3e4c0 Compare December 1, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant