chore: add Drizzle schema and generated migrations for default and per-server databases#7395
Conversation
…r-server databases
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (4)**/*.{js,jsx,ts,tsx,json}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (3)📚 Learning: 2026-04-30T17:07:51.020ZApplied to files:
📚 Learning: 2026-02-05T13:55:00.974ZApplied to files:
📚 Learning: 2026-05-07T17:47:14.516ZApplied to files:
🔇 Additional comments (17)
WalkthroughThis PR introduces Drizzle ORM infrastructure for SQLite persistence in a React Native app, adding two independent databases (app and servers) with 16 total tables, SQL migrations, type definitions, and validation tests to enable structured ORM-based data access. ChangesDrizzle ORM Database Foundation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
diegolmello
requested a deployment
to
android_build
GitHub Actions
Waiting
diegolmello
left a comment
diegolmello
left a comment
There was a problem hiding this comment.
Structural review (NATIVE-1275). Schema is column-complete vs the WatermelonDB source and the generated migrations are clean drizzle-kit output (no hand-edits). Two findings worth acting on before merge are inline below.
Minor notes, not blocking: the two near-identical drizzle.app.config.ts / drizzle.servers.config.ts exist only because drizzle-kit takes one schema target per config — a one-line comment saying so would stop a future "dedupe" that breaks generation. The mixed indentation in migrations/*/migrations.js is drizzle-kit's own Expo-template output (both files are prettier-ignored), so it's not actionable.
| avatar_etag: text('avatar_etag') | ||
| }); | ||
|
|
||
| export const messagesTable = sqliteTable( |
There was a problem hiding this comment.
slop: heavy — column block copy-pasted across three tables. messagesTable, threadsTable and threadMessagesTable repeat the same ~30 columns verbatim; they differ by only ~6 fields total. Drizzle supports column-object spreading: define a messageColumns object once and spread it into each table, adding only the table-specific fields. Collapses this file from ~300 to ~160 lines and makes a future column addition a one-place change instead of three.
| msg: text('msg'), | ||
| t: text('t'), | ||
| rid: text('rid'), | ||
| ts: real('ts'), |
There was a problem hiding this comment.
bug: med — no .notNull() anywhere in the schema. Every column here (and in servers.ts) is nullable, but the WatermelonDB source marks many as required — e.g. on messages alone ts, u, _updated_at, alias, parse_urls; also uploads.size/progress, rooms.encrypted/ro, subscriptions.ts/unread, frequently_used_emojis.count. Writes that would have thrown in WMDB will silently store NULL. Add .notNull() to every field that isn't isOptional in the WMDB source and re-run drizzle-kit generate. If relaxing the constraints is intentional for the clean break, say so in a comment.
|
|
||
| // ---- app (per-server) database row types ---- | ||
|
|
||
| export type TSubscriptionRow = InferSelectModel<typeof subscriptionsTable>; |
There was a problem hiding this comment.
slop: nit — speculative TXxxInsert aliases. 13 InferInsertModel aliases are exported but none are consumed in this PR. Defer the Insert variants until a call site actually needs them, or keep only if the next stacked PR consumes them immediately.
diegolmello
left a comment
diegolmello
left a comment
There was a problem hiding this comment.
Review: Drizzle schema + generated migrations
Schema-only foundation; nothing imports it yet. Quality is high — I diffed all 16 tables (13 per-server + 3 servers-db) column-by-column against the WatermelonDB schemas in app/lib/database/schema/ and found full parity: no missing tables, no missing columns, correct SQLite affinities (incl. real() for the WMDB number/boolean cases), and every WMDB index reproduced. The deliberate choices (universal nullability, keeping verbatim SQLite column names) are clearly motivated.
Findings are all minor / process-level, no inline anchors needed:
🟡 [medium] No db:generate npm script for either config — the test instructions tell reviewers to run pnpm exec drizzle-kit generate --config drizzle.app.config.ts (and the servers variant) by hand. A "db:generate:app" / "db:generate:servers" pair makes the regenerate→diff loop repeatable for the next PR author.
🟡 [medium] The servers schema (serversTable, usersServersTable, serversHistoryTable) has zero query-shape test coverage — queryShapes.test.ts imports only from ./app. Add at least one serversTable and one usersServersTable lookup so a future divergence is caught.
🟢 [low] Generated files (*.sql, _journal.json, migrations.js, 0000_snapshot.json) have no trailing newline and are in .eslintignore/.prettierignore, so the hook won't fix them — expect noisy diffs on every regeneration unless normalised.
🟢 [low] All *_at/ts/ls timestamp columns are real() (correct WMDB parity — they store integer epoch-ms in a REAL-affinity column). Worth a one-line schema comment so accessor authors don't expect a Date.
Open question for the wipe-and-restore migration (not this PR): WMDB non-optional string columns default to '' on insert, never NULL, but the Drizzle schema makes them all nullable; and users_count was a WMDB string historically, now read as a number. When legacy rows are copied, watch for '' → NULL and '' → NaN/0 drift where downstream code does === '' checks.
Reviewed by an automated pass; treat comments as suggestions, not blockers.
diegolmello
changed the base branch from
develop
to
feat/native-1272-sqlcipher-migration
1 participant
Proposed changes
First code step of the WatermelonDB → expo-sqlite + Drizzle migration: adds the Drizzle ORM schema definitions and generated SQL migrations for both databases, plus the drizzle-kit tooling configs. No runtime behavior changes — nothing imports these modules yet.
app/lib/database/driver/schema/):app.ts— the 13 per-server tables (messages,subscriptions,rooms,threads,thread_messages,custom_emojis,frequently_used_emojis,uploads,settings,roles,permissions,slash_commands,users) with exact WatermelonDB table/column name parity.servers.ts— the 3 default-database tables (servers,users,servers_history).index.ts— typed exports and table registries for both database kinds.app/lib/database/driver/migrations/): drizzle-kit output (SQL + snapshots + journal +migrations.jsfor the expo driver), one folder per database.drizzle.app.config.ts/drizzle.servers.config.ts(dialectsqlite, driverexpo).drizzle-orm0.45.2 (runtime),drizzle-kit0.31.10 (dev).Schema decisions (from the migration PRD):
_status,_changed) are dropped — the new layer does not use WatermelonDB's sync protocol.idtext primary keys, matching WatermelonDB's actual SQLite DDL (it emits noNOT NULLconstraints on data columns), so the wipe-and-restore migration can re-insert any legacy row without constraint failures.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1275
How to test or reproduce
TZ=UTC pnpm test app/lib/database/driver— schema tests assert table/column parity against the WatermelonDB schema and pin the generated SQL shapes of representative queries.pnpm exec tsc --noEmit -p .— type-checks the schema exports.pnpm exec drizzle-kit generate --config drizzle.app.config.ts(and the servers config) produces no diff.Screenshots
N/A — no UI changes.
Types of changes
Checklist
Further comments
Part of the WatermelonDB replacement track. The driver adapter (connection lifecycle, SQLCipher keying, live-query hooks) follows in a separate PR built on top of this one.
Summary by CodeRabbit
New Features
Chores