feat: Add support for and-or inline array aggregate filters #779
Conversation
Was incorrectly typing the filter parameter as the underlyering type, not a list. This feature can be added in properly later (also only added for nillable types, non-nillables fail the isLeaf type check for some horrible reason).
AndrewSisley
added
feature
AndrewSisley
force-pushed
the
sisley/feat/I621-agg-filter-and-or
branch
from
e7a1c47 to
e8fdc7f
Compare
AndrewSisley
changed the title
Codecov Report
@@ Coverage Diff @@
## develop #779 +/- ##
===========================================
+ Coverage 58.53% 58.63% +0.09%
===========================================
Files 150 150
Lines 16900 16967 +67
===========================================
+ Hits 9893 9948 +55
- Misses 6083 6093 +10
- Partials 924 926 +2
|
query/graphql/schema/generate.go
Outdated
| Name: fmt.Sprintf("%s%s", filterTypeName, "FilterArg"), | ||
| } | ||
|
|
||
| fieldThunk := (gql.InputObjectConfigFieldMapThunk)( |
There was a problem hiding this comment.
question: why the extra parenthesis arount the function name?
There was a problem hiding this comment.
It is casting the anon function, although I think the cast might be avoidable with explicit typing of the variable declaration - will give it a quick go before merging - thanks :)
There was a problem hiding this comment.
Ah yes I missed that it was a type and not a function. But yes indeed explicit typing would work:
var fieldThunk gql.InputObjectConfigFieldMapThunk
fieldThunk = func() (gql.InputObjectConfigFieldMap, error) {There was a problem hiding this comment.
actually you can avoid the type casting and the explicit typing. Just do:
fieldThunk := func() (gql.InputObjectConfigFieldMap, error) {There was a problem hiding this comment.
Nevermind. fieldThunk needs an explicit type. I though for a minute that it would work cuz it compiled but we get a runtime error.
There was a problem hiding this comment.
yeah, the thunk stuff is a bit unsafe like that :( merging with the explicit type
AndrewSisley
force-pushed
the
sisley/feat/I621-agg-filter-and-or
branch
from
e8fdc7f to
8ccc4a7
Compare
| Name: fmt.Sprintf("%s%s", filterTypeName, "FilterArg"), | ||
| } | ||
|
|
||
| var fieldThunk gql.InputObjectConfigFieldMapThunk = func() (gql.InputObjectConfigFieldMap, error) { |
There was a problem hiding this comment.
praise: I like this much better :)
There was a problem hiding this comment.
thanks for spotting it
…twork#779) * Remove broken inline array filter param Was incorrectly typing the filter parameter as the underlyering type, not a list. This feature can be added in properly later (also only added for nillable types, non-nillables fail the isLeaf type check for some horrible reason). * Add and-or support for inline array agg filters
Relevant issue(s)
Resolves #621
Description
Adds support for and-or in inline array aggregate filters. Also fixes a bug where the underling type (e.g. Int) was set as the type param for normal-non-aggregate filters on inline arrays (tested via tests when adding the main feature, fix-commit does not contain tests that covers this).
Specify the platform(s) on which this was tested: