From 92152743d77fec664266555a25d4d2c3f9c8f3f2 Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Mon, 3 Aug 2020 13:36:53 +0200 Subject: [PATCH] Fixes #2960 - Ticket removal of merged / linked tickets doesn't remove references --- .../_application_controller_generic.coffee | 10 +++- app/models/ticket.rb | 1 + app/models/ticket/merge_history.rb | 28 +++++++++ spec/models/ticket_spec.rb | 24 +++++++- spec/system/ticket/update_spec.rb | 59 +++++++++++++++++-- 5 files changed, 111 insertions(+), 11 deletions(-) create mode 100644 app/models/ticket/merge_history.rb diff --git a/app/assets/javascripts/app/controllers/_application_controller_generic.coffee b/app/assets/javascripts/app/controllers/_application_controller_generic.coffee index 471bc42b6..f61f99004 100644 --- a/app/assets/javascripts/app/controllers/_application_controller_generic.coffee +++ b/app/assets/javascripts/app/controllers/_application_controller_generic.coffee @@ -645,11 +645,17 @@ class App.GenericHistory extends App.ControllerModal content = "#{ @T( item.type ) } #{ @T( 'sent to' ) } '#{ item.value_to }'" else if item.type is 'received_merge' ticket = App.Ticket.find( item.id_from ) - ticket_link = "##{ ticket.number }" + ticket_link = if ticket + "##{ ticket.number }" + else + item.value_from content = "#{ @T( 'Ticket' ) } #{ ticket_link } #{ @T( 'was merged into this ticket' ) }" else if item.type is 'merged_into' ticket = App.Ticket.find( item.id_to ) - ticket_link = "##{ ticket.number }" + ticket_link = if ticket + "##{ ticket.number }" + else + item.value_to content = "#{ @T( 'This ticket was merged into' ) } #{ @T( 'ticket' ) } #{ ticket_link }" else content = "#{ @T( item.type ) } #{ @T(item.object) } " diff --git a/app/models/ticket.rb b/app/models/ticket.rb index f09ca42e3..3fbbe4108 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -20,6 +20,7 @@ class Ticket < ApplicationModel include Ticket::Assets include Ticket::SearchIndex include Ticket::Search + include Ticket::MergeHistory store :preferences before_create :check_generate, :check_defaults, :check_title, :set_default_state, :set_default_priority diff --git a/app/models/ticket/merge_history.rb b/app/models/ticket/merge_history.rb new file mode 100644 index 000000000..2c97659f0 --- /dev/null +++ b/app/models/ticket/merge_history.rb @@ -0,0 +1,28 @@ +module Ticket::MergeHistory + extend ActiveSupport::Concern + + included do + after_destroy :merge_history_cleanup + end + + private + + def merge_history_cleanup + cleanup_type :received_merge, :id_from, :value_from + cleanup_type :merged_into, :id_to, :value_to + end + + def cleanup_type(history_type_name, lookup_attribute_name, target_attribute_name) + type = History.type_lookup history_type_name + object = History.object_lookup self.class.name + + History + .where(history_object_id: object, history_type_id: type) + .find_by(lookup_attribute_name => id) + &.update!(target_attribute_name => replacement_title) + end + + def replacement_title + "##{number} #{title}" + end +end diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 4c4523dc5..d300f65f3 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -81,7 +81,6 @@ RSpec.describe Ticket, type: :model do end end - # Issue #2469 - Add information "Ticket merged" to History context 'when merging' do let(:merge_user) { create(:user) } @@ -91,11 +90,12 @@ RSpec.describe Ticket, type: :model do # when creating the history entries target_ticket travel 5.minutes + + ticket.merge_to(ticket_id: target_ticket.id, user_id: merge_user.id) end + # Issue #2469 - Add information "Ticket merged" to History it 'creates history entries in both the origin ticket and the target ticket' do - ticket.merge_to(ticket_id: target_ticket.id, user_id: merge_user.id) - expect(target_ticket.history_get.size).to eq 2 target_history = target_ticket.history_get.last @@ -124,6 +124,24 @@ RSpec.describe Ticket, type: :model do expect(ExternalSync).to have_received(:migrate).with('Ticket', ticket.id, target_ticket.id) end + + # Issue #2960 - Ticket removal of merged / linked tickets doesn't remove references + context 'and deleting the origin ticket' do + it 'adds reference number and title to the target ticket' do + expect { ticket.destroy } + .to change { target_ticket.history_get.find { |elem| elem.fetch('type') == 'received_merge' }.dig('value_from') } + .to("##{ticket.number} #{ticket.title}") + end + end + + # Issue #2960 - Ticket removal of merged / linked tickets doesn't remove references + context 'and deleting the target ticket' do + it 'adds reference number and title to the origin ticket' do + expect { target_ticket.destroy } + .to change { ticket.history_get.find { |elem| elem.fetch('type') == 'merged_into' }.dig('value_to') } + .to("##{target_ticket.number} #{target_ticket.title}") + end + end end end diff --git a/spec/system/ticket/update_spec.rb b/spec/system/ticket/update_spec.rb index 3d782ab4d..f3d7016db 100644 --- a/spec/system/ticket/update_spec.rb +++ b/spec/system/ticket/update_spec.rb @@ -170,14 +170,17 @@ RSpec.describe 'Ticket Update', type: :system do end end - # Issue #2469 - Add information "Ticket merged" to History context 'when merging tickets' do - it 'tickets history of both tickets should show the merge event' do - user = create :user - origin_ticket = create :ticket, group: group - target_ticket = create :ticket, group: group - origin_ticket.merge_to(ticket_id: target_ticket.id, user_id: user.id) + let!(:user) { create(:user) } + let!(:origin_ticket) { create :ticket, group: group } + let!(:target_ticket) { create :ticket, group: group } + before do + origin_ticket.merge_to(ticket_id: target_ticket.id, user_id: user.id) + end + + # Issue #2469 - Add information "Ticket merged" to History + it 'tickets history of both tickets should show the merge event' do visit "#ticket/zoom/#{origin_ticket.id}" within(:active_content) do expect(page).to have_css('.js-actions .dropdown-toggle', wait: 3) @@ -200,6 +203,50 @@ RSpec.describe 'Ticket Update', type: :system do expect(modal).to have_link "##{origin_ticket.number}", href: "#ticket/zoom/#{origin_ticket.id}" end end + + # Issue #2960 - Ticket removal of merged / linked tickets doesn't remove references + context 'when the merged origin ticket is deleted' do + before do + origin_ticket.destroy + end + + it 'shows the target ticket history' do + visit "#ticket/zoom/#{target_ticket.id}" + within(:active_content) do + expect(page).to have_css('.js-actions .dropdown-toggle', wait: 3) + click '.js-actions .dropdown-toggle' + click '.js-actions .dropdown-menu [data-type="ticket-history"]' + end + + modal_ready + + within('.modal-body') do + expect(page).to have_text "##{origin_ticket.number} #{origin_ticket.title}" + end + end + end + + # Issue #2960 - Ticket removal of merged / linked tickets doesn't remove references + context 'when the merged target ticket is deleted' do + before do + target_ticket.destroy + end + + it 'shows the origin history' do + visit "#ticket/zoom/#{origin_ticket.id}" + within(:active_content) do + expect(page).to have_css('.js-actions .dropdown-toggle', wait: 3) + click '.js-actions .dropdown-toggle' + click '.js-actions .dropdown-menu [data-type="ticket-history"]' + end + + modal_ready + + within('.modal-body') do + expect(page).to have_text "##{target_ticket.number} #{target_ticket.title}" + end + end + end end context 'when using text modules' do