Skip to content

AO3-3764 Paginate tag set nominations review page#5752

Open
pmonfort wants to merge 7 commits intootwcode:masterfrom
pmonfort:AO3-3764-paginate-tag-set-nominations-review
Open

AO3-3764 Paginate tag set nominations review page#5752
pmonfort wants to merge 7 commits intootwcode:masterfrom
pmonfort:AO3-3764-paginate-tag-set-nominations-review

Conversation

@pmonfort
Copy link
Copy Markdown
Contributor

Pull Request Checklist

Issue

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

Purpose

When a tag set has more than 30 unreviewed nominations, moderators currently see a random subset with the message "There are too many nominations to show at once." This makes it impossible to systematically review all nominations.

This PR replaces the random selection with proper pagination using pagy:

  • Paginate nominations by unique tagname, grouping duplicates (same tag nominated by multiple users) into a single entry
  • Each tag type (fandom, character, relationship, freeform) paginates independently with its own page param
  • Heading shows "X - Y of Z [Type]" when paginated, or "[Type] (N left to review)" when everything fits on one page
  • Pagination links include anchors to scroll to the correct fieldset
  • Order by created_at ascending for deterministic results
  • Removed the random selection logic and the "too many nominations" flash message

Testing Instructions

  1. Log in as a moderator of a tag set with more than 25 unreviewed nominations
  2. Go to Review Nominations
  3. Verify pagination links appear and each tag type paginates independently
  4. Verify that navigating pages does not affect other tag types (e.g. going to fandom page 2 keeps freeform on page 1)
  5. Verify duplicate nominations (same tagname from different users) appear only once
  6. Verify approving/rejecting tags updates counts correctly
  7. Verify tag sets with fewer than 25 nominations show no pagination links and display the "(N left to review)" heading
  8. Verify the old "There are too many nominations to show at once" message no longer appears

Credit

Pablo Monfort (he/him)

@pmonfort pmonfort force-pushed the AO3-3764-paginate-tag-set-nominations-review branch from 11f0ab0 to 0bb47cc Compare April 22, 2026 05:31
<%= check_all_none("Approve All", "Approve None", "approve") %>
<% if @paginations&.dig(tag_type) %>
<%== pagy_nav(@paginations[tag_type],
anchor_string: "id=\"#{tag_type}_nominations\"") %>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Question! Does the anchor_string here mean that the pagination links should be like ?fandom_page=3#fandom_nominations?

I swear I saw that behavior when I first pulled this code to try it out, but it has disappeared on me, so I'm confused. 😆

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's the intended behavior! The anchor_string parameter name is a bit misleading, it refers to the HTML anchor element ( tag) not URL anchors/fragments. I've pushed a fix using the correct fragment parameter.
Good catch! Thanks!

Copy link
Copy Markdown
Collaborator

@sarken sarken left a comment

Choose a reason for hiding this comment

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

Just a couple of nitpicks/questions about the tests! Aside from that, this looks good and I'm excited. 🎉

Comment on lines +167 to +173
Scenario: Owner of a tag set with over 30 nominations does not see the randomized selection message
Given I am logged in as "tagsetter"
And I set up the nominated tag set "Nominated Tags" with 6 fandom noms and 6 character noms
When there are 36 unreviewed nominations
Given I am logged in as "tagsetter"
And I review nominations for "Nominated Tags"
Then I should not see "There are too many nominations to show at once"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd be okay with removing this test. We can just confirm that the message has been removed during manual testing; we don't need to test it every time we run the tests for the rest of time. 😆

Comment on lines +279 to +280
tsn = TagSetNomination.create!(owned_tag_set: owned_tag_set, pseud: nominator.default_pseud)
FandomNomination.create!(tag_set_nomination: tsn, tagname: "Paginated Fandom #{i}")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we use factories for the tag set nominations and tag nominations? I think it would be something like:

                      tsn = create(:tag_set_nomination, owned_tag_set: owned_tag_set, pseud: nominator.default_pseud)
                      create(:tag_nomination, tag_set_nomination: tsn, tagname: "Paginated Fandom #{i}", type: "FandomNomination")

context 'unreviewed fandom nominations <= 30' do
let(:fandom_nom_num) { 30 }
context "unreviewed fandom nominations within per page limit" do
let(:fandom_nom_num) { 20 }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there a reason it needs to be 20? If so, it would be a good idea to document why we do that instead of something like the per_page + 1 we do elsewhere.

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