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

Mongoid 4.0 support #407

Merged
merged 38 commits into from
Nov 3, 2014
Merged

Mongoid 4.0 support #407

merged 38 commits into from
Nov 3, 2014

Conversation

Zhomart
Copy link
Contributor

@Zhomart Zhomart commented Aug 4, 2014

issue #120

Hi, I've added support for mongoid, but without associations.

Here what I've done:

  • active_record, arel codes moved into adapters/active_record
    • ransack classes that hardly depend on AR/Arel are moved inside adapters/active_record/ransack
  • mongoid codes are inside adapters/mongoid
  • created Attribute class for mongoid that imitates Arel::Attributes::Attribute
  • all existing ransack specs remained, added specs for mongoid
  • disabled joins for mongoid. there are no specs for mongoid/joins
  • added rake tasks
    • $ rake runs active_record specs. default
    • $ DB=mongodb rake runs mongoid specs

@Zhomart Zhomart mentioned this pull request Aug 4, 2014
@jonatack
Copy link
Contributor

@Zhomart sorry for replying so slowly and thanks for the PR. This is impressive. Will review soon.

@Zhomart
Copy link
Contributor Author

Zhomart commented Oct 6, 2014

I've created demo app http://ransack-mongodb-demo.herokuapp.com/

@maran
Copy link

maran commented Oct 7, 2014

I've successfully implemented some basic searching into my app using some simple field_cont and collection select search filters. Worked flawlessly for me.

@jonatack
Copy link
Contributor

Thanks, guys.

@max-konin
Copy link

Hi, Does it support search by embedded document?
I have:

class Band
  include Mongoid::Document
  embeds_one :label
end

class Label
  include Mongoid::Document
  field :name, type: String
  embedded_in :band
end 

Can I find band with specific label.name?

@Zhomart
Copy link
Contributor Author

Zhomart commented Oct 17, 2014

No, this patch was quick solution. But I will add it if I have time.

@maran
Copy link

maran commented Oct 31, 2014

@jonatack Is anything else blocking the merge of this PR. Anything we can help with?

@jonatack
Copy link
Contributor

@maran just a question of time (there are many changes to review in this PR), and getting some urgent regressions in Ransack fixed first and improving the gem's test coverage, because missing tests were the reason why the regression occured. This looks like an interesting PR that is super worthy, and I'm not against merging it in if it doesn't degrade Ransack performance too much and won't be too difficult to maintain.

* master:
  Patch Rails commit c1a118a
  Remove extra space [skip ci]
  Use newer Ruby hash syntax [skip ci]
  Update change log [skip ci]
  Wrap 80 characters [skip ci]
@sbounmy
Copy link

sbounmy commented Nov 2, 2014

@Zhomart I just gave a try with mongoid 4 a simple query on id field does not work :

Category.search(:_id_in => ['544d26a44d61630b5c070000']).result
=> #<Mongoid::Criteria
  selector: {}
  options:  {}
  class:    Category
  embedded: false>

Category.search(:_id_in => ['544d26a44d61630b5c070000']).result
=> #<Mongoid::Criteria
  selector: {}
  options:  {}
  class:    Category
  embedded: false>

Category.search(:id_in => ['544d26a44d61630b5c070000']).result
D, [2014-11-02T17:35:41.787651 #12628] DEBUG -- :   MOPED: 127.0.0.1:27017 COMMAND      database=food_development command={:count=>"categories", :query=>{}} runtime: 0.4760ms
D, [2014-11-02T17:35:41.788500 #12628] DEBUG -- :   MOPED: 127.0.0.1:27017 COMMAND      database=food_development command={:count=>"categories", :query=>{}} runtime: 0.3480ms
NoMethodError: undefined method `type' for nil:NilClass

Tested with a field name it works :

irb(main):041:0> Category.search(:name_in => 'Foo').result
D, [2014-11-02T17:37:14.124320 #12628] DEBUG -- :   MOPED: 127.0.0.1:27017 COMMAND      database=food_development command={:count=>"categories", :query=>{}} runtime: 0.3820ms
D, [2014-11-02T17:37:14.125242 #12628] DEBUG -- :   MOPED: 127.0.0.1:27017 COMMAND      database=food_development command={:count=>"categories", :query=>{}} runtime: 0.3090ms
D, [2014-11-02T17:37:14.126129 #12628] DEBUG -- :   MOPED: 127.0.0.1:27017 COMMAND      database=food_development command={:count=>"categories", :query=>{}} runtime: 0.3080ms
D, [2014-11-02T17:37:14.126711 #12628] DEBUG -- :   MOPED: 127.0.0.1:27017 COMMAND      database=food_development command={:count=>"categories", :query=>{}} runtime: 0.2330ms
D, [2014-11-02T17:37:14.127326 #12628] DEBUG -- :   MOPED: 127.0.0.1:27017 COMMAND      database=food_development command={:count=>"categories", :query=>{}} runtime: 0.2370ms
=> #<Mongoid::Criteria
  selector: {"name"=>{"$in"=>["Foo"]}}
  options:  {}
  class:    Category
  embedded: false>

@Zhomart
Copy link
Contributor Author

Zhomart commented Nov 2, 2014

@sbounmy, it must work now. Category.search(:id_in => ['544d26a44d61630b5c070000']).result

@sbounmy
Copy link

sbounmy commented Nov 3, 2014

thanks a lot @Zhomart , can't wait to get this merged

jonatack added a commit that referenced this pull request Nov 3, 2014
@jonatack jonatack merged commit 438fe2f into activerecord-hackery:master Nov 3, 2014
@jonatack
Copy link
Contributor

jonatack commented Nov 3, 2014

Let's give this a shot.

@Zhomart, thank you for your persistence with this feature. Before releasing it in a new version of Ransack, could you update the documentation (README, wiki) where needed, and the change log?

Everyone, please try using Ransack master and let us know if any issues.

@vanboom
Copy link

vanboom commented Nov 10, 2014

Thanks for implementing Mongoid support... I am finding some issues

  1. The "not null" or "is null" predicates do not seem to work.
  2. Ransack doesn't handle searching for embedded documents.

I'll get into the code and see if I can contribute!

@jonatack
Copy link
Contributor

@vanboom thanks for the feedback, this is exactly what we need before releasing this feature in the official gem. Any contributions would be great!

@vanboom
Copy link

vanboom commented Nov 11, 2014

Suggestion: make dynamic fields searchable. If I return the name of my dynamic fields from ransackable_attributes, it works... is there an option to allow all fields to pass? vs. having to specify the dynamic fields individually (which is not possible)

I think having a way to pass all fields will enable Ransack searches for dynamic fields...which would be pretty darn cool imho.

Thanks - sorry I haven't been able to figure this one out yet...

@pencilcheck
Copy link
Contributor

Mongoid stores its relational and embedded attributes in class arrays, I wonder if we can take advantage of that somehow ... And if the only blocker from getting it done is join spec, but not any technical difficulties, someone knowledgeable should come up with a more detail join spec. Mongoid does some denormalization internally I believe.

This pull request was closed.
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.

7 participants