Fix "requiring authorization" shared examples in our test suite

[coalest]

Description

We have a set of shared examples called "required authorization" (link to source). They are meant to be a DRY way to ensure that the controller actions or requests require authorization.

The problem is there a bug in the implementation which means that the shared specs will always pass.

To illustrate, running the following specs:

FooController = Class.new(ActionController::Base)

RSpec.describe FooController, type: :controller do
  let(:object) { create(:item) }

  include_examples "requiring authorization"
end

RSpec.describe "foo", type: :request do
  let(:object) { create(:item) }

  include_examples "requiring authorization"
end

produces this output:

Randomized with seed 56820

foo
  redirects the user to the sign-in page for CRUD actions

FooController
  redirects the user to the sign-in page for CRUD actions

Finished in 0.73823 seconds (files took 3.71 seconds to load)
2 examples, 0 failures

Let's fix this so that these shared examples are actually testing authorization.

Issues to resolve

• Fix the bug causing every example block to be skipped. (Hint: Look at how the constraints passed in are defined/mutated.)
• These shared examples are used in both controller specs and request specs, but look to be written exclusively for controller specs. Let's either create separate shared examples for request specs or refactor the specs to work for both controller and request specs.

[coalest]

@dorner Do you mind taking at a look at this issue and seeing if it's described correctly and that you think this is worth fixing? (Maybe this isn't a priority and we just want to delete these examples entirely since they aren't doing anything.) I wouldn't want someone to start working on this if a PR isn't wanted.

[dorner]

I think it's worth fixing, yep. I know we have some specific specs that test this in addition... I think we need to decide either to fix this and remove those, or remove this and add more specific specs where they don't exist right now.

[McEileen]

Hi, does this issue need someone to work on it? If so, I'm interested.

[cielf]

@McEileen -- I think it's in a state where we need to make a decision about it before going further? - @dorner is that your intent -- or are you looking for someone to investigate whether we should fix and remove, or remove and add?

[dorner]

I think it makes sense for us to have a general authorization spec that does not need to be repeated for every single controller (unless the controller has some custom auth logic). Then we can remove those additional specs from individual controllers.

[cielf]

@McEileen Assigning it to you, then -- if you need more details let us know.

[github-actions[bot]]

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

[McEileen]

I am still interested in working on this, but haven't started it.

[github-actions[bot]]

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

[cielf]

Still holding for @McEileen

[github-actions[bot]]

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

[github-actions[bot]]

Automatically unassigned after 7 days of inactivity.

[McEileen]

I will be taking a pause from contributing to Human Essentials for a bit. I encourage anyone who is interested to pick up this issue.