Skip to content

fix: allow provides on abstract types - #1552

Merged
Noroth merged 6 commits into
masterfrom
ludwig/eng-9831-engine-allow-provides-to-be-defined-on-abstract-types
Jul 1, 2026
Merged

fix: allow provides on abstract types#1552
Noroth merged 6 commits into
masterfrom
ludwig/eng-9831-engine-allow-provides-to-be-defined-on-abstract-types

Conversation

@Noroth

@Noroth Noroth commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

The planner currently attempts to retrieve an external field that is defined as provided in the same subgraph via an entity fetch from the other subgraph. This is incorrect behavior. The field should be inlined and instead of having an additional fetch to retrieve the field, it should be inlined.

In this PR the validation logic now properly checks if a field was provided, even when it is part of an inline fragment.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.

Open Source AI Manifesto

This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.

@Noroth
Noroth requested a review from a team as a code owner June 29, 2026 14:24
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 93f6e036-ff0e-41e3-aa4c-e1867f2ff85e

📥 Commits

Reviewing files that changed from the base of the PR and between 72863bb and 41878f9.

📒 Files selected for processing (1)
  • v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_provides_test.go
💤 Files with no reviewable changes (1)
  • v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_provides_test.go

📝 Walkthrough

Walkthrough

A lookup in collectNodesDSVisitor.EnterField now keys provided fields by path without fragments, and three federation tests cover @provides behavior over union types, interface types, and nested abstract selection rewriting.

Changes

@provides fix for abstract type inline fragments

Layer / File(s) Summary
providesEntries key fix
v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go
The provided-field membership check now uses info.currentPathWithoutFragments instead of info.currentPath.
Federation @provides tests for abstract types
v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_provides_test.go
Adds tests for union-typed fields, interface-typed fields, and nested abstract selection rewriting, covering both provided-field inlining and planned entity fetches.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: fixing provides handling on abstract types.
Description check ✅ Passed The description is directly about the planner incorrectly refetching provided fields and fixing inline-fragment handling.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ludwig/eng-9831-engine-allow-provides-to-be-defined-on-abstract-types

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

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_provides_test.go`:
- Line 1123: The test schema definition contains a typo in the GraphQL type
declaration, so it won’t parse and the planner path in this test cannot run.
Update the schema string in graphql_datasource_federation_provides_test.go where
the Media type is declared, changing the malformed “inteface Media” entry to the
correct “interface Media” so the test setup is valid and the intended case is
exercised.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7ab1eef0-1911-411d-b96d-471820b57941

📥 Commits

Reviewing files that changed from the base of the PR and between 478f3c0 and f96a2af.

📒 Files selected for processing (2)
  • v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_provides_test.go
  • v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go

@Noroth
Noroth merged commit fb2d52a into master Jul 1, 2026
10 checks passed
@Noroth
Noroth deleted the ludwig/eng-9831-engine-allow-provides-to-be-defined-on-abstract-types branch July 1, 2026 12:02
dkorittki pushed a commit that referenced this pull request Jul 2, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.6.0](v2.5.1...v2.6.0)
(2026-07-02)


### Features

* add SubscriptionOnCreate hook
([#1538](#1538))
([3f30cb2](3f30cb2))


### Bug Fixes

* allow provides on abstract types
([#1552](#1552))
([fb2d52a](fb2d52a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: wundergraph-bot[bot] <285992168+wundergraph-bot[bot]@users.noreply.github.com>
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