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

Add for_actions composite matcher to permit/forbid_mass_assignment_of #23

Open
chrisalley opened this issue May 26, 2018 · 2 comments
Open

Comments

@chrisalley
Copy link
Collaborator

Currently, to check that the mass assignment of one or more attributes is permitted or forbidden for different actions, we have to declare this as two seperate statements:

it { is_expected.to permit_mass_assignment_of(:publish).for_action(:create) }
it { is_expected.to permit_mass_assignment_of(:publish).for_action(:update) }

But it would be nicer to check that the attribute is permitted with a single statement.

My proposal is that we add a new composite matcher to permit_mass_assignment_of and forbid_mass_assignment_of called for_actions which accepts an array of actions to permit/forbid the atttibute(s) in. E.g.

it { is_expected.to permit_mass_assignment_of(:publish).for_actions(%i[create update]) }

or another example:

it { is_expected.to forbid_mass_assignment_of(%i[foo bar]).for_actions(%i[destroy]) }

The spec should pass if and only if all of the listed attributes are permitted for all of the listed actions, or all of the listed attributes are forbidden for all of the listed actions, for the permit_mass_assignment_of and forbid_mass_assignment_of matchers respectively.

@tagliala
Copy link
Collaborator

tagliala commented May 25, 2023

Open points:

The error message should change

expect(policy).to fodbid_mass_assignment_of(:param).for_actions(:a1, :a2)
"Expected x to be forbidden by actions :a1, :a2, but it was permitted by :a1".

This would increase the complexity of for_only_actions (permitted by a1 but forbidden by a2)

How to deal with the default action

  • Should it considered an action?
  • Should for_action(:update) check for permitted_attributes also?

for_actions should work like policies

  • for_action = for_actions with one argument
  • for_actions should check only the given actions
  • for_all_actions should check all the actions, including the base action
  • for_only_actions should check that only the given actions allow the given param

Otherwise, the implementation would be ambiguous

/cc @chrisalley

@chrisalley
Copy link
Collaborator Author

chrisalley commented May 25, 2023

Added comment about for_all_actions to #24.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants