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.

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

fix: Handle filter input field argument being nil #787

Merged
merged 1 commit into from
Sep 19, 2022

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Sep 12, 2022

Relevant issue(s)

Resolves #784
Similar to #701 (PR), #700 (ISSUE)

Description

  • In generate.go.genTypeFilterArgInput where if a [field.Type + "FilterArg"] key didn't exist in the typeMap it would return nil and pass that to InputObjectFieldConfig as Type.
  • Additionally, with help from @jsimnz we modify the way AppendType was being used in genTypeQueryableFieldList. We do direct manipulation of the TypeMap, such that it won't cause any errors. We do still get the benefits of AppendType because of our ResolveType function (we want to avoid resolving FieldThunk too early),
  • Another schema type generation bug was fixed by the above change where the OrderArg for the relation field name would have the type as empty string "" instead of "XOrderArg".

How has this been tested?

Local & CI

Specify the platform(s) on which this was tested:

  • Arch Linux

@shahzadlone shahzadlone added bug Something isn't working area/schema Related to the schema system action/no-benchmark Skips the action that runs the benchmark. labels Sep 12, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.3.1 milestone Sep 12, 2022
@shahzadlone shahzadlone requested a review from a team September 12, 2022 17:39
@shahzadlone shahzadlone self-assigned this Sep 12, 2022
Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

Possible query test gap leaking a probable bug (might hide gql intelisense types in client even if query works)

@shahzadlone shahzadlone requested review from AndrewSisley and a team September 13, 2022 11:40
@shahzadlone shahzadlone force-pushed the lone/fix/nil-filter-input-type branch 3 times, most recently from 9d10cbd to b9ec3d1 Compare September 18, 2022 06:53
@shahzadlone shahzadlone marked this pull request as ready for review September 18, 2022 06:56
@shahzadlone shahzadlone force-pushed the lone/fix/nil-filter-input-type branch from b9ec3d1 to 244c10a Compare September 18, 2022 06:59
@shahzadlone shahzadlone requested a review from jsimnz September 18, 2022 07:16
@shahzadlone
Copy link
Member Author

Possible query test gap leaking a probable bug (might hide gql intelisense types in client even if query works)

Have updated PR description with the new changes with John's help which fixes the bug.

…eldList according to Johns suggestion. Update the schema tests that also fixes an order field.
@shahzadlone shahzadlone force-pushed the lone/fix/nil-filter-input-type branch from 244c10a to dae09a5 Compare September 18, 2022 15:02
Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

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

LGTM 😎

@@ -281,7 +281,7 @@ var defaultBookArgsWithoutFilter = trimFields(
buildOrderArg("book", []argDef{
{
fieldName: "author",
typeName: "",
typeName: "authorOrderArg",
Copy link
Member Author

Choose a reason for hiding this comment

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

thought: Potentially fixes another bug with Order input type.

Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@shahzadlone shahzadlone dismissed AndrewSisley’s stale review September 19, 2022 10:09

All comments resolved

@shahzadlone shahzadlone merged commit bbb9b42 into develop Sep 19, 2022
@shahzadlone shahzadlone deleted the lone/fix/nil-filter-input-type branch September 19, 2022 10:12
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
* Resolves: sourcenetwork#784 

* Description:
- In `generate.go`.genTypeFilterArgInput where if a [field.Type + "FilterArg"] key didn't exist in the `typeMap` it would return nil and pass that to `InputObjectFieldConfig` as Type.
- Modify the way `AppendType` was being used in `genTypeQueryableFieldList`. We do direct manipulation of the TypeMap, such that it won't cause any errors. We do still get the benefits of `AppendType` because of our `ResolveType` function (we want to avoid resolving `FieldThunk` too early),
- Another schema type generation bug was fixed by the above change where the `OrderArg` for the relation field name would have the type as empty string `""` instead of `"XOrderArg"`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. area/schema Related to the schema system bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter Field type must be input type but got nil
4 participants