manyfold3d · Floppy · Oct 11, 2024 · Oct 11, 2024
diff --git a/app/models/model.rb b/app/models/model.rb
@@ -52,10 +52,15 @@ def merge_into!(target)
     relative_path = Pathname.new(path).relative_path_from(Pathname.new(target.path))
     # Move files
     model_files.each do |f|
-      f.update(
-        filename: File.join(relative_path, f.filename),
-        model: target
-      )
+      new_filename = File.join(relative_path, f.filename)
+      if target.model_files.exists?(filename: new_filename)
+        f.delete # Don't run callbacks, just remove the database record
+      else
+        f.update(
+          filename: new_filename,
+          model: target
+        )
+      end
     end
     Scan::CheckModelIntegrityJob.set(wait: 5.seconds).perform_later(target.id)
     reload

diff --git a/spec/models/model_spec.rb b/spec/models/model_spec.rb
@@ -101,8 +101,9 @@
   context "when nested inside another" do
     around do |ex|
       MockDirectory.create([
-        "parent/part.stl",
-        "parent/child/part.stl"
+        "parent/parent_part.stl",
+        "parent/child/child_part.stl",
+        "parent/child/duplicate.stl"
       ]) do |path|
         @library_path = path
         ex.run
@@ -128,10 +129,10 @@
 
     context "when merging into parent" do
       it "moves files" do # rubocop:todo RSpec/MultipleExpectations
-        file = create(:model_file, model: child, filename: "part.stl")
+        file = create(:model_file, model: child, filename: "child_part.stl")
         child.merge_into! parent
         file.reload
-        expect(file.filename).to eql "child/part.stl"
+        expect(file.filename).to eql "child/child_part.stl"
         expect(file.model).to eql parent
       end
 
@@ -141,6 +142,33 @@
         }.to change(described_class, :count).from(2).to(1)
       end
     end
+
+    context "when merging models that have duplicated files" do
+      before do
+        create(:model_file, model: parent, filename: "parent_part.stl")
+        create(:model_file, model: parent, filename: "child/duplicate.stl")
+        create(:model_file, model: child, filename: "duplicate.stl")
+        create(:model_file, model: child, filename: "child_part.stl")
+      end
+
+      it "removes duplicated file" do
+        expect {
+          child.merge_into! parent
+        }.to change(ModelFile, :count).by(-1)
+      end
+
+      it "rehomes distinct file" do
+        child.merge_into! parent
+        expect(parent.model_files.exists?(filename: "child/child_part.stl")).to be true
+      end
+
+      it "keeps all real files intact" do
+        child.merge_into! parent
+        parent.model_files.each do |file|
+          expect(file.exists_on_storage?).to be true
+        end
+      end
+    end
   end
 
   context "when nested inside another with underscores in the name" do