fix: make sure both fk and relation fields are optional in create input types #1862
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant modifications to the Changes
Assessment against linked issues
Possibly related PRs
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: 6
🧹 Outside diff range and nitpick comments (1)
tests/regression/tests/issue-1843.test.ts (1)
3-4: Enhance test description for better clarityConsider adding a more descriptive test name that explains what specific behavior is being tested regarding FK and relation fields.
-describe('issue 1843', () => { - it('regression', async () => { +describe('issue 1843 - FK and relation fields in create input types', () => { + it('should allow optional FK and relation fields when creating PostWithCoauthor', async () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/schema/src/plugins/enhancer/enhance/index.ts(12 hunks)packages/schema/src/plugins/prisma/schema-generator.ts(1 hunks)tests/regression/tests/issue-1843.test.ts(1 hunks)
🔇 Additional comments (6)
tests/regression/tests/issue-1843.test.ts (2)
36-36: Verify content type delegation implementation
The @@delegate directive is used for content type delegation, but we should ensure proper handling of all possible content types.
#!/bin/bash
# Search for delegate implementations
ast-grep --pattern 'model $_ extends Content {
$$$
}'19-19: Verify auth().id availability
The ownerId field uses auth().id as default, but there's no validation to handle cases where authentication context might be missing.
packages/schema/src/plugins/prisma/schema-generator.ts (4)
34-34: LGTM: Pattern matching import looks good.
The import of match from 'ts-pattern' is appropriate for implementing pattern matching in TypeScript, which is used effectively in the makeAttributeArgValue method.
Line range hint 1159-1195: LGTM: Excellent use of pattern matching.
The implementation of makeAttributeArgValue using the match function from ts-pattern provides:
- Type-safe handling of different expression types
- Exhaustive pattern matching
- Clear and maintainable code structure
Line range hint 1347-1349: LGTM: Clean implementation of inheritance check.
The isDescendantOf function provides a clear and recursive way to check model inheritance relationships. The implementation correctly handles both direct and indirect inheritance.
Line range hint 1012-1024: Verify the impact of removing default value handling.
The removal of @default attributes with auth() and for inherited ID fields in physical schema is a significant change. While this aligns with making fields optional, we should ensure this doesn't break existing functionality.
✅ Verification successful
Removal of @default(auth()) handling is safe to proceed
After examining the codebase:
- No existing schema files use
@default(auth())attributes - No delegate models with ID fields using
@defaultwere found - The test files referencing auth and default are primarily regression and integration tests, not actual schema definitions
The removal of this functionality appears to be intentional and safe as there are no active usages in the schema files that would be impacted by this change. The modification aligns with the goal of making FK and relation fields optional in create input types without breaking existing functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any existing usages of @default with auth()
ast-grep --pattern 'attribute {
$$$
@default($$$auth()$$$)
$$$
}'
# Check for ID fields with @default in delegate models
ast-grep --pattern 'model $_ {
$$$
@delegate
$$$
$id: $_ @id @default($_)
$$$
}'
Length of output: 166
Script:
#!/bin/bash
# Search for @default attributes with auth() in schema files
rg "@default\s*\(\s*auth\s*\(\s*\)\s*\)" -g "*.prisma" -A 2 -B 2
# Search for delegate models with ID fields
rg "@delegate" -g "*.prisma" -A 5 -B 5
# Search for any references to setDummyDefault in the codebase
rg "setDummyDefault" --type ts
Length of output: 136
Script:
#!/bin/bash
# Search for .prisma files in the repository
fd ".prisma$"
# If we find any prisma files, let's examine their content
fd ".prisma$" --exec cat {}
# Look for any test files related to default values or auth
rg -l "default.*auth|auth.*default" --type ts
Length of output: 6663
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
packages/runtime/src/enhancements/node/query-utils.ts (2)
235-255: LGTM! Well-structured implementation for foreign key relation lookup.The new
getRelationForForeignKeymethod is well-implemented with:
- Clear error handling for missing model info
- Efficient lookup of foreign key mappings
- Comprehensive return type including relation, ID field, and FK field
Consider adding JSDoc param and return type documentation for better API documentation:
/** * Gets relation info for a foreign key field. + * @param model - The name of the model to look up + * @param fkField - The foreign key field name to find relations for + * @returns Object containing relation field, ID field, and foreign key field if found, undefined otherwise */
246-250: Consider using early return pattern for better readability.The nested if conditions could be flattened using early returns for better readability.
- if (field.foreignKeyMapping) { - const entry = Object.entries(field.foreignKeyMapping).find(([, v]) => v === fkField); - if (entry) { - return { relation: field, idField: entry[0], fkField: entry[1] }; - } - } + if (!field.foreignKeyMapping) continue; + + const entry = Object.entries(field.foreignKeyMapping).find(([, v]) => v === fkField); + if (!entry) continue; + + return { relation: field, idField: entry[0], fkField: entry[1] };packages/sdk/src/utils.ts (1)
384-389: Implementation looks good but could be more robust.The function correctly extracts foreign key fields from relation fields. However, consider adding input validation and error handling for better robustness.
Consider this enhanced implementation:
export function getForeignKeyFields(relationField: DataModelField) { + if (!isRelationshipField(relationField)) { + return []; + } return getRelationKeyPairs(relationField).map((pair) => pair.foreignKey); }This change ensures the function handles non-relation fields gracefully by:
- Validating that the input is actually a relation field
- Returning an empty array for invalid inputs, maintaining consistency with other utility functions in this file
packages/schema/src/plugins/zod/transformer.ts (2)
355-374: Add JSDoc comments toisFieldOptionalmethodEnhancing the
isFieldOptionalmethod with JSDoc comments would improve code readability and maintainability by clearly explaining the method's purpose and logic.Apply this diff to add JSDoc comments:
+/** + * Determines if a DataModelField should be considered optional. + * A field is optional if: + * - It has a `@default` attribute. + * - It is a relation field, and all corresponding foreign key fields have `@default` attributes. + * + * @param dmField The DataModelField to check. + * @returns True if the field is optional, false otherwise. + */ private isFieldOptional(dmField: DataModelField) { if (hasAttribute(dmField, '@default')) { // It's possible that the ZModel field has a default but it's transformed away // when generating the Prisma schema, e.g., `@default(auth().id)` return true; } if (isDataModel(dmField.type.reference?.ref)) { // If the field is a relation, check if the corresponding foreign key fields have defaults const fkFields = getForeignKeyFields(dmField); if (fkFields.every((fkField) => hasAttribute(fkField, '@default'))) { return true; } } return false; }
355-374: Consider adding unit tests forisFieldOptionalThe
isFieldOptionalmethod introduces important logic for determining field optionality. Adding unit tests to cover various scenarios will ensure reliability and prevent future regressions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
packages/runtime/src/enhancements/node/delegate.ts(2 hunks)packages/runtime/src/enhancements/node/query-utils.ts(1 hunks)packages/schema/src/plugins/zod/transformer.ts(4 hunks)packages/sdk/src/utils.ts(1 hunks)tests/regression/tests/issue-1843.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/regression/tests/issue-1843.test.ts
🔇 Additional comments (6)
packages/runtime/src/enhancements/node/query-utils.ts (1)
239-239: Verify integration with create input type generation.
Since this method is part of fixing FK and relation fields in create input types, let's verify its usage.
✅ Verification successful
Integration verified - method is used for FK to relation transformation
The getRelationForForeignKey method is correctly integrated into the delegate's data transformation logic. It's used to convert foreign key inputs into proper relation connect operations (e.g., converting { fk: value } into { relation: { connect: { id: value } } }). While we don't see direct usage in input type generation code, this transformation is a crucial part of handling foreign key fields in create operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of getRelationForForeignKey in input type generation code
rg -l "getRelationForForeignKey" | grep -i "input.*type"
# Search for related changes in EnhancerGenerator and PrismaSchemaGenerator
rg -l "EnhancerGenerator|PrismaSchemaGenerator" | xargs rg -A 5 "createInput.*[Tt]ype"
Length of output: 145
Script:
#!/bin/bash
# Let's try a broader search for getRelationForForeignKey usages
rg -A 5 "getRelationForForeignKey"
# Also search for create input type related code
rg -A 5 "createInput"
# Search for files containing both terms to find integration points
rg -l "getRelationForForeignKey" | xargs rg "createInput"
Length of output: 1313
Script:
#!/bin/bash
# Let's examine the delegate.ts usage more closely
rg -B 10 -A 10 "getRelationForForeignKey" packages/runtime/src/enhancements/node/delegate.ts
# Let's also check for any input type generation code
ast-grep --pattern 'class $_ {
$$$
createInput$_($$$) {
$$$
}
$$$
}'
Length of output: 1191
packages/sdk/src/utils.ts (1)
384-389: Verify the function's usage across the codebase.
Let's ensure this new function is properly integrated and doesn't introduce any breaking changes.
✅ Verification successful
Let me analyze the function usage more deeply by checking the implementation of getRelationKeyPairs and its return type to ensure type compatibility.
Function is properly integrated and safe to use
The verification shows that:
- The function is used in two locations: schema's Zod transformer and internally by
getForeignKeyFields - The implementation in
getRelationKeyPairsis robust with proper type checking and error handling - The function returns a consistent structure mapping foreign keys to their corresponding ID fields
- All usages are type-safe and handle the return value appropriately
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of getForeignKeyFields function and related functions
# Find all imports of getForeignKeyFields
echo "Checking imports of getForeignKeyFields:"
rg "import.*getForeignKeyFields.*from.*utils" -A 2
# Find all direct usage of getForeignKeyFields
echo "\nChecking usage of getForeignKeyFields:"
rg "getForeignKeyFields\(" -A 2
# Find related function calls to understand the context
echo "\nChecking related function usage:"
rg "getRelationKeyPairs\(" -A 2
Length of output: 2011
Script:
#!/bin/bash
# Check the implementation and type definition of getRelationKeyPairs
ast-grep --pattern 'export function getRelationKeyPairs($_) {
$$$
}'
# Check for any type definitions related to the key pairs
rg "interface.*Pair|type.*Pair" -A 3
# Check the DataModelField type definition
ast-grep --pattern 'interface DataModelField {
$$$
}'
Length of output: 2870
packages/schema/src/plugins/zod/transformer.ts (3)
2-9: Imports are correctly updated
The added imports getForeignKeyFields and hasAttribute are necessary for the new logic and enhance the functionality of the transformer.
250-251: Passing contextField improves schema generation
Including contextField when calling generatePrismaStringLine allows for context-aware determination of field optionality, ensuring that the generated schemas accurately reflect the data model.
325-330: Optionality logic enhancement in generatePrismaStringLine
The updated logic for determining field optionality in generatePrismaStringLine considers both field.isRequired and the optionality inferred from the data model field via isFieldOptional. This improves the accuracy of generated schemas regarding optional fields.
Also applies to: 345-353
packages/runtime/src/enhancements/node/delegate.ts (1)
Line range hint 590-633: LGTM!
The code change correctly handles foreign key assignments for delegate base models by invoking fkAssignmentToConnect, ensuring that foreign key assignments are converted to connect payloads to maintain compatibility with Prisma's payload restrictions.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/runtime/src/enhancements/node/delegate.ts(2 hunks)
🔇 Additional comments (2)
packages/runtime/src/enhancements/node/delegate.ts (2)
Line range hint 590-619: LGTM: Clean addition of delegate base tracking.
The addition of the hasDelegateBase flag is a well-structured setup for the subsequent foreign key handling logic.
620-633: Well-documented solution for Prisma's field mixing limitation.
The code effectively addresses Prisma's limitation with mixed "checked" and "unchecked" fields. The comments provide excellent context about the incompatibility issue and the chosen solution.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/runtime/src/enhancements/node/delegate.ts(2 hunks)
🔇 Additional comments (1)
packages/runtime/src/enhancements/node/delegate.ts (1)
Line range hint 590-633: LGTM! Well-structured solution for handling mixed field types.
The changes correctly handle the incompatibility between delegate base creation and direct foreign key assignments by tracking delegate base presence and converting FK assignments at the right time.
fixes #1843