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

                  @current_journal
                end
+             	# Returns the current journal or nil if it's not initialized
+               def current_journal
+                 @current_journal
+               end
                # Returns the id of the last journal or nil
                def last_journal_id
                  if new_record?
                  return unless copy? && !@after_create_from_copy_handled
                  if (@copied_from.project_id == project_id || Setting.cross_project_issue_relations?) && @copy_options[:link] != false
+                   if @current_journal
+                     @copied_from.init_journal(@current_journal.user)
+                   end
                    relation = IssueRelation.new(:issue_from => @copied_from, :issue_to => self, :relation_type => IssueRelation::TYPE_COPIED_TO)
                    unless relation.save
                      logger.error "Could not create relation while copying ##{@copied_from.id} to ##{id} due to validation errors: #{relation.errors.full_messages.join(', ')}" if logger
                        next
                      end
                      copy = Issue.new.copy_from(child, copy_options)
+                     if @current_journal
+                       copy.init_journal(@current_journal.user)
+                     end
                      copy.author = author
                      copy.project = project
                      copy.parent_issue_id = copied_issue_ids[child.parent_id]
                  end
                end
+               # Called after a relation is added
+               def relation_added(relation)
+                 if @current_journal
+                   @current_journal.details << JournalDetail.new(
+                     :property  => 'relation',
+                     :prop_key  => relation.relation_type_for(self),
+                     :value => relation.other_issue(self).try(:id)
+                   )
+                   @current_journal.save
+                 end
+               end
+               # Called after a relation is removed
+               def relation_removed(relation)
+                 if @current_journal
+                   @current_journal.details << JournalDetail.new(
+                     :property  => 'relation',
+                     :prop_key  => relation.relation_type_for(self),
+                     :old_value => relation.other_issue(self).try(:id)
+                   )
+                   @current_journal.save
+                 end
+               end
                # Default assignment based on category
                def default_assign
                  if assigned_to.nil? && category && category.assigned_to

app/models/issue_relation.rb +21 -26

                attr_protected :issue_from_id, :issue_to_id
                before_save :handle_issue_order
-               after_create  :create_journal_after_create
-               after_destroy :create_journal_after_delete
+               after_create  :call_issues_relation_added_callback
+               after_destroy :call_issues_relation_removed_callback
                def visible?(user=User.current)
                  (issue_from.nil? || issue_from.visible?(user)) && (issue_to.nil? || issue_to.visible?(user))
                  r == 0 ? id <=> relation.id : r
                end
+               def init_journals(user)
+                 issue_from.init_journal(user) if issue_from
+                 issue_to.init_journal(user) if issue_to
+               end
                private
                # Reverses the relation if needed so that it gets stored in the proper way
                  end
                end
-               def create_journal_after_create
-                 journal = issue_from.init_journal(User.current)
-                 journal.details << JournalDetail.new(:property => 'relation',
-                                                      :prop_key => relation_type_for(issue_from),
-                                                      :value    => issue_to.id)
-                 journal.save
-                 journal = issue_to.init_journal(User.current)
-                 journal.details << JournalDetail.new(:property => 'relation',
-                                                      :prop_key => relation_type_for(issue_to),
-                                                      :value    => issue_from.id)
-                 journal.save
-               end
-               def create_journal_after_delete
-                 journal = issue_from.init_journal(User.current)
-                 journal.details << JournalDetail.new(:property  => 'relation',
-                                                      :prop_key  => relation_type_for(issue_from),
-                                                      :old_value => issue_to.id)
-                 journal.save
-                 journal = issue_to.init_journal(User.current)
-                 journal.details << JournalDetail.new(:property  => 'relation',
-                                                      :prop_key  => relation_type_for(issue_to),
-                                                      :old_value => issue_from.id)
-                 journal.save
+               def call_issues_relation_added_callback
+                 call_issues_callback :relation_added
+               end
+               def call_issues_relation_removed_callback
+                 call_issues_callback :relation_removed
+               end
+               def call_issues_callback(name)
+                 [issue_from, issue_to].each do |issue|
+                   if issue
+                     issue.send name, self
+                   end
+                 end
                end
              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
-                 assert_equal 1, issue.journals.size
-                 journal = issue.journals.first
-                 assert_equal 2, journal.details.size
+                 journal = Journal.new
+                 journal.details << JournalDetail.new(:property => 'relation', :prop_key => 'relates', :value => visible_issue.id)
+                 journal.details << JournalDetail.new(:property => 'relation', :prop_key => 'relates', :value => hidden_issue.id)
                  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