Feature #2621 » 0001-Handle-the-StaleObjectError-when-two-users-edit-the-.patch
app/controllers/application_controller.rb | ||
---|---|---|
218 | 218 |
unsaved = [] |
219 | 219 |
if attachments && attachments.is_a?(Hash) |
220 | 220 |
attachments.each_value do |attachment| |
221 |
file = attachment['file'] |
|
222 |
next unless file && file.size > 0 |
|
223 |
a = Attachment.create(:container => obj, |
|
224 |
:file => file, |
|
225 |
:description => attachment['description'].to_s.strip, |
|
226 |
:author => User.current) |
|
227 |
a.new_record? ? (unsaved << a) : (attached << a) |
|
221 |
# Attachment was already saved, but not associated with the |
|
222 |
# journal (e.g. StaleObjectError) |
|
223 |
if attachment['id'] && Attachment.find_by_id(attachment['id']) |
|
224 |
saved_attachment = Attachment.find_by_id(attachment['id']) |
|
225 |
if saved_attachment.container == obj |
|
226 |
attached << saved_attachment |
|
227 |
else |
|
228 |
unsaved << saved_attachment |
|
229 |
end |
|
230 |
# New attachment |
|
231 |
elsif file = attachment['file'] && attachment['file'].size > 0 |
|
232 |
file = attachment['file'] |
|
233 |
a = Attachment.create(:container => obj, |
|
234 |
:file => file, |
|
235 |
:description => attachment['description'].to_s.strip, |
|
236 |
:author => User.current) |
|
237 |
a.new_record? ? (unsaved << a) : (attached << a) |
|
238 |
else |
|
239 |
next |
|
240 |
end |
|
241 | ||
228 | 242 |
end |
229 | 243 |
if unsaved.any? |
230 | 244 |
flash[:warning] = l(:warning_attachments_not_saved, unsaved.size) |
app/controllers/issues_controller.rb | ||
---|---|---|
206 | 206 |
end |
207 | 207 |
rescue ActiveRecord::StaleObjectError |
208 | 208 |
# Optimistic locking exception |
209 |
flash.now[:error] = l(:notice_locking_conflict) |
|
210 |
# Remove the previously added attachments if issue was not updated |
|
211 |
attachments.each(&:destroy) |
|
209 |
@issue.reload # Get new lock_version |
|
210 |
@stale_object = StaleObject.new(@issue) |
|
211 |
@attachments_not_on_journal = attachments |
|
212 | ||
213 |
# Pull in updated attribute changes |
|
214 |
# User can change issue attributes only if he has :edit permission or if a workflow transition is allowed |
|
215 |
if (@edit_allowed || !@allowed_statuses.empty?) && params[:issue] |
|
216 |
attrs = params[:issue].dup |
|
217 |
attrs.delete_if {|k,v| !UPDATABLE_ATTRS_ON_TRANSITION.include?(k) } unless @edit_allowed |
|
218 |
attrs.delete(:status_id) unless @allowed_statuses.detect {|s| s.id.to_s == attrs[:status_id].to_s} |
|
219 |
attrs.delete(:lock_version) |
|
220 |
@issue.attributes = attrs |
|
221 |
end |
|
222 | ||
223 |
flash.now[:error] = l(:notice_locking_conflict) + @stale_object.difference_messages(@issue) |
|
212 | 224 |
end |
213 | 225 | |
214 | 226 |
def reply |
app/views/attachments/_upload_conflict.html.erb | ||
---|---|---|
1 |
<div class="flash warning"> |
|
2 |
<ul> |
|
3 |
<% attachments.each do |attachment| %> |
|
4 |
<%= hidden_field_tag "attachments[#{attachment.id}][id]", h(attachment.id) %> |
|
5 |
<li> |
|
6 |
<%= l(:notice_file_saved_temporary, {:filename => h(attachment.filename)}) %> |
|
7 |
</li> |
|
8 |
<% end %> |
|
9 |
</ul> |
|
10 |
</div> |
app/views/issues/_edit.rhtml | ||
---|---|---|
37 | 37 |
<%= wikitoolbar_for 'notes' %> |
38 | 38 |
<%= call_hook(:view_issues_edit_notes_bottom, { :issue => @issue, :notes => @notes, :form => f }) %> |
39 | 39 |
|
40 |
<% if @attachments_not_on_journal && @attachments_not_on_journal.present? %> |
|
41 |
<%= render :partial => 'attachments/upload_conflict', :locals => {:attachments => @attachments_not_on_journal } %> |
|
42 |
<% end %> |
|
43 | ||
40 | 44 |
<p><%=l(:label_attachment_plural)%><br /><%= render :partial => 'attachments/form' %></p> |
41 | 45 |
</fieldset> |
42 | 46 |
</div> |
config/locales/en.yml | ||
---|---|---|
147 | 147 |
notice_account_pending: "Your account was created and is now pending administrator approval." |
148 | 148 |
notice_default_data_loaded: Default configuration successfully loaded. |
149 | 149 |
notice_unable_delete_version: Unable to delete version. |
150 |
notice_file_saved_temporary: "File '{{filename}}' was saved. To permanently save it with a note, please submit this form." |
|
150 | 151 |
|
151 | 152 |
error_can_t_load_default_data: "Default configuration could not be loaded: {{value}}" |
152 | 153 |
error_scm_not_found: "The entry or revision was not found in the repository." |
... | ... | |
823 | 824 |
text_wiki_page_nullify_children: "Keep child pages as root pages" |
824 | 825 |
text_wiki_page_destroy_children: "Delete child pages and all their descendants" |
825 | 826 |
text_wiki_page_reassign_children: "Reassign child pages to this parent page" |
827 |
text_changed_to: "{{label}} changed to {{new}}" |
|
826 | 828 |
|
827 | 829 |
default_role_manager: Manager |
828 | 830 |
default_role_developper: Developer |
lib/stale_object.rb | ||
---|---|---|
1 |
class StaleObject |
|
2 |
include Redmine::I18n |
|
3 |
include ERB::Util |
|
4 |
attr_accessor :attributes |
|
5 |
attr_accessor :changes |
|
6 |
|
|
7 |
def initialize(stale_object) |
|
8 |
raise ArgumentError.new("Call with an ActiveRecord object") unless stale_object.respond_to?(:attributes) |
|
9 |
@attributes = stale_object.attributes.dup |
|
10 |
end |
|
11 |
|
|
12 |
def difference_messages(fresh_object, options = { }) |
|
13 |
options = { |
|
14 |
:wrap => "ul", |
|
15 |
:item => "li" |
|
16 |
}.merge(options) |
|
17 | ||
18 |
changes = self.changes(fresh_object) |
|
19 |
|
|
20 |
if changes.empty? |
|
21 |
error_messages = '' |
|
22 |
else |
|
23 |
error_messages = "<#{options[:wrap]}>" |
|
24 |
changes.each do |key,value| |
|
25 |
if key.match(/(.*)(_id)$/) |
|
26 |
association = fresh_object.class.reflect_on_association($1.to_sym) |
|
27 |
if association |
|
28 |
field = 'field_' + key.sub($2,'') |
|
29 |
if value.nil? |
|
30 |
data_value = l(:label_none) |
|
31 |
else |
|
32 |
data_value = association.klass.find(value) |
|
33 |
end |
|
34 |
end |
|
35 |
end |
|
36 |
field ||= 'field_' + key |
|
37 |
data_value ||= value || l(:label_none) |
|
38 | ||
39 |
error_messages << "<#{options[:item]}>#{l(:text_changed_to, :label => l(field.to_sym), :new => html_escape(data_value))}</#{options[:item]}>" |
|
40 |
end |
|
41 |
error_messages << "</#{options[:wrap]}>" |
|
42 |
end |
|
43 |
|
|
44 |
return error_messages |
|
45 |
end |
|
46 | ||
47 |
def changes(fresh_object) |
|
48 |
@changes ||= @attributes.diff(fresh_object.attributes).except('lock_version') || { } |
|
49 |
end |
|
50 |
end |
test/unit/lib/stale_object_test.rb | ||
---|---|---|
1 |
# Redmine - project management software |
|
2 |
# Copyright (C) 2006-2008 Jean-Philippe Lang |
|
3 |
# |
|
4 |
# This program is free software; you can redistribute it and/or |
|
5 |
# modify it under the terms of the GNU General Public License |
|
6 |
# as published by the Free Software Foundation; either version 2 |
|
7 |
# of the License, or (at your option) any later version. |
|
8 |
# |
|
9 |
# This program is distributed in the hope that it will be useful, |
|
10 |
# but WITHOUT ANY WARRANTY; without even the implied warranty of |
|
11 |
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
|
12 |
# GNU General Public License for more details. |
|
13 |
# |
|
14 |
# You should have received a copy of the GNU General Public License |
|
15 |
# along with this program; if not, write to the Free Software |
|
16 |
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. |
|
17 | ||
18 |
require File.dirname(__FILE__) + '/../../test_helper' |
|
19 | ||
20 |
class StaleObjectTest < Test::Unit::TestCase |
|
21 | ||
22 |
# TODO: context |
|
23 |
# initialize |
|
24 |
def test_attributes_set |
|
25 |
issue = Issue.find(:first) |
|
26 |
stale = StaleObject.new(issue) |
|
27 |
assert stale.attributes == issue.attributes |
|
28 |
end |
|
29 | ||
30 |
def test_invalid_stale_object_should_raise_an_exception |
|
31 |
object = Object.new |
|
32 |
assert_raise ArgumentError do |
|
33 |
stale = StaleObject.new(object) |
|
34 |
end |
|
35 |
end |
|
36 |
|
|
37 |
# difference_messages |
|
38 |
def test_difference_message_with_no_changes_should_be_an_empty_string |
|
39 |
issue = Issue.find(:first) |
|
40 |
stale = StaleObject.new(issue) |
|
41 |
assert stale.difference_messages(issue).empty? |
|
42 |
end |
|
43 | ||
44 |
def test_difference_message_with_changes_on_fields_should_return_the_field_names_and_values |
|
45 |
issue = Issue.find(:first) |
|
46 |
stale = StaleObject.new(issue) |
|
47 |
issue.subject = "Changed" |
|
48 |
|
|
49 |
message = stale.difference_messages(issue) |
|
50 |
assert_match /subject/i, message |
|
51 |
assert_match /changed/i, message |
|
52 |
end |
|
53 | ||
54 |
def test_difference_message_with_changes_on_associations_should_return_the_association_names |
|
55 |
issue = Issue.find(:first) |
|
56 |
stale = StaleObject.new(issue) |
|
57 |
issue.status = IssueStatus.find(6) |
|
58 |
|
|
59 |
message = stale.difference_messages(issue) |
|
60 |
assert_match /status/i, message |
|
61 |
assert_match /new/i, message |
|
62 |
end |
|
63 |
|
|
64 |
def test_difference_message_with_changes_on_associations_should_work_with_nils |
|
65 |
issue = Issue.find(:first) |
|
66 |
issue.category = IssueCategory.find(:last) |
|
67 |
stale = StaleObject.new(issue) |
|
68 |
issue.category = nil # Set association to nil |
|
69 |
|
|
70 |
message = stale.difference_messages(issue) |
|
71 |
assert_match /category/i, message |
|
72 |
assert_match /printing/i, message |
|
73 |
end |
|
74 | ||
75 |
def test_difference_message_with_nil_set_on_a_field_should_show_none |
|
76 |
issue = Issue.find(:first) |
|
77 |
issue.estimated_hours = 6 |
|
78 |
stale = StaleObject.new(issue) |
|
79 |
issue.estimated_hours = nil |
|
80 |
|
|
81 |
message = stale.difference_messages(issue) |
|
82 |
assert_match /est/i, message |
|
83 |
assert_match /6/i, message |
|
84 |
end |
|
85 | ||
86 |
def test_difference_message_should_allow_for_a_different_wrap_tag |
|
87 |
issue = Issue.find(:first) |
|
88 |
stale = StaleObject.new(issue) |
|
89 |
issue.subject = "Changed" |
|
90 |
|
|
91 |
message = stale.difference_messages(issue, :wrap => "span") |
|
92 |
assert_match /<span>/i, message |
|
93 |
assert_match /<\/span>/i, message |
|
94 |
end |
|
95 | ||
96 |
def test_difference_message_should_allow_for_a_different_item_tag |
|
97 |
issue = Issue.find(:first) |
|
98 |
stale = StaleObject.new(issue) |
|
99 |
issue.subject = "Changed" |
|
100 |
|
|
101 |
message = stale.difference_messages(issue, :item => "span") |
|
102 |
assert_match /<span>/i, message |
|
103 |
assert_match /<\/span>/i, message |
|
104 |
end |
|
105 |
end |
|
106 |
- « Previous
- 1
- 2
- Next »