fix for codegen rejecting operations that contain fragments with type conditions on interfaces

dsnam commented 1 month ago

Description

Given a schema like this:

type Query {
    foo: Foo
}

interface Foo {
    fooField: String!
}

interface Bar implements Foo {
    fooField: String!
    barField: String!
}

type Baz implements Bar & Foo {
    barField: String!
    fooField: String!
}

codegen would reject queries with fragments like this:

foo {
    fooField
    ... on Bar {
        barField
    }
}

=========================== short test summary info ============================
FAILED tests/test_operation_codegen.py::test_operation_gen_nested_interface - SystemExit: type Bar not possible for Foo at tests/test-schemas/op-gen.gql:3:9
========================= 1 failed, 94 passed in 1.49s =========================

The check for whether or not a type in a type condition was valid was only based on the interface's possibleTypes, which cannot include other interfaces. This adds checking whether the type implements the interface. I believe interfaces used to not be allowed to implement interfaces according to the spec, but this was added some time ago.

Related Issues

I did not open an issue or spot a related one. If there are issues with the approach in this PR I can close this and open an issue instead to keep track of it.

Progress

Pull request checklist

[x] Tests: This PR includes tests for covering the features or bug fixes (if applicable).
[ ] Docs: This PR updates/creates the necessary documentation.
[ ] CI: Make sure your Pull Request passes all CI checks. If not, clarify the motif behind that and the action plan to solve it (may reference a ticket)

How to test it

I added a test that uses a schema and operation file in this PR that reproduces the issue.

dsnam commented 3 weeks ago

@barbieri any chance you will have time to take a look at this soon?

barbieri commented 3 weeks ago

sorry taking so long (busy at work), I'll look at it during the weekend. Thanks for your contribution!!!

barbieri commented 1 day ago

just a heads up I didn't forget about this, seems the CI is broken unrelated to this PR... I'll disable older python to see if that comes back to work

barbieri commented 1 day ago

sorry, I did click the "update branch as merge", but can you do a rebase? Let me know if that works for you

coveralls commented 1 day ago

Pull Request Test Coverage Report for Build 10801479412

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals
Change from base Build 10801456318:	0.0%
Covered Lines:	1615
Relevant Lines:	1615

💛 - Coveralls

dsnam commented 1 day ago

sorry, I did click the "update branch as merge", but can you do a rebase? Let me know if that works for you

Yep, had to reinstall deps to get the pre-commit tests passing but I think it's set now.

profusion / sgqlc