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

app/controllers/application_controller.rb

@@ -354,6 +354,52 @@ def load_collection
     @collection = Collection.find_by(name: params[:collection_id]) if params[:collection_id]
   end
 
+  # obj_name: "work", "bookmark", or "collection" (used in the respective controllers)
+  def load_owner(obj_name)
+    @work = Work.find(params[:work_id]) if params[:work_id].present?
+    @collection = Collection.find_by!(name: params[:collection_id]) if params[:collection_id].present?
+    @language = Language.find_by(short: params[:language_id]) if params[:language_id].present?
+
+    if params[:user_id].present?
+      @user = User.find_by(login: params[:user_id])
+      raise ActiveRecord::RecordNotFound, "Couldn't find user named '#{params[:user_id]}'" if @user.blank?
+
+      if params[:pseud_id].present?
+        @pseud = @user.pseuds.find_by(name: params[:pseud_id])
+        raise ActiveRecord::RecordNotFound, "Couldn't find pseud named '#{params[:pseud_id]}'" if @pseud.blank? && obj_name == "bookmark"
+      end
+    end
+
+    if params[:tag_id].present?
+      @tag = Tag.find_by_name(params[:tag_id])
+      raise ActiveRecord::RecordNotFound, "Couldn't find tag named '#{params[:tag_id]}'" unless @tag.is_a?(Tag)
+
+      unless @tag.canonical?
+        redirect_to tag_path(@tag) and return if @tag.merger.blank?
+
+        work_redirect_path = @collection.present? ? collection_tag_works_path(@collection, @tag.merger) : tag_works_path(@tag.merger)
+        case obj_name
+        when "collection"
+          redirect_to tag_collections_path(@tag.merger) and return
+        when "bookmark"
+          redirect_to tag_bookmarks_path(@tag.merger) and return
+        when "work"
+          redirect_to work_redirect_path and return
+        end
+      end
+    end
+
+    # can't factor out shared user/collection/tag variables because precedence matters
+    @owner = case obj_name
+             when "work"
+               @pseud || @user || @collection || @tag || @language
+             when "bookmark"
+               @bookmarkable || @pseud || @user || @collection || @tag
+             when "collection"
+               @user || @work || @collection || @tag
+             end
+  end
+
   def privileged_collection_admin?
     policy(Collection).access?
   end

app/controllers/bookmarks_controller.rb

@@ -1,6 +1,6 @@
 class BookmarksController < ApplicationController
   before_action :load_collection
-  before_action :load_owner, only: [:index]
+  before_action :load_bookmark_owner, only: [:index]
   before_action :load_bookmarkable, only: [:index, :new, :create]
   before_action :users_only, only: [:new, :create, :edit, :update]
   before_action :check_user_status, only: [:new, :create, :edit, :update]
@@ -251,33 +251,8 @@ def destroy
 
   protected
 
-  def load_owner
-    if params[:user_id].present?
-      @user = User.find_by(login: params[:user_id])
-      unless @user
-        raise ActiveRecord::RecordNotFound, "Couldn't find user named '#{params[:user_id]}'"
-      end
-      if params[:pseud_id].present?
-        @pseud = @user.pseuds.find_by(name: params[:pseud_id])
-        unless @pseud
-          raise ActiveRecord::RecordNotFound, "Couldn't find pseud named '#{params[:pseud_id]}'"
-        end
-      end
-    end
-    if params[:tag_id]
-      @tag = Tag.find_by_name(params[:tag_id])
-      unless @tag
-        raise ActiveRecord::RecordNotFound, "Couldn't find tag named '#{params[:tag_id]}'"
-      end
-      unless @tag.canonical?
-        if @tag.merger.present?
-          redirect_to tag_bookmarks_path(@tag.merger) and return
-        else
-          redirect_to tag_path(@tag) and return
-        end
-      end
-    end
-    @owner = @bookmarkable || @pseud || @user || @collection || @tag
+  def load_bookmark_owner
+    load_owner("bookmark")
   end
 
   def index_page_title

app/controllers/collections_controller.rb

@@ -1,4 +1,5 @@
 class CollectionsController < ApplicationController
+  before_action :load_collection_owner, only: [:index]
   before_action :users_only, only: [:new, :create]
   before_action :load_collection_from_id, only: [:show, :edit, :update, :destroy, :confirm_delete]
   before_action :collection_owners_or_privileged_admins_only, only: [:edit]
@@ -31,32 +32,48 @@ def check_parent_visible
   end
 
   def index
+    base_options = {
+      page: params[:page] || 1
+    }
     options = params[:collection_search].present? ? collection_filter_params : {}
-    if params[:work_id]
-      @work = Work.find(params[:work_id])
-      @collections = @work.approved_collections
-        .by_title
-        .for_blurb
-        .paginate(page: params[:page])
-    elsif params[:collection_id]
-      @collection = Collection.find_by!(name: params[:collection_id])
-      @search = CollectionSearchForm.new({ parent_id: @collection.id, sort_column: "title.keyword" }.merge(page: params[:page]))
-      @collections = @search.search_results.scope(:for_search)
-      flash_search_warnings(@collections)
-      @page_subtitle = t(".subcollections_page_title", collection_title: @collection.title)
-    elsif params[:user_id]
-      @sort_and_filter = true
-      @user = User.find_by!(login: params[:user_id])
-      @search = CollectionSearchForm.new(options.merge({ maintainer_id: @user.id }.merge(page: params[:page])))
-      @collections = @search.search_results.scope(:for_search)
-      flash_search_warnings(@collections)
-      @page_subtitle = ts("%{username} - Collections", username: @user.login)
+    options.merge!(base_options)
+
+    @page_subtitle = case @owner
+                     when Collection
+                       t(".page_title.subcollection", collection_title: @collection.title)
+                     when User
+                       t(".page_title.user", username: @user.login)
+                     when Tag
+                       t(".page_title.tag", tag_name: @tag.name)
+                     else
+                       t(".page_title.general")
+                     end
+
+    if @owner.present?
+      if @work.present?
+        @sort_and_filter = false
+        @collections = @work.approved_collections
+          .by_title
+          .for_blurb
+          .page(params[:page])
+      elsif @collection.present?
+        @sort_and_filter = false
+        @search = CollectionSearchForm.new({ parent_id: @collection.id, sort_column: "title.keyword" }.merge(base_options))
+        @collections = @search.search_results.scope(:for_search)
+      elsif @user.present? || @tag.present?
+        @sort_and_filter = true
+        add_owner options
+        @search = CollectionSearchForm.new(options)
+        @collections = @search.search_results.scope(:for_search)
+      end
     else
       @sort_and_filter = true
-      @search = CollectionSearchForm.new(options.merge(page: params[:page]))
+      @search = CollectionSearchForm.new(options)
       @collections = @search.search_results.scope(:for_search)
-      flash_search_warnings(@collections)
     end
+
+    flash_search_warnings(@collections) if @work.blank?
+    @pagy = pagy_query_result(@collections) if @collections.respond_to?(:total_pages)
   end
 
   # display challenges that are currently taking signups
@@ -194,6 +211,22 @@ def destroy
     redirect_to(collections_path)
   end
 
+  protected
+
+  def load_collection_owner
+    load_owner("collection")
+  end
+
+  def add_owner(options)
+    case @owner
+    when Tag
+      options[:filter_ids] ||= []
+      options[:filter_ids] << @owner.id
+    when User
+      options[:maintainer_id] = @owner.id
+    end
+  end
+
   private
 
   def collection_filter_params

app/controllers/works_controller.rb

@@ -5,7 +5,7 @@ class WorksController < ApplicationController
 
   # only registered users and NOT admin should be able to create new works
   before_action :load_collection
-  before_action :load_owner, only: [:index]
+  before_action :load_work_owner, only: [:index]
   before_action :users_only, except: [:index, :show, :navigate, :search, :collected, :edit, :update, :drafts, :share]
   before_action :check_user_status, except: [:index, :edit, :edit_multiple, :confirm_delete_multiple, :delete_multiple, :confirm_delete, :destroy, :show, :show_multiple, :navigate, :search, :collected, :share]
   before_action :check_user_not_suspended, only: [:edit, :confirm_delete, :destroy, :show_multiple, :edit_multiple, :confirm_delete_multiple, :delete_multiple]
@@ -723,32 +723,8 @@ def mark_as_read
 
   protected
 
