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

Enable filters in model resolver #9

Closed
randomiser opened this issue Apr 24, 2022 · 5 comments
Closed

Enable filters in model resolver #9

randomiser opened this issue Apr 24, 2022 · 5 comments
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request released

Comments

@randomiser
Copy link

randomiser commented Apr 24, 2022

Howdy again,

Following the docs for filtering I'm attempting to filter at a deep level only.

When I attempt to call queries that involve several levels of joins such as the following the query isn't amended to include the filter I've included and returns all of the records.

I've switched on filter: true for all models and set it at the resolver level, and updated the GQL to include the filter options.

I notice your docs state "TODO" against some of the extra filtering options. Should this case work currently? I've tried other variants like status__in: [pending] and adding a modifier to the model directly and calling that.

{
	"query": "query NotWorkingExample { 
			companies { 
				offices { 
					staffMembers { 
						firstName 
						lastName
						expenses(filter: {status: pending}) { 
							id 
							status 
						} 
				}
			} 
		} 
	}"
}

EDIT: added code formatting - Ilya

IlyaSemenov added a commit that referenced this issue Apr 26, 2022
@IlyaSemenov
Copy link
Owner

@randomiser I added demo test for the case (see the link to commit above) - nested filter works for me. Please check what's different with your case.

I notice your docs state "TODO" against some of the extra filtering options. Should this case work currently?

It should, but it's currently either "all in" or fully disabled (which is the default). Ideally it should allow fine-grained setup so that filtering only on certain fields could be enabled. Otherwise, it might introduce a security risk in certain cases.

@IlyaSemenov IlyaSemenov added bug Something isn't working need info Further information is requested labels Apr 26, 2022
@randomiser
Copy link
Author

Hi Ilya,

I was basing my assumptions on how this worked on this document:

https://github.com/IlyaSemenov/objection-graphql-resolver/blob/master/docs/filters.md

I hadn't realised I would need to explicitly add a RelationResolver into the ModelResolver to get this working. Now I've made the change as per your test it's working fine. Thank you!

Suggest you could add this RelationResolver information into the filters.md documentation. Would stop questions like this happening again.

Thanks again, great library!
Matt

@IlyaSemenov
Copy link
Owner

I see. So two problems here:

  1. The documentation is plain wrong.
  2. Enabling filters on model resolver level (or even on graph resolver level) actually makes sense.

I'll take care of this.

@IlyaSemenov IlyaSemenov added documentation Improvements or additions to documentation enhancement New feature or request and removed bug Something isn't working need info Further information is requested labels Apr 27, 2022
@IlyaSemenov IlyaSemenov changed the title Deeply nested filters seem to be ignored Enable filters in model resolver Apr 27, 2022
@IlyaSemenov IlyaSemenov added the bug Something isn't working label Apr 27, 2022
IlyaSemenov added a commit that referenced this issue May 7, 2023
BREAKING CHANGE:

Introduce the new graph, model, field, relation, page API and deprecate
the older GraphResolver, ModelResolver, etc. functions.

Add allowAllFields, allowAllFilters graph and model-level options.

Resolves #9, resolves #11, resolves #12.
@github-actions
Copy link
Contributor

github-actions bot commented May 7, 2023

🎉 This issue has been resolved in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@IlyaSemenov
Copy link
Owner

It's now possible to set allowAllFilters: true on model and graph level (see examples).

Unfortunately, that means migrating to the new API. (In fact, it's possible to back-port that to the old API which is still supported.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request released
Projects
None yet
Development

No branches or pull requests

2 participants