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

Allow Active Record Ransack aliases with the same name on different models #717

Merged
merged 2 commits into from
Aug 31, 2016
Merged

Allow Active Record Ransack aliases with the same name on different models #717

merged 2 commits into from
Aug 31, 2016

Conversation

laserlemon
Copy link
Contributor

This fixes two bugs:

  1. Subclasses were not properly inheriting their parents' Ransack
    aliases in Mongoid because each class defined its very own set of
    aliases.
  2. In Active Record, Ransack aliases were defined in such a way that the
    parent's (and grandparent's, etc.) aliases were overwritten by the
    child, meaning that all aliases were ultimately kept on
    ActiveRecord::Base. This has the unfortunate effect of enforcing
    uniqueness of Ransack alias names across all models rather than per
    model. Depending on the load order of models, earlier definitions of
    an alias in other models would be clobbered.

This fixes two bugs:

1. Subclasses were not properly inheriting their parents' Ransack
   aliases in Mongoid because each class defined its very own set of
   aliases.
2. In Active Record, Ransack aliases were defined in such a way that the
   parent's (and grandparent's, etc.) aliases were overwritten by the
   child, meaning that all aliases were ultimately kept on
   ActiveRecord::Base. This has the unfortunate effect of enforcing
   uniqueness of Ransack alias names across all models rather than per
   model. Depending on the load order of models, earlier definitions of
   an alias in other models would be clobbered.
@laserlemon
Copy link
Contributor Author

laserlemon commented Aug 26, 2016

There's more background as to why this fix works in Rails' documentation of class_attribute (as of version 5.0.0.1).

@laserlemon laserlemon changed the title Ransack alias name collision Allow Active Record Ransack aliases with the same name on different models Aug 26, 2016
@@ -23,7 +23,8 @@ def ransacker(name, opts = {}, &block)
end

def ransack_alias(new_name, old_name)
self._ransack_aliases.store(new_name.to_s, old_name.to_s)
self._ransack_aliases = _ransack_aliases.merge new_name.to_s =>
Copy link
Contributor Author

@laserlemon laserlemon Aug 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth noting that this is exactly how the ransacker method already works.

@laserlemon
Copy link
Contributor Author

💚 ❕

@jonatack jonatack merged commit 2ec5e86 into activerecord-hackery:master Aug 31, 2016
@jonatack
Copy link
Contributor

jonatack commented Aug 31, 2016

Thanks @laserlemon ❤️ 💚 💙 💛 💜

jonatack added a commit that referenced this pull request Sep 4, 2016
prasadsurase pushed a commit to prasadsurase/ransack that referenced this pull request Sep 22, 2016
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