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

Ability.concat #42

Closed
stalniy opened this issue Mar 13, 2018 · 1 comment
Closed

Ability.concat #42

stalniy opened this issue Mar 13, 2018 · 1 comment
Labels
status: define requirements Need to define requirements and ensure that feature is useful status: invalid

Comments

@stalniy
Copy link
Owner

stalniy commented Mar 13, 2018

Create possibility to concatenate 2 and more abilities together. Can be useful for cases when user want to define separate ability instances for different purposes (e.g., feature flags checks, hardware checks, user permission checks). The resulting Ability instance is composed of n Ability instances and its can method returns true only if all underlying abilities returns true.

Only Ability with the same options (i.e., subjectName) can be concatenated together, otherwise exception should be thrown

const ability = AbilityBuilder.define(can => {
  can('read', 'Book')
})

const anotherAbility = Ability.define(can => {
  cannot('read', 'Book', { private: true })
})

const userAbility = ability.concat(anotherAbility)

userAbility.can('read', book) // if ability.can('read', book) && userAbility.can('read', book)
userAbility.rules 
/*
[
  [{ actions: 'read', subject: 'Book' }], 
  [{ actions: 'read', subject: 'Book', conditions: { private: true } }]\
]
*/
@stalniy stalniy added this to the 2.1 milestone Mar 13, 2018
@stalniy stalniy added the status: define requirements Need to define requirements and ensure that feature is useful label Mar 13, 2018
@stalniy
Copy link
Owner Author

stalniy commented Mar 23, 2018

The main issue with the idea is that for some cases we would like to define not all actions or not all subjects.

Lets consider an example with hardware capabilities and user permissions. For example, there are devices which supports wifi and which not. So, we define ability like this:

const hardwareAbility = AbilityBuilder.define(can => {
  if (hardware.supportsWiFi) {
     can('manage', 'WiFi')
  }
})

const userAbility = AbilityBuilder.define(can => {
  if (user.is('admin')) {
    can('manage', 'WiFi')
    can(['read', 'update'], 'User', { id: user.id })
  }
})

const ability = hardwareAbility.concat(userAbility)

Now when we do ability.can('read', 'Wifi') all works as expected but if we do ability.can('read', 'User') it will return false because hardware ability doesn't have rules for User (and shouldn't have them).

So, I decline this feature in favor of #45 . With conditional rules it may look like this:

const userAbility = AbilityBuilder.define(can => {
  if (user.is('admin')) {
    can('manage', 'WiFi').onlyIf(() => hardware.supportsWiFi, { error: 'This device does not have WiFi card' })
    can(['read', 'update'], 'User', { id: user.id })
  }
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: define requirements Need to define requirements and ensure that feature is useful status: invalid
Projects
None yet
Development

No branches or pull requests

1 participant