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

Filters on different models can't share the same name #30

Closed
zolrath opened this issue Jun 15, 2018 · 4 comments
Closed

Filters on different models can't share the same name #30

zolrath opened this issue Jun 15, 2018 · 4 comments

Comments

@zolrath
Copy link
Collaborator

zolrath commented Jun 15, 2018

While attempting to make a custom search filter that was shared across multiple models I found that once a specific filter name was used multiple times that filter would fail.

For example, in your tests you have an IsNew filter defined on your Posts.
If you create an IsNew filter on your Comments (and add a DateCreated field on Comments to allow the filter to function) Sieve will not be able to use this filter properly.

It does not attempt to find the IsNew filter defined for a given type, it simply searches for a method matching that name.

I've submitted a fix as PR #29 but thought I should create an Issue in case you wanted to solve it differently.

@Biarity
Copy link
Owner

Biarity commented Jun 16, 2018

An alternative solution would be to allow multiple separate container classes for custom filters/sorts, one per model. This would make for a convenient way of isolating filters/sorts for different models into different classes whilst also taking care this problem.

Fix #29 would still be needed though, because I was thinking this would be implemented alongside the current "monolith filter/sort class" method to give the developer a choice (where a monolith class would be more convenient for smaller projects).

@zolrath
Copy link
Collaborator Author

zolrath commented Jun 16, 2018

Ah yeah that would be sensible!
Thanks for the responsiveness on the two pull requests/one bug report in the past couple weeks, this project has saved me a ton of headache!

@zolrath
Copy link
Collaborator Author

zolrath commented Jun 16, 2018

Any chance of a 2.1.3 with this change on NuGet?

@Biarity
Copy link
Owner

Biarity commented Jun 16, 2018

Published as 2.1.3, though it'll take a while for nuget's "validation and indexing" before it's publicly available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants