Fixes several minor issues when building the GraphQL schema #16151
Conversation
|
If anybody with more knowledge of GraphQL wants to review, please do. Otherwise, I can do it to an extent, but @gvkries please help by adding PR comments on the sections, pointing which one fixes which issue. That would help a lot. |
|
@hyzx86 your review please |
...OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/DynamicContentTypeBuilder.cs
Show resolved
Hide resolved
|
I corrected the fix for #14592. Dynamically added fields are now correctly assigned to the corresponding part in GraphQL, instead of always being added to the content type (as if they were collapsed). |
|
Let's merge #16184 before this one so have a more unique diff. |
|
This pull request has merge conflicts. Please resolve those before requesting a review. |
|
@gvkries please resolve the conflicts and let us merge this too |
|
I'll do a quick review. |
...OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/DynamicContentTypeBuilder.cs
Outdated
Show resolved
Hide resolved
Do you still need that? Haven't had time to do it yet. |
|
Not anymore :). |
Okay, thanks. And I'm sorry that I gave you more work 😇 |
src/docs/releases/2.0.0.md
Outdated
| @@ -246,6 +246,11 @@ Here are the updated signatures: | |||
|
|
|||
| These adjustments ensure compatibility and adherence to the latest conventions within the `SectionDisplayDriver` class. | |||
|
|
|||
| ### GraphQL Module | |||
|
|
|||
| The GraphQL schema may change, because fields are now always added to the correct part. Previously, additional fields may have been added to the parent content item type directly. | |||
There was a problem hiding this comment.
additional fields may have been added to the parent content item type directly
Not sure what it means, but having content type fields attached to the dynamic part (the implicit one matching the type name) was by design. Does it mean that an Article type will have to access an ArticlePart property in graphql to get the fields?
There was a problem hiding this comment.
This only affects fields that have been added to a statically typed part. E.g. if you add fields to the HtmlBodyPart they were added to the content type directly, which will have failed often because of duplicated field names. The dynamic part itself has not been changed and also collapsing fields to the parent content item type is left unchanged as well. So the schema will only change in rare cases, not commonly.
This is fixing bug #14592.
src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Extensions/GraphQLTypeExtensions.cs
Show resolved
Hide resolved
...OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/DynamicContentTypeBuilder.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Hisham Bin Ateya <[email protected]>
There was a problem hiding this comment.
@hishamco please merge if you're OK with it. @sebastienros do you have any other feedback?
…Types/DynamicContentTypeBuilder.cs Co-authored-by: Hisham Bin Ateya <[email protected]>
Fixes #5420
Fixes #6253
Fixes #14592
Fixes #12423
Fixes #14592