-  def load_owner
-    if params[:user_id].present?
-      @user = User.find_by!(login: params[:user_id])
-      if params[:pseud_id].present?
-        @pseud = @user.pseuds.find_by(name: params[:pseud_id])
-      end
-    end
-    if params[:tag_id]
-      @tag = Tag.find_by_name(params[:tag_id])
-      unless @tag && @tag.is_a?(Tag)
-        raise ActiveRecord::RecordNotFound, "Couldn't find tag named '#{params[:tag_id]}'"
-      end
-      unless @tag.canonical?
-        if @tag.merger.present?
-          if @collection.present?
-            redirect_to(collection_tag_works_path(@collection, @tag.merger)) && return
-          else
-            redirect_to(tag_works_path(@tag.merger)) && return
-          end
-        else
-          redirect_to(tag_path(@tag)) && return
-        end
-      end
-    end
-    @language = Language.find_by(short: params[:language_id]) if params[:language_id].present?
-    @owner = @pseud || @user || @collection || @tag || @language
+  def load_work_owner
+    load_owner("work")
   end
 
   def load_work

app/helpers/search_helper.rb

@@ -52,22 +52,20 @@ def works_original_path
     )
   end
 
-  def collections_original_path
+  def bookmarks_original_path
     url_for(
-      controller: :collections,
+      controller: :bookmarks,
       action: :index,
       only_path: true,
-      **params.slice(:title, :challenge_type, :moderated, :multifandom, :closed, :tag,
-                     :sort_column, :sort_direction).permit!
+      **params.slice(:tag_id, :collection_id, :pseud_id, :user_id).permit!
     )
   end
 
-  def bookmarks_original_path
+  def collections_original_path
     url_for(
-      controller: :bookmarks,
+      controller: :collections,
       action: :index,
-      only_path: true,
-      **params.slice(:tag_id, :collection_id, :pseud_id, :user_id).permit!
+      only_path: true
     )
   end
 end

app/views/bookmarks/index.html.erb

@@ -31,6 +31,7 @@
     <% if @tag %>
       <li><%= span_if_current t(".navigation.works"), tag_works_path(@tag) %></li>
       <li><%= span_if_current t(".navigation.bookmarks"), tag_bookmarks_path(@tag) %></li>
+      <li><%= span_if_current t(".navigation.collections"), tag_collections_path(@tag) %></li>
     <% end %>
     <% if @facets.present? %>
       <% # Filters button for narrow screens jumps to filters when JavaScript is disabled and opens filters when JavaScript is enabled %>

app/views/collection_profile/show.html.erb

@@ -19,7 +19,7 @@
         <dd>
           <ul class="tags commas">
             <% @collection.tags.each do |tag| %>
-              <li><%= link_to_tag_works(tag) %></li>
+              <li><%= link_to(tag.display_name, tag_collections_path(tag), class: "tag") %></li>
             <% end %>
           </ul>
         </dd>

app/views/collections/_collection_blurb.html.erb

@@ -17,7 +17,7 @@
       <% unless collection.tag_string.blank? %>
         <h5 class="tags heading">
           <span class="landmark"><%= t(".collection_tags") %></span>
-          <%= safe_join(collection.tags.collect{ |tag| link_to_tag_works(tag) }, t("support.array.words_connector")) %>
+          <%= safe_join(collection.tags.collect { |tag| link_to(tag.display_name, tag_collections_path(tag), class: "tag") }, t("support.array.words_connector")) %>
         </h5>
       <% end %>
     </div>

app/views/collections/_filters.html.erb

@@ -1,5 +1,11 @@
 <%= form_for @search, as: :collection_search,
