-
-
Notifications
You must be signed in to change notification settings - Fork 90
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(polymorphism): incorrect typing generated for nested create/update input #1145
Conversation
WalkthroughWalkthroughThis update introduces significant enhancements to the schema plugin's enhancer, focusing on improving code structure, readability, and functionality. Key changes include the addition of new utility functions, a shift of the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (6)
- packages/schema/src/plugins/enhancer/enhance/index.ts (4 hunks)
- packages/schema/src/plugins/enhancer/index.ts (2 hunks)
- packages/schema/src/plugins/prisma/schema-generator.ts (5 hunks)
- tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts (2 hunks)
- tests/integration/tests/enhancements/with-delegate/issue-1135.test.ts (1 hunks)
- tests/integration/tests/enhancements/with-delegate/utils.ts (1 hunks)
Additional comments: 9
tests/integration/tests/enhancements/with-delegate/utils.ts (1)
- 45-75: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-75]
The schema definitions for
Video
andImage
models extendingAsset
are correctly implemented to test polymorphic many-to-many relationships. This aligns well with the PR objectives. However, it would be beneficial to add comments explaining the purpose of these models and their relationships for better clarity and maintainability.packages/schema/src/plugins/enhancer/index.ts (1)
- 1-7: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-22]
The reorganization of imports and the update in the usage of the
EnhancerGenerator
class contribute to better code structure and readability. This change aligns with best practices for maintainability and code organization.tests/integration/tests/enhancements/with-delegate/issue-1135.test.ts (1)
- 1-85: The test case for regression testing related to issue 1135 is correctly implemented and aligns with the PR objectives. Adding more detailed comments explaining the purpose of the test and the specific regression it addresses would enhance understanding and maintainability.
packages/schema/src/plugins/enhancer/enhance/index.ts (1)
- 44-537: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-536]
The refactoring and enhancements within the
EnhancerGenerator
class, including the addition of new utility functions and the reorganization of code, significantly improve the code's structure, readability, and functionality. Ensure that all new utility functions are used appropriately and their implementations are correct. Adding comments to complex logic sections would further enhance understanding and maintainability.packages/schema/src/plugins/prisma/schema-generator.ts (3)
- 38-38: The addition of
getAttributeArgLiteral
import aligns with the PR objectives to enhance the handling of relation fields and their attributes. This utility function will likely be used to improve the accuracy of attribute argument retrieval, which is crucial for the correct generation of schema definitions.- 399-442: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [389-426]
The logic added here for expanding polymorphic relations in the logical schema mode is a significant enhancement. It correctly identifies delegate model types and generates auxiliary opposite relation fields for each concrete model. This approach ensures that the schema accurately represents polymorphic many-to-many relationships, addressing the PR's primary objective.
However, it's important to ensure that the generated field names and relation attributes adhere to Prisma's conventions and limitations. Additionally, the use of
upperCaseFirst
for foreign key fields (line 418) enhances readability and follows the naming convention updates mentioned in the PR objectives.One potential improvement could be to add more comments explaining the purpose of each block within this complex logic, especially for future maintainability.
Consider adding more detailed comments within this block to explain the logic behind generating auxiliary opposite relation fields for polymorphic relationships. This will improve code readability and maintainability.
- 482-504: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [485-538]
This block of code introduces logic to name relations inherited from delegate base models for disambiguation. This is a crucial enhancement for handling polymorphic scenarios, ensuring that relation names are unique and meaningful, especially when dealing with inherited fields and relations.
The use of
getAttribute
andgetAttributeArgLiteral
to construct the relation name (lines 501, 537) is consistent with the PR's objectives to improve attribute handling. This approach ensures that the generated relation names are accurate and follow the new naming conventions.However, similar to the previous comment, adding explanatory comments would enhance the understandability of this logic, given its complexity and importance in achieving the PR's objectives.
Adding comments to explain the logic for naming relations inherited from delegate models would enhance code readability and maintainability, especially for those unfamiliar with the intricacies of polymorphic relationships.
tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts (2)
- 5-5: The addition of
POLYMORPHIC_MANY_TO_MANY_SCHEMA
alongsidePOLYMORPHIC_SCHEMA
is noted. This change aligns with the PR's objective to enhance testing coverage for polymorphic scenarios.- 1006-1040: The 'many to many' test case is well-structured and directly addresses the PR's objective of enhancing test coverage for polymorphic scenarios. It effectively tests the creation of entities and their connections, followed by assertions to validate the expected behavior. This is a crucial addition to ensure the robustness of the system's handling of polymorphic many-to-many relationships.
Fixes #1135 |
Summary by CodeRabbit
New Features
Refactor
EnhancerGenerator
class.Tests