Follow-up from PR #60 review (non-blocking; the code is correct today). Two small robustness improvements to assignment_store.go:
1. Make the unique-violation path symmetric with the FK path
CreateAssignment (assignment_store.go:53-55) maps any 23505 to ErrAssignmentExists -> HTTP 409 via isUniqueViolation, which only inspects pgErr.Code and ignores pgErr.ConstraintName. The adjacent FK path deliberately switches on the constraint name and routes anything unrecognized to a wrapped 500 ("schema drift = server bug"). This is correct today because the composite PK (user_id, vehicle_id) is the only unique constraint on the table (migration 000006). But if a future migration adds another unique index, a violation of that constraint would silently surface as a misleading 409 "assignment already exists" with no slog.Error.
Suggested: verify the constraint name (user_vehicles_pkey) before mapping to ErrAssignmentExists, and fall through to a wrapped %w 500 otherwise -- mirroring the FK default case.
2. End-to-end coverage for the unrecognized-FK default branch
TestPgErrorHelpers covers the classification helpers with an unknown constraint name, but the default: arm inside CreateAssignment itself (assignment_store.go:64-66) is never executed end-to-end. Consider extracting the error-classification switch into a standalone classifyAssignmentError(err) error helper so the default arm (and the %w wrap that keeps errors.Is/errors.As working) can be unit-tested directly.
Neither item blocks anything currently shipping; filing so they aren't lost. Source: review of PR #60.
Follow-up from PR #60 review (non-blocking; the code is correct today). Two small robustness improvements to
assignment_store.go:1. Make the unique-violation path symmetric with the FK path
CreateAssignment(assignment_store.go:53-55) maps any23505toErrAssignmentExists-> HTTP 409 viaisUniqueViolation, which only inspectspgErr.Codeand ignorespgErr.ConstraintName. The adjacent FK path deliberately switches on the constraint name and routes anything unrecognized to a wrapped 500 ("schema drift = server bug"). This is correct today because the composite PK(user_id, vehicle_id)is the only unique constraint on the table (migration 000006). But if a future migration adds another unique index, a violation of that constraint would silently surface as a misleading409 "assignment already exists"with noslog.Error.Suggested: verify the constraint name (
user_vehicles_pkey) before mapping toErrAssignmentExists, and fall through to a wrapped%w500 otherwise -- mirroring the FKdefaultcase.2. End-to-end coverage for the unrecognized-FK
defaultbranchTestPgErrorHelperscovers the classification helpers with an unknown constraint name, but thedefault:arm insideCreateAssignmentitself (assignment_store.go:64-66) is never executed end-to-end. Consider extracting the error-classification switch into a standaloneclassifyAssignmentError(err) errorhelper so the default arm (and the%wwrap that keepserrors.Is/errors.Asworking) can be unit-tested directly.Neither item blocks anything currently shipping; filing so they aren't lost. Source: review of PR #60.