diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb index 67f0c29a9b..d39b61842a 100644 --- a/app/models/mail_handler.rb +++ b/app/models/mail_handler.rb @@ -508,8 +508,7 @@ class MailHandler < ActionMailer::Base def update_work_package(work_package) attributes = collect_wp_attributes_from_email_on_update(work_package) - - attributes.merge!(attachment_ids: create_attachments_from_mail.map(&:id)) + attributes[:attachment_ids] = work_package.attachment_ids + create_attachments_from_mail.map(&:id) service_call = WorkPackages::UpdateService .new(user: user, diff --git a/spec/models/mail_handler_spec.rb b/spec/models/mail_handler_spec.rb index 67dacb011b..4331131846 100644 --- a/spec/models/mail_handler_spec.rb +++ b/spec/models/mail_handler_spec.rb @@ -77,6 +77,11 @@ describe MailHandler, type: :model do let!(:mail_user) { FactoryBot.create :admin, mail: 'user@example.org' } let!(:work_package) { FactoryBot.create :work_package, project: project } + before do + # Avoid trying to extract text + allow(OpenProject::Database).to receive(:allows_tsv?).and_return false + end + it 'should update a work package with attachment' do expect(WorkPackage).to receive(:find_by).with(id: 123).and_return(work_package) @@ -93,6 +98,21 @@ describe MailHandler, type: :model do expect(work_package.attachments.count).to eq(1) expect(work_package.attachments.first.filename).to eq('Photo25.jpg') end + + + context 'with existing attachment' do + let!(:attachment) { FactoryBot.create(:attachment, container: work_package) } + + it 'does not replace it (Regression #29722)' do + work_package.reload + expect(WorkPackage).to receive(:find_by).with(id: 123).and_return(work_package) + + # Mail with two attachemnts, one of which is skipped by signature.asc filename match + submit_email 'update_ticket_with_attachment_and_sig.eml', issue: { project: 'onlinestore' } + + expect(work_package.attachments.length).to eq 2 + end + end end describe '#category' do