From 43e5bb75d21f4654414e025b4fd49803507eba3c Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sun, 11 Apr 2010 16:27:37 +0000
Subject: [PATCH] Fixed: issue optimistic locking broken by r3308 (#5280).
git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3663 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
app/controllers/issues_controller.rb | 6 ------
app/models/issue.rb | 20 ++++++++++++++------
test/fixtures/issues.yml | 1 +
test/functional/issues_controller_test.rb | 23 +++++++++++++++++++++++
4 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index b97937a48..1a2f96d56 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -212,12 +212,6 @@ class IssuesController < ApplicationController
format.xml { render :xml => @issue.errors, :status => :unprocessable_entity }
end
end
-
- rescue ActiveRecord::StaleObjectError
- # Optimistic locking exception
- flash.now[:error] = l(:notice_locking_conflict)
- # Remove the previously added attachments if issue was not updated
- attachments[:files].each(&:destroy) if attachments[:files]
end
def reply
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 012661cad..e2c85e1db 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -212,6 +212,7 @@ class Issue < ActiveRecord::Base
done_ratio
estimated_hours
custom_field_values
+ lock_version
) unless const_defined?(:SAFE_ATTRIBUTES)
# Safely sets attributes
@@ -481,6 +482,7 @@ class Issue < ActiveRecord::Base
end
# Saves an issue, time_entry, attachments, and a journal from the parameters
+ # Returns false if save fails
def save_issue_with_child_records(params, existing_time_entry=nil)
if params[:time_entry] && params[:time_entry][:hours].present? && User.current.allowed_to?(:log_time, project)
@time_entry = existing_time_entry || TimeEntry.new
@@ -498,14 +500,20 @@ class Issue < ActiveRecord::Base
attachments[:files].each {|a| @current_journal.details << JournalDetail.new(:property => 'attachment', :prop_key => a.id, :value => a.filename)}
# TODO: Rename hook
Redmine::Hook.call_hook(:controller_issues_edit_before_save, { :params => params, :issue => self, :time_entry => @time_entry, :journal => @current_journal})
- if save
- # TODO: Rename hook
- Redmine::Hook.call_hook(:controller_issues_edit_after_save, { :params => params, :issue => self, :time_entry => @time_entry, :journal => @current_journal})
- return true
+ begin
+ if save
+ # TODO: Rename hook
+ Redmine::Hook.call_hook(:controller_issues_edit_after_save, { :params => params, :issue => self, :time_entry => @time_entry, :journal => @current_journal})
+ return true
+ else
+ return false
+ end
+ rescue ActiveRecord::StaleObjectError
+ attachments[:files].each(&:destroy)
+ errors.add_to_base l(:notice_locking_conflict)
+ return false
end
end
- # failure, returns false
-
end
# Unassigns issues from +version+ if it's no longer shared with issue's project
diff --git a/test/fixtures/issues.yml b/test/fixtures/issues.yml
index 6ec671e58..3adda3121 100644
--- a/test/fixtures/issues.yml
+++ b/test/fixtures/issues.yml
@@ -37,6 +37,7 @@ issues_002:
root_id: 2
lft: 1
rgt: 2
+ lock_version: 3
issues_003:
created_on: 2006-07-19 21:07:27 +02:00
project_id: 1
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 757f36c19..37561ec69 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -980,6 +980,29 @@ class IssuesControllerTest < ActionController::TestCase
assert_redirected_to :controller => 'issues', :action => 'show', :id => issue.id
end
+ def test_put_update_stale_issue
+ issue = Issue.find(2)
+ @request.session[:user_id] = 2
+
+ assert_no_difference 'Journal.count' do
+ assert_no_difference 'Attachment.count' do
+ put :update,
+ :id => issue.id,
+ :issue => {
+ :fixed_version_id => 4,
+ :lock_version => (issue.lock_version - 1)
+ },
+ :notes => '',
+ :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}}
+ end
+ end
+
+ assert_response :success
+ assert_template 'edit'
+ assert_tag :tag => 'div', :attributes => { :id => 'errorExplanation' },
+ :content => /Data has been updated by another user/
+ end
+
def test_get_bulk_edit
@request.session[:user_id] = 2
get :bulk_edit, :ids => [1, 2]
--
GitLab