Add tests for our policies

[chvp]

The only changes to our actual app code is some hardening in the permitted attributes. This shouldn't result in any behaviour changes.

• I've added tests relevant to my changes.

[robbevp]

All for hardening and testing the policies! I'll keep this open, until we resolve the PR earlier in the stack.

A few open suggestions:

• There's a gem (that I wrote) that provides some custom assertions for policies, so we can just write assert_permitted .... Though I also understand not wanting to add a gem for rather trivial things like this
• I would avoid the confusion of anonymous user and use something like guest (since an anonymous user is in fact the absence of a user)
• I have the habit of checking all user types for each action/scope/method, just for clarity. So often do something like:

  test 'should allow index for users and moderators' do
    assert_predicate Pundit.policy!(@user, Album), :index?
    assert_predicate Pundit.policy!(@moderator, Album), :index?
    assert_not_predicate Pundit.policy!(nil, Album), :index?

[chvp]

@robbevp I updated the AlbumPolicyTest as you suggested (although I kept the test for permitted attributes separate per user class, since they felt quite disparate). Can you check that one to see if it matches what you would expect before I do the rest?

[robbevp]

@robbevp I updated the AlbumPolicyTest as you suggested (although I kept the test for permitted attributes separate per user class, since they felt quite disparate). Can you check that one to see if it matches what you would expect before I do the rest?

Yes, I think is going in the right direction. I don't have a strong opinion about which assertions to group together, but I can now better orientate myself in the test file than before

[robbevp]

👍 I'd just split off the changes in the workflows and flake to keep the PR focused on one thing