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

Upgrade GraphQL to 8.2 #16736

Merged
merged 37 commits into from
Nov 22, 2024
Merged

Upgrade GraphQL to 8.2 #16736

merged 37 commits into from
Nov 22, 2024

Conversation

MikeAlhayek
Copy link
Member

@MikeAlhayek MikeAlhayek commented Sep 16, 2024 •
edited by Piedone
Loading

Fixes #16826.

@MikeAlhayek
Copy link
Member Author

@hyzx86 you upgraded GraphQL package last time. You may be more familiar with this. Can you please check this out?

@MikeAlhayek MikeAlhayek mentioned this pull request Sep 17, 2024
Closed
@hishamco
Copy link
Member

IMHO we need to stop bumping the packages ourselves like you said before until the Dependabot is fixed

@MikeAlhayek
Copy link
Member Author

IMHO we need to stop bumping the packages ourselves like you said before until the Dependabot is fixed

The Dependabot is fixed. But it can't do breaking changes like this one. These will need to always be done manually.

@hishamco
Copy link
Member

I thought it was straightforward

Anyhow, to try to fix the broken unit tests. @hyzx86 your help appreciated

@hyzx86
Copy link
Contributor

hyzx86 commented Sep 18, 2024

This problem seems to be related to dynamically generated filtering conditions.

Sorry, I'm busy with other projects recently. It's at a critical stage, and I may not have time to deal with this problem for some time.

@hishamco
Copy link
Member

Thanks @hyzx86

@MikeAlhayek can you handle this? Or we might need help from @carlwoodhouse or @mdameer who worked recently on some GraphQL PRs

@MikeAlhayek
Copy link
Member Author

This is going to be a breaking change and should not be part of 2.x. We should wait until 3 before doing this as we have to set ResolvedType instead of Type on all fields (including custom fields in external projects).

@hishamco
Copy link
Member

So, let us defer this PR then

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

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

@hishamco
Copy link
Member

The build is still fail

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

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

@gvkries gvkries changed the title Upgrade GraphQL to 8.0.2 Upgrade GraphQL to 8.2 Nov 22, 2024
gvkries
gvkries reviewed Nov 22, 2024
View reviewed changes
Copy link
Contributor

@gvkries gvkries left a comment

Choose a reason for hiding this comment

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

Nice work, thanks @MikeAlhayek. I've added some comments, but I've not fully tested this yet.

src/OrchardCore.Modules/OrchardCore.Apis.GraphQL/Services/SchemaService.cs Outdated Show resolved Hide resolved
src/OrchardCore.Modules/OrchardCore.ContentFields/GraphQL/Fields/ContentFieldsProvider.cs Show resolved Hide resolved
src/OrchardCore.Modules/OrchardCore.ContentFields/GraphQL/Fields/ContentFieldsProvider.cs Show resolved Hide resolved
src/OrchardCore.Modules/OrchardCore.Media/GraphQL/MediaAssetQuery.cs Show resolved Hide resolved
src/OrchardCore.Modules/OrchardCore.Queries/Sql/GraphQL/SqlQueryFieldTypeProvider.cs Outdated Show resolved Hide resolved
src/OrchardCore/OrchardCore.Apis.GraphQL.Abstractions/Queries/WhereInputObjectGraphType.cs Show resolved Hide resolved
src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/DynamicPartGraphType.cs Show resolved Hide resolved
@MikeAlhayek MikeAlhayek merged commit 5dc244b into main Nov 22, 2024
33 checks passed
@MikeAlhayek MikeAlhayek deleted the ma/upgeade-graphql branch November 22, 2024 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gvkries gvkries gvkries approved these changes

@sebastienros sebastienros Awaiting requested review from sebastienros

Assignees
No one assigned
Labels
GraphQL
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.

Upgrade GraphQL to 8.x.x
5 participants
@MikeAlhayek @hishamco @hyzx86 @Piedone @gvkries