Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Migrates the app’s data layer from Prisma/Postgres to Drizzle (libsql/Turso) and updates the frontend/tooling stack (Next.js 16, Tailwind v4), including adapting e2e tests and server actions to the new database APIs.
Changes:
- Replaced Prisma client usage with Drizzle
db+ new SQLite schema and migration artifacts. - Updated server actions and Playwright tests to use Drizzle queries and a file-based SQLite test DB.
- Migrated Tailwind to v4 (CSS-first) and updated various UI classes and build tooling/dependencies.
Reviewed changes
Copilot reviewed 45 out of 49 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adjusts TS/JSX compiler settings and include paths for updated Next/tooling. |
| tests/global-setup.ts / tests/global-teardown.ts | Sets up/tears down a SQLite test DB and pushes schema for e2e. |
| tests/**/*.spec.ts | Updates tests from Prisma to Drizzle queries and types. |
| src/lib/database.ts | Switches DB client to Drizzle libsql/Turso connection. |
| src/lib/schema.ts | Introduces Drizzle SQLite schema + relations for app tables. |
| src/lib/db.error.ts | Replaces Prisma error mapping with libsql error mapping. |
| src/app/**/actions.tsx | Rewrites server actions to Drizzle queries and new error handling. |
| src/proxy.ts | Renames exported function (previously middleware) and keeps matcher config. |
| src/app/globals.css / postcss.config.mjs / tailwind.config.ts | Moves to Tailwind v4 CSS-first and new PostCSS plugin; removes old config file. |
| package.json / pnpm-workspace.yaml / docker-compose.yaml / .env.test | Updates scripts, deps, pnpm config, removes Postgres from compose, and updates env vars for Turso/libsql. |
Comments suppressed due to low confidence (1)
src/proxy.ts:4
- Next.js middleware is only picked up when exported as
middlewarefrommiddleware.ts(orsrc/middleware.ts). Renaming the function here toproxy(and not having amiddleware.ts) will prevent this logic from running. Consider restoring the expectedmiddlewareexport/name/location, or add amiddleware.tsthat re-exports this function asmiddleware.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const userRelations = relations(user, ({ many, one }) => ({ | ||
| attendants: many(attendant), | ||
| rooms: many(attendanceRoom), | ||
| activeSemester: one(semester, { |
There was a problem hiding this comment.
relations(...) is declared before the referenced tables (attendant, attendanceRoom, attendanceRecord) are initialized. In JS/TS, referencing those const bindings before their declarations can throw at module load time. Reorder this file to declare all sqliteTable(...) tables first, then define all relations(...) after the table constants exist (or otherwise avoid forward references in relation callbacks).
| const rawData = await db.query.attendanceRecord.findMany({ | ||
| where: and( | ||
| eq(attendanceRecord.attendanceRoomId, id), | ||
| gte(attendanceRecord.createdAt, today.toISOString()) | ||
| ), |
There was a problem hiding this comment.
getAttendanceRecordByRoomId no longer verifies that the requested room belongs to the authenticated user. As written, any authenticated user who can guess a room id could fetch attendance records for that room. Add an ownership check (e.g., fetch the room with createdBy = session.id first, or join attendance_room and filter by createdBy) before returning records/count.
| "dev": "next dev", | ||
| "build": "bun db:migrate && next build", |
There was a problem hiding this comment.
The build script runs pnpm db:migrate as part of pnpm build. Running migrations during build can cause unexpected schema changes during CI builds, require DB connectivity at build time, and can be unsafe with concurrent deploys. Prefer running migrations as an explicit deploy step (or in CI/CD) separate from next build.
| where: and( | ||
| eq(attendanceRecord.attendantId, attendantId), | ||
| eq(attendanceRecord.attendanceRoomId, roomId), | ||
| gte(attendanceRecord.createdAt, today.toISOString()) | ||
| ), |
There was a problem hiding this comment.
Same timestamp format issue as elsewhere: created_at is stored via SQLite CURRENT_TIMESTAMP text, but this compares using today.toISOString() in gte(...). This can misclassify “today” records. Use consistent storage (ISO 8601) or use SQLite datetime comparison functions in the predicate.
| volumes: | ||
| postgres_data: |
There was a problem hiding this comment.
The postgres service was removed, but the postgres_data volume is still declared. Either remove the unused volume or rename it to reflect the current services to avoid confusion in local dev.
| volumes: | |
| postgres_data: |
| eq(attendanceRecord.attendanceRoomId, id), | ||
| gte(attendanceRecord.createdAt, today.toISOString()) | ||
| ), |
There was a problem hiding this comment.
created_at is stored as SQLite CURRENT_TIMESTAMP text (e.g. YYYY-MM-DD HH:MM:SS), but this compares it against today.toISOString() (YYYY-MM-DDTHH:MM:SS.sssZ). A plain string gte comparison is unreliable across these formats. Store timestamps in a comparable format (e.g. ISO 8601 consistently) or compare using SQLite datetime functions (e.g. datetime(created_at) >= datetime(?)) via a sql predicate.
| attendant, | ||
| attendanceRecord, | ||
| attendanceRoom, | ||
| user, |
There was a problem hiding this comment.
user is imported from @/lib/schema but never used in this file. Remove the unused import to avoid lint/TS unused-import failures.
| user, |
| for (const room of rooms) { | ||
| await db.insert(attendanceRoom).values(room); | ||
| } |
There was a problem hiding this comment.
This inserts 21 rows one-by-one, which is noticeably slower in Playwright runs than a single bulk insert. Drizzle supports insert(...).values(arrayOfRows) (or wrapping inserts in a transaction) to create these rooms in one call.
Autopilot PR Check IssuesThe following potential issues were detected in this PR:
|
No description provided.