AO3-7144 AO3-7147 Add collection listings for tags (with corrected page headings)

[techlingyi]

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-7144

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

Purpose

7147: Implement collection listings for tags -- now tags have index pages for collections like they do for works and bookmarks with that tag. Links have been updated accordingly.

7144: Collections index page headings are now consistent with other page headings and i18n'd.

References

Adds to search_header, which has a PR for internationalization here: AO3-7357 #5695

Credit

Ling-Yi (any pronouns)

[Bilka2]

Not a full review, just a note: The code for AO3-7357 needs to change approach, so I think to avoid merge conflicts/doing extra work here, it would be better to go with ts for now (and just don't mind that reviewdog will complain)

[techlingyi]

Just done, thank you! c:

[techlingyi]

Actually: since the kudos page for an individual work (ao3.org/works/[id]/kudos) links back to the work in its header, should I do that for the header of the collections page (ao3.org/works/[id]/collections) too? It does look a lot better as a link when the work title is in lowercase.

[Bilka2]

Thank you for working on this!

Recommendation for how to action this review:

1. Read/skim the comments for what I say regarding merges - there's a few spots where master and this PR disagree and specifically the master or this PR's version need to be picked
2. Merge master according to that
3. Action the rest of the review

I want to note that I didn't yet review the newly added test file - there are already a lot of comments so I think it will be simpler to do that as a second round

[techlingyi]

@Bilka2 Ready for review again, thank you! :D

[Bilka2]

This is looking very good! Mostly just minor comments from me, plus could you take a look at and respond to https://github.com/otwcode/otwarchive/pull/5733/files#r3579071085 from the previous round (I assume it's just more complicated and not cleaner to merge the tag loading?)

For my own tracking, I've looked at everything except:

• subnavigation on the collection index, pending master merge for that because it's modified so heavily in master

(If the dates on the comments are a bit confusing, I had half the review done, got pulled away and then got back now to finish it off. I edited some of the earlier comments today as I went over the files again, so basically all comments are from today, even though I started reviewing over a month ago...)

[Bilka2]

I believe it should say "N Collections including WORK_TITLE", like on the main collections index described in AO3-7144 (that's what I meant with my initial comment). I suppose you can achieve that in the same way you achieved it for the main collections index, with the collections_in_the directly on the index page?

(I don't like having the text outside search_header (we'll need it inside to i18n it properly) but right now I can't come up with a better solution that isn't a hack and since we're not i18n'ing it now and i18n will rewrite the method anyway it's probably fine)

[techlingyi]

@Bilka2 Hi! Sorry for the delay on this - I'm finally getting back to it, but I've been stuck on this specification:

I believe it should say "N Collections including WORK_TITLE", like on the main collections index described in AO3-7144 (that's what I meant with my initial comment). I suppose you can achieve that in the same way you achieved it for the main collections index, with the collections_in_the directly on the index page?

I tried to plug it in the same way I did for everything else, but since the code for fetching the collections of a work isn't reliant on the search form, it doesn't seem to respond to total_pages and so just shows "Recent Collections including WORK_TITLE" rather than a number, which I'm not sure how to fix.

if @owner.present?
      if @work.present?
        @sort_and_filter = false
        @collections = @work.approved_collections
          .by_title
          .for_blurb

Should I try to get around this (since it's not in the main body of AO3-7144), and if so, could I have any help? I can't quite figure out how the search scopes work ^^; Thank you so much!!

[Bilka2]

@techlingyi I see the issue, it's because of the switch to pagy: will_paginate would set total_pages (and more, like offset) but pagy doesn't touch @collections at all. So we basically need a whole new search_header method that can handle pagy or we need to rewrite pagy so that it sets all the things on @collections that we need for search_header. I think that is out of scope of this already large PR. Since the switch to pagy is not required here, I would recommend to switch back to will_paginate for all branches of CollectionsController#index.

If you switch back to will_paginate then it works to replace the current header

otwarchive/app/views/collections/index.html.erb

Lines 7 to 9 in 2958a17

	<% elsif @work %>
	<%= t(".page_heading.collections_including", work: @work.title) %>
	<% else %>

with this:

  <% elsif @work %>
    <%= t(".page_heading.collections_including", collections: search_header(@collections, @query, t(".page_heading.collection"), @owner), work: @work.title) %>
  <% else %>

with the locale collections_including: "%{collections} including %{work}"

[techlingyi]

@Bilka2 Ready for review again, thank you! I can’t get Github to let me reply to or resolve either of the relevant comments, but I factored out the load_owner method and fixed the navigation - I switched to will_paginate, which told me to add an extra .page to the transformations on @collections, and then that turned out to work even when I switched back to pagy :D