feat(orm)!: Add typed typed mutation hooks #2365#2740
Conversation
📝 WalkthroughWalkthroughChangesEntity mutation hooks
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
3ebbffb to
ce01ceb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/e2e/orm/plugin-infra/entity-mutation-hooks.test.ts (1)
806-831: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest can silently pass without executing any assertion.
All
expectcalls are insideafterEntityMutation; there's no outer check confirming the hook actually ran. If per-model after-hook dispatch regresses, this test would still pass with zero assertions executed.✅ Proposed fix
it('per-model hooks infer correct entity types', async () => { + expect.assertions(2); const client = _client.$use({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/orm/plugin-infra/entity-mutation-hooks.test.ts` around lines 806 - 831, Add an outer execution flag to the “per-model hooks infer correct entity types” test, set it inside User.afterEntityMutation after the hook runs, and assert it after client.user.create completes. Keep the existing entity type and email assertions unchanged.packages/orm/src/client/executor/zenstack-query-executor.ts (1)
376-386: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: extract shared per-model hook lookup.
The
(onEntityMutation as Record<string, any>)[mutationInfo.model]lookup pattern is duplicated betweencallBeforeMutationHooksandcallAfterMutationHooks. A small private helper (e.g.getModelHooks(onEntityMutation, model)) would remove the duplication, though the current repetition is small and low-risk.Also applies to: 417-429
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/orm/src/client/executor/zenstack-query-executor.ts` around lines 376 - 386, Extract the duplicated per-model hook lookup into a private helper, such as getModelHooks, and use it in both callBeforeMutationHooks and callAfterMutationHooks. Keep the helper’s behavior equivalent to the existing onEntityMutation Record lookup for mutationInfo.model.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/orm/src/client/executor/zenstack-query-executor.ts`:
- Around line 125-132: Update the hasEntityMutationPluginsWithAfterMutationHooks
getter to accept the mutationModel and check only that model’s per-model
afterEntityMutation hook, while preserving catch-all hook detection. Update the
needLoadAfterMutationEntities call site to pass the existing mutationModel,
preventing hooks registered for other models from triggering entity loading.
---
Nitpick comments:
In `@packages/orm/src/client/executor/zenstack-query-executor.ts`:
- Around line 376-386: Extract the duplicated per-model hook lookup into a
private helper, such as getModelHooks, and use it in both
callBeforeMutationHooks and callAfterMutationHooks. Keep the helper’s behavior
equivalent to the existing onEntityMutation Record lookup for
mutationInfo.model.
In `@tests/e2e/orm/plugin-infra/entity-mutation-hooks.test.ts`:
- Around line 806-831: Add an outer execution flag to the “per-model hooks infer
correct entity types” test, set it inside User.afterEntityMutation after the
hook runs, and assert it after client.user.create completes. Keep the existing
entity type and email assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c7f981f2-b6a7-49fb-a7d5-79f609339101
📒 Files selected for processing (3)
packages/orm/src/client/executor/zenstack-query-executor.tspackages/orm/src/client/plugin.tstests/e2e/orm/plugin-infra/entity-mutation-hooks.test.ts
fixes: #2365
Summary by CodeRabbit
New Features
Bug Fixes
Breaking Changes
onEntityMutation.$all.Tests