Skip to content

no repeat sightings #5

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.

Sign up for GitHub

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

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

no repeat sightings #5

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion app/api/sightings/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,28 @@ export async function POST(request: Request) {
);
}

// 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();

Comment on lines +58 to +64

Copilot AI Dec 14, 2025

Copy link

Choose a reason for hiding this comment

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

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.

Suggested change
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;
}

Copilot uses AI. Check for mistakes.
if (existingSighting) {
return NextResponse.json(
{ error: 'You have already recorded a sighting for this book' },
{ status: 409 }
);
}
Comment on lines +57 to +70

Copilot AI Dec 14, 2025

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +57 to +70

Copilot AI Dec 14, 2025

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

// Insert: Use User Client (respects RLS)
const { data, error } = await supabase
.from('sightings')
.insert({
book_id: bookId,
user_id: user.id,
location,
// date: Date.now(), // Schema handles created_at
sighting_type: 'SIGHTING'
})
.select()
Expand Down