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.merge #43

Closed
stalniy opened this issue Mar 13, 2018 · 3 comments
Closed

Ability.merge #43

stalniy opened this issue Mar 13, 2018 · 3 comments
Labels
casl/ability status: define requirements Need to define requirements and ensure that feature is useful

Comments

@stalniy
Copy link
Owner

stalniy commented Mar 13, 2018

Allows to merge 2 and more Ability instances into. Only Ability instances with the same options can be merged together, otherwise exception should be thrown.

This feature will allow to merge rules of few abilities, basically this is a helper method for doing the next thing:

ability.update(ability.rules.concat(anotherAbility.rules))

This may be useful if you want to define some rules dynamically in the database and some statically in the code

Design:

Merged abilities are added at the end

const hardwareAbility = AbilityBuilder.define(can => {
   ...
})

const userAbility = AbilityBuilder.define(can => {
  ...
})

const ability = userAbility.merge(hardwareAbility)

In this case, hardwareAbility may enforce/overwrite any userAbility. ability is modified and hardwareAbility is not changed, update and updated events are emitted on Ability instance.

In case userAbility and hardwareAbility has different options for subject detection (i.e., subjectName) Ability.merge should throw an error telling that merged abilities are not compatible

@stalniy stalniy added the status: define requirements Need to define requirements and ensure that feature is useful label Mar 13, 2018
@stalniy stalniy added this to the 2.1 milestone Mar 13, 2018
@stalniy stalniy added enhancement and removed status: define requirements Need to define requirements and ensure that feature is useful labels Apr 16, 2018
@stalniy stalniy removed this from the 2.1 milestone Apr 19, 2018
@stalniy stalniy added status: define requirements Need to define requirements and ensure that feature is useful and removed enhancement labels Apr 19, 2018
@stalniy
Copy link
Owner Author

stalniy commented Apr 19, 2018

After some thinking and writing code I decided that such function just adds complexity and requires to answer on additional questions, like:

  1. When we merge abilities how should be they merged? At the end of existing Ability rules or at the beginning (i.e., push vs unshift).
  2. Should this function accept 1 or endless amount of arguments?
  3. Should it accept arrays?

In most cases, such functionality will be needed only backend side, so it would be good not to pollute frontend bundle with code which will never be used.

To achieve the same functionality we can do this:

// should define `cannot` rules
function hardwareRules() {
  const { can, cannot, rules } = AbilityBuilder.extract()

  if (!hardware.supports('wifi')) {
    cannot('manage', 'wifi')
  }

  cannot(...)

  return rules
}

function userRules(user) {
  const { can, cannot, rules } = AbilityBuilder.extract()

  can(...)
  cannot(...)
  can(...)

  return rules
}

const ability = new Ability(userRules(user).concat(hardwareRules()))

So, a user is able to split rules (instead of Ability instances) into different functions and combine them in whatever way is better for his app.

@stalniy
Copy link
Owner Author

stalniy commented May 29, 2018

I will update documentation with the example above and close this issue

@stalniy
Copy link
Owner Author

stalniy commented May 29, 2018

Example and explanation is added in https://stalniy.github.io/casl/abilities/2017/07/23/use-cases.html

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

No branches or pull requests

2 participants