Fix setting the @user in UsersController

[emcoding]

Related issue: User Registrations Overhaul
#991 .5 in Overhaul Break Down
Branch off of #992

This is an example for the authorisation issues mentioned in # 991 5).
Setting the @user happened in two before_actions: both in Cancancan's load_resource method and in set_user . The two before_actions were confusing, because some crud actions were called in both of them; so: sometimes with authorization, sometimes without. That caused some weird behaviour in the authorisations. (Possibly the reason for some of the extra 'signed_in` checks in Ability.)

• Now the @user is set only once in each controller action.
• Specs were flaky, because of a GitHub user name validation error in the Factory. Changed that to a sequence.

@pgaspar
When logged in as an unconfirmed user, see the difference between the behaviour on the Teams pages compared to the new behaviour on the Users pages.
This is what I meant here: #989 (comment)

• Follow up: Find all the other controllers that have the same ambiguity and solve them.

[pgaspar]

Hey @F3PiX!

Back in #989 (comment) I was referring specifically to what I expected before_action :authenticate_user! to do, since we were trying to figure out if that before action was blocking access to unconfirmed users or not.

That being said, I think I understand your changes in this PR, but I want to double check. Correct me if I'm wrong: index and show were not being authorized and were rendering without any ability check. Other than that, edit, update and destroy were setting @user twice because of the two before_actions. Am I right that these are the issue we're fixing here?

I tried checking how User pages (index, show, editing) compare with the Team pages (index, show, editing, creating) when it comes to confirmed and unconfirmed users and I couldn't see differences - Read actions were available for both confirmed and unconfirmed users while Create, Update (and Delete I assume) actions were only available for confirmed users, redirecting unconfirmed users with an authorization error. Both seemed to work similarly in this sense at least from my testing. I'm sorry if I'm missing the point you were trying to make! 😓

[emcoding]

@pgaspar Thanks for checking!!
First:

That being said, I think I understand your changes in this PR, but I want to double check. Correct me if I'm wrong: index and show were not being authorized and were rendering without any ability check. Other than that, edit, update and destroy were setting @user twice because of the two before_actions. Am I right that these are the issue we're fixing here?
That is exactly the point 🎯

That you don't find differences in the UI is because all the extra authorisation calls in Ability.
I'd say: let's finish that first, because it will make all the other changes, including this one, super clear.

Also, I should have given a bit more background info where to find the changes. The UI doesn't change (because that was taken care with workarounds in Cancancan. ) (Except o joy, it is sooo much faster!)
So, for future reference:
The changed behaviour can be reveiled by adding a byebug at the start of the Ability initializer.
Before this PR: /users/yourid/edit will go multiple times through the authorisation process.
After: same request will go directly to the edit page, without hitting authorisation - as it should!
BUT I just checked this in my PR with the make over of Ability.

TL;DR It is best to set this PR on hold untill after the make over.

[emcoding]

Closing this, to be continued in new PR.