refactor: migrate user related models to drizzle - #2798
Conversation
📝 WalkthroughWalkthroughAdds Drizzle-backed template and templateFavorite tables and schemas; removes legacy Sequelize user models/exports; implements a Drizzle-based UserTemplateRepository and BaseRepository.count(); updates DI, router, service, tests, snapshots, and simplifies the Sequelize DB provider. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Router as Router (dashboard / templates)
participant Repo as UserTemplateRepository
participant DB as Postgres (Drizzle)
rect rgba(135,206,235,0.5)
Client->>Router: HTTP requests (GET /stats, template ops)
end
rect rgba(144,238,144,0.5)
Router->>Repo: call repository methods (count / findById / upsert / addFavorite / ...)
end
rect rgba(255,182,193,0.5)
Repo->>DB: execute Drizzle cursor queries (select/insert/update/join)
DB-->>Repo: return rows / ids / affected counts
end
Repo-->>Router: return shaped TemplateOutput / counts
Router-->>Client: JSON response (includes totalTemplateCount / templates)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/user/services/user-templates/user-templates.service.spec.ts (1)
134-149:⚠️ Potential issue | 🟡 MinorTest description doesn't match the actual assertion.
The test description says "calls repository updateById" but the test actually asserts
updateTemplate. Update the description to match the refactored method name.📝 Suggested fix
describe("update", () => { - it("calls repository updateById with correct parameters", async () => { + it("calls repository updateTemplate with correct parameters", async () => { const { service, userTemplateRepository } = setup();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/user/services/user-templates/user-templates.service.spec.ts` around lines 134 - 149, The test description is out of sync with the assertion: update() spec says "calls repository updateById" but the test actually asserts userTemplateRepository.updateTemplate; update the test's it() description string to accurately reflect the refactored method name (e.g., "calls repository updateTemplate with correct parameters") so it matches the assertion in the update test for service.update and the mocked userTemplateRepository.updateTemplate.
🧹 Nitpick comments (1)
apps/api/drizzle/0028_right_red_ghost.sql (1)
16-24: Consider adding an index ontemplateFavorite.userIdfor efficient user favorite lookups.The unique index on
(userId, templateId)is useful for preventing duplicate favorites, but queries that fetch all favorites for a specific user (e.g.,WHERE userId = ?) may not efficiently use this composite index depending on the query planner. A dedicated index onuserIdalone would optimize such lookups.💡 Suggested addition
CREATE UNIQUE INDEX IF NOT EXISTS "templateFavorite_userId_templateId_unique" ON "templateFavorite" ("userId","templateId"); +--> statement-breakpoint +CREATE INDEX IF NOT EXISTS "templateFavorite_userId_idx" ON "templateFavorite" ("userId");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/drizzle/0028_right_red_ghost.sql` around lines 16 - 24, Add a dedicated index on templateFavorite.userId to optimize queries that fetch all favorites for a user; specifically, create an index (e.g., "templateFavorite_userId_idx") on the "templateFavorite" table for the "userId" column in addition to the existing UNIQUE index "templateFavorite_userId_templateId_unique" so single-column user lookups use the index efficiently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/user/repositories/user-template/user-template.repository.ts`:
- Around line 97-99: The addFavorite method currently calls the global
crypto.randomUUID(); update the file to use a named import for consistency by
importing { randomUUID } from 'crypto' and replace crypto.randomUUID() with
randomUUID() in addFavorite (which inserts into this.favoriteTable via
this.cursor). Ensure any eslint/formatting rules pass after updating the import
and usage.
---
Outside diff comments:
In `@apps/api/src/user/services/user-templates/user-templates.service.spec.ts`:
- Around line 134-149: The test description is out of sync with the assertion:
update() spec says "calls repository updateById" but the test actually asserts
userTemplateRepository.updateTemplate; update the test's it() description string
to accurately reflect the refactored method name (e.g., "calls repository
updateTemplate with correct parameters") so it matches the assertion in the
update test for service.update and the mocked
userTemplateRepository.updateTemplate.
---
Nitpick comments:
In `@apps/api/drizzle/0028_right_red_ghost.sql`:
- Around line 16-24: Add a dedicated index on templateFavorite.userId to
optimize queries that fetch all favorites for a user; specifically, create an
index (e.g., "templateFavorite_userId_idx") on the "templateFavorite" table for
the "userId" column in addition to the existing UNIQUE index
"templateFavorite_userId_templateId_unique" so single-column user lookups use
the index efficiently.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2798 +/- ##
==========================================
- Coverage 53.73% 52.80% -0.94%
==========================================
Files 1018 985 -33
Lines 23588 22741 -847
Branches 5759 5661 -98
==========================================
- Hits 12676 12009 -667
+ Misses 9516 9345 -171
+ Partials 1396 1387 -9
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
e6e0f49 to
03bee40
Compare
03bee40 to
1e84f00
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api/src/user/repositories/user-template/user-template.repository.integration.ts`:
- Around line 469-472: The setup function currently has no parameters; update
the setup declaration to accept a single inline-typed parameter (e.g., a
destructured options object with its type defined inline) so it matches test
guidelines; modify function setup(...) to take that single inline-typed param
and use its fields as needed while still resolving UserTemplateRepository via
container.resolve(UserTemplateRepository), ensuring callers pass or omit the new
param as appropriate (provide a default if necessary).
1e84f00 to
e1a9fe9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/user/repositories/user-template/user-template.repository.ts`:
- Around line 141-145: The code is calling crypto.randomUUID() but only
randomUUID is imported from node:crypto; replace the undefined crypto usage by
calling the imported randomUUID() directly in the insert values (the block using
this.cursor.insert(this.table).values({...})). Update the values object to use
randomUUID() for id (consistent with the pattern already used elsewhere, e.g.
line ~99) so runtime/type-checking no longer reference the missing crypto
symbol.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/api/drizzle/0028_right_red_ghost.sqlapps/api/drizzle/meta/0028_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/chain/providers/sequelize.provider.tsapps/api/src/core/repositories/base.repository.tsapps/api/src/routers/dashboardRouter.tsapps/api/src/user/model-schemas/index.tsapps/api/src/user/model-schemas/template-favorite/template-favorite.schema.tsapps/api/src/user/model-schemas/template/template.schema.tsapps/api/src/user/model-schemas/user/user.schema.tsapps/api/src/user/repositories/user-template/user-template.repository.integration.tsapps/api/src/user/repositories/user-template/user-template.repository.tsapps/api/src/user/services/user-templates/user-templates.service.spec.tsapps/api/src/user/services/user-templates/user-templates.service.tspackages/database/dbSchemas/index.tspackages/database/dbSchemas/user/index.tspackages/database/dbSchemas/user/template.tspackages/database/dbSchemas/user/templateFavorite.tspackages/database/dbSchemas/user/userSetting.ts
💤 Files with no reviewable changes (5)
- packages/database/dbSchemas/index.ts
- packages/database/dbSchemas/user/index.ts
- packages/database/dbSchemas/user/userSetting.ts
- packages/database/dbSchemas/user/templateFavorite.ts
- packages/database/dbSchemas/user/template.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/api/src/core/repositories/base.repository.ts
- apps/api/drizzle/meta/_journal.json
- apps/api/src/user/services/user-templates/user-templates.service.ts
- apps/api/src/user/model-schemas/user/user.schema.ts
- apps/api/src/user/services/user-templates/user-templates.service.spec.ts
- apps/api/src/user/model-schemas/template/template.schema.ts
- apps/api/drizzle/meta/0028_snapshot.json
e1a9fe9 to
7abecff
Compare
7abecff to
c8300e1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/api/src/user/repositories/user-template/user-template.repository.integration.ts (1)
469-471:⚠️ Potential issue | 🟡 MinorAdjust setup() to accept a single inline-typed parameter.
This still violates the test setup guideline and was flagged previously.
Based on learnings, “Use `setup` function instead of `beforeEach` in test files. The `setup` function must … accept a single parameter with inline type definition…”.🔧 Suggested fix
-function setup() { +function setup({}: {} = {}) { const userTemplateRepository = container.resolve(UserTemplateRepository); return { userTemplateRepository }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/user/repositories/user-template/user-template.repository.integration.ts` around lines 469 - 471, The setup helper currently takes no args; change the setup function to accept a single inline-typed parameter (e.g. { userTemplateRepository?: UserTemplateRepository }) and use that to override or resolve dependencies: keep resolving UserTemplateRepository via container.resolve(UserTemplateRepository) when not provided, and return { userTemplateRepository } from setup(); update the function signature named setup to declare the inline type so tests can call setup({ userTemplateRepository: ... }) per the guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/user/repositories/user-template/user-template.repository.ts`:
- Around line 128-145: The upsert function may set copiedFromId to a
non-existent or unauthorized template id; modify upsert to verify that the
provided id exists and belongs to the same user (reuse the existing variable
existing from the Templates.findFirst check or add a separate query) before
including copiedFromId in the insert values, and only set copiedFromId when that
check passes; ensure the verification uses Templates.findFirst (or similar) and
eq conditions on this.table.id and this.table.userId to avoid dangling
references.
---
Duplicate comments:
In
`@apps/api/src/user/repositories/user-template/user-template.repository.integration.ts`:
- Around line 469-471: The setup helper currently takes no args; change the
setup function to accept a single inline-typed parameter (e.g. {
userTemplateRepository?: UserTemplateRepository }) and use that to override or
resolve dependencies: keep resolving UserTemplateRepository via
container.resolve(UserTemplateRepository) when not provided, and return {
userTemplateRepository } from setup(); update the function signature named setup
to declare the inline type so tests can call setup({ userTemplateRepository: ...
}) per the guideline.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/api/drizzle/0028_right_red_ghost.sqlapps/api/drizzle/meta/0028_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/chain/providers/sequelize.provider.tsapps/api/src/core/repositories/base.repository.tsapps/api/src/routers/dashboardRouter.tsapps/api/src/user/model-schemas/index.tsapps/api/src/user/model-schemas/template-favorite/template-favorite.schema.tsapps/api/src/user/model-schemas/template/template.schema.tsapps/api/src/user/model-schemas/user/user.schema.tsapps/api/src/user/repositories/user-template/user-template.repository.integration.tsapps/api/src/user/repositories/user-template/user-template.repository.tsapps/api/src/user/services/user-templates/user-templates.service.spec.tsapps/api/src/user/services/user-templates/user-templates.service.tspackages/database/dbSchemas/index.tspackages/database/dbSchemas/user/index.tspackages/database/dbSchemas/user/template.tspackages/database/dbSchemas/user/templateFavorite.tspackages/database/dbSchemas/user/userSetting.ts
💤 Files with no reviewable changes (5)
- packages/database/dbSchemas/index.ts
- packages/database/dbSchemas/user/userSetting.ts
- packages/database/dbSchemas/user/index.ts
- packages/database/dbSchemas/user/template.ts
- packages/database/dbSchemas/user/templateFavorite.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/api/src/core/repositories/base.repository.ts
- apps/api/src/user/model-schemas/index.ts
- apps/api/drizzle/meta/0028_snapshot.json
- apps/api/drizzle/meta/_journal.json
- apps/api/src/user/services/user-templates/user-templates.service.spec.ts
Why
What
Summary by CodeRabbit
New Features
Chores