jro_apps/redmine Commit - r13152:32b79b6fd4e3

Fixed that IssueRelation should not be responsible for calling Issue#init_journal (#18237)....

Jean-Philippe Lang -

r13152:32b79b6fd4e3

parent child

Context file:

r13152:32b79b6fd4e3

Collapse all files

app/controllers/issue_relations_controller.rb +2 0

                 if params[:relation] && m = params[:relation][:issue_to_id].to_s.strip.match(/^#?(\d+)$/)
                   @relation.issue_to = Issue.visible.find_by_id(m[1].to_i)
                 end
+                @relation.init_journals(User.current)
                 saved = @relation.save
                 respond_to do |format|
               def destroy
                 raise Unauthorized unless @relation.deletable?
+                @relation.init_journals(User.current)
                 @relation.destroy
                 respond_to do |format|

app/controllers/issues_controller.rb +1 0

               def build_new_issue_from_params
                 if params[:id].blank?
                   @issue = Issue.new
+                  @issue.init_journal(User.current)
                   if params[:copy_from]
                     begin
                       @copy_from = Issue.visible.find(params[:copy_from])

app/models/issue.rb +35 0

@@ -667,6 +667,11 class Issue < ActiveRecord::Base
667	@current_journal	667	@current_journal
668	end	668	end
669		669
		670	# Returns the current journal or nil if it's not initialized
		671	def current_journal
		672	@current_journal
		673	end
		674
670	# Returns the id of the last journal or nil	675	# Returns the id of the last journal or nil
671	def last_journal_id	676	def last_journal_id
672	if new_record?	677	if new_record?
@@ -1308,6 +1313,9 class Issue < ActiveRecord::Base
1308	return unless copy? && !@after_create_from_copy_handled	1313	return unless copy? && !@after_create_from_copy_handled
1309		1314
1310	if (@copied_from.project_id == project_id \|\| Setting.cross_project_issue_relations?) && @copy_options[:link] != false	1315	if (@copied_from.project_id == project_id \|\| Setting.cross_project_issue_relations?) && @copy_options[:link] != false
		1316	if @current_journal
		1317	@copied_from.init_journal(@current_journal.user)
		1318	end
1311	relation = IssueRelation.new(:issue_from => @copied_from, :issue_to => self, :relation_type => IssueRelation::TYPE_COPIED_TO)	1319	relation = IssueRelation.new(:issue_from => @copied_from, :issue_to => self, :relation_type => IssueRelation::TYPE_COPIED_TO)
1312	unless relation.save	1320	unless relation.save
1313	logger.error "Could not create relation while copying ##{@copied_from.id} to ##{id} due to validation errors: #{relation.errors.full_messages.join(', ')}" if logger	1321	logger.error "Could not create relation while copying ##{@copied_from.id} to ##{id} due to validation errors: #{relation.errors.full_messages.join(', ')}" if logger
@@ -1328,6 +1336,9 class Issue < ActiveRecord::Base
1328	next	1336	next
1329	end	1337	end
1330	copy = Issue.new.copy_from(child, copy_options)	1338	copy = Issue.new.copy_from(child, copy_options)
		1339	if @current_journal
		1340	copy.init_journal(@current_journal.user)
		1341	end
1331	copy.author = author	1342	copy.author = author
1332	copy.project = project	1343	copy.project = project
1333	copy.parent_issue_id = copied_issue_ids[child.parent_id]	1344	copy.parent_issue_id = copied_issue_ids[child.parent_id]
@@ -1477,6 +1488,30 class Issue < ActiveRecord::Base
1477	end	1488	end
1478	end	1489	end
1479		1490
		1491	# Called after a relation is added
		1492	def relation_added(relation)
		1493	if @current_journal
		1494	@current_journal.details << JournalDetail.new(
		1495	:property => 'relation',
		1496	:prop_key => relation.relation_type_for(self),
		1497	:value => relation.other_issue(self).try(:id)
		1498	)
		1499	@current_journal.save
		1500	end
		1501	end
		1502
		1503	# Called after a relation is removed
		1504	def relation_removed(relation)
		1505	if @current_journal
		1506	@current_journal.details << JournalDetail.new(
		1507	:property => 'relation',
		1508	:prop_key => relation.relation_type_for(self),
		1509	:old_value => relation.other_issue(self).try(:id)
		1510	)
		1511	@current_journal.save
		1512	end
		1513	end
		1514
1480	# Default assignment based on category	1515	# Default assignment based on category
1481	def default_assign	1516	def default_assign
1482	if assigned_to.nil? && category && category.assigned_to	1517	if assigned_to.nil? && category && category.assigned_to

app/models/issue_relation.rb +21 -26

@@ -72,8 +72,8 class IssueRelation < ActiveRecord::Base
72		72
73	attr_protected :issue_from_id, :issue_to_id	73	attr_protected :issue_from_id, :issue_to_id
74	before_save :handle_issue_order	74	before_save :handle_issue_order
75	after_create :create_journal_after_create	75	after_create :call_issues_relation_added_callback
76	after_destroy :create_journal_after_delete	76	after_destroy :call_issues_relation_removed_callback
77		77
78	def visible?(user=User.current)	78	def visible?(user=User.current)
79	(issue_from.nil? \|\| issue_from.visible?(user)) && (issue_to.nil? \|\| issue_to.visible?(user))	79	(issue_from.nil? \|\| issue_from.visible?(user)) && (issue_to.nil? \|\| issue_to.visible?(user))
@@ -168,6 +168,11 class IssueRelation < ActiveRecord::Base
168	r == 0 ? id <=> relation.id : r	168	r == 0 ? id <=> relation.id : r
169	end	169	end
170		170
		171	def init_journals(user)
		172	issue_from.init_journal(user) if issue_from
		173	issue_to.init_journal(user) if issue_to
		174	end
		175
171	private	176	private
172		177
173	# Reverses the relation if needed so that it gets stored in the proper way	178	# Reverses the relation if needed so that it gets stored in the proper way
@@ -182,29 +187,19 class IssueRelation < ActiveRecord::Base
182	end	187	end
183	end	188	end
184		189
185	def create_journal_after_create	190	def call_issues_relation_added_callback
186	journal = issue_from.init_journal(User.current)	191	call_issues_callback :relation_added
187	journal.details << JournalDetail.new(:property => 'relation',	192	end
188	:prop_key => relation_type_for(issue_from),	193
189	:value => issue_to.id)	194	def call_issues_relation_removed_callback
190	journal.save	195	call_issues_callback :relation_removed
191	journal = issue_to.init_journal(User.current)	196	end
192	journal.details << JournalDetail.new(:property => 'relation',	197
193	:prop_key => relation_type_for(issue_to),	198	def call_issues_callback(name)
194	:value => issue_from.id)	199	[issue_from, issue_to].each do \|issue\|
195	journal.save	200	if issue
196	end	201	issue.send name, self
197		202	end
198	def create_journal_after_delete	203	end
199	journal = issue_from.init_journal(User.current)
200	journal.details << JournalDetail.new(:property => 'relation',
201	:prop_key => relation_type_for(issue_from),
202	:old_value => issue_to.id)
203	journal.save
204	journal = issue_to.init_journal(User.current)
205	journal.details << JournalDetail.new(:property => 'relation',
206	:prop_key => relation_type_for(issue_to),
207	:old_value => issue_from.id)
208	journal.save
209	end	204	end
210	end	205	end

test/unit/issue_relation_test.rb +4 -2

                 assert_not_equal [], r.errors[:base]
               end
-              def test_create_should_make_journal_entry
+              def test_create_with_initialized_journals_should_create_journals
                 from = Issue.find(1)
                 to   = Issue.find(2)
                 from_journals = from.journals.size
                 to_journals   = to.journals.size
                 relation = IssueRelation.new(:issue_from => from, :issue_to => to,
                                              :relation_type => IssueRelation::TYPE_PRECEDES)
+                relation.init_journals User.find(1)
                 assert relation.save
                 from.reload
                 to.reload
                 assert_nil   to.journals.last.details.last.old_value
               end
-              def test_delete_should_make_journal_entry
+              def test_destroy_with_initialized_journals_should_create_journals
                 relation = IssueRelation.find(1)
                 from = relation.issue_from
                 to   = relation.issue_to
                 from_journals = from.journals.size
                 to_journals   = to.journals.size
+                relation.init_journals User.find(1)
                 assert relation.destroy
                 from.reload
                 to.reload

test/unit/journal_test.rb +5 -7

               def test_visible_details_should_include_relations_to_visible_issues_only
                 issue = Issue.generate!
                 visible_issue = Issue.generate!
-                IssueRelation.create!(:issue_from => issue, :issue_to => visible_issue, :relation_type => 'relates')
                 hidden_issue = Issue.generate!(:is_private => true)
-                IssueRelation.create!(:issue_from => issue, :issue_to => hidden_issue, :relation_type => 'relates')
-                issue.reload
+                journal = Journal.new
-                assert_equal 1, issue.journals.size
+                journal.details << JournalDetail.new(:property => 'relation', :prop_key => 'relates', :value => visible_issue.id)
-                journal = issue.journals.first
+                journal.details << JournalDetail.new(:property => 'relation', :prop_key => 'relates', :value => hidden_issue.id)
-                assert_equal 2, journal.details.size
                 visible_details = journal.visible_details(User.anonymous)
                 assert_equal 1, visible_details.size
-                assert_equal visible_issue.id.to_s, visible_details.first.value
+                assert_equal visible_issue.id.to_s, visible_details.first.value.to_s
                 visible_details = journal.visible_details(User.find(2))
                 assert_equal 2, visible_details.size

test/unit/mailer_test.rb +1 0

               def test_issue_edit_with_relation_should_notify_users_who_can_see_the_related_issue
                 issue = Issue.generate!
+                issue.init_journal(User.find(1))
                 private_issue = Issue.generate!(:is_private => true)
                 IssueRelation.create!(:issue_from => issue, :issue_to => private_issue, :relation_type => 'relates')
                 issue.reload

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages