-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[1685] Move from Metasearch to Ransack #1979
Conversation
@daxter - thanks for the tip on the branch name. |
how's this going Daxter? I'm creating a new rails 4 app and it's hard to not have my trusty AA at the ready. Is there much more work to do here? After this, do you have any idea how much more work is needed to get to rails 4? cheers |
@jackdempsey I haven't tried working on this for a while now, but it shouldn't take too much longer. As far as I'm aware, meta_search is the only thing in the way of Rails 4 compatibility. |
There's this fork that might be a good starting point to solving this. |
thx jasper. I'm somewhat inclined to put some $ behind this desire using https://www.bountysource.com |
Haha, I was going to do it nonetheless. Hence this PR. :) |
:-) Do you have any sense how hard it'll be to switch to Ransack? If it's a Course switching multiple things at the same time (rails 4 AND ransack) On Tue, Apr 2, 2013 at 9:33 AM, Sean Ian Linsley
|
It's mostly plug-and-play for Ransack, but there's one unresolved issue (a la #1811) that needs to be resolved. I can't be sure about Rails 4, but Ransack should be integrated this week. |
Did that really just happen?
Oh man. That's what I get for being a nonconformist. |
Everything is working now, except the index page scenario for AA Comments. As in: ActiveAdmin.register ActiveAdmin::Comment, as: 'Comment' do
# ...
end The basic problem is the polymorphic association between comments and your models are getting corrupted (not sure where it's happening). I'm attempting to re-use the Issue that @pavelbrylov opened earlier: activerecord-hackery/ransack#173 |
I've got this branch added to the Gemfile, but bundle returns:
Any ideas? |
@lupinglade looks like you edited your comment after the fact. Earlier you were asking how to get Bundler to recognize the unusual branch name, and just FYI you can get around that by specifying a commit ID instead. How exactly do you have it set up now? I could see you getting that error if you're using a local copy of Active Admin instead of using Bundler. |
I have it wrapped in escaped quotes and that gets the right branch, but running bundle install gives the error above. I've also tried separately checking out the branch and running bundle on the source and the same error occurs. This is with rails 4.0.0.beta1, rvm head, ruby 2.0.0 Here's what I have in the Gemfile: gem 'activeadmin', github: "gregbell/active_admin", branch: ""metasearch->ransack"" I've tried with the commit ID and the same thing happens (bundle error as above). |
I don't understand where activeadmin >= 0 depends on activerecord (~> 3.0)... I've looked at the value of the detect_rails_version method and it returns 4.0.0.beta1 properly. It seems its a problem with the ransack gem and maybe some other dependencies still being stuck with rails 3? |
Bundler is being a bit inaccurate in its error message. The real problem is that Ransack specifies Rails 3.x
There's an open PR to fix this. Let me know if it works for you. gem 'ransack', github: 'avit/ransack', branch: 'rails4-dependencies' |
That fixed it, thank you! |
Strange, now I am getting another error:
|
I could get past it by marking out that include ::Sass::Rails::Helpers, but then the next error is:
Even if I add protected_attributes gem to the Gemfile (and bundle install), that same error comes up. How do you have this working? :-/ |
Nevermind, I got it. You're doing the migration on rails 3 and I'm testing on rails 4 :) |
How exactly did you get around that error? |
Do you mean the one regarding attr_accessible? I haven't yet, but I am looking into it. |
Any idea why the Sass module is uninitialized? If I can fix that maybe I can get the rest of it working on rails 4. |
To fix that attr_accessible error, we need to remove the attr_accessible definitions in comment.rb and then add a strong parameters definition to the controller responsible for the comments. Something along the lines of: private
def comment_params
params.require(:resource).permit(:resource_id, :resource_type, :body, :namespace)
end Instead of this in comment.rb attr_accessible :resource, :resource_id, :resource_type, :body, :namespace Then the create implementation should be something like: def create
@comment = Comment.new(comment_params)
# ...
end Just not sure where to place this since it looks like the controller is dynamically generated. I am thinking lib/active_admin/comments.rb? |
Is there no way to support either or, depending on whether the app is using Rails 4? |
Well, requiring the 'protected_attributes' gem should work at least until rails 5. It did not seem to work if I add it to the project Gemfile, so I think it needs to be done in active_admin itself. Its the only place that its used though from what I can tell, so it might be worthwhile to switch to strong_parameters dynamically based on what version of rails active_admin is running on? Not sure which is better. |
If we can get the Sass issue fixed, I can check to see what other things we might run into on Rails 4. |
@lupinglade could you please discuss Rails 4 compatibility in #1963? This really isn't the place for it. |
Could you elaborate? If you specify this branch in your Gemfile, it should just work. gem 'activeadmin', github: 'gregbell/active_admin', branch: '"metasearch->ransack"' If you're trying to use Rails 4, you should check out #1963. |
Ah, I had missed putting the double quotes inside the single. That said, I still get the following:
Yeah, obv using rails 4 - thought this had been merged in with the other |
Last time I checked no one's paying me to do this, so you shouldn't expect me to have all the answers. As it so happens @lupinglade already mentioned that problem and I ran down the solution. Though that appears to be out of date |
Chill, dude, I obviously wasn't clear - when I said "thought this had been merged in with the other" I was meaning that I had misunderstood, thinking the two threads/pull reqs were more related than they were. Using the other thread you referenced, I got it to work by doing the following:
|
Yeah my bad. I was getting annoyed because the answer was right there. But I guess if I happened across long threads like these I wouldn't actually read them unless absolutely necessary. I updated the description text for this PR to be more useful. |
Huh. Apparently with Ransack you can change between -- User.search(username_cont: 'jim%').result.count
SELECT COUNT(*) FROM "users" WHERE ("users"."username" LIKE '%jim\%%')
-- User.search(username_cont: 'jim').result.count
SELECT COUNT(*) FROM "users" WHERE ("users"."username" LIKE '%jim%')
-- User.search(username_matches: 'jim%').result.count
SELECT COUNT(*) FROM "users" WHERE ("users"."username" LIKE 'jim%') |
I'm currently remaking this PR to catch it up with master. I should have it working today, but for now you can always refer to the old commit hash: d3aa5a2 |
I have no idea why Travis won't correctly resolve the Bundler dependencies for Rails 4:
It works for me locally |
I ran into this problem because of meta_search still being included in the gemspec. |
Ahh... of course. I don't know how that escaped me. |
Conflicts: activeadmin.gemspec undoes 1d29474, to apply the full fix
#### Removed outdated version-checking conditions Modules affected: + CSV parser + asset registration + class reloading in development #### Removed 99% of our custom class reloader in favor of Rails reloading Note: I removed the code that purges `app/admin` from Rails' `eager_load_paths` since there were no bad effects of doing so. Only the code that does this for Rails' `autoload_paths` seems necessary. #### Gemspec dependencies updated + fastercsv removed + coffee-rails added + sass replaced by sass-rails
Adds conditionals to the admin user generator, and to the AA Comments model & controller to handle both Protected Attributes and Strong Paramaters Also updates dependencies so the test Rails 4 app will at least bundle
Note that while Ransack renames many query methods ("predicates"), this commit creates aliases for the more plain-english predicates so they still work as expected. To see the full list of Ransack predicates, check out `lib/ransack/constants.rb` in the Ransack source code. This commit also removes polymorphic foreign types from the default list of filters for a resource, since Ransack seems to barf on them. We don't have a good filter UI for them anyway, so it's not a huge issue.
Are there any examples that show how to update the old Metasearch search_method approach for filters Specifically, trying to upgrade something like this so that it can work with ransack: In Active Admin resource: filter :month, :as => :select, :collection => (1..12) In model: scope :month_eq, lambda{ |month| where("strftime('%m', birthday) + 0 = ?", month.to_i) }
search_methods :month_eq How would one do this with ransack? Example stolen from: http://stackoverflow.com/a/12045146/2308991 Thanks! |
The documentation for Ransack is terrible. The best resource is discussions that people have had on the Issue tracker like this: activerecord-hackery/ransack#36 I'll try to write something up for this. |
@boontdustie , did you find solution ? |
@Fivell - Sorry, no. We ended up just going a different route for the filters that doesn't involve ransack. |
I just figured it out: # The scope version
scope :month_eq, lambda{ |month| where("strftime('%m', created_at) + 0 = ?", month.to_i) }
# the Ransack version
ransacker :month, formatter: proc(&:to_i) do |parent|
Arel::Nodes::SqlLiteral.new "strftime('%m', #{parent.table_name}.created_at) + 0"
end
# So both of these work:
Post.month_eq(8)
Post.search(month_eq: 8).result
# So this AA filter should now work as expected:
filter :month, :as => :select, :collection => (1..12) It bugs me that this causes code duplication... UPDATE: Hmm, looks like using that as a filter raises an odd error. The error only happens with equals, not greater or lesser than. See http://stackoverflow.com/questions/12605518 for more info |
@daxter , you are right, almost no docs for ransacker....Can't figure out how to rewrite simple module to use with AA and rails4 module CurrencyEqScopes
def self.included(base)
base.scope :currency_id_eq, lambda { |currency|
base.joins(:currency).where("currencies.id = ?", currency)
}
base.send :search_methods, :currency_id_eq
end
end |
#2326 includes the code from this PR, so I'm closing this. |
And now #2326 has been merged into master :D |
I think you should use filter with predicate like this: # /app/models/project.rb
class Project < ActiveRecord::Base
ransacker :month, formatter: proc(&:to_i) do |parent|
Arel::Nodes::SqlLiteral.new "strftime('%m', #{parent.table_name}.created_at) + 0"
end
end
# /app/admin/projects.rb
ActiveAdmin.register Project do
filter :month_eq, as: :select, collection: (1..12)
end If you run it like |
Is that really all that was needed? xD |
This PR is for #1685, and is a continuation of the work @pavelbrylov did in #1811.
BTW
'"metasearch->ransack"'
Using this with Rails 4
This doesn't currently work 100% with RaIls 4, but it's close. You need to specify these gems: