From 54d160b147aeb1379ca951060501da9bf675185a Mon Sep 17 00:00:00 2001 From: aidewoode Date: Tue, 7 May 2024 15:48:44 +0800 Subject: [PATCH] Avoid using global params in view partial --- app/helpers/application_helper.rb | 12 ------- app/presenters/filter_sort_presenter.rb | 31 +++++++++++++++++++ app/views/albums/_filters.html.erb | 6 ++-- app/views/albums/filter/genres/index.html.erb | 2 +- app/views/albums/filter/years/index.html.erb | 2 +- app/views/albums/index.html.erb | 6 ++-- app/views/artists/index.html.erb | 2 +- app/views/layouts/application.html.erb | 4 +-- app/views/playlists/index.html.erb | 2 +- app/views/shared/_filter_options.html.erb | 4 +-- app/views/shared/_nav_bar.html.erb | 4 +-- app/views/shared/_search_bar.html.erb | 8 ++--- app/views/shared/_sort_select.html.erb | 8 ++--- app/views/songs/_filters.html.erb | 6 ++-- app/views/songs/filter/genres/index.html.erb | 2 +- app/views/songs/filter/years/index.html.erb | 2 +- app/views/songs/index.html.erb | 6 ++-- 17 files changed, 65 insertions(+), 42 deletions(-) create mode 100644 app/presenters/filter_sort_presenter.rb diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index fb773347..90f47263 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -76,10 +76,6 @@ def format_number(number) number_to_human(number, units: {thousand: "K", million: "M", billion: "B"}, precision: 1, significant: false) end - def is_active?(controller: "", path: "") - params[:controller].in?(Array(controller)) || (path.is_a?(Regexp) ? (path =~ request.path) : (path == request.path)) - end - def page_title_tag(title) title_suffix = " - #{t(:app_name)}" title = "#{title}#{title_suffix unless native_app? || dialog?}" @@ -90,12 +86,4 @@ def page_title_tag(title) def current_url url_for(only_path: false) end - - def filter_sort_params(args = {}) - filter_params = params[:filter].presence || {} - filter_params = filter_params.merge(args.delete(:filter) || {}) - new_params = params.merge(args).merge(filter: filter_params) - - new_params.slice(:filter, :sort, :sort_direction).permit! - end end diff --git a/app/presenters/filter_sort_presenter.rb b/app/presenters/filter_sort_presenter.rb new file mode 100644 index 00000000..40e06e57 --- /dev/null +++ b/app/presenters/filter_sort_presenter.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class FilterSortPresenter + def initialize(params) + @params = params + end + + def has_filter? + params[:filter].present? + end + + def filter_value(key) + params[:filter].fetch(key, nil) + end + + def sort_value + params[:sort] + end + + def sort_direction_value + params[:sort_direction] + end + + def params(other_hash = {}) + filter_params = @params[:filter].presence || {} + filter_params = filter_params.merge(other_hash.delete(:filter) || {}) + new_params = @params.merge(other_hash).merge(filter: filter_params) + + new_params.slice(:filter, :sort, :sort_direction).permit! + end +end diff --git a/app/views/albums/_filters.html.erb b/app/views/albums/_filters.html.erb index fd2d2c50..ba901f0e 100644 --- a/app/views/albums/_filters.html.erb +++ b/app/views/albums/_filters.html.erb @@ -1,4 +1,4 @@ -<% if filter_sort_params[:filter].present? %> +<% if filter_sort_presenter.has_filter? %> <%= link_to albums_path, class: "c-button c-button--outline c-button--small u-mr-small", data: {"turbo-action" => ("replace" if native_app?)} do %> <%= icon_tag "close" %> @@ -15,7 +15,7 @@
- <%= turbo_frame_tag "turbo-genre-filter", src: albums_filter_genres_path(**filter_sort_params), loading: "lazy" do %> + <%= turbo_frame_tag "turbo-genre-filter", src: albums_filter_genres_path(**filter_sort_presenter.params), loading: "lazy" do %>
<%= loader_tag size: "small" %>
@@ -31,7 +31,7 @@
- <%= turbo_frame_tag "turbo-year-filter", src: albums_filter_years_path(**filter_sort_params), loading: "lazy" do %> + <%= turbo_frame_tag "turbo-year-filter", src: albums_filter_years_path(**filter_sort_presenter.params), loading: "lazy" do %>
<%= loader_tag size: "small" %>
diff --git a/app/views/albums/filter/genres/index.html.erb b/app/views/albums/filter/genres/index.html.erb index 4b1a2660..4321396a 100644 --- a/app/views/albums/filter/genres/index.html.erb +++ b/app/views/albums/filter/genres/index.html.erb @@ -1 +1 @@ -<%= render partial: "shared/filter_options", locals: {filter_name: "genre", options: @genres, filter_controller: "/albums"} %> +<%= render partial: "shared/filter_options", locals: {filter_name: "genre", options: @genres, filter_controller: "/albums", filter_sort_presenter: FilterSortPresenter.new(params)} %> diff --git a/app/views/albums/filter/years/index.html.erb b/app/views/albums/filter/years/index.html.erb index 4b056da1..c03f271c 100644 --- a/app/views/albums/filter/years/index.html.erb +++ b/app/views/albums/filter/years/index.html.erb @@ -1 +1 @@ -<%= render partial: "shared/filter_options", locals: {filter_name: "year", options: @years, filter_controller: "/albums"} %> +<%= render partial: "shared/filter_options", locals: {filter_name: "year", options: @years, filter_controller: "/albums", filter_sort_presenter: FilterSortPresenter.new(params)} %> diff --git a/app/views/albums/index.html.erb b/app/views/albums/index.html.erb index b3bd7e7f..7278a293 100644 --- a/app/views/albums/index.html.erb +++ b/app/views/albums/index.html.erb @@ -1,13 +1,15 @@ <% page_title_tag t("label.albums") %> +<% filter_sort_presenter = FilterSortPresenter.new(params) %> +
<% unless native_app? %>

<%= t("label.albums") %>

<% end %>
- <%= render partial: "albums/filters" %> - <%= render partial: "shared/sort_select", locals: {option: @sort_option} %> + <%= render partial: "albums/filters", locals: {filter_sort_presenter: filter_sort_presenter} %> + <%= render partial: "shared/sort_select", locals: {option: @sort_option, sort_controller: controller_name, filter_sort_presenter: filter_sort_presenter} %>
<% if @albums.empty? %> diff --git a/app/views/artists/index.html.erb b/app/views/artists/index.html.erb index f42d040c..0e67b374 100644 --- a/app/views/artists/index.html.erb +++ b/app/views/artists/index.html.erb @@ -6,7 +6,7 @@

<%= t("label.artists") %>

<% end %>
- <%= render partial: "shared/sort_select", locals: {option: @sort_option} %> + <%= render partial: "shared/sort_select", locals: {option: @sort_option, sort_controller: controller_name, filter_sort_presenter: FilterSortPresenter.new(params)} %>
<% if @artists.empty? %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 0057fe47..daf9f992 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -27,8 +27,8 @@
<%= render "shared/flash" %> - <%= render "shared/search_bar" %> - <%= render "shared/nav_bar" %> + <%= render partial: "shared/search_bar", locals: {current_user: Current.user, current_session: Current.session, query: params[:query]} %> + <%= render partial: "shared/nav_bar", locals: {current_controller: controller_name} %>
<%= yield %> diff --git a/app/views/playlists/index.html.erb b/app/views/playlists/index.html.erb index 095dadf5..0a7b0d20 100644 --- a/app/views/playlists/index.html.erb +++ b/app/views/playlists/index.html.erb @@ -10,7 +10,7 @@
- <%= render partial: "shared/sort_select", locals: {option: @sort_option} %> + <%= render partial: "shared/sort_select", locals: {option: @sort_option, sort_controller: controller_name, filter_sort_presenter: FilterSortPresenter.new(params)} %>
diff --git a/app/views/shared/_filter_options.html.erb b/app/views/shared/_filter_options.html.erb index b238d23f..570601b3 100644 --- a/app/views/shared/_filter_options.html.erb +++ b/app/views/shared/_filter_options.html.erb @@ -1,7 +1,7 @@ <%= turbo_frame_tag "turbo-#{filter_name}-filter".dasherize do %> <% options.each do |option| %> <%= link_to( - url_for(controller: filter_controller, **filter_sort_params(filter: {filter_name => option})), + url_for(controller: filter_controller, **filter_sort_presenter.params(filter: {filter_name => option})), class: "c-dropdown__item", data: { "turbo-frame" => "_top", @@ -10,7 +10,7 @@ ) do %> <%= option %> - <% if params[:filter]&.fetch(filter_name, nil) == option.to_s %> + <% if filter_sort_presenter.filter_value(filter_name) == option.to_s %> <%= icon_tag("check", size: "small", class: "u-ml-narrow") %> <% end %> diff --git a/app/views/shared/_nav_bar.html.erb b/app/views/shared/_nav_bar.html.erb index 493fcad7..4a0596fd 100644 --- a/app/views/shared/_nav_bar.html.erb +++ b/app/views/shared/_nav_bar.html.erb @@ -1,9 +1,9 @@
- <%= text_field_tag "query", params[:query], class: "c-input", data: {"search-target" => "input", "test-id" => "search_input"}, autocomplete: "on" %> + <%= text_field_tag "query", query, class: "c-input", data: {"search-target" => "input", "test-id" => "search_input"}, autocomplete: "on" %>
<%= loader_tag size: "small" %> @@ -15,12 +15,12 @@
- <%= avatar_tag Current.user %> + <%= avatar_tag current_user %>
<%= link_to t("label.settings"), setting_path, class: "c-dropdown__item" %> <%= link_to t("label.manage_users"), users_path, class: "c-dropdown__item" if is_admin? %> - <%= link_to t("label.update_profile"), edit_user_path(Current.user), class: "c-dropdown__item" %> - <%= button_to t("button.logout"), Current.session, method: :delete, form_class: "c-dropdown__item" %> + <%= link_to t("label.update_profile"), edit_user_path(current_user), class: "c-dropdown__item" %> + <%= button_to t("button.logout"), current_session, method: :delete, form_class: "c-dropdown__item" %>
diff --git a/app/views/shared/_sort_select.html.erb b/app/views/shared/_sort_select.html.erb index 73fa38fc..5b7cf5fb 100644 --- a/app/views/shared/_sort_select.html.erb +++ b/app/views/shared/_sort_select.html.erb @@ -4,10 +4,10 @@
<% option.values.each do |sort_value| %> - <%= link_to url_for(controller: params[:controller], action: :index, **filter_sort_params(sort: sort_value)), class: "c-dropdown__item", data: {"turbo-action" => ("replace" if native_app?)} do %> + <%= link_to url_for(controller: sort_controller, action: :index, **filter_sort_presenter.params(sort: sort_value)), class: "c-dropdown__item", data: {"turbo-action" => ("replace" if native_app?)} do %> <%= t("field.#{sort_value}") %> - <% if params[:sort] == sort_value || (params[:sort].blank? && sort_value == option.default.name) %> + <% if filter_sort_presenter.sort_value == sort_value || (filter_sort_presenter.sort_value.blank? && sort_value == option.default.name) %> <%= icon_tag("check", size: "small", class: "u-ml-narrow") %> <% end %> @@ -17,10 +17,10 @@
<% %w[asc desc].each do |sort_direction| %> - <%= link_to url_for(controller: params[:controller], action: :index, **filter_sort_params(sort: params[:sort] || option.default.name, sort_direction: sort_direction)), class: "c-dropdown__item", data: {"turbo-action" => ("replace" if native_app?)} do %> + <%= link_to url_for(controller: sort_controller, action: :index, **filter_sort_presenter.params(sort: filter_sort_presenter.sort_value || option.default.name, sort_direction: sort_direction)), class: "c-dropdown__item", data: {"turbo-action" => ("replace" if native_app?)} do %> <%= t("label.#{sort_direction}") %> - <% if params[:sort_direction] == sort_direction || (params[:sort_direction].blank? && option.default.direction == sort_direction) %> + <% if filter_sort_presenter.sort_direction_value == sort_direction || (filter_sort_presenter.sort_direction_value.blank? && option.default.direction == sort_direction) %> <%= icon_tag("check", size: "small", class: "u-ml-narrow") %> <% end %> diff --git a/app/views/songs/_filters.html.erb b/app/views/songs/_filters.html.erb index 6838358e..cf3bb5e3 100644 --- a/app/views/songs/_filters.html.erb +++ b/app/views/songs/_filters.html.erb @@ -1,4 +1,4 @@ -<% if filter_sort_params[:filter].present? %> +<% if filter_sort_presenter.has_filter? %> <%= link_to songs_path, class: "c-button c-button--outline c-button--small u-mr-small", data: {"turbo-action" => ("replace" if native_app?)} do %> <%= icon_tag "close" %> @@ -15,7 +15,7 @@
- <%= turbo_frame_tag "turbo-album-genre-filter", src: songs_filter_genres_path(**filter_sort_params), loading: "lazy" do %> + <%= turbo_frame_tag "turbo-album-genre-filter", src: songs_filter_genres_path(**filter_sort_presenter.params), loading: "lazy" do %>
<%= loader_tag size: "small" %>
@@ -31,7 +31,7 @@
- <%= turbo_frame_tag "turbo-album-year-filter", src: songs_filter_years_path(**filter_sort_params), loading: "lazy" do %> + <%= turbo_frame_tag "turbo-album-year-filter", src: songs_filter_years_path(**filter_sort_presenter.params), loading: "lazy" do %>
<%= loader_tag size: "small" %>
diff --git a/app/views/songs/filter/genres/index.html.erb b/app/views/songs/filter/genres/index.html.erb index f7cc4814..62d7121e 100644 --- a/app/views/songs/filter/genres/index.html.erb +++ b/app/views/songs/filter/genres/index.html.erb @@ -1 +1 @@ -<%= render partial: "shared/filter_options", locals: {filter_name: "album_genre", options: @genres, filter_controller: "/songs"} %> +<%= render partial: "shared/filter_options", locals: {filter_name: "album_genre", options: @genres, filter_controller: "/songs", filter_sort_presenter: FilterSortPresenter.new(params)} %> diff --git a/app/views/songs/filter/years/index.html.erb b/app/views/songs/filter/years/index.html.erb index 780e865f..3144a224 100644 --- a/app/views/songs/filter/years/index.html.erb +++ b/app/views/songs/filter/years/index.html.erb @@ -1 +1 @@ -<%= render partial: "shared/filter_options", locals: {filter_name: "album_year", options: @years, filter_controller: "/songs"} %> +<%= render partial: "shared/filter_options", locals: {filter_name: "album_year", options: @years, filter_controller: "/songs", filter_sort_presenter: FilterSortPresenter.new(params)} %> diff --git a/app/views/songs/index.html.erb b/app/views/songs/index.html.erb index adc87fc5..9f9e4512 100644 --- a/app/views/songs/index.html.erb +++ b/app/views/songs/index.html.erb @@ -1,13 +1,15 @@ <% page_title_tag t("label.songs") %> +<% filter_sort_presenter = FilterSortPresenter.new(params) %> +
<% unless native_app? %>

<%= t("label.songs") %>

<% end %>
- <%= render partial: "songs/filters" %> - <%= render partial: "shared/sort_select", locals: {option: @sort_option} %> + <%= render partial: "songs/filters", locals: {filter_sort_presenter: filter_sort_presenter} %> + <%= render partial: "shared/sort_select", locals: {option: @sort_option, sort_controller: controller_name, filter_sort_presenter: filter_sort_presenter} %>
<% if @songs.empty? %>