no repeat sightings - #5
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR implements duplicate sighting prevention by checking if a user has already recorded a sighting for a specific book before allowing a new one to be created. The change also includes cleanup of commented-out code.
- Added a database query to check for existing sightings by user and book combination
- Returns a 409 Conflict status when a duplicate sighting is attempted
- Removed unnecessary commented code
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Check for existing sighting by this user for this book | ||
| const { data: existingSighting } = await supabase | ||
| .from('sightings') | ||
| .select('id') | ||
| .eq('book_id', bookId) | ||
| .eq('user_id', user.id) | ||
| .single(); | ||
|
|
||
| if (existingSighting) { | ||
| return NextResponse.json( | ||
| { error: 'You have already recorded a sighting for this book' }, | ||
| { status: 409 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
There's a potential race condition between checking for existing sightings and inserting a new one. If two requests are processed simultaneously for the same user and book, both could pass the duplicate check before either completes the insert. Consider using a unique constraint on (user_id, book_id) in the database schema and handling the constraint violation error, or use a database transaction if Supabase supports it.
| // Check for existing sighting by this user for this book | ||
| const { data: existingSighting } = await supabase | ||
| .from('sightings') | ||
| .select('id') | ||
| .eq('book_id', bookId) | ||
| .eq('user_id', user.id) | ||
| .single(); | ||
|
|
||
| if (existingSighting) { | ||
| return NextResponse.json( | ||
| { error: 'You have already recorded a sighting for this book' }, | ||
| { status: 409 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
The new duplicate sighting prevention logic lacks test coverage. Since the repository has existing test coverage for other sightings endpoints (such as sightings/claim), tests should be added to verify that duplicate sightings are properly rejected and that the appropriate 409 status code and error message are returned.
| const { data: existingSighting } = await supabase | ||
| .from('sightings') | ||
| .select('id') | ||
| .eq('book_id', bookId) | ||
| .eq('user_id', user.id) | ||
| .single(); | ||
|
|
There was a problem hiding this comment.
The use of .single() here will throw an error if no matching record is found, which is not properly handled. This will cause the catch block to execute with a generic "Failed to create sighting" message when the user hasn't created a duplicate. Instead, use .maybeSingle() which returns null when no record exists, or check the error response from the query.
| const { data: existingSighting } = await supabase | |
| .from('sightings') | |
| .select('id') | |
| .eq('book_id', bookId) | |
| .eq('user_id', user.id) | |
| .single(); | |
| const { data: existingSighting, error: existingSightingError } = await supabase | |
| .from('sightings') | |
| .select('id') | |
| .eq('book_id', bookId) | |
| .eq('user_id', user.id) | |
| .maybeSingle(); | |
| if (existingSightingError) { | |
| throw existingSightingError; | |
| } |
No description provided.