Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scope on morphTo directive applied to source model, rather than target #2106

Open
dennis-koster opened this issue Apr 1, 2022 · 2 comments · May be fixed by #2110
Open

Scope on morphTo directive applied to source model, rather than target #2106

dennis-koster opened this issue Apr 1, 2022 · 2 comments · May be fixed by #2110
Labels
bug An error within Lighthouse

Comments

@dennis-koster
Copy link
Contributor

Describe the bug

When applying a scope to the morphTo directive, it appears that Lighthouse is trying to apply the scope to the root model, rather than then target relationship. For instance, given the following schema:

type Author {
    id: ID!
    publications: [Publication!]!
}

type Publication {
    id: ID!
    publication: Publishable @morphTo(scopes: ["active"])
}

type Magazine {
    id: ID!
    title: String!
    isActive: Boolean!
}

type Book {
    id: ID!
    title: String!
    isActive: Boolean!
}

union Publishable = Book | Magazine

Lighthouse is trying to apply the active scope to the Publication model, rather than the Magazine or Book model.

Expected behavior/Solution

The scope should be applied to the query responsible for fetching the Magazine or Book.

Steps to reproduce

See above example.

Lighthouse Version
v5.23.1

spawnia added a commit that referenced this issue Apr 7, 2022
@spawnia spawnia added the bug An error within Lighthouse label Apr 7, 2022
@spawnia spawnia linked a pull request Apr 7, 2022 that will close this issue
5 tasks
@spawnia
Copy link
Collaborator

spawnia commented Apr 7, 2022

Right, the current behaviour is somewhat nonsensical. To properly support scopes on polymorphic relations, the definition has to take the type of the related model into account. I got a PR on this going, but it needs a bunch more work to be finished. @dennis-koster please review #2110

@dennis-koster
Copy link
Contributor Author

@spawnia Hi Spawnia, thanks for drafting up a PR! I think the idea of specifying which model you want to apply what scope(s) to makes sense. I wonder if this would work in combination with the union type though, as in my example. Perhaps it would be beneficial to add a test for that scenario as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error within Lighthouse
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants