Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AO3-5889 Paginate pseuds using default per page setting #4569

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

neuroalien
Copy link
Contributor

@neuroalien neuroalien commented Jul 8, 2023

Pull Request Checklist

If cucumber tests are required, I would appreciate being paired with a coder who is okay with writing cucumber features, please.

Issue

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

Purpose

Adds pagination to the pseuds page.

Testing Instructions

How can the Archive's QA team verify that this is working as you intended?

If you have a Jira account with access, please update or comment on the issue
with any new or missing testing instructions instead.

References

Are there other relevant issues/pull requests/mailing list discussions?

Credit

What name and pronouns should we use to credit you in the Archive of Our Own's Release Notes?

alien, they/them

@@ -14,7 +14,7 @@ def load_user
# GET /pseuds.xml
def index
if @user
@pseuds = @user.pseuds
@pseuds = @user.pseuds.paginate(page: params[:page])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we set this to use the alphabetical scope? I noticed when I was playing with tests I'd sometimes get the wrong pseud on the second page if I didn't add that.

Speaking of tests, I added this right after

And I should not see "Slartibartfast" within "ul.expandable"

  When I go to my pseuds page
  Then I should not see "Zaphod (Zaphod)" within "ul.pseud.index"
    But I should see "Agrajag (Zaphod)" within "ul.pseud.index"
    And I should see "Betelgeuse (Zaphod)" within "ul.pseud.index"
    And I should see "Slartibartfast (Zaphod)" within "ul.pseud.index"
    And I should see "Next" within ".pagination"
  When I follow "Next" within ".pagination"
  Then I should see "Zaphod (Zaphod)" within "ul.pseud.index"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! Do we want alphabetical or default_alphabetical? Presumably what we use now will stick for the foreseeable 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the spec! 🙌

Copy link
Collaborator

Choose a reason for hiding this comment

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

alphabetical is good! That's what it seems to be on staging

@@ -11,6 +11,10 @@
<% end %>
<!--/subnav-->

<!--subnav-->
<%= will_paginate @pseuds %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wait, sorry, one more comment -- can you move <%= will_paginate @pseuds %> up above the <!--/subnav--> on line 12? We usually just have one subnav comment on the top and one on the bottom.

We're also adding alphabetical sorting to the pseuds.
@neuroalien neuroalien force-pushed the AO3-5889-pseud-pagination branch from 011654b to 1571a78 Compare July 10, 2023 21:41
Copy link
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.

🎉

@sarken sarken merged commit 7f43395 into otwcode:master Jul 11, 2023
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