search

ghiscoding / slickgrid-universal

Slickgrid-Universal is a monorepo which includes all Editors, Filters, Extensions, Services related to SlickGrid usage and is also Framework Agnostic
https://ghiscoding.github.io/slickgrid-universal/
Other
89 stars 29 forks source link

feat(GraphQL): add `filterQueryOverride` to GraphQL Service #1549

Closed ghiscoding closed 5 months ago

ghiscoding commented 5 months ago

image

stackblitz[bot] commented 5 months ago

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.8%. Comparing base (3882ce1) to head (6092bdd).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1549 +/- ## ======================================== + Coverage 99.8% 99.8% +0.1% ======================================== Files 198 198 Lines 21685 21691 +6 Branches 7112 7253 +141 ======================================== + Hits 21624 21630 +6 Misses 55 55 Partials 6 6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ghiscoding commented 5 months ago

@zewa666 so I think I got a similar approach to your OData PR #1536, however 1 major thing that makes it apart is that instead of returning a string, I'm actually asking the user to return an object of the shape { field: string; operator: string; value: any; } which is what we end up with in the GraphQL query.

However, while doing this PR, I noticed that my GraphqlFilterQueryOverrideArgs is an exact copy of your OdataFilterQueryOverrideArgs, so I think it would be best if we merge them both into 1 interface which could maybe be named as BackendServiceFilterQueryOverrideArgs and because it's generic then I think you should also move it to a backend service interface (or a new file) like backendServiceOption.interface.ts. So would you mind doing that change in your PR? I can update my PR afterward by simply using that new interface.

I know you're using OData and not GraphQL, but it would still be nice if you could take a look at my PR and tell me if it makes any sense (I don't even have a GraphQL server to test with) but I followed this SO answer that inspired my approach which I think is correct: https://stackoverflow.com/a/37981802/1212166

zewa666 commented 5 months ago

sure I'll take a look tonight. btw can you target my branch as destination for the merge? I'm genuinly interested if thats possible, but no probs I can move the files around as you mentioned anyways.

with regards to testing perhaps a quick way to ramp up a server with auto graphql features would be to look into Hasura. or if you just want to spin up a demo on top of json files perhaps https://github.com/marmelab/json-graphql-server