Skip to content

Conversation

@genu
Copy link
Contributor

@genu genu commented Nov 19, 2025

Fixes #422

Summary by CodeRabbit

  • Refactor

    • Improved auth type handling to correctly support nested relations and array relation shapes.
  • API Changes

    • Made a core model result type publicly available for more consistent ORM typings.
  • New Features

    • Added generated schema and model type aliases to simplify strongly-typed ORM usage.
  • Tests

    • Added a regression test verifying auth inference with nested related objects.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 19, 2025

Walkthrough

Updates ORM type definitions to include relation fields in auth typing by adding an internal recursive AuthModelType, refines AuthType to use it when authType is a model, exports DefaultModelResult publicly, and adds generated test/schema files exercising the change.

Changes

Cohort / File(s) Summary
Auth type change
packages/orm/src/client/contract.ts
Added imports (FieldIsArray, RelationFields, RelationFieldType, SchemaDef, DefaultModelResult); introduced internal AuthModelType<Schema, Model> that composes scalar and recursive relation fields (array-aware); updated AuthType<Schema> to return AuthModelType when Schema['authType'] is a model.
Export surface
packages/orm/src/client/crud-types.ts
Promoted DefaultModelResult from an internal alias to a publicly exported type.
Generated test & fixtures
tests/regression/test/issue-422/models.ts, tests/regression/test/issue-422/input.ts, tests/regression/test/issue-422/regression.test.ts, tests/regression/test/issue-422/schema.ts, tests/regression/test/issue-422/schema.zmodel
Added schema, model result aliases, typed helper arg aliases for Session/User/Profile, and a regression test that verifies auth inference allows nested relations when calling $setAuth.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review the recursive type logic in AuthModelType for correctness and termination (ensure no unintended distributive or circular behavior).
  • Verify FieldIsArray conditional wrapping produces correct array vs single-item types.
  • Confirm AuthType branching preserves previous behaviors for non-model authTypes.
  • Skim generated tests and schema to ensure they correctly exercise auth-with-relations scenarios and match the intended model shapes.

Possibly related PRs

Poem

🐰 I nibble on types one by one,
Tunnels of relations, bright and fun,
Recursion hops where fields entwine,
Arrays lined up in tidy brine,
Auth now hugs each nested sun.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'fix: enhance auth type' directly relates to the main change, which enhances the auth type system to include relations in the auth model payload.
Linked Issues check ✅ Passed The code changes successfully address the primary objective from issue #422 by allowing relation objects in $setAuth payloads through the new AuthModelType that recursively includes relations.
Out of Scope Changes check ✅ Passed All changes are scoped to implementing the auth type enhancement for relations: type system updates in contract.ts and crud-types.ts, plus test files validating the new behavior.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@genu genu changed the base branch from main to dev November 19, 2025 16:29
@genu genu marked this pull request as draft November 19, 2025 16:37
@genu genu marked this pull request as ready for review November 19, 2025 16:37
@genu
Copy link
Contributor Author

genu commented Nov 19, 2025

The target branch was initially set to main, I'm not sure how to get coderabbitai to reprocess the PR as most of its summaries are wrong.

Copy link
Member

@ymc9 ymc9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @genu , thanks for making the fix! Please check my comments.

@genu
Copy link
Contributor Author

genu commented Nov 20, 2025

@ymc9 I updated the solution to use DefaultModelResult. Let me know if this is what you had in mind

Copy link
Member

@ymc9 ymc9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/regression/test/issue-422/regression.test.ts (1)

13-16: Consider clarifying test data naming.

Lines 13 and 16 use 'user1' as the session ID, which may confuse readers since it suggests a user identifier rather than a session identifier. Consider using names like 'session1', 'session2' for session IDs to improve readability.

Example:

-        db.$setAuth({ id: 'user1', user: { id: 'user1' } });
+        db.$setAuth({ id: 'session2', user: { id: 'user1' } });

-        db.$setAuth({ id: 'user1', user: { id: 'user1', profile: { name: 'User1' } } });
+        db.$setAuth({ id: 'session3', user: { id: 'user1', profile: { name: 'User1' } } });
📜 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 d3c9e21 and 6e56e98.

📒 Files selected for processing (5)
  • tests/regression/test/issue-422/input.ts (1 hunks)
  • tests/regression/test/issue-422/models.ts (1 hunks)
  • tests/regression/test/issue-422/regression.test.ts (1 hunks)
  • tests/regression/test/issue-422/schema.ts (1 hunks)
  • tests/regression/test/issue-422/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/regression/test/issue-422/schema.ts
🧬 Code graph analysis (2)
tests/regression/test/issue-422/regression.test.ts (2)
packages/testtools/src/client.ts (1)
  • createTestClient (95-244)
tests/regression/test/issue-422/schema.ts (1)
  • schema (121-121)
tests/regression/test/issue-422/schema.ts (2)
packages/schema/src/expression-utils.ts (1)
  • ExpressionUtils (19-123)
packages/schema/src/schema.ts (1)
  • SchemaDef (11-19)
⏰ 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). (2)
  • GitHub Check: build-test (20.x, sqlite)
  • GitHub Check: build-test (20.x, postgresql)
🔇 Additional comments (2)
tests/regression/test/issue-422/regression.test.ts (1)

5-18: LGTM! Effective compile-time type test for Issue #422.

The test correctly verifies that $setAuth accepts auth payloads with relations (line 13) and nested relations (line 16), addressing the core issue. The absence of runtime assertions is appropriate since this primarily validates TypeScript type inference.

tests/regression/test/issue-422/schema.zmodel (1)

1-25: LGTM! Well-structured test schema for Issue #422.

The schema correctly defines the models needed to test auth with relations:

  • Session model with @@auth attribute (line 11) serves as the auth type
  • Proper relations configured between Session → User and User → Profile
  • Cascade delete behavior ensures referential integrity in tests
  • UUID defaults and unique constraints are appropriate

The schema effectively supports testing scalar fields, single-level relations, and nested relations in auth payloads.

@ymc9 ymc9 merged commit 8b4511b into zenstackhq:dev Nov 20, 2025
4 of 5 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.

Setting the auth doesn't include relations

2 participants