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
Support passing options to a rule condition #30
Support passing options to a rule condition #30
Conversation
This is a great addition. @vadimdemedes I think you should review this for sure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, this feature can be useful.
test.js
Outdated
const cancan = new CanCan(); | ||
const {can, allow} = cancan; | ||
|
||
allow(User, 'update', User, (user, target, payload) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clean this up? It's hard to follow.
test.js
Outdated
@@ -192,3 +192,24 @@ test('override instanceOf', t => { | |||
t.false(cannot(user, 'read', product)); | |||
t.false(can(user, 'create', product)); | |||
}); | |||
|
|||
test('allow can depend of an optional payload', t => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
=> "pass options to the rule"
readme.md
Outdated
@@ -126,11 +132,24 @@ const post = new Post(); | |||
can(user, 'view', post); | |||
``` | |||
|
|||
### cannot(instance, action, target) | |||
```js | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Examples in readme should only contain code. If there are comments, they should be in text before that example.
readme.md
Outdated
const admin = new User({roles: ['administrator']}); | ||
const moderator = new User({roles: ['moderator']}); | ||
|
||
can(admin, 'update', moderator, {fields: ["roles"]}); // true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use single quotes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And follow this format:
can(...);
//=> return value
readme.md
Outdated
|
||
Inverse of `.can()`. | ||
|
||
### authorize(instance, action, target) | ||
### authorize(instance, action, target[, payload]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
payload
=> options
.
index.js
Outdated
@@ -44,7 +44,7 @@ class CanCan { | |||
}); | |||
} | |||
|
|||
can(performer, action, target) { | |||
can(performer, action, target, payload) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
payload
=> options
- rename 'payload' parameter to 'options' - reformat comments from example code in readme.md - use single quotes - rewrite allow rule in "pass options to the rule" test
Hi @vadimdemedes ! |
Thanks for working on this! |
Appends an optional payload parameter on .can() method and condition callback passed to .allow() method.
It makes it possible to check additional informations when it's time to decide if an action can be performed or not. For instance, on an update action, permissions can now be granted according to updated fields, as shown on the updated readme.