Skip to content

Fix ParentRequires(string) dropping requirement on named fields#10004

Merged
michaelstaib merged 2 commits into
mainfrom
mst/parentRequires
Jun 29, 2026
Merged

Fix ParentRequires(string) dropping requirement on named fields#10004
michaelstaib merged 2 commits into
mainfrom
mst/parentRequires

Conversation

@michaelstaib

Copy link
Copy Markdown
Member

Fixes #10000

Copilot AI review requested due to automatic review settings June 29, 2026 14:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes a regression where ParentRequires(string) could be effectively ignored for named (non-member/inferred) fields, causing required parent properties (e.g., Name) to be omitted from projections and resulting in null/incorrect resolver behavior (as reported in #10000).

Changes:

  • Allow field requirements to be stored even when the entity type is not known at descriptor time (common for .Field("...")).
  • Make requirements processing resilient to missing entity type information during schema completion.
  • Add regression coverage in both Core descriptor tests and Data projection integration tests (with snapshot).

Reviewed changes

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

Show a summary per file
File Description
src/HotChocolate/Data/test/Data.Tests/ProjectableDataLoaderTests.cs Adds an integration test covering ParentRequires(nameof(Brand.Name)) on a named field (computedName).
src/HotChocolate/Data/test/Data.Tests/snapshots/ProjectableDataLoaderTests.Brand_Details_Requires_Brand_Name_With_Named_Field.md Adds snapshot validating the SQL projection includes Brand.Name and the resolver output is correct.
src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/ObjectFieldDescriptorTests.cs Adds a unit test ensuring ParentRequires(string) sets requirement flags/features for named fields.
src/HotChocolate/Core/src/Types/Types/Descriptors/FieldRequirementFeature.cs Enables nullable annotations and allows EntityType to be nullable.
src/HotChocolate/Core/src/Types/Types/Descriptors/Configurations/FieldConfiguration.cs Stops clearing requirements just because entityType is null; preserves requirement metadata for later use.
src/HotChocolate/Core/src/Types/Execution/Requirements/RequirementsTypeInterceptor.cs Adds a guard to skip requirement processing when no entity type can be determined.

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

@michaelstaib michaelstaib merged commit 024171a into main Jun 29, 2026
7 of 8 checks passed
@michaelstaib michaelstaib deleted the mst/parentRequires branch June 29, 2026 14:50
@github-actions

Copy link
Copy Markdown
Contributor

Patch coverage

60.0% of changed lines covered (3/5)

File Covered Changed Patch %
…/Types/Execution/Requirements/RequirementsTypeInterceptor.cs 1 3 33.3% 🔴
…/Types/Types/Descriptors/Configurations/FieldConfiguration.cs 1 1 100.0% 🟢
…/Core/src/Types/Types/Descriptors/FieldRequirementFeature.cs 1 1 100.0% 🟢
Uncovered changed lines (JSON)
{
  "sha": "63df907fd8a971e4f8b41ff795258f49ade056b2",
  "files": [
    { "path": "src/HotChocolate/Core/src/Types/Execution/Requirements/RequirementsTypeInterceptor.cs", "ranges": [[49, 50]] }
  ]
}

Project coverage: 52.4% (216241/412359 lines)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ParentRequires(string? requires) not working in v16.4.0-p.7

2 participants