fix(delegate): don't generate both "@@schema" attributes from base and sub models #1940
Conversation
📝 WalkthroughWalkthroughThe pull request addresses a bug related to schema inheritance in Prisma models. The changes modify the attribute filtering logic in the AST utility functions to allow sub-models to override the database schema attribute (@@Schema). This enables more flexible schema configurations, particularly in multi-tenant setups where base and extended models might need to be in different database schemas. Changes
Assessment against linked issues
Possibly related PRs
Sequence DiagramsequenceDiagram
participant BaseModel
participant SubModel
participant SchemaGenerator
BaseModel->>SchemaGenerator: Define initial @@schema
SubModel->>SchemaGenerator: Override @@schema if specified
SchemaGenerator->>SchemaGenerator: Apply attribute override logic
SchemaGenerator-->>SubModel: Generate model with appropriate schema
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/regression/tests/issue-1647.test.ts (1)
5-35: First test case is well-structured.
The test confirms correct inheritance of the default@@schema("public")across base and sub-models. As a best practice, consider also verifying unexpected schemas do not appear, ensuring negative checks are covered.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/schema/src/utils/ast-utils.ts(3 hunks)tests/regression/tests/issue-1647.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
🔇 Additional comments (5)
tests/regression/tests/issue-1647.test.ts (2)
1-2: Imports are correct and straightforward.
These imports properly reference test tools and the filesystem, which is standard and succinct.
36-68: Second test case thoroughly validates overrides.
Using a sub-model schema ensures that the override logic is tested. This is a solid example of verifying that only a single occurrence of@@schema("public")remains in the final output, emphasizing that@@schema("post")properly overrides the parent model’s schema.packages/schema/src/utils/ast-utils.ts (3)
19-26: Verified new import statements.
ImportinghasAttributefrom@zenstackhq/sdkis a clean approach that improves readability by eliminating the need to reimplement similar checks.
106-108: Definition of overrideAttributes is clear and maintainable.
This array-based approach allows for easy extension if future inherited attributes need to be overridden.
122-125: Conditional check for overridden attributes.
This logic correctly prevents inheriting@@schemawhen already present in the sub-model. It aligns with the issue’s goal by ensuring duplicate attributes are not generated.
fixes #1647