Skip to content

Commit

Permalink
Fixed issue #1216 - Race condition if agents merge ticket at same tim…
Browse files Browse the repository at this point in the history
…e but in different directions.
  • Loading branch information
rolfschmidt authored and martini committed Jul 25, 2017
1 parent 2be861e commit d636996
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 1 deletion.
7 changes: 6 additions & 1 deletion app/models/ticket.rb
Expand Up @@ -275,6 +275,11 @@ def self.process_escalation

def merge_to(data)

# prevent cross merging tickets
target_ticket = Ticket.find(data[:ticket_id])
raise 'no target ticket given' if !target_ticket
raise 'invalid state for target ticket' if target_ticket.state.name == 'merged'

# update articles
Transaction.execute do

Expand Down Expand Up @@ -317,7 +322,7 @@ def merge_to(data)
save!

# touch new ticket (to broadcast change)
Ticket.find(data[:ticket_id]).touch
target_ticket.touch
end
true
end
Expand Down
11 changes: 11 additions & 0 deletions spec/factories/ticket.rb
@@ -0,0 +1,11 @@
FactoryGirl.define do
factory :ticket do
title 'Test Ticket'
group { Group.lookup(name: 'Users') }
customer { FactoryGirl.create(:customer_user) }
state { Ticket::State.lookup(name: 'new') }
priority { Ticket::Priority.lookup(name: '2 normal') }
updated_by_id 1
created_by_id 1
end
end
4 changes: 4 additions & 0 deletions spec/factories/user.rb
Expand Up @@ -18,6 +18,10 @@
created_by_id 1
end

factory :customer_user, parent: :user do
role_ids { Role.signup_role_ids.sort }
end

factory :user_login_failed, parent: :user do
login_failed { (Setting.get('password_max_login_failed').to_i || 10) + 1 }
end
Expand Down
27 changes: 27 additions & 0 deletions spec/models/ticket_spec.rb
@@ -0,0 +1,27 @@
require 'rails_helper'

RSpec.describe Ticket do

describe '.merge_to' do

it 'prevents cross merging tickets' do
source_ticket = create(:ticket)
target_ticket = create(:ticket)

result = source_ticket.merge_to(
ticket_id: target_ticket.id,
user_id: 1,
)
expect(result).to be(true)

expect {
result = target_ticket.merge_to(
ticket_id: source_ticket.id,
user_id: 1,
)
}.to raise_error('invalid state for target ticket')
end

end

end

0 comments on commit d636996

Please sign in to comment.