search

silverstripe / silverstripe-graphql

Serves Silverstripe data as GraphQL representations
BSD 3-Clause "New" or "Revised" License
52 stars 61 forks source link

FIX: NestedInputBuilder doesn't work with nested '*' #538

Closed GuySartorelli closed 1 year ago

GuySartorelli commented 1 year ago

Supercedes https://github.com/silverstripe/silverstripe-graphql/pull/536 and includes requested changes mentioned in that PR

Issue

GuySartorelli commented 1 year ago

I thought the answer was going to be "yes, easy peasy" but it turns out that there's something weird about the way test schemas are built which differs in some way from normal schemas.... I've created a new issue to cover that. Until that issue is resolved, I don't think there's any way to add a test for this - but I don't think this bug fix should be held back by that, since it is likely that will be a complex issue to resolve.

sabina-talipova commented 1 year ago

@GuySartorelli, I've already approved. I couldn't reproduce issue by using provided code example in main ticket. But I have a question regarding new issue that you opened. Does not seem to you that you get the same Exception that was mentioned in the main ticket? Did you apply your test to this solution? Thank you.

GuySartorelli commented 1 year ago

I couldn't reproduce issue by using provided code example in main ticket

Did you use this example and did you both activate the default schema and tell graphql where to look for your models file?

I have a question regarding https://github.com/silverstripe/silverstripe-graphql/issues/539 ... Does not seem to you that you get the same Exception that was mentioned in the main ticket? Did you apply your test to this solution?

That's actually exactly the problem. In that issue, the schema example doesn't cause any exception when used outside of a test context. But when used in a test, it gets the exception mentioned in that issue - which is indeed a different exception than the one this PR is fixing. So there's something about the way tests set up or build schemas that's different from the normal execution, which is causing that exception when no exception should be thrown.

sabina-talipova commented 1 year ago

Did you use https://github.com/silverstripe/silverstripe-graphql/issues/535#issuecomment-1586558963 and did you both activate the default schema and tell graphql where to look for your models file? Yes, I did.