Migrate database operations to Drizzle ORM and clean up scripts - #166
Merged
Conversation
schema.ts is drizzle-kit's schema entry point; a fuller users definition there made drizzle-kit think it owns and should generate migrations for a table that's actually owned by the sibling FOSSBilling/extensions repo.
Also replaces every platform.getDatabase("DB_EXTENSIONS") call site in
index.ts with getExtensionsDb(c.env.DB_EXTENSIONS) - done in one pass since
all four v2 database classes pull from the same binding, even though
submissions-database.ts/developers-database.ts haven't been converted yet
(next).
Most queries use the idiomatic query builder. create() and approve()'s three-statement batch stay as raw sql templates (via db.run) rather than query-builder calls: their INSERT...SELECT...WHERE and changes()-gated UPDATE patterns aren't expressible through .insert()/.update() builders, and approve() specifically depends on D1 batch's per-statement changes() visibility between statements - safer to port byte-for-byte than to risk subtly losing that guarantee in a "more idiomatic" rewrite.
Uses the idiomatic query builder for straightforward single-table CRUD, and the raw D1 client ($client, Drizzle's documented escape hatch) for batched statements with changes()-gated writes or correlated subqueries - both patterns the query builder can't express, and the latter also works around a real drizzle-orm 0.45.2 bug where db.run(sql) with bound params can't be used inside db.batch() (confirmed via an isolated repro against real D1: SQLiteD1Session.batch() unconditionally reads .stmt off the prepared query, which raw sql queries don't have). Also fixes two correctness bugs this surfaced: - isOwnerConflict/isPendingClaimConflict/isPendingTargetConflict matched against error.message, but Drizzle wraps driver errors in a DrizzleError whose .message is a generic "Failed to run the query" wrapper - the real SQLite/D1 message (e.g. "UNIQUE constraint failed") is nested in .cause. Added errorMessageChain() to walk the full cause chain. - extension_submissions.created_at/reviewed_at were TEXT in schema.ts but DATETIME in the original migration - already-known, harmless divergence (see 0014_drizzle_baseline.sql), unrelated to this commit. Replaces test/services/extensions/v2/mock-db.ts (a ~1200-line hand-rolled SQL-string-matching D1 fake) with real local D1: db-fixtures.ts provides raw-SQL seed/read helpers, and db-interceptor.ts wraps the real D1 binding so a handful of tests can inject a fault or a mid-request race at an exact point (something no amount of pre-request state setup can reproduce, since the race is inside a single request's own execution) while everything else still executes for real. Testing against real D1 surfaced several behaviors the mock silently ignored: FK enforcement on owner_user_id/ submitted_by/claimant_id (fixed by auto-provisioning a users row per caller, matching how auth always precedes these calls in production) and developer_history's append-only triggers (test cleanup now drops/recreates them around the reset). One test asserting a "developer row missing" fallback was deleted: extensions.author_id has always been a hard FK to developers(id) in the real schema, so that scenario was never actually reachable in production - the old mock just never enforced it. All 318 workers-pool tests and 38 node-adapter tests pass; full project typecheck and lint are clean.
- Add db:generate:extensions/central-alerts (drizzle-kit generate) and migrate:central-alerts:* npm scripts, matching the existing migrate:extensions-v2:* naming. - Remove init:db and the now-superseded src/services/central-alerts/v1/scripts/init-db.ts - central_alerts is fully on wrangler d1 migrations now (see the earlier baseline commit). - Update README's local setup steps and test/mocks/README.md, which still documented the deleted mock-adapters.ts. - Confirmed IDatabase/IPreparedStatement removal (from the earlier "Replace IDatabase abstraction" commit) left no stragglers. - prettier --write on every file touched across this migration. Full verification: typecheck clean, lint clean (0 errors), format clean (aside from 2 pre-existing files this migration never touched), 318 workers-pool tests + 38 node-adapter tests passing.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
api | 7249d8b | Commit Preview URL Branch Preview URL |
Jul 30 2026, 04:45 AM |
Superseded by the drizzle-kit-managed migrations (0000/0001); nothing referenced it after init-db.ts was deleted.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Auto-approved: Refactors database layer to use Drizzle ORM, adds baseline migrations for existing schemas, and removes legacy adapters. No change to behavior or operational configuration.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
cubic-dev-ai
Bot
dismissed
their stale review
July 30, 2026 04:35
Dismissed because Cubic found issues in a newer review.
main gained "Let claimants withdraw their own pending developer-profile claim" (#165) after this branch diverged, touching the same three files rewritten here for Drizzle. Auto-merge pasted the old raw db.prepare().bind().run() implementation in verbatim (git can't know it needs rewriting), so cancelClaim() is reimplemented against the Drizzle query builder to match rejectClaim()'s pattern, and its route handler now goes through getExtensionsDb() like every other v2 route. Also resolves a modify/delete conflict on the now-deleted mock-db.ts (kept deleted) and rewrites the three ported tests to use the db-fixtures helpers instead of the removed tables.* Map-mock API.
Addresses PR review feedback: db-interceptor.ts's batch() necessarily fires every statement's hook before D1's real batch() call, since D1 batches are one opaque, atomic round-trip with no way to observe or inject a fault between individual statements. Documents that constraint and renames/reframes the affected test - it verifies D1's built-in all-or-nothing rollback on a failing statement, not any app-level revert logic, since no partial commit within a batch is reproducible or even possible.
Contributor
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Auto-approved: Refactors database access to Drizzle ORM with schemas and migrations, removing the custom adapter. This bounded, behavior-preserving migration improves type safety and maintainability.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Migrate all database access to
drizzle-ormfor the Extensions and Central Alerts D1 databases. Replace the custom DB layer, add migrations, and run tests against real D1 for correctness and maintainability, including porting the claim-cancel flow to Drizzle.Refactors
drizzle-orm; addgetExtensionsDbandgetCentralAlertsDb; removeIDatabase/adapters and simplify platform context.toD1Statementto work around a D1 batch issue.getExtensionsDb, consistent with other v2 endpoints.0014_drizzle_baseline.sql; central alerts0000create +0001seed); remove the legacy central-alertsinit.sql..causemessages.@cloudflare/vitest-pool-workers.Migration
db:generate:extensions,db:generate:central-alerts, andmigrate:*for both DBs; removeinit:db.npm run migrate:extensions-v2:localandnpm run migrate:central-alerts:local.wrangler.jsoncsetsmigrations_dirforDB_CENTRAL_ALERTS; tests auto-apply viaapplyTestMigrations.Written for commit 7249d8b. Summary will update on new commits.