Adding support for "can?" check multiple subjects #3

cefigueiredo · 2014-02-22T22:20:33Z

The behaviour when an array of subject is given, is check if there is a permission at least on one of the given subjects.

bryanrite · 2014-02-23T19:04:59Z

spec/cancan/ability_spec.rb

+    @ability.can :update, [String, Range]
+    expect(@ability.can?(:update, ["foo", 1..3])).to be_true
+    expect(@ability.can?(:update, [123, "foo"])).to be_true
+    expect(@ability.can?(:update, 123)).to be_false


Could this be updated to check be_false against an array, for example, if you sent in an array of [123, 1.0] it should be false?

Thanks!

sure... but I will not change that line... I will add another assuring that with an array where all subjects have no permission it should be false...

bryanrite · 2014-02-23T19:06:19Z

Oh, and add a line to the changelog as well please!

bryanrite · 2014-02-23T19:52:49Z

On further inspection, can we also test this will handle nested can? declarations?

You can declare a can? like:

can? :create, @category => Project

I want to ensure we could also send that into the array:

can? :create, [{@category => Project}, Category]

bryanrite · 2014-02-23T19:53:44Z

CHANGELOG.rdoc

@@ -1,6 +1,11 @@
 Master


+1.7.1 (February 22th, 2014)


Please drop this line, just add your changelog entry under master. When a new release is cut, we'll assign it to a version.

Thanks!

codyolsen · 2014-02-28T21:58:56Z

+1 on this feature. Among other things, it's going to make testing much cleaner. @cefigueiredo are you planning on completing this?

cefigueiredo · 2014-02-28T23:06:55Z

Sure... I'm working on some problems. Maybe I change a bit how it will deal with multiple subjects... but the functionality will be implemented!

During the test to permit pass an Object as a hash I needed to change how call can? to mantain the functionality to pass any object inclusive an instance of Array as a single subject. To assure that funciontality, now we need to pass a hash containing a key :any and the array of subjects as his value. The array deals with the same objects that an usual can? does... and obviously, that hash is only necessary when checking multiple subjects.

cefigueiredo · 2014-03-02T02:45:24Z

To ensure that can? can receive the same objects and in the same way a can? with single subject does; I changed how to pass the multple subjects.
To pass an array of subjects, now you need to pass them in a hash as the value of the key :any

can? :read, { :any => [{@category => Project}, Category] }

bryanrite · 2014-03-02T04:37:59Z

I like that! We can later add :all => [...] to match all rules.

cefigueiredo · 2014-03-02T05:01:45Z

Exactly what I thought! I would do that on that commit, but I saw that it was more complex than :any to test and implement.

codyolsen · 2014-03-03T16:33:47Z

@cefigueiredo This is great! Thanks for your contribution! I can't wait to throw this into my tests! @bryanrite is the plan to merge this or should I use @cefigueiredo's fork for the time being?

bryanrite · 2014-03-05T17:35:47Z

Sorry, didn't realize this was ready.

@cefigueiredo, @codyolsen I'm going to suggest a couple things to this PR to clean-up the code, afterwards I'll merge it in.

bryanrite · 2014-03-05T17:36:55Z

lib/cancan/ability.rb

+        end
+        matches.compact.first
+      else  
+        relevant_rules_for_match(action, subject).detect do |rule|


Can we extract the relevant_rules_for_match block into a single method so its not duplicated.

codyolsen · 2014-03-06T00:02:10Z

@bryanrite 10-4. Sounds like a plan.

The method can? was changed and does not pass an array of subject anymore to matches_subject? since last commit. So, I'm removing the method matches_subject_any? that only existed to deal with that array of subjects.

The method can? became a bit messy and repetitive dealing chosing paths almost identical to single and multiple subjects, when only the difference was iterate on the array of subjects. So, I created the method extract_subject that independently if given a single or multiple subjects, it will returns an array of subjects. So, for can? does not matter anymore if subject is single or multiple, it will deal on the same way.

bryanrite · 2014-03-12T16:32:58Z

Sorry for the delay on this, just been busy, I'll get this merged in, and try to add any functionality as well.

ekampp · 2014-04-10T12:06:16Z

This has been closed, not merged. State?

bryanrite · 2014-04-10T16:16:05Z

@ekampp The referenced PR was closed. This PR is still open. It is valid, I have just not had a ton of time to integrate it lately, as it will require a new minor version. I'll try and get at it in the next day or two.

ekampp · 2014-04-11T05:12:03Z

Ohh.. Never mind, then. I'm tiered 😫

cefigueiredo · 2014-04-11T12:19:55Z

Are there something I could do to make it easier?

On Fri, Apr 11, 2014 at 2:12 AM, Emil Kampp notifications@github.comwrote:

Ohh.. Never mind, then. I'm tiered [image: 😫]

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3#issuecomment-40171350
.

Sync with Master repository

bryanrite · 2014-04-30T22:15:29Z

Thanks! Sorry it took so long, but I had to spend some time refactoring all the tests before I wanted to add more features. Travis says failing but its due to their latest update and not code, so you can ignore.

