Skip to content

Fix for guid filter in GraphQL#1644

Merged
Aniruddh25 merged 16 commits intomainfrom
dev/agarwalayush/gqlFilterWithGuid
Aug 25, 2023
Merged

Fix for guid filter in GraphQL#1644
Aniruddh25 merged 16 commits intomainfrom
dev/agarwalayush/gqlFilterWithGuid

Conversation

@ayush3797
Copy link
Copy Markdown
Contributor

@ayush3797 ayush3797 commented Aug 22, 2023

Why make this change?

Fix #1598 to ensure GQL filter works with guid types.

What is this change?

Correctly resolved guid type to Hotchocolate type of UUID (per: https://chillicream.com/docs/hotchocolate/v12/defining-a-schema/scalars#uuid-type) which was earlier resolved to string. Accordingly parsed the guid values using Guid.Parse() method instead of parsing it as string.

How was this tested?

  • Unit Tests - SchemaConverterTests
  • Integration Tests - Added filter tests with GUID to GraphQLSupportedTypesTestsBase.QueryTypeColumnFilterAndOrderBy

@ayush3797 ayush3797 self-assigned this Aug 22, 2023
@ayush3797 ayush3797 added bug Something isn't working graphql labels Aug 22, 2023
@ayush3797 ayush3797 linked an issue Aug 22, 2023 that may be closed by this pull request
1 task
Copy link
Copy Markdown
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix! Few suggestions

Comment thread src/Core/Services/ResolverMiddleware.cs
Comment thread src/Service.GraphQLBuilder/GraphQLUtils.cs Outdated
Comment thread src/Service.Tests/DatabaseSchema-PostgreSql.sql Outdated
Copy link
Copy Markdown
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment thread src/Core/Services/ResolverMiddleware.cs Outdated
Comment thread src/Core/Services/ResolverMiddleware.cs Outdated
Comment thread src/Core/Services/TypeHelper.cs Outdated
Comment thread src/Service.GraphQLBuilder/GraphQLTypes/SupportedTypes.cs Outdated
Copy link
Copy Markdown
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

I'm confused why we're mapping mssql GUID to GraphQL Type ID.

Comment thread src/Service.GraphQLBuilder/Sql/SchemaConverter.cs Outdated
Comment thread src/Core/Services/ResolverMiddleware.cs Outdated
Comment thread src/Service.GraphQLBuilder/GraphQLTypes/SupportedTypes.cs
Copy link
Copy Markdown
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

just last two questions.

Comment thread src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs Outdated
Comment thread src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs Outdated
Comment thread src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs Outdated
Comment thread src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs Outdated
Copy link
Copy Markdown
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

thanks for addressing concerns!

@Aniruddh25 Aniruddh25 merged commit 8b05b5b into main Aug 25, 2023
@Aniruddh25 Aniruddh25 deleted the dev/agarwalayush/gqlFilterWithGuid branch August 25, 2023 22:40
Aniruddh25 added a commit that referenced this pull request Aug 25, 2023
## Why make this change?
Fix #1598 to ensure GQL
filter works with guid types.

## What is this change?
Correctly resolved guid type to Hotchocolate type of `UUID` (per:
https://chillicream.com/docs/hotchocolate/v12/defining-a-schema/scalars#uuid-type)
which was earlier resolved to string. Accordingly parsed the guid values
using `Guid.Parse()` method instead of parsing it as string.

## How was this tested?
- [x] Unit Tests - `SchemaConverterTests`
- [X] Integration Tests - Added filter tests with GUID to
`GraphQLSupportedTypesTestsBase.QueryTypeColumnFilterAndOrderBy`

---------

Co-authored-by: Aniruddh Munde <[email protected]>
Aniruddh25 added a commit that referenced this pull request Aug 26, 2023
- Cherry picks PR #1644 to 0.8 release as a bug fix patch.

---------

Co-authored-by: Ayush Agarwal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working graphql

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: Failed to convert parameter value from a String to a Guid.

4 participants