From ccc29be2976aba89365112122d02692c14b45831 Mon Sep 17 00:00:00 2001 From: James Smith Date: Mon, 13 Jan 2025 09:20:16 +0000 Subject: [PATCH 1/2] refactor renderable check into ModelFile --- app/components/model_component.html.erb | 2 +- app/helpers/application_helper.rb | 9 --------- app/models/model_file.rb | 4 ++++ app/views/application/_object_preview.html.erb | 2 +- app/views/collections/_collection.html.erb | 2 +- app/views/model_files/show.html.erb | 2 +- app/views/models/_file.html.erb | 2 +- spec/helpers/application_helper_spec.rb | 14 -------------- spec/models/model_file_spec.rb | 12 ++++++++++++ 9 files changed, 21 insertions(+), 28 deletions(-) diff --git a/app/components/model_component.html.erb b/app/components/model_component.html.erb index 8e2e32af7..b487ba190 100644 --- a/app/components/model_component.html.erb +++ b/app/components/model_component.html.erb @@ -4,7 +4,7 @@ <% if file.is_image? %> <%= content_tag :div, nil, class: "card-img-top card-img-top-background", style: "background-image: url(#{model_model_file_path(@model, file, format: file.extension)})" %> <%= image_tag model_model_file_path(@model, file, format: file.extension), class: "card-img-top image-preview #{"sensitive" if helpers.needs_hiding?(@model)}", alt: file.name %> - <% elsif helpers.renderable?(file.extension) %> + <% elsif file.is_renderable? %>
"> <%= render partial: "object_preview", locals: {model: @model, file: file} %>
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 12bf244ef..e73e262e0 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -81,15 +81,6 @@ def card(style, title = nil, options = {}, &content) end end - def renderable?(format) - case format - when "stl", "obj", "3mf", "ply", "gltf", "glb" - true - else - false - end - end - def text_input_row(form, name, options = {}) content_tag :div, class: "row mb-3 input-group" do safe_join [ diff --git a/app/models/model_file.rb b/app/models/model_file.rb index 990ec9586..32f783e9b 100644 --- a/app/models/model_file.rb +++ b/app/models/model_file.rb @@ -84,6 +84,10 @@ def is_3d_model? SupportedMimeTypes.model_extensions.include? extension end + def is_renderable? + ["stl", "obj", "3mf", "ply", "gltf", "glb"].include? extension + end + def mime_type Mime::Type.lookup_by_extension(extension) end diff --git a/app/views/application/_object_preview.html.erb b/app/views/application/_object_preview.html.erb index 9ef73ad3a..18efd6b50 100644 --- a/app/views/application/_object_preview.html.erb +++ b/app/views/application/_object_preview.html.erb @@ -1,4 +1,4 @@ -<% if renderable?(file.extension) %> +<% if file.is_renderable? %>
<%= content_tag :div, nil, class: "card-img-top card-img-top-background", style: "background-image: url(#{model_model_file_path(model, file, format: file.extension)})" %> <%= image_tag model_model_file_path(model, file, format: file.extension), class: "card-img-top image-preview ", alt: file.name %> - <% elsif renderable?(file.extension) %> + <% elsif file.is_renderable? %>
<%= render partial: "object_preview", locals: {model: model, file: file} %>
diff --git a/app/views/model_files/show.html.erb b/app/views/model_files/show.html.erb index 3be5a6ef9..1809f9279 100644 --- a/app/views/model_files/show.html.erb +++ b/app/views/model_files/show.html.erb @@ -15,7 +15,7 @@ <%= video_tag model_model_file_path(@model, @file, format: @file.extension), alt: @file.name, style: "width: 100%", controls: true %> <% elsif @file.is_document? %> <%= tag.iframe src: model_model_file_path(@model, @file, format: @file.extension), alt: @file.name, style: "width: 100%; aspect-ratio: 0.707" %> - <% elsif renderable?(@file.extension) %> + <% elsif @file.is_renderable? %> <% if @file.presupported_version || @file.unsupported_version %>
  • diff --git a/app/views/models/_file.html.erb b/app/views/models/_file.html.erb index 79a566089..7a4711452 100644 --- a/app/views/models/_file.html.erb +++ b/app/views/models/_file.html.erb @@ -3,7 +3,7 @@ <% if file.is_image? %> <%= content_tag :div, nil, class: "card-img-top card-img-top-background", style: "background-image: url(#{model_model_file_path(@model, file, format: file.extension)})" %> <%= image_tag model_model_file_path(@model, file, format: file.extension), class: "card-img-top image-preview", alt: file.name %> - <% elsif renderable?(file.extension) %> + <% elsif file.is_renderable? %>
    <%= render partial: "object_preview", locals: {model: @model, file: file} %>
    diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index e65fb3589..f63d9c80e 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -27,20 +27,6 @@ end end - describe "#renderable?" do - it "returns true for renderable formats" do # rubocop:todo RSpec/MultipleExpectations - expect(helper.renderable?("stl")).to be true - expect(helper.renderable?("obj")).to be true - expect(helper.renderable?("3mf")).to be true - expect(helper.renderable?("ply")).to be true - end - - it "returns false for non-renderable formats" do # rubocop:todo RSpec/MultipleExpectations - expect(helper.renderable?("txt")).to be false - expect(helper.renderable?("doc")).to be false - end - end - describe "#text_input_row" do it "returns the correct HTML for the text input row" do # rubocop:todo RSpec/MultipleExpectations form = ActionView::Helpers::FormBuilder.new(:test, nil, helper, {}) diff --git a/spec/models/model_file_spec.rb b/spec/models/model_file_spec.rb index 8ef0152a5..5e103cc1c 100644 --- a/spec/models/model_file_spec.rb +++ b/spec/models/model_file_spec.rb @@ -150,4 +150,16 @@ expect(unsupported.reload.presupported_version).to be_nil end end + + { + stl: true, + png: false, + pdf: false, + lys: false + }.each_pair do |extension, result| + it "shows that #{extension} files are#{"n't" if result == false} renderable" do + file = create(:model_file, filename: "test.#{extension}") + expect(file.is_renderable?).to be result + end + end end From a2a3f3cef0ddc6cd8d8dec3fcf66c8860ab2ab7b Mon Sep 17 00:00:00 2001 From: James Smith Date: Mon, 13 Jan 2025 09:28:10 +0000 Subject: [PATCH 2/2] choose a renderable preview file if available --- app/jobs/model_scan_job.rb | 8 +++++++- spec/jobs/model_scan_job_spec.rb | 8 ++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/jobs/model_scan_job.rb b/app/jobs/model_scan_job.rb index 3c828bf27..2a20cc0cb 100644 --- a/app/jobs/model_scan_job.rb +++ b/app/jobs/model_scan_job.rb @@ -28,7 +28,7 @@ def perform(model_id, include_all_subfolders: false) end # Set tags and default files model.model_files.reload - model.preview_file = model.model_files.min_by { it.is_image? ? 0 : 1 } unless model.preview_file + model.preview_file = model.model_files.min_by { preview_priority(it) } unless model.preview_file if model.tags.empty? model.generate_tags_from_directory_name! if SiteSettings.model_tags_tag_model_directory_name if SiteSettings.model_tags_auto_tag_new.present? @@ -41,4 +41,10 @@ def perform(model_id, include_all_subfolders: false) model.save! # Integrity check will run automatically after save end + + def preview_priority(file) + return 0 if file.is_image? + return 1 if file.is_renderable? + 100 + end end diff --git a/spec/jobs/model_scan_job_spec.rb b/spec/jobs/model_scan_job_spec.rb index a58aeb064..01387ae3a 100644 --- a/spec/jobs/model_scan_job_spec.rb +++ b/spec/jobs/model_scan_job_spec.rb @@ -5,7 +5,7 @@ context "with a simple model folder" do around do |ex| MockDirectory.create([ - "model_one/part_1.obj", + "model_one/part_1.lys", "model_one/part_2.obj" ]) do |path| @library_path = path @@ -21,13 +21,13 @@ it "detects model files" do # rubocop:todo RSpec/MultipleExpectations expect { described_class.perform_now(model.id) }.to change { model.model_files.count }.to(2) - expect(model.model_files.map(&:filename)).to eq ["part_1.obj", "part_2.obj"] + expect(model.model_files.map(&:filename)).to eq ["part_1.lys", "part_2.obj"] end - it "sets the preview file to the first scanned file by default" do # rubocop:todo RSpec/MultipleExpectations + it "sets the preview file to the first renderable scanned file by default" do # rubocop:todo RSpec/MultipleExpectations expect { described_class.perform_now(model.id) }.to change { model.model_files.count }.to(2) model.reload - expect(model.preview_file.filename).to eq "part_1.obj" + expect(model.preview_file.filename).to eq "part_2.obj" end it "queues up individual file scans" do