author Remo Fritzsche <remo.fritzsche@sitrox.com> 1612868567 +0100 committer Alessandro Rodi <coorasse@gmail.com> 1657114188 +0200 # This is a combination of 4 commits. # This is the 1st commit message: Preserve nil values as extra arguments # This is the commit message #2: Fix link (#744) # This is the commit message #3: Documentation fixes and improvements - fix typo, punctuations, and grammar - fix broken and misformatted links - fix code sample # This is the commit message #4: Format documentation/guides and fix linting issues The motivation of the changes is to make these documents neater when reading offline using a text or code editor. Reading the documentation offline is faster, and the source code is readily available for further learning. The changes do not affect the meaning of each documentation or instruction. The following changes were made: - remove extra and trailing spaces - add new lines to separate the title, explanation, code samples, etc - fix headings, links - specify code block language (bash, ruby) Updated Devise.md as exception is changed CanCan::Unathorized exception not exists anymore, CanCan::AccessDenied is used. Also added responses for different content types as a recommendation. Add support for non-hash conditions Co-authored-by: Juleffel <juleffel@protonmail.com> Improve ability checks with Hashes Add Ruby 3.0 and Ruby 3.1 to CI Drop ActiveRecord4 from CI (#778)

Do not run rails >= 7 on ruby < 3

* Update gitignore * This is the commit message #3: Do not run rails >= 7 on ruby < 3 * Remove jruby and truffleruby * Fix rubocop * Use rspec instead of rake

Adding support for "can?" check multiple subjects

d454cae

The behaviour when an array of subject is given, is check if there is a permission at least on one of the given subjects.

bryanrite reviewed Feb 23, 2014
View reviewed changes

cefigueiredo added 2 commits February 23, 2014 16:11

Assuring that with both subjects without permission can? returns false

10e9cdb

Adjust CHANGELOG

03d42e8

bryanrite reviewed Feb 23, 2014
View reviewed changes

bryanrite reviewed Mar 5, 2014
View reviewed changes

cefigueiredo added 2 commits March 7, 2014 00:02

Removing unused method matches_subject_any?

f7e2f02

The method can? was changed and does not pass an array of subject anymore to matches_subject? since last commit. So, I'm removing the method matches_subject_any? that only existed to deal with that array of subjects.

bryanrite mentioned this pull request Mar 12, 2014

authorize_resource with multiple classes(Models) ? #18

Closed

cefigueiredo added 2 commits April 13, 2014 19:53

Merge pull request #1 from CanCanCommunity/master

85daa93

Sync with Master repository

Merge branch 'master' into can_check_multiple_subjects

e418428

bryanrite closed this in 8d4279d Apr 30, 2014

coorasse added a commit that referenced this pull request May 23, 2024

This is the commit message #3:

c00f7d0

Do not run rails >= 7 on ruby < 3

coorasse added a commit that referenced this pull request May 23, 2024

This is the commit message #3:

fb1a506

Do not run rails >= 7 on ruby < 3

coorasse added a commit that referenced this pull request May 23, 2024

This is the commit message #3:

2482f51

Do not run rails >= 7 on ruby < 3

coorasse added a commit that referenced this pull request May 23, 2024

Update the tests matrix (#849)

d4ef85a

* Update gitignore * This is the commit message #3: Do not run rails >= 7 on ruby < 3 * Remove jruby and truffleruby * Fix rubocop * Use rspec instead of rake

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for "can?" check multiple subjects #3

Adding support for "can?" check multiple subjects #3

cefigueiredo commented Feb 22, 2014

bryanrite Feb 23, 2014

cefigueiredo Feb 23, 2014

bryanrite commented Feb 23, 2014

bryanrite commented Feb 23, 2014

bryanrite Feb 23, 2014

codyolsen commented Feb 28, 2014

cefigueiredo commented Feb 28, 2014

cefigueiredo commented Mar 2, 2014

bryanrite commented Mar 2, 2014

cefigueiredo commented Mar 2, 2014

codyolsen commented Mar 3, 2014

bryanrite commented Mar 5, 2014

bryanrite Mar 5, 2014

codyolsen commented Mar 6, 2014

bryanrite commented Mar 12, 2014

ekampp commented Apr 10, 2014

bryanrite commented Apr 10, 2014

ekampp commented Apr 11, 2014

cefigueiredo commented Apr 11, 2014

bryanrite commented Apr 30, 2014

Adding support for "can?" check multiple subjects #3

Adding support for "can?" check multiple subjects #3

Conversation

cefigueiredo commented Feb 22, 2014

bryanrite Feb 23, 2014

Choose a reason for hiding this comment

cefigueiredo Feb 23, 2014

Choose a reason for hiding this comment

bryanrite commented Feb 23, 2014

bryanrite commented Feb 23, 2014

bryanrite Feb 23, 2014

Choose a reason for hiding this comment

codyolsen commented Feb 28, 2014

cefigueiredo commented Feb 28, 2014

cefigueiredo commented Mar 2, 2014

bryanrite commented Mar 2, 2014

cefigueiredo commented Mar 2, 2014

codyolsen commented Mar 3, 2014

bryanrite commented Mar 5, 2014

bryanrite Mar 5, 2014

Choose a reason for hiding this comment

codyolsen commented Mar 6, 2014

bryanrite commented Mar 12, 2014

ekampp commented Apr 10, 2014

bryanrite commented Apr 10, 2014

ekampp commented Apr 11, 2014

cefigueiredo commented Apr 11, 2014

bryanrite commented Apr 30, 2014