diff --git a/Gemfile b/Gemfile index 234a0863a..85fc23156 100644 --- a/Gemfile +++ b/Gemfile @@ -97,3 +97,5 @@ gem "sqlite3_ar_regexp", "~> 2.2" gem "mittsu", github: "danini-the-panini/mittsu", ref: "7f44c46" gem "view_component", "~> 3.10" + +gem "rails-controller-testing", "~> 1.0", group: :test diff --git a/Gemfile.lock b/Gemfile.lock index 227d01dec..336c44fa2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -300,6 +300,10 @@ GEM activesupport (= 7.0.8) bundler (>= 1.15.0) railties (= 7.0.8) + rails-controller-testing (1.0.5) + actionpack (>= 5.0.1.rc1) + actionview (>= 5.0.1.rc1) + activesupport (>= 5.0.1.rc1) rails-dom-testing (2.2.0) activesupport (>= 5.0.0) minitest @@ -506,6 +510,7 @@ DEPENDENCIES public_suffix (~> 5.0) puma (~> 6.4) rails (~> 7.0.8) + rails-controller-testing (~> 1.0) rails-settings-cached (~> 2.9) redis (~> 5.0) rspec-rails diff --git a/app/controllers/problems_controller.rb b/app/controllers/problems_controller.rb index 293feb3d3..998537c4e 100644 --- a/app/controllers/problems_controller.rb +++ b/app/controllers/problems_controller.rb @@ -1,6 +1,6 @@ class ProblemsController < ApplicationController def index page = params[:page] || 1 - @problems = Problem.all.page(page).per(50).order([:category, :problematic_type]) + @problems = Problem.visible(current_user.problem_settings).page(page).per(50).order([:category, :problematic_type]) end end diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index a68e416f0..31aae141c 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -10,6 +10,7 @@ def update update_pagination_settings(params[:pagination]) update_renderer_settings(params[:renderer]) update_tag_cloud_settings(params[:tag_cloud]) + update_problem_settings(params[:problems]) @user.save! # Save site-wide settings if user is an admin if current_user.admin? @@ -56,6 +57,11 @@ def update_renderer_settings(settings) } end + def update_problem_settings(settings) + return unless settings + @user.problem_settings = settings + end + def update_folder_settings(settings) return unless settings SiteSettings.model_path_template = settings[:model_path_template].gsub(/^\//, "") # Remove leading slashes diff --git a/app/helpers/problems_helper.rb b/app/helpers/problems_helper.rb index ed6c6d996..8a8652620 100644 --- a/app/helpers/problems_helper.rb +++ b/app/helpers/problems_helper.rb @@ -1,18 +1,10 @@ module ProblemsHelper def problem_severity(problem) - case problem.category.to_sym - when :missing - "danger" - when :nesting, :duplicate - "warning" - else - "info" - end + current_user.problem_settings[problem.category]&.to_sym || :silent end def max_problem_severity - return "danger" if Problem.where(category: :missing).count > 0 - return "warning" if Problem.where(category: [:nesting, :duplicate]).count > 0 - "info" + severities = Problem.select(:category).distinct.map { |p| problem_severity(p) } + severities.max_by { |p| Problem::SEVERITIES.find_index(p) } end end diff --git a/app/models/problem.rb b/app/models/problem.rb index 3851138af..56a756556 100644 --- a/app/models/problem.rb +++ b/app/models/problem.rb @@ -3,7 +3,12 @@ class Problem < ApplicationRecord validates :category, uniqueness: {scope: :problematic}, presence: true - enum :category, [ + scope :visible, ->(settings) { + enabled = settings.select { |cat, sev| sev.to_sym != :silent } + where(category: enabled.keys) + } + + CATEGORIES = [ :missing, :empty, :destination_exists, # No longer used, but kept for compatibility @@ -11,6 +16,22 @@ class Problem < ApplicationRecord :inefficient, :duplicate ] + enum :category, CATEGORIES + + SEVERITIES = [ + :silent, + :info, + :warning, + :danger + ] + + DEFAULT_SEVERITIES = { + missing: :danger, + empty: :info, + nesting: :warning, + inefficient: :info, + duplicate: :warning + } def self.create_or_clear(problematic, cat, present, options = {}) if present diff --git a/app/views/application/_navbar.html.erb b/app/views/application/_navbar.html.erb index 18fcf59c1..557c45b55 100644 --- a/app/views/application/_navbar.html.erb +++ b/app/views/application/_navbar.html.erb @@ -50,7 +50,7 @@ <% end %> - <% if Problem.count > 0 %> + <% if Problem.visible(current_user.problem_settings).count > 0 %>
  • <%= nav_link 'exclamation-triangle-fill', Problem.model_name.human.pluralize, diff --git a/app/views/settings/_problem_settings.html.erb b/app/views/settings/_problem_settings.html.erb new file mode 100644 index 000000000..3e0188a92 --- /dev/null +++ b/app/views/settings/_problem_settings.html.erb @@ -0,0 +1,24 @@ +
    +

    <%= t(".title") %>

    +
    +

    + <%= t(".description") %> +

    + <% Problem::CATEGORIES.each do |category| %> + <% next if Problem::DEFAULT_SEVERITIES[category].nil? # Skip deprecated categories; they don't appear in the defaults list %> +
    + <%= form.label t("problems.categories.#{category}"), + for: "problems[#{category}]", + class: "col col-form-label" + %> +
    + <%= form.select "problems[#{category}]", + Problem::SEVERITIES.map { |s| [t("problems.severities.#{s}"), s] }, + {selected: @user.problem_settings[category.to_s]}, + class: "form-select" + %> +
    +
    + <% end %> +
    +
    diff --git a/app/views/settings/_renderer_settings.html.erb b/app/views/settings/_renderer_settings.html.erb index 40a32b3d2..13e739ae1 100644 --- a/app/views/settings/_renderer_settings.html.erb +++ b/app/views/settings/_renderer_settings.html.erb @@ -1,4 +1,4 @@ -
    +

    Renderer

    diff --git a/app/views/settings/_tag_cloud_settings.html.erb b/app/views/settings/_tag_cloud_settings.html.erb index bf4fe2728..69c2ff415 100644 --- a/app/views/settings/_tag_cloud_settings.html.erb +++ b/app/views/settings/_tag_cloud_settings.html.erb @@ -1,33 +1,36 @@ -

    +

    Tag Cloud

    -
    - <%= form.label "Minimum count of tag for display", for: "tag_cloud[threshold]", class: "col-sm-8 col-form-label"%> -
    +
    + <%= form.label "Minimum count of tag for display", for: "tag_cloud[threshold]", class: "col col-form-label"%> +
    <%= form.text_field "tag_cloud[threshold]", value: @user.tag_cloud_settings["threshold"], class: "form-control" %>
    -
    - <%= form.label "Display tag count for individual tags", for: "tag_cloud[heatmap]", class: "col-sm-8 col-form-label"%> -
    +
    + <%= form.label "Display tag count for individual tags", for: "tag_cloud[heatmap]", class: "col col-form-label"%> +
    <%= form.check_box "tag_cloud[heatmap]", checked: @user.tag_cloud_settings["heatmap"], class: "form-check-input" %>
    -
    - <%= form.label "Hide unrelated tags in filtered lists", for: "tag_cloud[hide_unrelated]", class: "col-sm-8 col-form-label"%> -
    +
    + <%= form.label "Hide unrelated tags in filtered lists", for: "tag_cloud[hide_unrelated]", class: "col col-form-label"%> +
    <%= form.check_box "tag_cloud[hide_unrelated]", checked: @user.tag_cloud_settings["hide_unrelated"], class: "form-check-input" %>
    -
    - <%= form.label "Sorting", for: "tag_cloud[sorting]", class: "col-sm-8 col-form-label"%> -
    - <%= form.select "tag_cloud[sorting]", ["frequency","alphabetical"], selected: @user.tag_cloud_settings["sorting"], class: "form-select" %> +
    + <%= form.label "Sorting", for: "tag_cloud[sorting]", class: "col col-form-label"%> +
    + <%= form.select "tag_cloud[sorting]", ["frequency","alphabetical"], + {selected: @user.tag_cloud_settings["sorting"]}, + class: "form-select" + %>
    - <%= form.label "Use delimiter to represent keypairs", for: "tag_cloud[keypair]", class: "col-sm-8 col-form-label"%> -
    + <%= form.label "Use delimiter to represent keypairs", for: "tag_cloud[keypair]", class: "col col-form-label"%> +
    <%= form.check_box "tag_cloud[keypair]", checked: @user.tag_cloud_settings["keypair"], class: "form-check-input" %>
    diff --git a/app/views/settings/show.html.erb b/app/views/settings/show.html.erb index 549242b5c..3f0106d23 100644 --- a/app/views/settings/show.html.erb +++ b/app/views/settings/show.html.erb @@ -9,6 +9,7 @@
    <%= render "renderer_settings", form: form %> + <%= render "problem_settings", form: form %>
    diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index d6a11b31c..17cce66c9 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -120,6 +120,7 @@ search: ignore_unused: - 'activerecord.attributes.*' - 'activerecord.models.*' +- 'problems.categories.*' # - '{devise,kaminari,will_paginate}.*' # - 'simple_form.{yes,no}' # - 'simple_form.{placeholders,hints,labels}.*' diff --git a/config/locales/en.yml b/config/locales/en.yml index 1854a45ba..b5adb1c3f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -48,6 +48,12 @@ en: message: This file is duplicated in other models. You may wish to remove some. title: Duplicated files problems: + categories: + duplicate: Duplicate files + empty: Model has no files + inefficient: Inefficient formats + missing: Missing files or folders + nesting: Model contains other models library: missing: Library folder not found model: @@ -60,11 +66,19 @@ en: description: Duplicated file model_file_inefficient: description: Inefficient file format + severities: + danger: Danger + info: Info + silent: Ignored + warning: Warning renderer: style: lambert: Realistically shaded normals: Surface direction settings: + problem_settings: + description: Change the severity of detected problems, or ignore them completely. + title: Problem Detection renderer_settings: auto_load_max_size: always: Always diff --git a/db/migrate/20240122114207_add_problem_settings_to_user.rb b/db/migrate/20240122114207_add_problem_settings_to_user.rb new file mode 100644 index 000000000..fd6b1d6be --- /dev/null +++ b/db/migrate/20240122114207_add_problem_settings_to_user.rb @@ -0,0 +1,5 @@ +class AddProblemSettingsToUser < ActiveRecord::Migration[7.0] + def change + add_column :users, :problem_settings, :json, default: Problem::DEFAULT_SEVERITIES + end +end diff --git a/db/schema.rb b/db/schema.rb index 2d9029242..8172ec55e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_07_11_111009) do +ActiveRecord::Schema[7.0].define(version: 2024_01_22_114207) do create_table "collections", force: :cascade do |t| t.string "name" t.text "notes" @@ -180,6 +180,7 @@ t.json "pagination_settings", default: {"models"=>true, "creators"=>true, "collections"=>true, "per_page"=>12} t.json "renderer_settings", default: {"grid_width"=>200, "grid_depth"=>200} t.json "tag_cloud_settings", default: {"threshold"=>0, "heatmap"=>true, "keypair"=>true, "sorting"=>"frequency", "hide_unrelated"=>true} + t.json "problem_settings", default: {"missing"=>"danger", "empty"=>"info", "nesting"=>"warning", "inefficient"=>"info", "duplicate"=>"warning"} t.index ["email"], name: "index_users_on_email", unique: true t.index ["username"], name: "index_users_on_username", unique: true end diff --git a/spec/factories/problem.rb b/spec/factories/problem.rb new file mode 100644 index 000000000..6a4e9f10c --- /dev/null +++ b/spec/factories/problem.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :problem do + category { :missing } + problematic { association :model_file } + end +end diff --git a/spec/helpers/problems_helper_spec.rb b/spec/helpers/problems_helper_spec.rb index 3c197f626..5ee6f8089 100644 --- a/spec/helpers/problems_helper_spec.rb +++ b/spec/helpers/problems_helper_spec.rb @@ -1,15 +1,29 @@ require "rails_helper" -# Specs in this file have access to a helper object that includes -# the ProblemsHelper. For example: -# -# describe ProblemsHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# expect(helper.concat_strings("this","that")).to eq("this that") -# end -# end -# end RSpec.describe ProblemsHelper do - pending "add some examples to (or delete) #{__FILE__}" + include Devise::Test::ControllerHelpers + let(:model) { create(:model) } + + before do + sign_in User.first + end + + it "converts a problem to a severity level" do + expect(helper.problem_severity( + Problem.new(category: :duplicate, problematic: model) + )).to eq :warning + end + + it "works out the maximum severity from a set of problems (warning)" do + Problem.create(category: :duplicate, problematic: model) + Problem.create(category: :inefficient, problematic: model) + expect(helper.max_problem_severity).to eq :warning + end + + it "works out the maximum severity from a set of problems (danger)" do + Problem.create(category: :missing, problematic: model) + Problem.create(category: :duplicate, problematic: model) + Problem.create(category: :inefficient, problematic: model) + expect(helper.max_problem_severity).to eq :danger + end end diff --git a/spec/models/problem_spec.rb b/spec/models/problem_spec.rb index 4b3838816..819615c24 100644 --- a/spec/models/problem_spec.rb +++ b/spec/models/problem_spec.rb @@ -1,5 +1,29 @@ require "rails_helper" RSpec.describe Problem do - pending "add some examples to (or delete) #{__FILE__}" + describe "querying visible scope" do + let(:settings) do + { + missing: :silent, + empty: :info, + nesting: :warning, + inefficient: :info, + duplicate: :warning + } + end + + before do + create_list(:problem, 3, :missing) + create_list(:problem, 3, :inefficient) + end + + it "lists visible problems" do + expect(described_class.visible(settings).length).to eq 3 + expect(described_class.visible(settings).map { |x| x.category.to_sym }).to include :inefficient + end + + it "does not include silenced problems" do + expect(described_class.visible(settings).map { |x| x.category.to_sym }).not_to include :missing + end + end end diff --git a/spec/requests/problems_spec.rb b/spec/requests/problems_spec.rb index c290be4c5..7ca982e69 100644 --- a/spec/requests/problems_spec.rb +++ b/spec/requests/problems_spec.rb @@ -2,9 +2,34 @@ RSpec.describe "Problems" do describe "GET /index" do - it "returns http success" do + before do + create_list(:problem, 2, category: :inefficient) + create_list(:problem, 3, category: :missing) + sign_in User.first + end + + it "returns success" do get "/problems/index" expect(response).to have_http_status(:success) end + + it "lists problems" do + get "/problems/index" + expect(assigns(:problems).length).to eq 5 + end + + context "with silenced problems" do + before do + u = User.first + u.problem_settings["missing"] = "silent" + u.save! + sign_in u + end + + it "doesn't show problems with silent severity" do + get "/problems/index" + expect(assigns(:problems).length).to eq 2 + end + end end end