Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Oct 1, 2025

fixes #2246

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

📝 Walkthrough

Walkthrough

Adds propagation of nested where clauses for delegated fields: after injecting base select/include hierarchy for a delegate field, the handler now calls injectWhereHierarchy on the nested type when a nested where exists. Adds a regression test covering polymorphic @@DeleGate nested where and _count behavior.

Changes

Cohort / File(s) Summary
Runtime: delegate where propagation
packages/runtime/src/enhancements/node/delegate.ts
Updated injectSelectIncludeHierarchy to, after injecting the base field hierarchy for a delegated field, detect a nested where and call injectWhereHierarchy on the nested type with that where object.
Regression test: polymorphic @@DeleGate with nested where
tests/regression/tests/issue-2246.test.ts
Adds a regression test that creates sample data and asserts nested include filtering and _count behavior when where is used on delegated polymorphic relations (checks "Inception" vs "Interstellar").

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Prisma as Prisma Client
    participant DPH as DelegateProxyHandler
    participant ISIH as injectSelectIncludeHierarchy
    participant IWH as injectWhereHierarchy

    Client->>Prisma: findMany/findFirst with include/select
    Prisma->>DPH: process delegated fields
    DPH->>ISIH: inject base select/include hierarchy
    Note over ISIH: Assemble base field hierarchy
    ISIH-->>DPH: base hierarchy injected
    alt nested include/select contains where
        DPH->>IWH: inject nested where hierarchy (NEW)
        Note over IWH: Traverse nested type and augment where clause
        IWH-->>DPH: where hierarchy injected
    end
    DPH-->>Prisma: modified args
    Prisma-->>Client: results
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly identifies that this change fixes a delegate-related filtering issue involving concrete relations and base fields, which matches the main objective of enabling nested where clause propagation in delegated models.
Linked Issues Check ✅ Passed The implementation adds explicit where-hierarchy injection for nested delegate fields and includes a regression test verifying filtering behavior, fully satisfying the requirements of issue #2246.
Out of Scope Changes Check ✅ Passed All changes are focused on delegate proxy handling and tests for the specific where propagation bug, with no unrelated code modifications.
Description Check ✅ Passed The description directly references fixing issue #2246, which is relevant to the delegated filtering logic being addressed in this pull request.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/issue-2246

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3f65262 and afd8a4b.

📒 Files selected for processing (1)
  • tests/regression/tests/issue-2246.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/regression/tests/issue-2246.test.ts (1)
packages/testtools/src/schema.ts (1)
  • loadSchema (172-248)
⏰ 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). (5)
  • GitHub Check: OSSAR-Scan
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
  • GitHub Check: dependency-review
  • GitHub Check: build-test (20.x)
🔇 Additional comments (5)
tests/regression/tests/issue-2246.test.ts (5)

1-1: LGTM! Clean import.

The import is correct and necessary for the test. The previously flagged unused import has been removed.


3-4: LGTM! Clear test structure.

The test suite and case naming clearly identify this as a regression test for issue #2246.


50-81: LGTM! Comprehensive test coverage.

The test assertions provide excellent coverage of the fix for issue #2246:

  • Nested where clauses on delegated relations (lines 50-58)
  • _count with nested where clauses (lines 60-66, 74-80)
  • Both positive and negative test cases (finding 'Inception' vs. not finding 'Interstellar')

This thoroughly exercises the where-clause propagation for delegated fields that was added to fix the PrismaClientValidationError.


16-21: Ignore access control suggestion for Movie model. Extended models inherit @@allow from their delegated base, so no explicit directive is required on Movie.

Likely an incorrect or invalid review comment.


36-48: Include explicit mediaType in nested Movie creation or confirm auto-population.
The schema’s mediaType field has no default, so the nested movies.create call in tests/regression/tests/issue-2246.test.ts may fail. Add

mediaType: 'Movie'

under the create data or verify that the Prisma client auto-populates the discriminator for delegated models.


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.

Copy link
Contributor

@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: 1

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

5-82: Good regression test coverage for the reported issue.

The test validates both include and _count scenarios with nested where filters on delegated fields, covering both matching and non-matching cases. This directly addresses the reported issue #2246.

Optional: Consider adding test cases for:

  • Using select instead of include with nested where filters.
  • Complex where clauses with logical operators (AND, OR, NOT) on delegated fields.
  • Filtering on fields defined directly in Movie (non-inherited) vs. fields from Media base (inherited) to ensure both scenarios work correctly.

These additions would strengthen confidence in edge cases, but the current coverage is sufficient for validating the fix.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 237bb59 and 3f65262.

📒 Files selected for processing (2)
  • packages/runtime/src/enhancements/node/delegate.ts (1 hunks)
  • tests/regression/tests/issue-2246.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/regression/tests/issue-2246.test.ts (1)
packages/testtools/src/schema.ts (1)
  • loadSchema (172-248)
⏰ 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). (5)
  • GitHub Check: build-test (20.x)
  • GitHub Check: dependency-review
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
  • GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
packages/runtime/src/enhancements/node/delegate.ts (1)

231-233: LGTM! Consistent where-clause propagation for nested delegate fields.

The addition correctly propagates where-clause injection for nested delegate fields during select/include processing. This aligns with the existing pattern for orderBy injection (lines 242-247) and fixes the reported issue where nested where filters on delegated fields were not being transformed.

@ymc9 ymc9 merged commit d543a0d into dev Oct 1, 2025
12 checks passed
@ymc9 ymc9 deleted the fix/issue-2246 branch October 1, 2025 02:12
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