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

Change with_any_role and with_all_roles to return an AR relation instead of an Array #441

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

dkniffin
Copy link
Collaborator

@dkniffin dkniffin commented Mar 3, 2017

Fixes #292

I couldn't run the tests locally, unfortunately, so I'm hoping they run on travis.

@wldcordeiro
Copy link
Member

Rerunning travis to see what our results are.

@dkniffin
Copy link
Collaborator Author

dkniffin commented Mar 3, 2017

Ok. I might have to merge master in first.

@wldcordeiro
Copy link
Member

@dkniffin I looked over the test failures and they appear to be legit. We're getting things like

expected: [#<Customer id: 1, login: "admin">]
got: [#<Customer id: 1, login: "admin">, #<Customer id: 1, login: "admin">]

So it looks like we're adding to the list twice.

@dkniffin
Copy link
Collaborator Author

dkniffin commented Mar 3, 2017

Yea. I'll look into it 👍

@dkniffin
Copy link
Collaborator Author

dkniffin commented Mar 3, 2017

@wldcordeiro Do you know why this might be happening when I try and run the tests locally?:

> bundle exec rake spec
Alias tip: be rake spec
/Users/dkniffin/.rbenv/versions/2.2.3/bin/ruby -I/Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/lib:/Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-support-3.5.0/lib /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/exe/rspec --pattern spec/generators/\*\*/\*_spec.rb
[Coveralls] Using SimpleCov's default settings.
/Users/dkniffin/Repositories/rolify/spec/support/adapters/utils/active_record.rb:1:in `require': cannot load such file -- active_record (LoadError)
	from /Users/dkniffin/Repositories/rolify/spec/support/adapters/utils/active_record.rb:1:in `<top (required)>'
	from /Users/dkniffin/Repositories/rolify/spec/generators_helper.rb:20:in `load'
	from /Users/dkniffin/Repositories/rolify/spec/generators_helper.rb:20:in `<top (required)>'
	from /Users/dkniffin/Repositories/rolify/spec/generators/rolify/rolify_activerecord_generator_spec.rb:1:in `require'
	from /Users/dkniffin/Repositories/rolify/spec/generators/rolify/rolify_activerecord_generator_spec.rb:1:in `<top (required)>'
	from /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1435:in `load'
	from /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1435:in `block in load_spec_files'
	from /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1433:in `each'
	from /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1433:in `load_spec_files'
	from /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:100:in `setup'
	from /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:86:in `run'
	from /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:71:in `run'
	from /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:45:in `invoke'
	from /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/exe/rspec:4:in `<main>'
/Users/dkniffin/.rbenv/versions/2.2.3/bin/ruby -I/Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/lib:/Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-support-3.5.0/lib /Users/dkniffin/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.5.4/exe/rspec --pattern spec/generators/\*\*/\*_spec.rb failed

@dkniffin dkniffin changed the title Change with_any_role to return an AR relation instead of an Array Change with_any_role and with_all_roles to return an AR relation instead of an Array Mar 3, 2017
@dkniffin
Copy link
Collaborator Author

dkniffin commented Mar 3, 2017

Hmm. I'm not sure I understand why this expectation exists. Here, we set up the subject to have the roles "admin" everywhere, and "moderator" on Forum.first, right? When you run subject.with_all_roles("admin", "moderator"), is that meant to say that they have all those roles, everywhere, or all those roles, anywhere?

Hopefully that makes sense.

@wldcordeiro
Copy link
Member

wldcordeiro commented Mar 3, 2017

@dkniffin it looks to me that we're testing in the general unscoped sense so we're saying "who has both admin and moderator" roles and the user has only moderator over the Forum.first so it doesn't count as true as that is scoped.

https://github.com/RolifyCommunity/rolify/wiki/Usage#finders-methods

@dkniffin
Copy link
Collaborator Author

dkniffin commented Mar 3, 2017

Ok, makes sense. I'll update the code to accomodate that. I'm very glad there's good test cases around all this :)

@coveralls
Copy link

Coverage Status

Coverage increased (+8.9%) to 92.179% when pulling d4276aa on dkniffin:with_any_role_AR into 038f9b9 on RolifyCommunity:master.

7 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+8.9%) to 92.179% when pulling d4276aa on dkniffin:with_any_role_AR into 038f9b9 on RolifyCommunity:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+8.9%) to 92.179% when pulling d4276aa on dkniffin:with_any_role_AR into 038f9b9 on RolifyCommunity:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+8.9%) to 92.179% when pulling d4276aa on dkniffin:with_any_role_AR into 038f9b9 on RolifyCommunity:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+8.9%) to 92.179% when pulling d4276aa on dkniffin:with_any_role_AR into 038f9b9 on RolifyCommunity:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+8.9%) to 92.179% when pulling d4276aa on dkniffin:with_any_role_AR into 038f9b9 on RolifyCommunity:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+8.9%) to 92.179% when pulling d4276aa on dkniffin:with_any_role_AR into 038f9b9 on RolifyCommunity:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+8.9%) to 92.179% when pulling d4276aa on dkniffin:with_any_role_AR into 038f9b9 on RolifyCommunity:master.

@dkniffin
Copy link
Collaborator Author

dkniffin commented Mar 4, 2017

Hmm. So it's failing now with two errors. The first is a stack level too deep, which I know happens during infinite loops, but I'm not sure what would be causing that here.

For the other, I'm not familiar with mongoid at all, so if you could provide some guidance, I'd appreciate it.

@coveralls
Copy link

coveralls commented Nov 20, 2017

Coverage Status

Coverage decreased (-0.7%) to 92.525% when pulling 317b895 on dkniffin:with_any_role_AR into 7c144ad on RolifyCommunity:master.

@coveralls
Copy link

coveralls commented Nov 20, 2017

Coverage Status

Coverage decreased (-0.9%) to 92.179% when pulling 899c41c on dkniffin:with_any_role_AR into dffb0f6 on RolifyCommunity:master.

@EppO
Copy link
Member

EppO commented Feb 27, 2019

Somehow it does work on Rails 4.2 but still return an Array using Rails 5.

Loading development environment (Rails 4.2.11)
irb(main):006:0> User.with_all_roles :toto
  User Load (0.8ms)  SELECT DISTINCT "users".* FROM (SELECT "users".* FROM "users" INNER JOIN "users_roles" ON "users_roles"."user_id" = "users"."id" INNER JOIN "roles" ON "roles"."id" = "users_roles"."role_id" WHERE (((roles.name IN ('toto')) AND (roles.resource_type IS NULL) AND (roles.resource_id IS NULL)))) AS users
=> #<ActiveRecord::Relation [#<User id: 1, email: "[email protected]", created_at: "2019-02-27 18:16:18", updated_at: "2019-02-27 18:16:18">]>
Loading development environment (Rails 5.2.2)
irb(main):001:0> User.with_all_roles :toto
  User Load (0.2ms)  SELECT "users".* FROM (SELECT "users".* FROM "users" INNER JOIN "users_roles" ON "users_roles"."user_id" = "users"."id" INNER JOIN "roles" ON "roles"."id" = "users_roles"."role_id" WHERE (((roles.name IN ('toto')) AND (roles.resource_type IS NULL) AND (roles.resource_id IS NULL)))) AS users
=> [#<User id: 1, email: "[email protected]", created_at: "2019-02-27 15:52:58", updated_at: "2019-02-27 15:52:58">, #<User id: 2, email: "[email protected]", created_at: "2019-02-27 15:54:14", updated_at: "2019-02-27 15:54:14">]

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.

with_any_role(:role1, :role2) on User Object returns Array instead of ActiveRecord::Relation
5 participants