AO3-6218 Allow certain admins to access all collection and challenge pages usually reserved for owners by not-varram · Pull Request #5592 · otwcode/otwarchive

not-varram · 2026-02-18T03:55:06Z

Pull Request Checklist

Have you read "How to write the perfect pull request"?
Have you read the contributing guidelines?
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.

Issue

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

Purpose

Allow admins with support, policy_and_abuse, or superadmin roles to access owner/maintainer collection and challenge pages for viewing/troubleshooting, without granting write permissions.

Implemented via shared controller helpers and action-level filter changes so read routes are opened for those roles while create/update/destroy paths remain owner/maintainer-only.

Testing Instructions

Automated coverage was added/updated for the affected controllers to verify:

allowed admin roles can access read pages
other admin roles cannot
privileged admins still cannot perform protected updates

(Functional QA flow is already documented in the Jira ticket.)

Credit

varram (he/him)

…pages usually reserved for owners

Bilka2

Please use pundit for the access control. You can find our existing policies in the app/policies folder if you'd like examples. The other issues on the admin role Epic also have examples of pundit implementations.

Since this is a rather big PR already just with the permission changes, I would prefer if you didn't action unrelated rubocop issues, like the i18n changes you did here. The risk that something accidentally breaks is just too big there to include it all with the permission changes.

For the specs, please use the an action only authorized admins can access shared example if possible, it tests a lot of admin roles automatically. Examples for it's use can be found all across the admin role tests.

not-varram · 2026-03-12T05:50:47Z

Hey- I dont have a way to get this out of "Coder has reviewed action" tag but I'm still working on it.

not-varram · 2026-03-13T03:23:39Z

it's ready to be reviewed :)

Bilka2

Thank you for working on this! This looks pretty good already, I just have a few nitpicks

I wanted to note that this doesn't touch anything in the views, so there are a bunch of links/buttons that will show for owners but not authorized admins, e.g. the links the collection sidebar. Please keep it that way! We'll make separate Jira issues to add these links into the pages for the authorized admins later, since this PR is already quick big to code/review/test

Bilka2 · 2026-03-19T19:49:14Z

app/controllers/collections_controller.rb

+  before_action :users_or_privileged_collection_admin_only, only: [:edit]
  before_action :load_collection_from_id, only: [:show, :edit, :update, :destroy, :confirm_delete]
-  before_action :collection_owners_only, only: [:edit, :update, :destroy, :confirm_delete]
+  before_action :collection_owners_or_privileged_admins_only, only: [:edit]


Shouldn't it be enough to list edit only in collection_owners_or_privileged_admins_only?

I suppose the duplication comes from edit being listed in users_only, but from what I can tell both it and update can be removed from there

Bilka2 · 2026-03-19T20:12:29Z

app/controllers/collection_items_controller.rb

+    elsif logged_in_as_admin?
+      admin_only_access_denied and return
    else
      flash[:error] = ts("You don't have permission to see that, sorry!")
      redirect_to collections_path and return


Suggested change

elsif logged_in_as_admin?

admin_only_access_denied and return

else

flash[:error] = ts("You don't have permission to see that, sorry!")

redirect_to collections_path and return

else

admin_only_access_denied and return if logged_in_as_admin?

flash[:error] = ts("You don't have permission to see that, sorry!")

redirect_to collections_path and return

is more readable to me? very style/opinion based though, so definitely tell me no here if you disagree, the existing code is also fine

Bilka2 · 2026-03-19T20:14:14Z

app/controllers/collection_participants_controller.rb

 class CollectionParticipantsController < ApplicationController
-  before_action :users_only
+  before_action :users_only, except: [:index]
+  before_action :users_or_privileged_collection_admin_only, only: [:index]


Shouldn't it be enough to list index only in collection_maintainers_or_privileged_admins_only?

Bilka2 · 2026-03-19T20:32:35Z

app/controllers/challenge_requests_controller.rb

      redirect_to "/" and return
    end
-    unless @collection.challenge_type == "PromptMeme" || (@collection.challenge_type == "GiftExchange" && @collection.challenge.user_allowed_to_see_requests_summary?(current_user))
+    unless @collection.challenge_type == "PromptMeme" || privileged_collection_admin? || (@collection.challenge_type == "GiftExchange" && @collection.challenge.user_allowed_to_see_requests_summary?(current_user))


