Set comment attachment replacements on edit

When a comment is updated, replace attachments.

https://community.openproject.org/wp/63384

Author: akabiru

app/controllers/work_packages/activities_tab_controller.rb

@@ -129,6 +129,7 @@ def update
       call = update_journal_service_call
 
       if call.success? && call.result
+        claim_journal_attachments_for(call.result)
         update_item_show_component(journal: call.result, grouped_emoji_reactions: grouped_emoji_reactions_for_journal)
       else
         handle_failed_create_or_update_call(call)

spec/features/activities/work_package/activity_tab_comment_editor_spec.rb

@@ -147,5 +147,43 @@ def expect_editor_to_be_dismissed_with_confirmation(&)
       journal = work_package.reload.journals.last
       expect(journal.attachments).to contain_exactly(attachment)
     end
+
+    context "when editing an existing comment" do
+      let(:comment) do
+        create(:work_package_journal,
+               user: admin,
+               notes: notes_with_attachment(existing_attachment),
+               journable: work_package,
+               version: 2).tap do |journal|
+                 journal.attachments << existing_attachment
+                 journal.save(validate: false)
+               end
+      end
+
+      let!(:existing_attachment) { create(:attachment, author: admin, container: nil) }
+
+      it "updates the comment with new attachments" do
+        expect(comment.reload.attachments).to contain_exactly(existing_attachment)
+
+        activity_tab.edit_comment(comment, text: "Some notes", save: false)
+
+        editor.drag_attachment(image_fixture.path, "An image caption")
+        editor.wait_until_upload_progress_toaster_cleared
+
+        click_on "Save"
+
+        expect(page).to have_content("An image caption")
+        expect(comment.reload.attachments).to contain_exactly(Attachment.where(author: admin).last)
+        expect(comment.reload.attachments).not_to include(existing_attachment)
+      end
+
+      def notes_with_attachment(attachment)
+        <<~HTML
+          <img class="op-uc-image op-uc-image_inline" src="/api/v3/attachments/#{attachment.id}/content">
+
+          First attachment
+        HTML
+      end
+    end
   end
 end

spec/services/work_packages/activities_tab/comment_attachments_claims/claims_service_spec.rb

@@ -104,5 +104,53 @@
         expect(journal.reload.attachments).to contain_exactly(attachment1, attachment2, attachment3)
       end
     end
+
+    context "with existing comment attachments" do
+      let(:comment) do
+        work_package.add_journal(user:, notes:)
+        work_package.save(validate: false)
+        work_package.journals.last
+      end
+
+      let(:existing_attachment) { create(:attachment, author: user, container: nil) }
+      let(:existing_attachment_excluded) { create(:attachment, author: user, container: nil) }
+      let(:newly_attached) { create(:attachment, author: user, container: nil) }
+
+      let(:notes) do
+        <<~HTML
+          <img class="op-uc-image op-uc-image_inline" src="/api/v3/attachments/#{existing_attachment.id}/content">
+
+          First attachment
+
+          <br>
+
+          <img class="op-uc-image op-uc-image_inline" src="/api/v3/attachments/#{newly_attached.id}/content">
+
+          Second attachment
+        HTML
+      end
+
+      before do
+        comment.attachments << [existing_attachment, existing_attachment_excluded]
+        comment.save(validate: false)
+      end
+
+      subject(:attachment_claims_service) do
+        described_class.new(
+          user: user,
+          model: comment
+        )
+      end
+
+      it "replaces the comment attachments with newly update ones" do
+        expect(comment.reload.attachments).to contain_exactly(existing_attachment, existing_attachment_excluded)
+
+        claim_result = attachment_claims_service.call
+        expect(claim_result).to be_success
+
+        expect(comment.reload.attachments).to contain_exactly(existing_attachment, newly_attached)
+        expect(Attachment.exists?(existing_attachment_excluded.id)).to be(false)
+      end
+    end
   end
 end