diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-07-14 08:40:08 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-07-14 08:40:08 +0000 |
commit | 90e47dd949abdff410d20db2ed908494a5248cf4 (patch) | |
tree | 8ed5b338cf318bd4a518d7526cd063ae04f8f217 /app | |
parent | 5c53dc5a1e7cbae3e7c281cfb0740d44c771958f (diff) | |
parent | f3d4767d0c78daf315e6b653bed3a3a3ee308072 (diff) | |
download | gitlab-ce-90e47dd949abdff410d20db2ed908494a5248cf4.tar.gz |
Merge branch 'rs-issue-1773' into 'master'
Fix mentions not being created upon issue/merge request update
New cross-references weren't being added when they were made in an issue or merge request update.
This happened because the relevant `UpdateService`s were making the `notice_added_references` call
after the model had already been updated and saved, so the `changes` attribute was empty and no
cross-references were made at all.
This fixes the bug and adds a bit of testing and a bit of refactoring.
Closes #1773
See merge request !974
Diffstat (limited to 'app')
-rw-r--r-- | app/models/concerns/mentionable.rb | 42 | ||||
-rw-r--r-- | app/models/note.rb | 2 | ||||
-rw-r--r-- | app/services/issues/update_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/update_service.rb | 2 |
4 files changed, 31 insertions, 17 deletions
diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index 56849f28ff0..5b0ae411642 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -79,22 +79,36 @@ module Mentionable end end - # If the mentionable_text field is about to change, locate any *added* references and create cross references for - # them. Invoke from an observer's #before_save implementation. - def notice_added_references(p = project, a = author) - ch = changed_attributes - original, mentionable_changed = "", false - self.class.mentionable_attrs.each do |attr| - if ch[attr] - original << ch[attr] - mentionable_changed = true - end - end + # When a mentionable field is changed, creates cross-reference notes that + # don't already exist + def create_new_cross_references!(p = project, a = author) + changes = detect_mentionable_changes + + return if changes.empty? - # Only proceed if the saved changes actually include a chance to an attr_mentionable field. - return unless mentionable_changed + original_text = changes.collect { |_, vals| vals.first }.join(' ') - preexisting = references(p, self.author, original) + preexisting = references(p, self.author, original_text) create_cross_references!(p, a, preexisting) end + + private + + # Returns a Hash of changed mentionable fields + # + # Preference is given to the `changes` Hash, but falls back to + # `previous_changes` if it's empty (i.e., the changes have already been + # persisted). + # + # See ActiveModel::Dirty. + # + # Returns a Hash. + def detect_mentionable_changes + source = (changes.present? ? changes : previous_changes).dup + + mentionable = self.class.mentionable_attrs + + # Only include changed fields that are mentionable + source.select { |key, val| mentionable.include?(key) } + end end diff --git a/app/models/note.rb b/app/models/note.rb index 68b9d433ae0..62567f471dc 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -356,7 +356,7 @@ class Note < ActiveRecord::Base end def set_references - notice_added_references(project, author) + create_new_cross_references!(project, author) end def editable? diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index f848ecedd6b..eabab65c9b0 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -35,7 +35,7 @@ module Issues create_title_change_note(issue, issue.previous_changes['title'].first) end - issue.notice_added_references(issue.project, current_user) + issue.create_new_cross_references!(issue.project, current_user) execute_hooks(issue, 'update') end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index e5c5368f5d6..589fad16165 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -59,7 +59,7 @@ module MergeRequests merge_request.mark_as_unchecked end - merge_request.notice_added_references(merge_request.project, current_user) + merge_request.create_new_cross_references!(merge_request.project, current_user) execute_hooks(merge_request, 'update') end |