What do you think about combining the admin check into user_allowed_to_see_requests_summary? (and maybe renaming the method)?

That would make this a bit shorter here

Bilka2 · 2026-03-19T20:37:11Z

app/controllers/potential_matches_controller.rb


-  before_action :users_only
+  before_action :users_only, except: [:index, :show]
+  before_action :users_or_privileged_collection_admin_only, only: [:index, :show]


Shouldn't it be enough to list index and show only in collection_maintainers_or_privileged_admins_only?

In fact the whole before_action :users_only also seems useless to me, since everything is either collection_maintainers_or_privileged_admins_only or collection_maintainers_only

Also, shouldn't it only be index that's allowed for admins?

Bilka2 · 2026-03-19T20:53:25Z

app/controllers/application_controller.rb

+    policy(Collection).access?
+  end
+
+  def users_or_privileged_collection_admin_only


Suggested change

def users_or_privileged_collection_admin_only

def users_or_privileged_collection_admins_only

it's inconsistent :(

Bilka2 · 2026-03-19T20:56:21Z

app/controllers/challenge_claims_controller.rb

+      not_allowed(@collection) unless user_scoped? || @challenge.user_allowed_to_see_assignments?(current_user) || privileged_collection_admin?

      @claims = ChallengeClaim.unposted_in_collection(@collection)
      @claims = @claims.where(claiming_user_id: current_user.id) if user_scoped?


This will error 500 for admins, won't it?

Bilka2 · 2026-03-21T21:06:26Z

spec/controllers/challenge_assignments_controller_spec.rb

+  describe "admin access to assignments pages" do
+    authorized_roles = %w[support policy_and_abuse superadmin].freeze
+    let(:gift_exchange) { create(:gift_exchange, assignments_sent_at: Faker::Time.backward) }
+    let(:user) { other_user }


Is this required?

Bilka2 · 2026-03-21T21:09:44Z

spec/controllers/challenge_requests_controller_spec.rb

while you're in this file, could you check whether bookmark_search: true, collection_search: true, work_search: true could be removed from the topmost describe? I have no idea why those are there

Bilka2 · 2026-03-21T21:11:05Z

spec/controllers/challenge_assignments_controller_spec.rb

+    let(:gift_exchange) { create(:gift_exchange, assignments_sent_at: Faker::Time.backward) }
+    let(:user) { other_user }
+
+    before { fake_logout }


Is this required? I'd expect the admin login from the it_behaves_like to override the user login that's define in this file

not-varram added 2 commits February 17, 2026 19:15

AO3-6218 Allow certain admins to access all collection and challenge …

7140910

…pages usually reserved for owners

Fix Rubocob issues

69c0d42

github-actions bot added the Awaiting Review label Feb 18, 2026

not-varram added 3 commits February 17, 2026 20:39

fix rubocop

e93f335

fix otwcode#3

1aae43d

try 4

e408677

Bilka2 requested changes Feb 18, 2026

View reviewed changes

Bilka2 added Reviewed: Action Needed and removed Awaiting Review labels Feb 18, 2026

github-actions bot added Coder Has Actioned Review and removed Reviewed: Action Needed labels Mar 12, 2026

not-varram force-pushed the AO3-6218-Allow-certain-admins-to-access-all-collection branch from b01187c to e408677 Compare March 12, 2026 04:38

adress PR comments.

2b6e386

Bilka2 requested changes Mar 21, 2026

View reviewed changes

Bilka2 added Reviewed: Action Needed and removed Coder Has Actioned Review labels Mar 21, 2026

	def users_or_privileged_collection_admin_only
	def users_or_privileged_collection_admins_only

Conversation

not-varram commented Feb 18, 2026

Pull Request Checklist

Issue

Purpose

Testing Instructions

Credit

Uh oh!

Bilka2 left a comment

Choose a reason for hiding this comment

Uh oh!

not-varram commented Mar 12, 2026

Uh oh!

not-varram commented Mar 13, 2026

Uh oh!

Bilka2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants