feat: add user-vehicle assignment CRUD endpoints

[diveshpatil9104]

Summary

• Add user_vehicles join table (migration 000008) with composite PK, explicitly named FK constraints, CASCADE deletes, and vehicle_id index
• Add 4 admin-only endpoints behind authMiddleware + adminMiddleware
• Store layer uses sqlc-generated queries with FK/unique constraint error mapping
• 37 unit tests + 12 integration tests

Why

Milestone 2 requires admin endpoints to manage many-to-many relationships between users and vehicles (README line 264). The user_vehicles join table is defined in the README database design but does not exist yet.

Without assignment endpoints, admins cannot connect drivers to vehicles — the start-trip endpoint (PR #55) will always fail its assignment check. This PR is the missing link:

1. Admin creates vehicle (feat: add admin vehicle CRUD endpoints (list, get, upsert, deactivate) #49)
2. Admin creates driver ( feat: add admin user CRUD endpoints  #57)
3. Admin assigns driver to vehicle (this PR)
4. Driver starts trip ( feat: add trip management endpoints (start/end trips #55) — assignment check passes

What Changed

migrations/000008_add_user_vehicles.up.sql — Creates user_vehicles table with PK (user_id, vehicle_id), explicitly named FK constraints with ON DELETE CASCADE, created_at timestamp, and index on vehicle_id.

assignment_store.go — Store methods using sqlc-generated queries (matching store_vehicles.go pattern). Four interfaces for testability. Detects unique violations (duplicate → 409) and FK violations with constraint name switch including default case (user/vehicle not found → 404). List queries capped at LIMIT 1000.

assignment_handlers.go — 4 handlers following existing closure pattern. Content-Type validation, MaxBytesReader (1KB), DisallowUnknownFields, trailing data check. Reuses existing validateVehicleID(). Nil-slice guards ensure [] not null in JSON responses.

assignment_handlers_test.go — 37 unit tests with mock store: validation, boundary tests (50/51 char vehicle_id), Content-Type, JSON edge cases, FK violations, duplicates. All assert error messages, not just status codes.

assignment_store_test.go — 12 integration tests: round-trip, duplicate, delete, FK violations with rollback verification, empty lists, ordering with explicit created_at offsets (no time.Sleep), CASCADE behavior.

main.go — Register 4 routes under /api/v1/admin/ wrapped with authMiddleware(adminMiddleware(...)).

Routes

Method	Route	What
POST	/api/v1/admin/assignments	Assign driver to vehicle
DELETE	/api/v1/admin/users/{userID}/vehicles/{vehicleID}	Remove assignment
GET	/api/v1/admin/users/{id}/vehicles	List vehicles for a driver
GET	/api/v1/admin/vehicles/{id}/users	List drivers for a vehicle

[aaronbrethorst]

Divesh, the implementation quality here is impressive — the interface segregation, the compile-time satisfaction checks, the thorough input validation discipline (Content-Type, MaxBytesReader, DisallowUnknownFields, trailing-data rejection), and the 49 tests across unit and integration layers all show real care. The scanAssignments shared helper, the non-nil empty slice initialization, and the pgconn error code detection are all well-executed.

That said, there are a couple of issues that need to be resolved before this can merge.

Critical Issues (2 found)

1. Admin endpoints have no authorization check — any authenticated user can manage assignments (main.go:84-89). The routes live under /api/v1/admin/ but are protected only by authMiddleware (requireAuth), which validates the JWT but does not check the role claim. This means any authenticated driver can create/delete/list assignments for any user. The JWT already includes a role claim (see auth.go), and requireAdminToken already exists in the codebase (used by the GTFS upload endpoint on the same branch). Shipping admin-scoped CRUD endpoints without role-based authorization is a security gap. The TODO comment acknowledges this, but the fix is trivial — either use the existing requireAdminToken middleware or add an inline role check. This should not ship without authorization.

2. Migration number conflict — 000004 is already taken on main (migrations/000004_add_user_vehicles.{up,down}.sql). PR #40 was merged to main and occupies migration 000004_add_vehicle_received_index. This PR also uses 000004. When both exist, golang-migrate will fail at startup because it expects unique, sequential migration numbers. You'll need to rebase against main and renumber to 000005.

Important Issues (4 found)

1. Raw SQL queries instead of sqlc (assignment_store.go). The existing codebase uses sqlc for all database queries (see db/query.sql.go and store.go which delegates to s.queries.*). This PR writes raw SQL directly against s.pool and manually scans results, creating two divergent data access patterns. The sqlc approach catches SQL errors at generation time and keeps the schema definition in sync. Please add the assignment queries to db/query.sql and regenerate.

2. FK constraint name mismatch silently converts a 4xx into a 500 (assignment_store.go:59-66). The switch on fkConstraintName(err) matches against hardcoded strings "user_vehicles_user_id_fkey" and "user_vehicles_vehicle_id_fkey". If the constraint name doesn't match either case (e.g., due to a migration rename or explicit naming), the switch falls through and returns a generic error. The handler then maps that to a 500 instead of a 404. Two fixes: (a) explicitly name the constraints in the migration DDL so the names are guaranteed, and (b) add a default case that logs the unrecognized constraint name and returns a 4xx-mappable error rather than silently falling through to 500.

3. Missing pagination on list endpoints (assignment_store.go:90-108). Neither ListAssignmentsByUser nor ListAssignmentsByVehicle has a LIMIT. If a vehicle has thousands of assignments, the response is unbounded. At minimum, add a safety cap (e.g., LIMIT 1000) to the SQL queries.

4. Nil-slice serialization risk in list handlers (assignment_handlers.go:140,160). The store currently returns make([]AssignmentResponse, 0) (non-nil), so json.Encode produces []. But the handler trusts this — if a future refactor returns nil, nil, the API silently changes from [] to null. Add a nil guard in the handler: if assignments == nil { assignments = []AssignmentResponse{} }.

Suggestions (2 found)

• Inconsistent validation messages across endpoints. handleCreateAssignment provides specific error messages for each vehicle_id validation failure ("vehicle_id is required", "must be at most 50 characters", "must contain only alphanumeric characters..."), but handleDeleteAssignment and handleListVehicleUsers collapse all three checks into a single generic "invalid vehicle id" message. Consider extracting a shared validateVehicleID function.

• Inconsistent 4xx logging. handleDeleteAssignment logs at slog.Warn when a delete targets a nonexistent assignment (line 112), but the other handlers don't log 4xx responses. Either remove the Warn from delete-not-found to match, or add equivalent logging to all 4xx paths.

Strengths

• Clean interface segregation (AssignmentCreator, AssignmentDeleter, AssignmentListerByUser, AssignmentListerByVehicle) with compile-time satisfaction checks
• Thorough input validation: Content-Type, MaxBytesReader (1KB), DisallowUnknownFields, trailing-data rejection via json.RawMessage
• 49 tests total — 37 unit tests with mock store covering happy paths, validation, JSON edge cases, FK violations, and duplicates; 12 integration tests with real PostgreSQL including cascade delete verification and explicit timestamp ordering
• Non-nil empty slice initialization ensures [] not null in JSON
• INSERT ... RETURNING created_at uses DB-generated timestamp rather than fabricating time.Now()
• Proper error wrapping with distinct messages for diagnostic differentiation
• DELETE uses path params instead of body — good REST practice

Recommended Action

1. Add admin authorization (use requireAdminToken or equivalent) — this is a security blocker
2. Rebase against main and renumber migration to 000005
3. Address the sqlc consistency, FK constraint naming, and pagination issues
4. Re-run review after fixes

[diveshpatil9104]

All changes addressed:

• Wrapped all routes with adminMiddleware
• Converted from raw pool to sqlc-generated queries
• Renumbered migration to 000008
• Added explicit FK constraint names with default case
• Added LIMIT 1000 to list queries
• Added nil-slice guards in handlers
• Reused existing validateVehicleID()
• Removed inconsistent warn log on delete-not-found