Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Nov 6, 2025

Summary by CodeRabbit

  • New Features
    • Relations can now automatically inherit default values when all their foreign key fields have defaults, enabling simplified creation of related records with pre-filled references.
    • Added comprehensive end-to-end test validating default authentication behavior across related models.

Copilot AI review requested due to automatic review settings November 6, 2025 02:04
@coderabbitai
Copy link

coderabbitai bot commented Nov 6, 2025

Walkthrough

This PR introduces relation-level default detection to the schema system. It adds a hasDefault flag to RelationInfo that indicates when all foreign key fields in a relation have default values, updates schema field helpers to consider this new metadata, and includes an end-to-end test suite validating default authentication behavior with generated schema models (User, Profile, Address).

Changes

Cohort / File(s) Summary
Core schema type definitions
packages/schema/src/schema.ts
Added optional hasDefault flag to RelationInfo; updated FieldHasDefault to return true for relations with hasDefault: true; reworked FieldIsRelationArray to derive result from FieldIsRelation instead of object type check.
Relation object generation
packages/sdk/src/ts-schema-generator.ts
Added FK field tracking in createRelationObject to collect field names from relation arguments; conditionally emits hasDefault: true on relation object when all referenced FK fields have @default attributes.
End-to-end test case
tests/e2e/orm/client-api/default-auth.test.ts
New test validating default authentication behavior: creates users without auth fields, sets auth context, creates related profiles, and asserts correct user association via defaults.
Generated schema assets
tests/e2e/orm/schemas/default-auth/schema.zmodel, tests/e2e/orm/schemas/default-auth/schema.ts
New ZModel and generated TypeScript ORM schema defining User, Profile, and Address models with relations; includes unique constraints, foreign keys, and auth-based defaults (e.g., userId defaults to auth().id).
Generated ORM type definitions
tests/e2e/orm/schemas/default-auth/models.ts, tests/e2e/orm/schemas/default-auth/input.ts
Generated TypeScript type aliases for User, Profile, and Address models; exports ORM argument types (FindMany, Create, Update, etc.) and payload getters for all three entities.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • packages/sdk/src/ts-schema-generator.ts: Verify FK field tracking logic correctly identifies all referenced fields and that hasDefault emission triggers only when all FK fields have defaults.
  • packages/schema/src/schema.ts: Confirm the updated FieldHasDefault and FieldIsRelationArray logic correctly integrate relation defaults without breaking existing field-level default detection.
  • Test schema files: Validate that generated schema correctly reflects the hasDefault metadata and that the end-to-end test properly exercises the auth-based default scenario across multiple relations.

Possibly related PRs

Poem

🐰 A hop through relations so fine,
Where defaults align with each foreign line,
Auth IDs dance where users must go,
Profile and Profile together do flow!
hasDefault flags bright, now the schema will know.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing a typing issue where relation fields with foreign keys having default values were not properly handled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/relation-default

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7f191c and 7fbe58f.

📒 Files selected for processing (7)
  • packages/schema/src/schema.ts (2 hunks)
  • packages/sdk/src/ts-schema-generator.ts (2 hunks)
  • tests/e2e/orm/client-api/default-auth.test.ts (1 hunks)
  • tests/e2e/orm/schemas/default-auth/input.ts (1 hunks)
  • tests/e2e/orm/schemas/default-auth/models.ts (1 hunks)
  • tests/e2e/orm/schemas/default-auth/schema.ts (1 hunks)
  • tests/e2e/orm/schemas/default-auth/schema.zmodel (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.

Applied to files:

  • tests/e2e/orm/schemas/default-auth/schema.ts
🧬 Code graph analysis (3)
tests/e2e/orm/client-api/default-auth.test.ts (2)
packages/testtools/src/client.ts (1)
  • createTestClient (52-169)
tests/e2e/orm/schemas/default-auth/schema.ts (1)
  • schema (9-121)
packages/sdk/src/ts-schema-generator.ts (1)
packages/sdk/src/model-utils.ts (1)
  • hasAttribute (59-64)
tests/e2e/orm/schemas/default-auth/schema.ts (2)
packages/schema/src/expression-utils.ts (1)
  • ExpressionUtils (19-123)
packages/schema/src/schema.ts (1)
  • SchemaDef (10-18)
⏰ 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, sqlite)
  • GitHub Check: build-test (20.x, postgresql)
  • GitHub Check: claude-review
🔇 Additional comments (10)
packages/schema/src/schema.ts (2)

55-55: LGTM! Clean addition of relation-level default indicator.

The optional hasDefault flag appropriately captures when all foreign key fields have default values, enabling better type inference for relation fields.


257-259: LGTM! Correctly extends default detection to relation fields.

The type logic properly treats a relation field as having a default when all its foreign key fields have defaults, fixing the typing issue mentioned in the PR title.

packages/sdk/src/ts-schema-generator.ts (3)

710-710: LGTM! Clean variable initialization.

The fkFields array is properly initialized to collect foreign key field names for subsequent default detection.


717-719: LGTM! Correctly captures FK field names.

The logic properly collects foreign key field names from the fields parameter of the @relation attribute for later default detection.


740-749: LGTM! Solid default detection logic.

The implementation correctly:

  • Only processes relations that have FK fields defined
  • Verifies each FK field exists and has a @default attribute
  • Safely handles missing fields (would evaluate to false via short-circuit)
  • Emits hasDefault: true only when all FK fields have defaults

The logic aligns with the type definition changes in schema.ts.

tests/e2e/orm/schemas/default-auth/schema.zmodel (1)

1-24: Auth-based relation default looks correct. Profile.userId carries the auth() default and the relation wiring matches the test scenario, so the schema aligns with the new default-handling behavior.

tests/e2e/orm/client-api/default-auth.test.ts (1)

5-20: Great coverage on relation defaults with auth(). The test hits both bare profile creation and the nested connect path, ensuring the new default behavior is enforced end-to-end.

tests/e2e/orm/schemas/default-auth/models.ts (1)

8-12: Model result aliases line up with the schema. The generated typings mirror the new models, so nothing further is needed here.

tests/e2e/orm/schemas/default-auth/schema.ts (1)

8-121: Relation metadata correctly sets hasDefault. The generated schema mirrors the ZModel, and the Profile↔User relation carries hasDefault: true, which is the key for the new behavior.

tests/e2e/orm/schemas/default-auth/input.ts (1)

8-70: Input arg aliases are consistent with the schema. The generated types cover the full operation surface for User, Profile, and Address, matching the new defaults-backed relations.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Nov 6, 2025

Claude finished @ymc9's task —— View job


Code Review Complete ✅

Reviewing PR #371: "fix(orm): typing issue with relation fields with fk having default value"

Todo List:

  • Read and analyze all changed files
  • Review core schema generation changes
  • Analyze TypeScript schema generator modifications
  • Review test implementation and coverage
  • Assess code quality and best practices
  • Check for potential bugs or security issues
  • Provide comprehensive feedback

Review Summary

This PR successfully addresses a TypeScript typing issue with relation fields that have foreign keys with default values. The implementation is well-structured and comprehensive.

Key Changes Analyzed

1. Core Schema Enhancement (packages/schema/src/schema.ts:55)

  • Enhancement: Added hasDefault?: boolean property to RelationInfo type
  • Impact: Enables tracking whether all foreign key fields in a relation have default values
  • Assessment: ✅ Clean, minimal API extension that maintains backward compatibility

2. Schema Generator Logic (packages/sdk/src/ts-schema-generator.ts:740-749)

  • Enhancement: Added logic to detect when all FK fields have default values:
    if (fkFields.length > 0) {
        const allHaveDefault = fkFields.every((fieldName) => {
            const fieldDef = field.$container.fields.find((f) => f.name === fieldName);
            return fieldDef && hasAttribute(fieldDef, '@default');
        });
        if (allHaveDefault) {
            relationFields.push(ts.factory.createPropertyAssignment('hasDefault', ts.factory.createTrue()));
        }
    }
  • Assessment: ✅ Robust implementation with proper field resolution and default detection

3. Test Coverage (tests/e2e/orm/schemas/default-auth/)

  • ZModel Schema: Realistic test case with userId Int @unique @default(auth().id)
  • Generated Schema: Properly shows hasDefault: true for the user relation (schema.ts:65)
  • Integration Test: Validates that Profile creation works without requiring userId field
  • Assessment: ✅ Comprehensive test coverage including both unit and integration tests

Code Quality Assessment

Strengths:

  1. Type Safety: The FieldHasDefault utility type correctly handles relation defaults (schema.ts:257-258)
  2. Proper Logic: FK default detection correctly checks all fields in the relation
  3. Test Quality: Test covers the exact use case - creating Profile without specifying userId
  4. Schema Generation: Generated TypeScript schema properly reflects the hasDefault flag

Technical Soundness:

  • Logic: The allHaveDefault check ensures ALL FK fields must have defaults (not just some)
  • Type System: The schema type system properly propagates the hasDefault information
  • Edge Cases: Handles optional FK fields and complex relation scenarios

Security & Performance

Security: ✅ No Concerns

  • Changes are purely type-system related
  • No runtime behavior changes that could introduce vulnerabilities
  • Auth default handling (@default(auth().id)) is already secure

Performance: ✅ No Impact

  • Schema generation is build-time only
  • Runtime performance unaffected
  • No unnecessary iterations or expensive operations

Potential Issues: None Identified

The implementation is solid with no apparent bugs or edge cases missed.

Recommendations for Future

  1. Documentation: Consider adding JSDoc comments to the hasDefault property in RelationInfo
  2. Edge Case Testing: Future tests could cover mixed scenarios (some FK fields with defaults, some without)

Final Assessment: ✅ APPROVED

This is a well-implemented fix that:

  • Solves the specific typing issue with relation FK defaults
  • Maintains backward compatibility
  • Includes comprehensive test coverage
  • Follows established code patterns
  • Has no security or performance concerns

The code quality is high and the solution is technically sound.

Copy link
Contributor

Copilot AI left a 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 adds support for relation fields with default foreign key values, specifically enabling the auth().id function to be used as a default value for foreign key fields. This allows creating related entities without explicitly providing the foreign key value when it can be inferred from the authentication context.

  • Added hasDefault property to relation metadata when all foreign key fields have default values
  • Updated type system to recognize relation fields with defaults as having default values
  • Added schema and test files demonstrating auth-based default values in relations

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/e2e/orm/schemas/default-auth/schema.zmodel New test schema defining User/Profile/Address models with auth-based FK default
tests/e2e/orm/schemas/default-auth/schema.ts Generated TypeScript schema with hasDefault metadata for Profile.user relation
tests/e2e/orm/schemas/default-auth/models.ts Generated TypeScript model type exports
tests/e2e/orm/schemas/default-auth/input.ts Generated TypeScript input type definitions for the test schema
tests/e2e/orm/client-api/default-auth.test.ts Test verifying profile creation without explicit userId when using auth default
packages/sdk/src/ts-schema-generator.ts Logic to detect and mark relations with all FK fields having defaults
packages/schema/src/schema.ts Type system updates to recognize relations with hasDefault as having defaults

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ymc9 ymc9 merged commit 5355474 into dev Nov 6, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants