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

Support filtering :id type fields #429

Closed
wants to merge 2 commits into from
Closed

Conversation

matsu911
Copy link
Contributor

This change supports filtering for id type fields such as belongs_to.

@cpjolicoeur
Copy link
Member

@matsu911 Thanks for this pull request submission.

Will you please provide a little more information on why this change is happening and what the use cases are? Is the use case simply to allow the default filters to allow filtering on model reference/relationship fields?

Also, if we do make this addition, we will need some test cases added to this pull request to support it. Currently, your changes here are failing the current test suite, and we'd need to ensure the existing tests pass, and add any new tests needed for the code changes.

@cpjolicoeur
Copy link
Member

As a side note, you also can customize the filter config on your own by defining your own filter_config/1 function in the module that you use Torch.Pagination inside of. This way you can add or customize which filters you like for your specific use case.

@matsu911
Copy link
Contributor Author

I attempted to use filter_assoc_select to filter by a related object, but I encountered an error stating that the corresponding _equals filter was not defined. I resolved this issue by overwriting filter_config/1 to define the filter. However, it seems that this type of filtering is a commonly encountered scenario. Therefore, it would be beneficial for Torch users if Torch allowed the default filtering by id.

@cpjolicoeur
Copy link
Member

@matsu911 I've made a few tweaks and additions to your code and created a new PR #430 and am closing this PR as a result. Your commit authorship still remains in the new PR #430.

@cpjolicoeur
Copy link
Member

@matsu911 Torch version 5.1.2 has been released that includes your changes here. Thanks for the pull request and work on this.

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

Successfully merging this pull request may close these issues.

2 participants