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

[938] Conditional logic for filters #1801

Merged
merged 1 commit into from
Jan 15, 2013
Merged

[938] Conditional logic for filters #1801

merged 1 commit into from
Jan 15, 2013

Conversation

seanlinsley
Copy link
Contributor

Fixes longstanding Issue #938, replacing Pull Request #1452 which was never merged.

+ followed the way conditional procs are built for Scopes
+ by way of Array.group_by, we now only recognize the last-defined
  `filter` call for each individual attribute
+ update the specs to allow `filter` to be called multiple times on
  the same pseudo-document
@seanlinsley
Copy link
Contributor Author

Note that I updated this PR with tests, and found a much more robust way to add this functionality than what I had before.

Bonus: if someone calls filter for the same resource multiple times, we'll now only utilize the last one.

One more thing: I updated the unit tests to allow filter to be called multiple times. I added this so my tests could be more succinct.

@macfanatic
Copy link
Contributor

Agree, this functionality was submitted 6 months ago and still hasn't been merged in, but it looks like it would still be a clean merge as of this morning.

@seanlinsley
Copy link
Contributor Author

@macfanatic to be fair, the original pull request was lacking test coverage and didn't follow the codebase's conventions. Though hopefully this gets merged in soon.

gregbell added a commit that referenced this pull request Jan 15, 2013
[938] Conditional logic for filters
@gregbell gregbell merged commit 4e291e3 into activeadmin:master Jan 15, 2013
@ghost
Copy link

ghost commented Jan 27, 2013

is this in the current version? conditional filter doenst work for me

@seanlinsley
Copy link
Contributor Author

You need to use the latest version from github:

# in your Gemfile
gem 'activeadmin', github: 'gregbell/active_admin'

@seanlinsley seanlinsley deleted the 938-conditional-filters branch February 8, 2013 01:08
@lporras
Copy link
Contributor

lporras commented Feb 12, 2014

I can not use conditional filter, I'm using the master version.

@seanlinsley
Copy link
Contributor Author

@lporras can you go into more detail? Can you provide an example that isn't working?

@lporras
Copy link
Contributor

lporras commented Feb 12, 2014

ops, sorry it works, I just realize that my conditios was incorrect.

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.

4 participants