-
-
Notifications
You must be signed in to change notification settings - Fork 13
merge dev to main (v3.0.0-beta.30) #489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: ymc9 <[email protected]>
* fix(delegate): exclude uninheritable attributes properly * fix tests
WalkthroughThis PR updates many package versions from 3.0.0-beta.29 → 3.0.0-beta.30, adjusts attribute inheritance/filtering in the language and Prisma schema generator, narrows delegate-field inheritance checks, adds an index to an Asset test schema, and enables Prisma Push in a regression test. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
Claude finished @ymc9's task —— View job PR Review Complete ✅This PR merges dev → main for v3.0.0-beta.30, containing primarily version bumps and one important delegate inheritance fix. Review Analysis:
Overall Assessment: APPROVED ✅✅ Version Management
✅ Key Bug Fix (Commit eb1a0c2)Fix: Delegate inheritance properly excludes uninheritable attributes Changes analyzed:
Code Quality:
✅ Test Coverage
✅ Security & Performance
✅ Code Standards
Recommendations:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR merges the dev branch to main for version 3.0.0-beta.30, bumping the version from beta.29 to beta.30 across all packages. The key change is a refactoring of how non-inheritable model-level attributes are handled in delegate inheritance. Previously, only the @@map attribute was filtered out during Prisma schema generation. Now, the filtering is done more comprehensively at the language utilities level, preventing @@map, @@unique, and @@index attributes from being inherited from base delegate models.
- Refactored attribute inheritance logic to centralize filtering of non-inheritable attributes (
@@map,@@unique,@@index) in the language utilities layer - Added test coverage for delegate inheritance with multiple non-inheritable attributes
- Updated delegate schema examples to demonstrate proper behavior with non-inherited
@@indexattributes
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Version bump to 3.0.0-beta.30 |
| packages/auth-adapters/better-auth/package.json | Version bump to 3.0.0-beta.30 |
| packages/cli/package.json | Version bump to 3.0.0-beta.30 |
| packages/clients/tanstack-query/package.json | Version bump to 3.0.0-beta.30 |
| packages/common-helpers/package.json | Version bump to 3.0.0-beta.30 |
| packages/config/eslint-config/package.json | Version bump to 3.0.0-beta.30 |
| packages/config/typescript-config/package.json | Version bump to 3.0.0-beta.30 |
| packages/config/vitest-config/package.json | Version bump to 3.0.0-beta.30 |
| packages/create-zenstack/package.json | Version bump to 3.0.0-beta.30 |
| packages/language/package.json | Version bump to 3.0.0-beta.30 |
| packages/language/src/utils.ts | Added isNonInheritableAttribute function to filter @@Map, @@unique, and @@index from inherited attributes |
| packages/orm/package.json | Version bump to 3.0.0-beta.30 |
| packages/plugins/policy/package.json | Version bump to 3.0.0-beta.30 |
| packages/schema/package.json | Version bump to 3.0.0-beta.30 |
| packages/sdk/package.json | Version bump to 3.0.0-beta.30 |
| packages/sdk/src/prisma/prisma-schema-generator.ts | Removed isInheritedMapAttribute method; filtering now handled centrally in utils.ts |
| packages/server/package.json | Version bump to 3.0.0-beta.30 |
| packages/testtools/package.json | Version bump to 3.0.0-beta.30 |
| packages/zod/package.json | Version bump to 3.0.0-beta.30 |
| samples/next.js/package.json | Version bump to 3.0.0-beta.30 |
| samples/orm/package.json | Version bump to 3.0.0-beta.30 |
| tests/e2e/package.json | Version bump to 3.0.0-beta.30 |
| tests/e2e/orm/schemas/delegate/schema.zmodel | Added @@index([ownerId]) to Asset model to demonstrate non-inheritance |
| tests/e2e/orm/schemas/delegate/schema.ts | Generated schema updated with @@index attribute on Asset (not inherited by Video/Image) |
| tests/regression/package.json | Version bump to 3.0.0-beta.30 |
| tests/regression/test/v2-migrated/issue-1243.test.ts | Added usePrismaPush option to properly test delegate inheritance with non-inheritable attributes |
| tests/runtimes/bun/package.json | Version bump to 3.0.0-beta.30 |
| tests/runtimes/edge-runtime/package.json | Version bump to 3.0.0-beta.30 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Claude finished @ymc9's task —— View job PR Review - v3.0.0-beta.30 ReleaseReviewing this PR for code quality, potential bugs, performance, security, and test coverage. Review Tasks:
Overall Assessment: APPROVED ✅✅ Version Management
✅ Key Bug Fix (Commit eb1a0c2)Fix: Delegate inheritance properly excludes uninheritable attributes Changes analyzed:
Code Quality:
✅ Test Coverage
✅ Security & Performance
✅ Code Standards
Recommendations:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/language/src/validators/datamodel-validator.ts (1)
433-436: Doc/semantics mismatch: “directly or indirectly” vs actual check.
The helper currently checks only whether the field’s declaring container is a delegate model and differs fromcontextModel. If “indirectly” is intended (multi-level inheritance), that’s probably still fine as long as the declaring container is always the original delegate ancestor—but the comment should reflect the actual criterion (declared-on-delegate-and-not-context), not imply traversal.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/language/src/validators/datamodel-validator.ts(2 hunks)packages/language/test/delegate.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
packages/language/test/delegate.test.ts
⏰ 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). (3)
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: claude-review
🔇 Additional comments (2)
packages/language/test/delegate.test.ts (1)
118-166: Nice regression coverage for delegate relation opposites.
The two-step schema (error then no-error) matches the validator behavior change and should prevent this from regressing.packages/language/src/validators/datamodel-validator.ts (1)
247-256: Good: opposite-relation check is now context-aware (but relies on$containersemantics).
PassingcontextModelintoisFieldInheritedFromDelegateModelseems to match the intended behavior change (delegate model’s own relation fields now require opposites; inherited ones don’t). Please double-check thatgetAllFields(dm)returns inheritedDataFields with$containerstill pointing at the base model (not the derived model), otherwise this condition can silently misclassify fields.#!/bin/bash # Verify how getAllFields constructs/returns inherited fields and whether it preserves DataField.$container. rg -n --type=ts "function getAllFields\\b|const getAllFields\\b|export function getAllFields\\b" -S rg -n --type=ts "getAllFields\\(" packages/language/src -S
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.