From 7fc774dc82050a07e22fa10a18ed87e0f6372e3e Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 16:58:38 +0100 Subject: [PATCH 01/23] use default pagination settings if there is no current user --- app/controllers/collections_controller.rb | 4 ++-- app/controllers/creators_controller.rb | 4 ++-- app/controllers/models_controller.rb | 4 ++-- app/helpers/application_helper.rb | 4 ++++ app/views/collections/index.html.erb | 6 +++--- app/views/collections/show.html.erb | 4 ++-- app/views/creators/index.html.erb | 6 +++--- app/views/models/index.html.erb | 4 ++-- 8 files changed, 20 insertions(+), 16 deletions(-) diff --git a/app/controllers/collections_controller.rb b/app/controllers/collections_controller.rb index 93ade2fce..5726d9f21 100644 --- a/app/controllers/collections_controller.rb +++ b/app/controllers/collections_controller.rb @@ -23,9 +23,9 @@ def index @collections.order(name: :asc) end - if current_user.pagination_settings["collections"] + if helpers.pagination_settings["collections"] page = params[:page] || 1 - @collections = @collections.page(page).per(current_user.pagination_settings["per_page"]) + @collections = @collections.page(page).per(helpers.pagination_settings["per_page"]) end # Eager load @collections = @collections.includes :collections, :collection, :links, models: [:preview_file, :library] diff --git a/app/controllers/creators_controller.rb b/app/controllers/creators_controller.rb index b9376a7ca..d7b92b0e0 100644 --- a/app/controllers/creators_controller.rb +++ b/app/controllers/creators_controller.rb @@ -23,9 +23,9 @@ def index @creators.order(name: :asc) end - if current_user.pagination_settings["creators"] + if helpers.pagination_settings["creators"] page = params[:page] || 1 - @creators = @creators.page(page).per(current_user.pagination_settings["per_page"]) + @creators = @creators.page(page).per(helpers.pagination_settings["per_page"]) end # Eager load data @creators = @creators.includes(:links, :models) diff --git a/app/controllers/models_controller.rb b/app/controllers/models_controller.rb index a02f8f766..75074bd76 100644 --- a/app/controllers/models_controller.rb +++ b/app/controllers/models_controller.rb @@ -24,9 +24,9 @@ def index @models.order(name: :asc) end - if current_user.pagination_settings["models"] + if helpers.pagination_settings["models"] page = params[:page] || 1 - @models = @models.page(page).per(current_user.pagination_settings["per_page"]) + @models = @models.page(page).per(helpers.pagination_settings["per_page"]) end @tags, @unrelated_tag_count = generate_tag_list(@filters.empty? ? nil : @models, @filter_tags) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 9039e6c43..6449cb23f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -150,4 +150,8 @@ def translate_with_locale_wrapper(key, **options) end end alias_method :t, :translate_with_locale_wrapper + + def pagination_settings + current_user&.pagination_settings || SiteSettings::UserDefaults::PAGINATION + end end diff --git a/app/views/collections/index.html.erb b/app/views/collections/index.html.erb index 769918049..ca4d5d161 100644 --- a/app/views/collections/index.html.erb +++ b/app/views/collections/index.html.erb @@ -5,17 +5,17 @@ <% content_for :items do %>
<%= skip_link "sidebar", t(".skip_collections") %> - <% if current_user.pagination_settings["collections"] %> + <% if pagination_settings["collections"] %> <%= paginate @collections %> <% end %>
- <%= render "unassigned" if (!current_user.pagination_settings["collections"] || @collections.first_page?) && !@filters[:collection] %> + <%= render "unassigned" if (!pagination_settings["collections"] || @collections.first_page?) && !@filters[:collection] %> <% if @filters[:collection] && @collection %> <%= render @collection %> <% end %> <%= render @collections %>
- <% if current_user.pagination_settings["collections"] %> + <% if pagination_settings["collections"] %> <%= paginate @collections %> <% end %>
diff --git a/app/views/collections/show.html.erb b/app/views/collections/show.html.erb index 601c7d7e6..5d19056ae 100644 --- a/app/views/collections/show.html.erb +++ b/app/views/collections/show.html.erb @@ -13,13 +13,13 @@
- <% if current_user.pagination_settings["models"] %> + <% if pagination_settings["models"] %> <%= paginate @models %> <% end %>
<%= render partial: "model", collection: @models %>
- <% if current_user.pagination_settings["models"] %> + <% if pagination_settings["models"] %> <%= paginate @models %> <% end %>
diff --git a/app/views/creators/index.html.erb b/app/views/creators/index.html.erb index 15eaaf296..cc802edd5 100644 --- a/app/views/creators/index.html.erb +++ b/app/views/creators/index.html.erb @@ -5,14 +5,14 @@ <% content_for :items do %>
<%= skip_link "sidebar", t(".skip_creators") %> - <% if current_user.pagination_settings["creators"] %> + <% if pagination_settings["creators"] %> <%= paginate @creators %> <% end %>
- <%= render "unassigned" if !current_user.pagination_settings["creators"] || @creators.first_page? %> + <%= render "unassigned" if !pagination_settings["creators"] || @creators.first_page? %> <%= render @creators %>
- <% if current_user.pagination_settings["creators"] %> + <% if pagination_settings["creators"] %> <%= paginate @creators %> <% end %>
diff --git a/app/views/models/index.html.erb b/app/views/models/index.html.erb index badff2bdc..4d8925700 100644 --- a/app/views/models/index.html.erb +++ b/app/views/models/index.html.erb @@ -5,13 +5,13 @@ <% content_for :items do %>
<%= skip_link "sidebar", t(".skip_models") %> - <% if current_user.pagination_settings["models"] %> + <% if pagination_settings["models"] %> <%= paginate @models %> <% end %>
<%= render ModelComponent.with_collection(@models, can_show: @can_show, can_edit: @can_edit, can_destroy: @can_destroy) %>
- <% if current_user.pagination_settings["models"] %> + <% if pagination_settings["models"] %> <%= paginate @models %> <% end %>
From 4253f5110e48471964b45c191205421e14dde850 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 16:59:07 +0100 Subject: [PATCH 02/23] handle logged-out state in scope resolve --- app/policies/application_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index 6e7db6613..4295f8d32 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -57,7 +57,7 @@ def initialize(user, scope) end def resolve - return scope.all if user.is_moderator? || !scope.respond_to?(:granted_to) + return scope.all if user&.is_moderator? || !scope.respond_to?(:granted_to) result = scope.granted_to(["view", "edit", "own"], [user, nil]) result = result.or(scope.granted_to(["view", "edit", "own"], user.roles)) if user From fd319c7235f8b87f9165fed0c0919d83b89f568f Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 17:00:55 +0100 Subject: [PATCH 03/23] use default tag list settings if logged out --- app/controllers/concerns/tag_listable.rb | 6 +++--- app/helpers/application_helper.rb | 4 ++++ app/views/application/_tag_list.html.erb | 2 +- app/views/models/index.html.erb | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/controllers/concerns/tag_listable.rb b/app/controllers/concerns/tag_listable.rb index b67f7c841..b03e28078 100644 --- a/app/controllers/concerns/tag_listable.rb +++ b/app/controllers/concerns/tag_listable.rb @@ -1,13 +1,13 @@ module TagListable def generate_tag_list(models = nil, filter_tags = nil) # All tags bigger than threshold - tags = all_tags = ActsAsTaggableOn::Tag.where(taggings_count: current_user.tag_cloud_settings["threshold"]..) + tags = all_tags = ActsAsTaggableOn::Tag.where(taggings_count: helpers.tag_cloud_settings["threshold"]..) # Ignore any tags that have been applied as filters tags = all_tags = tags.where.not(id: filter_tags) if filter_tags # Generate a list of tags shared by the list of models tags = tags.includes(:taggings).where("taggings.taggable": models.map(&:id)) if models # Apply tag sorting - tags = case current_user.tag_cloud_settings["sorting"] + tags = case helpers.tag_cloud_settings["sorting"] when "alphabetical" tags.order(name: :asc) else @@ -23,7 +23,7 @@ def generate_tag_list(models = nil, filter_tags = nil) def split_key_value_tags(tags) # Split into plain tags and key-value tags - if current_user.tag_cloud_settings["keypair"] + if helpers.tag_cloud_settings["keypair"] plain_tags = tags.where.not("name LIKE '%:%'") kv_tags = tags.where("name LIKE '%:%'") else diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6449cb23f..27e3de966 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -154,4 +154,8 @@ def translate_with_locale_wrapper(key, **options) def pagination_settings current_user&.pagination_settings || SiteSettings::UserDefaults::PAGINATION end + + def tag_cloud_settings + current_user&.tag_cloud_settings || SiteSettings::UserDefaults::TAG_CLOUD + end end diff --git a/app/views/application/_tag_list.html.erb b/app/views/application/_tag_list.html.erb index 6788c5a03..b4f0e27dc 100644 --- a/app/views/application/_tag_list.html.erb +++ b/app/views/application/_tag_list.html.erb @@ -9,7 +9,7 @@ end %> <% end %> <%- tag_html_opts = {data: {bulk_item_tags: defined?(model_id) ? model_id&.to_s : nil}} %> -<%- heatmap = defined?(show_count) ? heatmap : current_user.tag_cloud_settings["heatmap"] %> +<%- heatmap = defined?(show_count) ? heatmap : tag_cloud_settings["heatmap"] %> <%= safe_join(tags.map { |tag| render TagComponent.new(tag: tag, filters: @filters, html_options: tag_html_opts, show_count: heatmap) }, " ") if tags %> <% if defined?(kv_tags) && kv_tags %>
    diff --git a/app/views/models/index.html.erb b/app/views/models/index.html.erb index 4d8925700..42801b211 100644 --- a/app/views/models/index.html.erb +++ b/app/views/models/index.html.erb @@ -20,7 +20,7 @@ <% content_for :sidebar do %> <%= card :secondary, t(".actions_heading") do %> <%= link_to t(".bulk_edit"), edit_models_path(@filters), class: "btn btn-secondary mb-3 me-3" if policy(:model).edit? %> - <% if current_user.tag_cloud_settings["heatmap"] && Model.where("(select count(*) from taggings where taggings.taggable_id=models.id and taggings.context='tags')<1").count > 0 %> + <% if tag_cloud_settings["heatmap"] && Model.where("(select count(*) from taggings where taggings.taggable_id=models.id and taggings.context='tags')<1").count > 0 %> <%= link_to t(".untagged"), models_path(tag: [""]), class: "btn btn-secondary mb-3 me-3" %> <% end %> <% if Model.where("(select count(*) from links where linkable_id=models.id and linkable_type='Model')<1").count > 0 %> From 8ec2e20cfbf98479360f151256b9713e5ce2d496 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 17:03:05 +0100 Subject: [PATCH 04/23] use default renderer settings if logged out --- app/helpers/application_helper.rb | 4 ++++ app/views/application/_object_preview.html.erb | 16 ++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 27e3de966..a3aedae8e 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -158,4 +158,8 @@ def pagination_settings def tag_cloud_settings current_user&.tag_cloud_settings || SiteSettings::UserDefaults::TAG_CLOUD end + + def renderer_settings + current_user&.renderer_settings || SiteSettings::UserDefaults::RENDERER + end end diff --git a/app/views/application/_object_preview.html.erb b/app/views/application/_object_preview.html.erb index 593a75c3b..ef7e18b82 100644 --- a/app/views/application/_object_preview.html.erb +++ b/app/views/application/_object_preview.html.erb @@ -5,14 +5,14 @@ data-worker-url="<%= javascript_path "offscreen_renderer.js" %>" data-format="<%= file.extension %>" data-y-up="<%= file.y_up %>" - data-grid-size-x="<%= current_user.renderer_settings["grid_width"] %>" - data-grid-size-z="<%= current_user.renderer_settings["grid_depth"] %>" - data-show-grid="<%= current_user.renderer_settings["show_grid"] %>" - data-enable-pan-zoom="<%= current_user.renderer_settings["enable_pan_zoom"] %>" - data-background-colour="<%= current_user.renderer_settings["background_colour"] %>" - data-object-colour="<%= current_user.renderer_settings["object_colour"] %>" - data-render-style="<%= current_user.renderer_settings["render_style"] %>" - data-auto-load="<%= ((file.size || 9_999_999.megabytes) < ((current_user.renderer_settings["auto_load_max_size"] || 9_999_999).megabytes)) ? "true" : "false" %>"> + data-grid-size-x="<%= renderer_settings["grid_width"] %>" + data-grid-size-z="<%= renderer_settings["grid_depth"] %>" + data-show-grid="<%= renderer_settings["show_grid"] %>" + data-enable-pan-zoom="<%= renderer_settings["enable_pan_zoom"] %>" + data-background-colour="<%= renderer_settings["background_colour"] %>" + data-object-colour="<%= renderer_settings["object_colour"] %>" + data-render-style="<%= renderer_settings["render_style"] %>" + data-auto-load="<%= ((file.size || 9_999_999.megabytes) < ((renderer_settings["auto_load_max_size"] || 9_999_999).megabytes)) ? "true" : "false" %>">
    - <%= link_to t(".open_button.text"), @model, {class: "btn btn-primary btn-sm", "aria-label": translate(".open_button.label", name: @model.name)} if @can_show %> + <%= link_to t(".open_button.text"), @model, {class: "btn btn-primary btn-sm", "aria-label": translate(".open_button.label", name: @model.name)} %> <%= link_to helpers.icon("pencil-fill", t(".edit_button.text")), edit_model_path(@model), {class: "btn btn-outline-secondary btn-sm", "aria-label": translate(".edit_button.label", name: @model.name)} if @can_edit %> <%= link_to helpers.icon("trash", t(".delete_button.text")), model_path(@model), {method: :delete, class: "btn btn-outline-danger btn-sm", "aria-label": translate(".delete_button.label", name: @model.name), data: {confirm: translate("models.destroy.confirm")}} if @can_destroy %>
    diff --git a/app/components/model_component.rb b/app/components/model_component.rb index af83e3b40..a758796a9 100644 --- a/app/components/model_component.rb +++ b/app/components/model_component.rb @@ -1,10 +1,9 @@ # frozen_string_literal: true class ModelComponent < ViewComponent::Base - def initialize(model:, can_show: false, can_edit: false, can_destroy: false) + def initialize(model:, can_edit: false, can_destroy: false) @model = model @can_destroy = can_destroy - @can_show = can_show @can_edit = can_edit end end diff --git a/app/controllers/models_controller.rb b/app/controllers/models_controller.rb index 25e6e2c69..8f31c0b35 100644 --- a/app/controllers/models_controller.rb +++ b/app/controllers/models_controller.rb @@ -10,7 +10,6 @@ class ModelsController < ApplicationController def index # Work out policies for showing buttons up front - @can_show = policy(Model).show? @can_destroy = policy(Model).destroy? @can_edit = policy(Model).edit? diff --git a/app/views/models/index.html.erb b/app/views/models/index.html.erb index 42801b211..81b4b6201 100644 --- a/app/views/models/index.html.erb +++ b/app/views/models/index.html.erb @@ -9,7 +9,7 @@ <%= paginate @models %> <% end %>
    - <%= render ModelComponent.with_collection(@models, can_show: @can_show, can_edit: @can_edit, can_destroy: @can_destroy) %> + <%= render ModelComponent.with_collection(@models, can_edit: @can_edit, can_destroy: @can_destroy) %>
    <% if pagination_settings["models"] %> <%= paginate @models %> From 34b5e1fb478a66209eed7e64c0a582ac94f428db Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:06:58 +0100 Subject: [PATCH 10/23] fix following policy --- app/policies/federails/following_policy.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/policies/federails/following_policy.rb b/app/policies/federails/following_policy.rb index 001ac5cb4..44fbb0f35 100644 --- a/app/policies/federails/following_policy.rb +++ b/app/policies/federails/following_policy.rb @@ -1,16 +1,16 @@ class Federails::FollowingPolicy < ApplicationPolicy def create? - any_of( - SiteSettings.multiuser_enabled?, - SiteSettings.federation_enabled? + all_of( + one_of( + SiteSettings.multiuser_enabled?, + SiteSettings.federation_enabled? + ), + @user ) end def destroy? - any_of( - SiteSettings.multiuser_enabled?, - SiteSettings.federation_enabled? - ) + create? end class Scope < ApplicationPolicy::Scope From 21bbd8801c18a7f7314f5d9efb4fc3c3ea687ec7 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:10:15 +0100 Subject: [PATCH 11/23] fix policy check for 3mf conversion --- app/views/model_files/show.html.erb | 2 +- app/views/problems/model_file/_inefficient.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/model_files/show.html.erb b/app/views/model_files/show.html.erb index 82c3b2623..d5fc92960 100644 --- a/app/views/model_files/show.html.erb +++ b/app/views/model_files/show.html.erb @@ -63,7 +63,7 @@ <%= card :secondary, t(".actions_heading") do %> <%= link_to safe_join([icon("cloud-download", t("general.download")), t("general.download")], " "), model_model_file_path(@model, @file, @file.extension.to_sym), {class: "btn btn-secondary"} %> - <% if policy(:model_file).create? && ["stl", "obj"].include?(@file.extension) %> + <% if policy(@file).edit? && ["stl", "obj"].include?(@file.extension) %> <%= link_to safe_join([icon("arrow-left-right", t(".convert")), t(".convert")], " "), model_model_files_path(@model, convert: {id: @file.id, to: "threemf"}), method: :post, class: "btn btn-warning" %> <% end %> <% end %> diff --git a/app/views/problems/model_file/_inefficient.html.erb b/app/views/problems/model_file/_inefficient.html.erb index a4ccae1ca..07b4595b4 100644 --- a/app/views/problems/model_file/_inefficient.html.erb +++ b/app/views/problems/model_file/_inefficient.html.erb @@ -2,7 +2,7 @@ <%= link_to problem.problematic.name, [problem.problematic.model, problem.problematic], class: "link-body-emphasis" %> <%= t ".title" %>: <%= problem.note %> - <% if policy(problem.problematic).show? && policy(:model_file).create? && ["stl", "obj"].include?(problem.problematic.extension) %> + <% if policy(problem.problematic).show? && policy(problem.problematic).create? && ["stl", "obj"].include?(problem.problematic.extension) %> <%= link_to t("model_files.show.convert"), model_model_files_path(problem.problematic.model, convert: {id: problem.problematic.id, to: "threemf"}), method: :post, class: "btn btn-warning" %> <% end %> From f9cf16bd642d50a9826eeaa13df21047595f17e8 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:14:24 +0100 Subject: [PATCH 12/23] fix problem with default settings when logged out --- app/models/problem.rb | 4 ++-- app/models/site_settings.rb | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/models/problem.rb b/app/models/problem.rb index 7bc6ff9d5..07da78571 100644 --- a/app/models/problem.rb +++ b/app/models/problem.rb @@ -31,7 +31,7 @@ class Problem < ApplicationRecord :danger ] - DEFAULT_SEVERITIES = { + DEFAULT_SEVERITIES = ActiveSupport::HashWithIndifferentAccess.new( missing: :danger, empty: :info, nesting: :warning, @@ -41,7 +41,7 @@ class Problem < ApplicationRecord no_3d_model: :silent, non_manifold: :warning, inside_out: :warning - } + ) def self.create_or_clear(problematic, cat, present, options = {}) if present diff --git a/app/models/site_settings.rb b/app/models/site_settings.rb index c332ed5cb..4f52e6740 100644 --- a/app/models/site_settings.rb +++ b/app/models/site_settings.rb @@ -58,7 +58,7 @@ def self.ignored_file?(pathname) end module UserDefaults - RENDERER = { + RENDERER = ActiveSupport::HashWithIndifferentAccess.new( grid_width: 200, grid_depth: 200, show_grid: true, @@ -66,24 +66,24 @@ module UserDefaults background_colour: "#000000", object_colour: "#cccccc", render_style: "normals" - } + ) - PAGINATION = { + PAGINATION = ActiveSupport::HashWithIndifferentAccess.new( models: true, creators: true, collections: true, per_page: 12 - } + ) - TAG_CLOUD = { + TAG_CLOUD = ActiveSupport::HashWithIndifferentAccess.new( threshold: 2, heatmap: true, keypair: true, sorting: "frequency" - } + ) - FILE_LIST = { + FILE_LIST = ActiveSupport::HashWithIndifferentAccess.new( hide_presupported_versions: true - } + ) end end From 541376e63094b81e19d5d150952efea3d44eb77e Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:15:35 +0100 Subject: [PATCH 13/23] lint --- app/policies/model_file_policy.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/policies/model_file_policy.rb b/app/policies/model_file_policy.rb index 7e2ed349a..24e5ab4dc 100644 --- a/app/policies/model_file_policy.rb +++ b/app/policies/model_file_policy.rb @@ -1,5 +1,4 @@ class ModelFilePolicy < ApplicationPolicy - def show? ModelPolicy.new(@user, @record.model).show? end From 420921c326d1b73671f6a06a96747e0fcf5b50d4 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:18:18 +0100 Subject: [PATCH 14/23] hide unpermitted buttons --- app/views/models/show.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/models/show.html.erb b/app/views/models/show.html.erb index 161cdd30a..6d9475168 100644 --- a/app/views/models/show.html.erb +++ b/app/views/models/show.html.erb @@ -60,7 +60,7 @@ <%= @model.path %> <% unless @model.contains_other_models? %> - <%= button_tag(t(".organize.button_text"), class: "btn btn-warning btn-sm float-end", "data-bs-toggle": "modal", "data-bs-target": "#confirm-move") if @model.needs_organizing? %> + <%= button_tag(t(".organize.button_text"), class: "btn btn-warning btn-sm float-end", "data-bs-toggle": "modal", "data-bs-target": "#confirm-move") if @model.needs_organizing? && policy(@model).edit? %> <% end %> @@ -71,7 +71,7 @@ <%= link_to safe_join([icon("pencil", t("general.edit")), t("general.edit")], " "), edit_model_path(@model), class: "btn btn-primary" if policy(@model).edit? %> <%= link_to safe_join([icon("trash", t("general.delete")), t("general.delete")], " "), model_path(@model), {method: :delete, class: "float-end btn btn-outline-danger", data: {confirm: translate("models.destroy.confirm")}} if policy(@model).destroy? %> - <%= render FollowButtonComponent.new(follower: current_user, target: @model) %> + <%= render FollowButtonComponent.new(follower: current_user, target: @model) if policy(Federails::Following).create? %> <% end %> <% if !@model.parents.empty? && policy(@model).merge? %> From d278a00c10a7d90d7f1c97541e59e66ccb2bf1be Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:19:49 +0100 Subject: [PATCH 15/23] hide unusable buttons --- app/views/models/index.html.erb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/views/models/index.html.erb b/app/views/models/index.html.erb index 81b4b6201..94291a0a7 100644 --- a/app/views/models/index.html.erb +++ b/app/views/models/index.html.erb @@ -20,11 +20,13 @@ <% content_for :sidebar do %> <%= card :secondary, t(".actions_heading") do %> <%= link_to t(".bulk_edit"), edit_models_path(@filters), class: "btn btn-secondary mb-3 me-3" if policy(:model).edit? %> - <% if tag_cloud_settings["heatmap"] && Model.where("(select count(*) from taggings where taggings.taggable_id=models.id and taggings.context='tags')<1").count > 0 %> - <%= link_to t(".untagged"), models_path(tag: [""]), class: "btn btn-secondary mb-3 me-3" %> - <% end %> - <% if Model.where("(select count(*) from links where linkable_id=models.id and linkable_type='Model')<1").count > 0 %> - <%= link_to t(".missing_url"), models_path(link: ""), class: "btn btn-secondary mb-3 me-3" %> + <% if current_user %> + <% if tag_cloud_settings["heatmap"] && Model.where("(select count(*) from taggings where taggings.taggable_id=models.id and taggings.context='tags')<1").count > 0 %> + <%= link_to t(".untagged"), models_path(tag: [""]), class: "btn btn-secondary mb-3 me-3" %> + <% end %> + <% if Model.where("(select count(*) from links where linkable_id=models.id and linkable_type='Model')<1").count > 0 %> + <%= link_to t(".missing_url"), models_path(link: ""), class: "btn btn-secondary mb-3 me-3" %> + <% end %> <% end %> <% if @creator %> <%= render FollowButtonComponent.new(follower: current_user, target: @creator, name: @creator.name) %> From 76c63e6a129e24518e13e4bddbb2ba486045b313 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:32:05 +0100 Subject: [PATCH 16/23] add a "no results" info message --- app/views/models/index.html.erb | 29 ++++++++++++++++++----------- config/locales/models/en.yml | 1 + 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/app/views/models/index.html.erb b/app/views/models/index.html.erb index 94291a0a7..6dd2f97c7 100644 --- a/app/views/models/index.html.erb +++ b/app/views/models/index.html.erb @@ -3,18 +3,25 @@ <% end %> <% content_for :items do %> -
    - <%= skip_link "sidebar", t(".skip_models") %> - <% if pagination_settings["models"] %> - <%= paginate @models %> - <% end %> -
    - <%= render ModelComponent.with_collection(@models, can_edit: @can_edit, can_destroy: @can_destroy) %> + <% if @models.empty? %> +
    + <%= icon "person-arms-up", "" %> + <%= t(".no_results_html") %>
    - <% if pagination_settings["models"] %> - <%= paginate @models %> - <% end %> -
    + <% else %> +
    + <%= skip_link "sidebar", t(".skip_models") %> + <% if pagination_settings["models"] %> + <%= paginate @models %> + <% end %> +
    + <%= render ModelComponent.with_collection(@models, can_edit: @can_edit, can_destroy: @can_destroy) %> +
    + <% if pagination_settings["models"] %> + <%= paginate @models %> + <% end %> +
    + <% end %> <% end %> <% content_for :sidebar do %> diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml index 8dc2e8755..c66c89ca3 100644 --- a/config/locales/models/en.yml +++ b/config/locales/models/en.yml @@ -40,6 +40,7 @@ en: actions_heading: Actions bulk_edit: Bulk Edit missing_url: Missing URL + no_results_html: Sorry, we couldn't find anything to show you! Try changing your filters or search terms, or uploading some models. skip_models: Skip model list untagged: Untagged merge: From a4a904cac6e567fd72f2e677fde4b286a0c7ddf5 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:33:51 +0100 Subject: [PATCH 17/23] auto show/hide follow button based on permissions --- app/components/follow_button_component.rb | 2 +- app/views/models/show.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/follow_button_component.rb b/app/components/follow_button_component.rb index 08017cb7b..01652144c 100644 --- a/app/components/follow_button_component.rb +++ b/app/components/follow_button_component.rb @@ -14,7 +14,7 @@ def before_render end def render? - SiteSettings.multiuser_enabled? || SiteSettings.federation_enabled? + (SiteSettings.multiuser_enabled? || SiteSettings.federation_enabled?) && helpers.policy(Federails::Following).create? end erb_template <<-ERB diff --git a/app/views/models/show.html.erb b/app/views/models/show.html.erb index 6d9475168..a92fa37b9 100644 --- a/app/views/models/show.html.erb +++ b/app/views/models/show.html.erb @@ -71,7 +71,7 @@ <%= link_to safe_join([icon("pencil", t("general.edit")), t("general.edit")], " "), edit_model_path(@model), class: "btn btn-primary" if policy(@model).edit? %> <%= link_to safe_join([icon("trash", t("general.delete")), t("general.delete")], " "), model_path(@model), {method: :delete, class: "float-end btn btn-outline-danger", data: {confirm: translate("models.destroy.confirm")}} if policy(@model).destroy? %> - <%= render FollowButtonComponent.new(follower: current_user, target: @model) if policy(Federails::Following).create? %> + <%= render FollowButtonComponent.new(follower: current_user, target: @model) %> <% end %> <% if !@model.parents.empty? && policy(@model).merge? %> From 53acb2fada6fef233c6bafa70a4486a467391fed Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:44:50 +0100 Subject: [PATCH 18/23] set policy scopes on model counts in creators and collections --- app/views/collections/_collection.html.erb | 4 ++-- app/views/collections/_unassigned.html.erb | 2 +- app/views/creators/_creator.html.erb | 2 +- app/views/creators/_unassigned.html.erb | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/collections/_collection.html.erb b/app/views/collections/_collection.html.erb index ae8605cf5..d67b55890 100644 --- a/app/views/collections/_collection.html.erb +++ b/app/views/collections/_collection.html.erb @@ -1,4 +1,4 @@ -<% model = collection.models.first %> +<% model = policy_scope(Model).where(collection: collection).first %>
    @@ -34,7 +34,7 @@
- <%= link_to "#{collection.models.count} #{Model.model_name.human count: collection.models.count}", {controller: "models", collection: collection.id}, class: "btn btn-primary", "aria-label": translate(".models_button.label", name: collection.name) if policy(collection).show? %> + <%= link_to "#{policy_scope(Model).where(collection: collection).count} #{Model.model_name.human count: policy_scope(Model).where(collection: collection).count}", {controller: "models", collection: collection.id}, class: "btn btn-primary", "aria-label": translate(".models_button.label", name: collection.name) if policy(collection).show? %> <%= link_to "#{collection.collections.count} #{Collection.model_name.human count: collection.collections.count}", (@filters || {}).merge(controller: "collections", collection: collection.id), class: "btn btn-primary", "aria-label": translate(".collections_button.label", name: collection.name) if collection.collections.count > 0 && policy(collection).show? %> <%= link_to icon("pencil-fill", t(".edit_button.text")), edit_collection_path(collection), {class: "btn btn-outline-secondary", "aria-label": translate(".edit_button.label", name: collection.name)} if policy(collection).edit? %>
diff --git a/app/views/collections/_unassigned.html.erb b/app/views/collections/_unassigned.html.erb index d6c08ffb7..813b1ac0f 100644 --- a/app/views/collections/_unassigned.html.erb +++ b/app/views/collections/_unassigned.html.erb @@ -4,7 +4,7 @@
<%= t(".name") %>
<%= t(".caption") %>
- <%= link_to "#{Model.where(collection: nil).count} #{Model.model_name.human count: Model.where(collection: nil).count}", models_path(collection: ""), {class: "btn btn-primary"} %> + <%= link_to "#{policy_scope(Model).where(collection: nil).count} #{Model.model_name.human count: policy_scope(Model).where(collection: nil).count}", models_path(collection: ""), {class: "btn btn-primary"} %>
diff --git a/app/views/creators/_creator.html.erb b/app/views/creators/_creator.html.erb index ec0d87ff7..38c94964c 100644 --- a/app/views/creators/_creator.html.erb +++ b/app/views/creators/_creator.html.erb @@ -13,7 +13,7 @@
  • <%= link_to t("sites.%{site}" % {site: link.site}), link.url %>
    • <% end %> - <%= link_to "#{creator.models.count} #{Model.model_name.human count: creator.models.count}", models_path(creator: creator.id), {class: "btn btn-primary", "aria-label": translate(".models_button.label", name: creator.name)} if policy(creator).show? %> + <%= link_to "#{policy_scope(Model).where(creator: creator).count} #{Model.model_name.human count: policy_scope(Model).where(creator: creator).count}", models_path(creator: creator.id), {class: "btn btn-primary", "aria-label": translate(".models_button.label", name: creator.name)} if policy(creator).show? %> <%= link_to icon("pencil-fill", t(".edit_button.text")), edit_creator_path(creator), {class: "btn btn-outline-secondary", "aria-label": translate(".edit_button.label", name: creator.name)} if policy(creator).edit? %> diff --git a/app/views/creators/_unassigned.html.erb b/app/views/creators/_unassigned.html.erb index 7faebb138..b38516078 100644 --- a/app/views/creators/_unassigned.html.erb +++ b/app/views/creators/_unassigned.html.erb @@ -3,7 +3,7 @@
    <%= t(".name") %>
    <%= t(".caption") %>
    - <%= link_to "#{Model.where(creator: nil).count} #{Model.model_name.human count: Model.where(creator: nil).count}", models_path(creator: ""), {class: "btn btn-primary"} %> + <%= link_to "#{policy_scope(Model).where(creator: nil).count} #{Model.model_name.human count: policy_scope(Model).where(creator: nil).count}", models_path(creator: ""), {class: "btn btn-primary"} %>
    From ad63e04b9b27a61e64f04c8de48014a1cc70bf91 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:55:47 +0100 Subject: [PATCH 19/23] don't show private things in activity feed --- app/views/activities/_create_actor.html.erb | 6 ++---- app/views/activities/_update_actor.html.erb | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/views/activities/_create_actor.html.erb b/app/views/activities/_create_actor.html.erb index bf28e0a9e..7ba18c341 100644 --- a/app/views/activities/_create_actor.html.erb +++ b/app/views/activities/_create_actor.html.erb @@ -1,7 +1,7 @@ <%- subject = activity&.actor %> <%- object = activity&.entity %> <%- thing = object&.entity %> -<% if thing %> +<% if thing && !thing.is_a?(User) && (thing.grants_permission_to?(["view", "edit", "own"], current_user) || thing.grants_permission_to?(["view", "edit", "own"], current_user&.roles)) %>
    @@ -9,9 +9,7 @@ <%= icon icon_for(thing.class), thing.class.model_name.human %>
    - <%- if thing.is_a?(User) %> - <%= object.name %> - <%- elsif object.local? %> + <%- if object.local? %> <%= link_to thing.name, thing %> <%- elsif object.profile_url %> <%= link_to object.name, object.profile_url %> diff --git a/app/views/activities/_update_actor.html.erb b/app/views/activities/_update_actor.html.erb index a53ddae77..dc7438ad2 100644 --- a/app/views/activities/_update_actor.html.erb +++ b/app/views/activities/_update_actor.html.erb @@ -1,7 +1,7 @@ <%- subject = activity&.actor %> <%- object = activity&.entity %> <%- thing = object&.entity %> -<% if thing && !thing.is_a?(User) %> +<% if thing && !thing.is_a?(User) && (thing.grants_permission_to?(["view", "edit", "own"], current_user) || thing.grants_permission_to?(["view", "edit", "own"], current_user&.roles)) %>
    From a6560808c2ffdf97a44c706adbc2dd341c68c3b8 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 22:56:48 +0100 Subject: [PATCH 20/23] don't show counts in tag cloud if logged out --- app/helpers/application_helper.rb | 2 +- app/views/models/index.html.erb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 0f9d0e602..18ce4c04d 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -156,7 +156,7 @@ def pagination_settings end def tag_cloud_settings - current_user&.tag_cloud_settings || SiteSettings::UserDefaults::TAG_CLOUD + current_user&.tag_cloud_settings || SiteSettings::UserDefaults::TAG_CLOUD.merge(heatmap: false) end def renderer_settings diff --git a/app/views/models/index.html.erb b/app/views/models/index.html.erb index 6dd2f97c7..e0a162d96 100644 --- a/app/views/models/index.html.erb +++ b/app/views/models/index.html.erb @@ -28,10 +28,10 @@ <%= card :secondary, t(".actions_heading") do %> <%= link_to t(".bulk_edit"), edit_models_path(@filters), class: "btn btn-secondary mb-3 me-3" if policy(:model).edit? %> <% if current_user %> - <% if tag_cloud_settings["heatmap"] && Model.where("(select count(*) from taggings where taggings.taggable_id=models.id and taggings.context='tags')<1").count > 0 %> + <% if tag_cloud_settings["heatmap"] && policy_scope(Model).where("(select count(*) from taggings where taggings.taggable_id=models.id and taggings.context='tags')<1").count > 0 %> <%= link_to t(".untagged"), models_path(tag: [""]), class: "btn btn-secondary mb-3 me-3" %> <% end %> - <% if Model.where("(select count(*) from links where linkable_id=models.id and linkable_type='Model')<1").count > 0 %> + <% if policy_scope(Model).where("(select count(*) from links where linkable_id=models.id and linkable_type='Model')<1").count > 0 %> <%= link_to t(".missing_url"), models_path(link: ""), class: "btn btn-secondary mb-3 me-3" %> <% end %> <% end %> From 651fca42da176df99c509bd90b2290bb9e0bbfc1 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 23:07:37 +0100 Subject: [PATCH 21/23] get follow button tests working with policies --- spec/components/follow_button_component_spec.rb | 3 +++ spec/support/view_components.rb | 10 +++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/spec/components/follow_button_component_spec.rb b/spec/components/follow_button_component_spec.rb index 39dbe3c9b..69fe0beb5 100644 --- a/spec/components/follow_button_component_spec.rb +++ b/spec/components/follow_button_component_spec.rb @@ -3,7 +3,10 @@ require "rails_helper" RSpec.describe FollowButtonComponent, type: :component do + let(:user) { create :user } + before do + sign_in(user) allow(SiteSettings).to receive(:multiuser_enabled?).and_return(true) end diff --git a/spec/support/view_components.rb b/spec/support/view_components.rb index c8b8b93f5..f38f20432 100644 --- a/spec/support/view_components.rb +++ b/spec/support/view_components.rb @@ -1,7 +1,15 @@ require "view_component/test_helpers" require "view_component/system_test_helpers" +# For devise in viewcomponents: https://github.com/ViewComponent/view_component/discussions/371 +module ManyfoldViewComponentTestHelpers + include ViewComponent::TestHelpers + def sign_in(user) + allow(vc_test_controller).to receive(:current_user).and_return(user) + end +end + RSpec.configure do |config| - config.include ViewComponent::TestHelpers, type: :component + config.include ManyfoldViewComponentTestHelpers, type: :component config.include ViewComponent::SystemTestHelpers, type: :component end From af5716696e321b1ff69cab80b57fd5fc7cd9c96a Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 23:10:09 +0100 Subject: [PATCH 22/23] remove requirement for authenticated access on every action --- app/controllers/application_controller.rb | 1 - app/controllers/health_controller.rb | 2 -- 2 files changed, 3 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index abfd1e26d..27e9b8e5a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -5,7 +5,6 @@ class ApplicationController < ActionController::Base after_action :verify_policy_scoped, only: :index, unless: :active_admin_controller? after_action :set_content_security_policy_header, if: -> { request.format.html? } - before_action :authenticate_user! around_action :switch_locale before_action :check_for_first_use before_action :show_security_alerts diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb index e09ad8dbd..23a9e6ff7 100644 --- a/app/controllers/health_controller.rb +++ b/app/controllers/health_controller.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class HealthController < ApplicationController - skip_before_action :authenticate_user! - after_action :verify_policy_scoped, except: :index def index From 461912c11f4ef78ef675b6ef8e7869dbeb761ffd Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 3 Sep 2024 23:11:39 +0100 Subject: [PATCH 23/23] use existing user in FollowButtonComponent test --- spec/components/follow_button_component_spec.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/spec/components/follow_button_component_spec.rb b/spec/components/follow_button_component_spec.rb index 69fe0beb5..bb8fb632c 100644 --- a/spec/components/follow_button_component_spec.rb +++ b/spec/components/follow_button_component_spec.rb @@ -3,16 +3,14 @@ require "rails_helper" RSpec.describe FollowButtonComponent, type: :component do - let(:user) { create :user } + let(:follower) { create(:user) } + let(:target) { create(:creator) } before do - sign_in(user) + sign_in(follower) allow(SiteSettings).to receive(:multiuser_enabled?).and_return(true) end - let(:follower) { create(:user) } - let(:target) { create(:creator) } - context "when the follower is not following the target" do let(:button) { allow(follower).to receive(:following?).with(target).and_return false