AO3-7468 Preload current_user roles

[y7nieSEl5]

Pull Request Checklist

• Have you read the contributing guidelines?
• Have you read "How to write the perfect pull request"?
• Have you added tests for any changed functionality?
• Have you added the Jira issue number
  as the first thing in your pull request title (e.g. AO3-1234 Fix thing)
• Do you have fewer than 5 pull requests already open? If not, please wait
  until they are reviewed and merged before creating new pull requests.
• I acknowledge that submitting code created or modified with the help of AI is a bannable offense.

Issue

https://otwarchive.atlassian.net/browse/AO3-7468

Purpose

This PR preloads a user's roles when Devise restores current_user from the session. As a result, subsequent has_role? permission checks use the loaded association instead of performing repeated role lookups throughout the request.

It also:

• Uses has_role? for named-role checks in the admin user form.
• Preloads roles when loading users for admin user pages that render the role collection.
• Adds regression coverage for users with roles, users without roles, invalid session salts, and admin user role preloading.

Credit

y7nieSEl5, she/her

[redsummernight]

Instead of checking that roles are preloaded like this, you should create an n+1 test where the number of roles is variable, but get admin_user_path(user) should perform a constant number of queries for role lookups.

We have similar tests for other controller actions under spec/requests.

[y7nieSEl5]

Thanks for the feedback! I replaced the controller preload assertion with a request-level N+1 test, preloaded roles for the history entries rendered on that page, and removed the two extra model expectations.
All three relevant spec files pass. Ready for another look!

[redsummernight]

You're limiting the number of roles to 5 or below. In populate you should create objects using the scale factor n, otherwise results won't be accurate.

Since we don't care what the roles actually are, you can do:

populate do |n|
  user.roles = create_list(:role, n)
end

[redsummernight]

You can inline admin in the before block - it won't be referred to elsewhere.

[redsummernight]

You can define the get action as a subject:

subject do
  proc do
    get admin_user_path(user)
  end
end

then use subject.call in both warmup and the actual test.

[redsummernight]

These expects don't need to be nested here. You can move them after the first expect for constant query count.

[redsummernight]

You should be able to skip matching and with_scale_factors, since these are the effects of only working with a max of 5 roles.

[redsummernight]

You should wrap the tests in a context block e.g. "with a logged in user with multiple roles". There may be other kinds of n+1 queries we want to test for this controller action in the future.