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

Handle Rspec negated be_able_to for actions list #807

Closed
wants to merge 1 commit into from
Closed

Handle Rspec negated be_able_to for actions list #807

wants to merge 1 commit into from

Conversation

uxxman
Copy link

@uxxman uxxman commented Nov 24, 2022

Currently, the rspec matcher handles list of actions correctly for to be_able_to case and returns successfully when the user is allowed to perform all the specified actions.

But, it doesn't work correctly for a list of actions for the negated cases.

# Ability
can [:read, :create, :update], Account

it { is_expected.to be_able_to([:read, :create], Account.new) }  # Passes when user is allowed to perform both actions (read, create)

it { is_expected.not_to be_able_to([:destroy, :read], Account.new) }  # Passes even though user is allowed to read

The underlying issue is this line

actions.all? { |action| ability.can?(action, *args[1..-1]) }

It will return false when we have some true values and even one false value.

PR fixes the issue by making use of => https://relishapp.com/rspec/rspec-expectations/docs/custom-matchers/define-a-custom-matcher#matcher-with-separate-logic-for-expect().to-and-expect().not-to

@coorasse
Copy link
Member

coorasse commented Mar 5, 2023

We are planning to disable negate able_to matcher for multiple actions. This lead to a lot of confusion in the past. See #796

@uxxman
Copy link
Author

uxxman commented May 2, 2023

@coorasse thanks for the update. Closing PR.

@uxxman uxxman closed this May 2, 2023
@uxxman uxxman deleted the patch-1 branch May 2, 2023 21:57
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.

2 participants