Skip to content

AO3-6218 Allow certain admins to access all collection and challenge pages usually reserved for owners#5694

Open
not-varram wants to merge 9 commits intootwcode:masterfrom
not-varram:AO3-6218-Allow-certain-admins-to-access-all-collection
Open

AO3-6218 Allow certain admins to access all collection and challenge pages usually reserved for owners#5694
not-varram wants to merge 9 commits intootwcode:masterfrom
not-varram:AO3-6218-Allow-certain-admins-to-access-all-collection

Conversation

@not-varram
Copy link
Copy Markdown
Contributor

Pull Request Checklist

Issue

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

Additional Notes

Failed on QA. Made necessary changes as per comments on jira.

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)

Copy link
Copy Markdown
Contributor

@Bilka2 Bilka2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the tests, I'm finally here for the updated review

And I invite the work "Test Work" to the collection "Moderated Collection"
When I am logged in as a "support" admin
And I view the awaiting collection approval collection items page for "Moderated Collection"
Then I should not see "You don't have permission"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this test locally without the changes in this PR and found it passing because the collection search error overrides this flash message. To get the flash message to show up, all indexing jobs have been run has to be done after creating the collection to get rid of the search error.

But I'd prefer to try to check for something else here, since that error also wouldn't show up if Support admins were forbidden from viewing the page like tag wrangling admins are down below. Maybe add the paths to paths.rb and then check with I should be on ...? Or check text on the page, though I'm guessing you would've gone for that if it had been viable...

Comment on lines +77 to +91
Scenario: Policy and abuse admin can access collection pages
Given I have a collection "PAB Collection"
And I am logged in as a "policy_and_abuse" admin
When I go to "PAB Collection" collection edit page
Then I should see "Edit Collection"
When I go to the "PAB Collection" participants page
Then I should see "Members of"

Scenario: Superadmin can access collection pages
Given I have a collection "Super Collection"
And I am logged in as a super admin
When I go to "Super Collection" collection edit page
Then I should see "Edit Collection"
When I go to the "Super Collection" participants page
Then I should see "Members of"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine to keep here since it's only two roles, but for the future: You can work with Scenario Outline and make the role a parameter, some other tests do that for example. Though I think in this case we don't have to exhaustively check every role since they behave the same and this is mostly about making sure the views don't error, so the way you organized this here is just fine

And I sign up for "GE Requests" with combination A
When I am logged in as a "support" admin
And I go to the "GE Requests" requests page
Then I should not see "You are not allowed to view the requests summary!"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue with this test not failing without the changes due to the error not showing up as above

And I open signups for "GE Matching"
When I am logged in as a "support" admin
And I go to "GE Matching" gift exchange matching page
Then I should not see "Sorry, you don't have permission"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And same here too :(

Then I should see "Sign-ups"
And I should see "signer1"

Scenario: Support admin can access an individual sign-up
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(this organization is nice for review, but I also wouldn't mind if you combine this test and the previous one)

And I press "Generate Potential Matches"
Then I should see "Please log out of your admin account first!"

Scenario: Support admin can access Assignments page
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing the test for accessing an individual assignment

And I sign up for Battle 12 with combination A
When I am logged in as a "support" admin
And I go to the "Battle 12" claims page
Then I should not see "Sorry, you don't have permission"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message problem again :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants