Merge pull request #860 from Floppy/problems

Add "Problems", which highlight missing files and other issues

app/controllers/models_controller.rb

@@ -55,6 +55,11 @@ def bulk_update
     redirect_to edit_library_models_path(@library, tag: params[:tag])
   end
 
+  def destroy
+    @model.destroy
+    redirect_to library_path(@library)
+  end
+
   private
 
   def bulk_update_params

app/controllers/problems_controller.rb

@@ -0,0 +1,5 @@
+class ProblemsController < ApplicationController
+  def index
+    @problems = Problem.all
+  end
+end

app/helpers/problems_helper.rb

@@ -0,0 +1,2 @@
+module ProblemsHelper
+end

app/jobs/library_scan_job.rb

@@ -21,14 +21,30 @@ def known_filenames(library)
 
   def clean_up_missing_models(library)
     library.models.each do |m|
-      m.destroy unless File.exist?(File.join(library.path, m.path))
+      if !File.exist?(File.join(library.path, m.path))
+        begin
+          m.problems.create(category: :missing)
+        rescue
+          nil
+        end
+      else
+        m.problems.where(category: :missing).destroy_all
+      end
     end
     nil
   end
 
   def clean_up_missing_model_files(library)
     library.model_files.each do |f|
-      f.destroy unless File.exist?(f.pathname)
+      if !File.exist?(f.pathname)
+        begin
+          f.problems.create(category: :missing)
+        rescue
+          nil
+        end
+      else
+        f.problems.where(category: :missing).destroy_all
+      end
     end
     nil
   end
@@ -43,6 +59,11 @@ def filter_out_common_subfolders(folders)
   end
 
   def perform(library)
+    if !File.exist?(library.path)
+      library.problems.create(category: :missing)
+    else
+      library.problems.where(category: :missing).destroy_all
+    end
     # Remove models with missing path
     clean_up_missing_models(library)
     clean_up_missing_model_files(library)

app/jobs/model_scan_job.rb

@@ -14,16 +14,30 @@ def self.file_pattern
   end
 
   def clean_up_missing_files(model)
-    model.model_files.select { |f|
-      !File.exist?(File.join(model.library.path, model.path, f.filename))
-    }.each(&:destroy)
+    model.model_files.each do |f|
+      if !File.exist?(File.join(model.library.path, model.path, f.filename))
+        begin
+          f.problems.create(category: :missing)
+        rescue
+          nil
+        end
+      else
+        f.problems.where(category: :missing).destroy_all
+      end
+    end
   end
 
   def perform(model)
     # Clean out missing files
     clean_up_missing_files(model)
     # For each file in the model, create a file object
     model_path = File.join(model.library.path, model.path)
+    if !File.exist?(model_path)
+      model.problems.create(category: :missing)
+      return
+    else
+      model.problems.where(category: :missing).destroy_all
+    end
     Dir.open(model_path) do |dir|
       Dir.glob([
         File.join(dir.path, ModelScanJob.file_pattern),
@@ -45,7 +59,15 @@ def perform(model)
         model.save!
       end
     end
-    # If this model has no files, self destruct
-    model.destroy if model.model_files.reload.count == 0
+    # If this model has no files, flag a problem
+    if model.model_files.reload.count == 0
+      begin
+        model.problems.create(category: :empty)
+      rescue
+        nil
+      end
+    else
+      model.problems.where(category: :empty).destroy_all
+    end
   end
 end

app/models/library.rb

@@ -1,6 +1,8 @@
 class Library < ApplicationRecord
   has_many :models, dependent: :destroy
   has_many :model_files, through: :models
+  has_many :problems, as: :problematic, dependent: :destroy
+
   validates :path, presence: true, uniqueness: true, existing_path: true
 
   default_scope { order(:path) }

app/models/model.rb

@@ -9,6 +9,7 @@ class Model < ApplicationRecord
   validates :path, presence: true, uniqueness: {scope: :library}
   validate :cannot_move_models_with_submodels, on: :update
   has_many :links, as: :linkable, dependent: :destroy
+  has_many :problems, as: :problematic, dependent: :destroy
   accepts_nested_attributes_for :links, reject_if: :all_blank, allow_destroy: true
 
   attr_accessor :organize

app/models/model_file.rb

@@ -1,11 +1,11 @@
 class ModelFile < ApplicationRecord
   belongs_to :model
+  has_many :problems, as: :problematic, dependent: :destroy
+
   validates :filename, presence: true, uniqueness: {scope: :model}
 
   default_scope { order(:filename) }
 
-  after_destroy :remove_file
-
   def file_format
     File.extname(filename).delete(".").downcase
   end
@@ -28,8 +28,6 @@ def calculate_digest
     nil
   end
 
-  private
-
   def remove_file
     File.delete(pathname) if File.exist?(pathname)
   end

app/models/problem.rb

@@ -0,0 +1,7 @@
+class Problem < ApplicationRecord
+  belongs_to :problematic, polymorphic: true
+
+  validates :category, uniqueness: {scope: :problematic}, presence: true
+
+  enum :category, [:missing, :empty]
+end

app/views/layouts/application.html.erb

@@ -43,6 +43,11 @@
             %>
           </li>
         <% end %>
+        <% if Problem.count > 0 %>
+          <li class="nav-item">
+            <%= link_to icon("exclamation-triangle-fill", "Problems"), problems_path, class: (current_page?(problems_path) ? "link-danger nav-link active" : "link-danger nav-link") %>
+          </li>
+        <% end %>
         <li class="nav-item">
           <%= link_to icon('sliders', 'Settings'), user_settings_path(current_user), class: (current_page?(user_settings_path(current_user)) ? "nav-link active" : "nav-link") %>
         </li>

app/views/problems/_empty_model.html copy.erb

@@ -0,0 +1,2 @@
+<td><%= link_to problem.problematic.name, [problem.problematic.library, problem.problematic] %></td>
+<td><%= link_to "Purge", [problem.problematic.library, problem.problematic], method: :delete, class: "btn btn-danger" %></td>

app/views/problems/_missing_library.html.erb

@@ -0,0 +1,2 @@
+<td><%= link_to problem.problematic.name, problem.problematic %></td>
+<td></td>

app/views/problems/_missing_model.html.erb

@@ -0,0 +1,2 @@
+<td><%= link_to problem.problematic.name, [problem.problematic.library, problem.problematic] %></td>
+<td><%= link_to "Purge", [problem.problematic.library, problem.problematic], method: :delete, class: "btn btn-danger" %></td>

app/views/problems/_missing_model_file.html.erb

@@ -0,0 +1,2 @@
+<td><%= link_to problem.problematic.name, [problem.problematic.model.library, problem.problematic.model, problem.problematic] %></td>
+<td><%= link_to "Purge", [problem.problematic.model.library, problem.problematic.model, problem.problematic], method: :delete, class: "btn btn-danger" %></td>

app/views/problems/index.html.erb

@@ -0,0 +1,14 @@
+<h1>Problems</h1>
+<p class='lead'><%= pluralize @problems.count, "problem" %> found</p>
+
+<table class="table table-striped">
+	<% @problems.each do |problem| %>
+		<tr>
+			<td>
+				<%= icon("file-x", problem.category) %>
+				<%= problem.category %>
+			</td>
+			<%= render partial: "#{problem.category}_#{problem.problematic_type.underscore}", locals: {problem: problem} %>
+		</tr>
+	<% end %>
+</table>

config/routes.rb

@@ -1,4 +1,5 @@
 Rails.application.routes.draw do
+  get "problems/index"
   devise_for :users, ActiveAdmin::Devise.config
   ActiveAdmin.routes(self)
   resources :users do
@@ -9,7 +10,7 @@
   post "/", controller: :search, action: :index
 
   resources :libraries do
-    resources :models, except: [:index, :destroy] do
+    resources :models, except: [:index] do
       member do
         post "merge"
       end
@@ -27,4 +28,5 @@
   end
   resources :creators
   resources :collections, only: [:index, :show]
+  resources :problems, only: [:index]
 end

db/migrate/20221210001132_create_problems.rb

@@ -0,0 +1,9 @@
+class CreateProblems < ActiveRecord::Migration[7.0]
+  def change
+    create_table :problems do |t|
+      t.references :problematic, polymorphic: true
+      t.integer :category
+      t.timestamps
+    end
+  end
+end

spec/helpers/problems_helper_spec.rb

@@ -0,0 +1,15 @@
+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, type: :helper do
+  pending "add some examples to (or delete) #{__FILE__}"
+end

spec/jobs/library_scan_job_spec.rb

@@ -29,9 +29,9 @@
     expect { LibraryScanJob.perform_now(library) }.to have_enqueued_job(ModelScanJob).exactly(3).times
   end
 
-  it "removes models with no files" do
+  it "flags models with no files as problems" do
     lib = create(:library, path: File.join("/", "tmp"))
     create(:model, library: lib, path: "missing")
-    expect { LibraryScanJob.perform_now(lib) }.to change { lib.models.count }.from(1).to(0)
+    expect { LibraryScanJob.perform_now(lib) }.to change { Problem.count }.from(0).to(1)
   end
 end

spec/jobs/model_scan_job_spec.rb

@@ -40,12 +40,11 @@
   end
 
   context "with already scanned files" do
-    it "removes files that don't exist on disk" do
+    it "flags up problems for files that don't exist on disk" do
       thing = create(:model, path: "model_one/nested_model", library: library)
       create(:model_file, filename: "missing.stl", model: thing)
       create(:model_file, filename: "gone.stl", model: thing)
-      expect { ModelScanJob.perform_now(thing) }.to change { thing.model_files.count }.from(2).to(1)
-      expect(thing.model_files.first.filename).to eq "part_one.stl"
+      expect { ModelScanJob.perform_now(thing) }.to change { Problem.count }.from(0).to(2)
     end
   end
 end

spec/models/problem_spec.rb

@@ -0,0 +1,5 @@
+require "rails_helper"
+
+RSpec.describe Problem, type: :model do
+  pending "add some examples to (or delete) #{__FILE__}"
+end

spec/requests/problems_spec.rb

@@ -0,0 +1,10 @@
+require "rails_helper"
+
+RSpec.describe "Problems", type: :request do
+  describe "GET /index" do
+    it "returns http success" do
+      get "/problems/index"
+      expect(response).to have_http_status(:success)
+    end
+  end
+end

spec/views/problems/index.html.erb_spec.rb

@@ -0,0 +1,5 @@
+require "rails_helper"
+
+RSpec.describe "problems/index.html.erb", type: :view do
+  pending "add some examples to (or delete) #{__FILE__}"
+end