-
-
Notifications
You must be signed in to change notification settings - Fork 92
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
fix(cli): regular relation fields are incorrectly recognized as self-relation #1790
Conversation
📝 Walkthrough<details>
<summary>📝 Walkthrough</summary>
## Walkthrough
This pull request introduces several modifications to the `DataModelValidator` class in the `datamodel-validator.ts` file, focusing on enhancing validation logic and error handling for relation fields. Key updates include the removal of an import, improved error messages, and refined method signatures. Additionally, the `datamodel-validation.test.ts` file has been updated with new test cases and refined assertions to ensure comprehensive coverage of validation scenarios, particularly related to relations and unique constraints.
## Changes
| File | Change Summary |
|----------------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------|
| packages/schema/src/language-server/validator/datamodel-validator.ts | - Removed `getLiteral` import.<br>- Updated `parseRelation` for better error handling and control flow.<br>- Modified `isSelfRelation` method signature.<br>- Refined error messages in validation methods. |
| packages/schema/tests/schema/validation/datamodel-validation.test.ts | - Added new test cases for error handling and edge cases.<br>- Updated error messages for clarity.<br>- Expanded relation handling tests.<br>- Added tests for abstract and delegate models. |
## Possibly related PRs
- **#1430**: This PR modifies the `DataModelValidator` class in `datamodel-validator.ts`, which is directly related to the changes made in the main PR regarding validation logic and error messages for relation fields.
</details> 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🔇 Additional comments (3)
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 (
|
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: 1
🧹 Outside diff range and nitpick comments (3)
packages/schema/src/language-server/validator/datamodel-validator.ts (1)
309-309
: Improve clarity of error messageConsider rephrasing the error message for better clarity and grammatical correctness. Suggested change:
-'Field for one side of relation must carry @relation attribute with both "fields" and "references"' +'A field on one side of a relation must have a @relation attribute with both "fields" and "references" specified.'packages/schema/tests/schema/validation/datamodel-validation.test.ts (2)
454-454
: Consider providing more specific error messages for clarityThe error message
"fields" and "references" must be provided together
could be enhanced by including the model and field names to help users quickly identify where the issue occurs.Consider updating the error message to include model and field context:
- ).toMatchObject(errorLike(`"fields" and "references" must be provided together`)); + ).toMatchObject(errorLike(`Model "B", field "a": "fields" and "references" must be provided together`));
544-544
: Enhance error message with model and field detailsThe error message
Field for one side of relation must carry @relation attribute with both "fields" and "references"
may be more helpful if it specifies the exact field and model involved. This aids in quicker debugging and improves user experience.Consider revising the error message:
- `Field for one side of relation must carry @relation attribute with both "fields" and "references"` + `Field "a" in model "B" must have a @relation attribute with both "fields" and "references"`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/schema/src/language-server/validator/datamodel-validator.ts (4 hunks)
- packages/schema/tests/schema/validation/datamodel-validation.test.ts (2 hunks)
🧰 Additional context used
🔇 Additional comments (1)
packages/schema/src/language-server/validator/datamodel-validator.ts (1)
144-147
: Early return inparseRelation
method simplifies control flowThe early return when both
fields
andreferences
are absent correctly simplifies the method and avoids unnecessary validation, ensuring that only relevant cases are processed further.Also applies to: 149-152
No description provided.