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 passing stringy booleans as scope args #460

Merged

Conversation

shekibobo
Copy link
Contributor

This implements the solution discussed in #403.

Fixes #403.

@shekibobo shekibobo force-pushed the fix-scope-boolean-values branch from a746330 to 0a2d956 Compare October 30, 2014 18:22
@jonatack
Copy link
Contributor

Thanks, @shekibobo! Would you/could you add tests for this and possibly update the change log?

@shekibobo
Copy link
Contributor Author

Probably. I didn't really see any tests for the scoping feature, so I wasn't quite sure where to add it.

@shekibobo
Copy link
Contributor Author

Trying to make this happen:

it "applies stringy boolean scopes with boolean values in an array" do
  search = Person.search('true_or_false' => ['true'])
  search.result.to_sql.should include "true_or_false = 1"
end

But it fails with:

1) Ransack::Adapters::ActiveRecord::Base#search with scopes applies stringy boolean scopes with boolean values in an array
     Failure/Error: search.result.to_sql.should include "true_or_false = 1"
       expected "SELECT \"people\".* FROM \"people\"  ORDER BY \"people\".\"id\" DESC" to include "true_or_false = 1"
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-expectations-2.14.5/lib/rspec/expectations/fail_with.rb:32:in `fail_with'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-expectations-2.14.5/lib/rspec/expectations/handler.rb:34:in `handle_matcher'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-expectations-2.14.5/lib/rspec/expectations/syntax.rb:53:in `should'
     # ./spec/ransack/adapters/active_record/base_spec.rb:38:in `block (4 levels) in <module:ActiveRecord>'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example.rb:114:in `instance_eval'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example.rb:114:in `block in run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example.rb:254:in `with_around_each_hooks'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example.rb:111:in `run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example_group.rb:390:in `block in run_examples'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example_group.rb:386:in `map'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example_group.rb:386:in `run_examples'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example_group.rb:371:in `run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example_group.rb:372:in `block in run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example_group.rb:372:in `map'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example_group.rb:372:in `run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example_group.rb:372:in `block in run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example_group.rb:372:in `map'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/example_group.rb:372:in `run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/command_line.rb:28:in `block (2 levels) in run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/command_line.rb:28:in `map'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/command_line.rb:28:in `block in run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/reporter.rb:58:in `report'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/command_line.rb:25:in `run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/runner.rb:80:in `run'
     # /Users/joshuakovach/.gem/ruby/2.1.4/gems/rspec-core-2.14.8/lib/rspec/core/runner.rb:17:in `block in autorun'

I've tested it, and it appears to work correctly on my machine, but I'm not sure what I might be missing on that one.

@shekibobo shekibobo force-pushed the fix-scope-boolean-values branch from 3b4bfe6 to 93033f7 Compare October 30, 2014 20:43
@jonatack
Copy link
Contributor

Thanks!

The new feature of detecting attribute names containing _and_ or _or_ is not working yet (change log and release notes updated today to mention that), which may be why that test using true_or_false isn't passing; Ransack is not seeing the attribute and creating the search query.

Thanks for removing the whitespace in the change log. Ransack v.1.5.1 is already released, so this PR should go in a new Unreleased section of the change log at the top. Don't worry about it though, if that isn't clear.

It's late, so I'll have another look at this tomorrow.

@shekibobo shekibobo force-pushed the fix-scope-boolean-values branch from 93033f7 to 314e28f Compare October 31, 2014 05:36
@shekibobo
Copy link
Contributor Author

Should I include those specs as pending?

@shekibobo
Copy link
Contributor Author

Also, I'm unsure what SQL clause we should be checking for. Is it true_or_false = 1, true_or_false = 't', true_or_false, NOT(true_or_false).... I could go on, but I'm not sure what exactly we should be looking for in these tests?

@shekibobo
Copy link
Contributor Author

Hey, it all actually works now! 😹

Specs for boolean scope arguments being passed in an array work, and scopes work if boolean arguments are passed as strings.

@shekibobo
Copy link
Contributor Author

Woo! Passing on all builds!

jonatack added a commit that referenced this pull request Oct 31, 2014
Allow passing stringy booleans as scope args. Fixes #403.
@jonatack jonatack merged commit 69d13b2 into activerecord-hackery:master Oct 31, 2014
@jonatack
Copy link
Contributor

Looks pretty good. Let's give it a try.

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.

Recent scopes addition broke #search(params[:q]) API
2 participants