diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3fa550fb219..c038d3722c5 100644 --- a/app/controllers/application_controller.rb +++ b/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 diff --git a/app/controllers/bookmarks_controller.rb b/app/controllers/bookmarks_controller.rb index c4f43b87f1c..17b24ee630e 100644 --- a/app/controllers/bookmarks_controller.rb +++ b/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 diff --git a/app/controllers/collections_controller.rb b/app/controllers/collections_controller.rb index 505a0791c35..9ed13baded8 100644 --- a/app/controllers/collections_controller.rb +++ b/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 diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 2a2929a326d..c18ee810a5e 100755 --- a/app/controllers/works_controller.rb +++ b/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 diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index a98848c890c..2e8b61dbbbb 100644 --- a/app/helpers/search_helper.rb +++ b/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 diff --git a/app/views/bookmarks/index.html.erb b/app/views/bookmarks/index.html.erb index 7293d1419f2..222b70f8a57 100755 --- a/app/views/bookmarks/index.html.erb +++ b/app/views/bookmarks/index.html.erb @@ -31,6 +31,7 @@ <% if @tag %>
  • <%= span_if_current t(".navigation.works"), tag_works_path(@tag) %>
  • <%= span_if_current t(".navigation.bookmarks"), tag_bookmarks_path(@tag) %>
  • +
  • <%= span_if_current t(".navigation.collections"), tag_collections_path(@tag) %>
  • <% end %> <% if @facets.present? %> <% # Filters button for narrow screens jumps to filters when JavaScript is disabled and opens filters when JavaScript is enabled %> diff --git a/app/views/collection_profile/show.html.erb b/app/views/collection_profile/show.html.erb index 606d638f81a..d51bc95bbc0 100644 --- a/app/views/collection_profile/show.html.erb +++ b/app/views/collection_profile/show.html.erb @@ -19,7 +19,7 @@
    diff --git a/app/views/collections/_collection_blurb.html.erb b/app/views/collections/_collection_blurb.html.erb index 4697a9952b2..f15a94724d9 100644 --- a/app/views/collections/_collection_blurb.html.erb +++ b/app/views/collections/_collection_blurb.html.erb @@ -17,7 +17,7 @@ <% unless collection.tag_string.blank? %>
    <%= t(".collection_tags") %> - <%= 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")) %>
    <% end %> diff --git a/app/views/collections/_filters.html.erb b/app/views/collections/_filters.html.erb index b6994f397e8..10809b5d523 100644 --- a/app/views/collections/_filters.html.erb +++ b/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", diff --git a/app/views/collections/index.html.erb b/app/views/collections/index.html.erb index 5582648c664..a0dcd6dff5d 100755 --- a/app/views/collections/index.html.erb +++ b/app/views/collections/index.html.erb @@ -1,29 +1,25 @@

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

    - -

    - <% if @collections.empty? %> - <%= t(".page_heading.no_results") %> - <% else %> - <%= search_header @collections, @query, t(".page_heading.search_header") %> - <% end %> -

    <%= t(".navigation.landmark_heading") %>

    -