-  url: (@user ? user_collections_path(@user) : collections_path),
+  url: (if @user
+          user_collections_path(@user)
+        elsif @tag
+          tag_collections_path(@tag)
+        else
+          collections_path
+        end),
   html: {
     method: :get,
     class: "narrow-hidden filters",

app/views/collections/index.html.erb

@@ -1,29 +1,25 @@
 <!--Descriptive page name and system messages, descriptions, and instructions.-->
 <h2 class="heading">
-  <% if @user %>
-    <%= t(".page_heading.users_collections", user: @user.login) %>
+  <% if @user || @tag %>
+    <%= search_header(@collections, @query, t(".page_heading.collection"), @owner) %>
   <% elsif @collection %>
-    <%= t(".page_heading.challenges_subcollections_in", collection: @collection.title) %>
+    <%= search_header(@collections, @query, t(".page_heading.subcollection"), @owner) %>
   <% elsif @work %>
-    <%= t(".page_heading.collections_including", work: @work.title) %>
+    <%= t(".page_heading.collections_including", work: @work.title, collections: search_header(@collections, @query, t(".page_heading.collection"), @owner)) %>
   <% else %>
-    <%= t(".page_heading.collections_in_the", archive: ArchiveConfig.APP_NAME) %>
+    <%= t(".page_heading.collections_in_the", archive: ArchiveConfig.APP_NAME, collections: search_header(@collections, @query, t(".page_heading.collection"), @owner)) %>
   <% end %>
 </h2>
-
-<h3 class="heading">
-  <% if @collections.empty? %>
-    <%= t(".page_heading.no_results") %>
-  <% else %>
-    <%= search_header @collections, @query, t(".page_heading.search_header") %>
-  <% end %>
-</h3>
 <!--/descriptions-->
 
 <!--Subnavigation, sorting and actions-->
 <h3 class="landmark heading"><%= t(".navigation.landmark_heading") %></h3>
-<ul class="navigation actions">
-  <% if @collection %>
+<ul class="navigation actions" role="navigation">
+  <% if @tag %>
+    <li><%= span_if_current t(".navigation.actions.tag.works"), tag_works_path(@tag) %></li>
+    <li><%= span_if_current t(".navigation.actions.tag.bookmarks"), tag_bookmarks_path(@tag) %></li>
+    <li><%= span_if_current t(".navigation.actions.tag.collections"), tag_collections_path(@tag) %></li>
+  <% elsif @collection %>
     <%# Collection subcollections index gets link for Subcollections %>
     <li><%= span_if_current t(".navigation.actions.subcollections"), collection_collections_path(@collection) %></li>
   <% elsif @user %>
@@ -34,15 +30,15 @@
       <li><%= link_to t(".navigation.actions.manage_items"), user_collection_items_path(@user) %></li>
     <% end %>
   <% else %>
-    <%# Collections and Open Challenges link unless on a user's collections page or collection's subcollections page %>
+    <%# Collections and Open Challenges link unless on a tag's/user's collections page or collection's subcollections page %>
     <li><%= span_if_current t(".navigation.actions.collections"), collections_path %></li>
     <li><%= link_to t(".navigation.actions.open_challenges"), list_challenges_collections_path %></li>
   <% end %>
   <% if logged_in? %>
     <%# Logged in collection maintainer on own collection gets link for New Subcollection and all other logged in users get New Collection link %>
     <% if @collection && !@collection.parent && @collection.user_is_maintainer?(current_user) %>
       <li><%= link_to t(".navigation.actions.new_subcollection"), new_collection_collection_path(@collection) %></li>
-    <% else %>
+    <% elsif !@tag %>
       <li><%= link_to t(".navigation.actions.new_collection"), new_collection_path %></li>
     <% end %>
   <% end %>
@@ -54,7 +50,7 @@
 
 <% unless @collections.blank? %>
   <!--pagination here-->
-  <%= will_paginate @collections %>
+  <%= pagy_nav @pagy %>
 
   <!--main content-->
   <h3 class="landmark heading"><%= t(".main_content.landmark_heading") %></h3>
@@ -70,5 +66,5 @@
 <% end %>
 
 <% unless @collections.blank? %>
-  <%= will_paginate @collections %>
+  <%= pagy_nav @pagy %>
 <% end %>

app/views/tag_wranglings/_wrangler_dashboard.html.erb

@@ -30,6 +30,11 @@
               <%= span_if_current t(".use_type_and_count", use_type: t(".use_type.external_works"), count: @counts[key]),
                     { controller: :bookmarks, action: :index, tag_id: @tag } %>
             </li>
+            <% elsif key == "Collections" %>
+              <li>
+              <%= span_if_current t(".use_type_and_count", use_type: t(".use_type.collections"), count: @counts[key]),
+                    tag_collections_path(@tag) %>
+            </li>
             <% else %>
               <li>
               <span>