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.

Sign up for GitHub

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

Jump to bottom

Collapsed fields collision in graphql #13927

Merged
merged 16 commits into from
Apr 16, 2024
Merged

Collapsed fields collision in graphql #13927

merged 16 commits into from
Apr 16, 2024

Conversation

lampersky
Copy link
Contributor

fixes #13926

@lampersky lampersky mentioned this pull request Jun 30, 2023
Closed
@lampersky lampersky changed the title Lampersky/collapsed fileds collision graphql collapsed fileds collision in graphql Jul 3, 2023
@agriffard agriffard changed the title collapsed fileds collision in graphql Collapsed fields collision in graphql Jul 24, 2023
@lampersky
simplify things a bit
c3c2aa2
@lampersky
Merge branch 'main' into lampersky/collapsed-fileds-collision-graphql
3a8c212
@lampersky
Merge branch 'main' into lampersky/collapsed-fileds-collision-graphql
67e9258 
# Conflicts:
#	src/OrchardCore.Modules/OrchardCore.ContentFields/GraphQL/Fields/ContentFieldsProvider.cs
#	src/OrchardCore.Modules/OrchardCore.ContentFields/GraphQL/Fields/ObjectGraphTypeFieldProvider.cs
@lampersky
Merge branch 'lampersky/collapsed-fileds-collision-graphql' of github…
8082183 
….com:lampersky/OrchardCore into lampersky/collapsed-fileds-collision-graphql
@lampersky
fix
a67246e
@lampersky
Copy link
Contributor Author

@sebastienros whenever you have time, please have a look into this PR, also here quick video was added to present those changes in PartSettings

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 8, 2024

This pull request has merge conflicts. Please resolve those before requesting a review.

@lampersky
Merge branch 'main' into lampersky/collapsed-fileds-collision-graphql
29b88ef 
# Conflicts:
#	src/OrchardCore.Modules/OrchardCore.ContentFields/GraphQL/Fields/ObjectGraphTypeFieldProvider.cs
sebastienros
sebastienros reviewed Feb 12, 2024
View reviewed changes
src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Options/GraphQLContentOptions.cs Outdated

if (settings.PreventFieldNameCollision)
{
return $"{partName.ToFieldName()}{fieldName.ToPascalCase()}";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No separators?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sebastienros yes, no separator, partName.ToFieldName() is producing camelCase, fieldName.ToPascalCase() is producing PascalCase when concatenated you will get some user friendly fields

example:
partName: MyCustomPart
fieldName: MyCustomField
will produce: myCustomMyCustomField graphQL field

image

@lampersky
Merge branch 'main' into lampersky/collapsed-fileds-collision-graphql
ba0c424 
# Conflicts:
#	src/OrchardCore.Modules/OrchardCore.ContentFields/GraphQL/Fields/ObjectGraphTypeFieldProvider.cs
@Piedone
Copy link
Member

Piedone commented Mar 21, 2024

Do you want to get back to the review of this, @sebastienros?

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 5, 2024

This pull request has merge conflicts. Please resolve those before requesting a review.

@Piedone
Copy link
Member

Piedone commented Apr 5, 2024

@sebastienros?

sebastienros
sebastienros reviewed Apr 5, 2024
View reviewed changes
...ore.Modules/OrchardCore.ContentTypes/GraphQL/Drivers/GraphQLContentTypePartSettingsDriver.cs Outdated

public GraphQLContentTypePartSettingsDriver(IOptions<GraphQLContentOptions> optionsAccessor)
public GraphQLContentTypePartSettingsDriver(IOptions<GraphQLContentOptions> optionsAccessor, IStringLocalizer<GraphQLContentTypePartSettingsDriver> stringLocalizer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see it used.

@lampersky lampersky requested a review from hishamco April 7, 2024 13:03
@hishamco
Copy link
Member

hishamco commented Apr 7, 2024

@hyzx86 do you have any feedback on this PR?

@Piedone
Copy link
Member

Piedone commented Apr 8, 2024

Please also merge soon before any conflicts can arise, if suitable.

@Piedone
Copy link
Member

Piedone commented Apr 10, 2024

Can this be merged, then, Hisham?

@github-actions GitHub Actions
Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

@lampersky
Copy link
Contributor Author

third merge conflict resolved, hurry up guys 😛

@Piedone Piedone merged commit 4e0f746 into OrchardCMS:main Apr 16, 2024
4 checks passed
@Piedone
Copy link
Member

Piedone commented Apr 16, 2024

So, I assume yes, this can be merged :D.

Sorry about that, and thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hishamco hishamco hishamco approved these changes

@sebastienros sebastienros sebastienros approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[graphQL] named part fields return nulls when Collapsed is checked
4 participants
@lampersky @Piedone @hishamco @sebastienros