Merge branch 'blackst0ne/gitlab-ce-add_system_note_for_editing_issuable'

20 files changed, 134 insertions, 13 deletions

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index fff57472a4f..6d6bcbaf88a 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -180,16 +180,16 @@ module ApplicationHelper
     element
   end
 
-  def edited_time_ago_with_tooltip(object, placement: 'top', html_class: 'time_ago', include_author: false)
-    return if object.updated_at == object.created_at
+  def edited_time_ago_with_tooltip(object, placement: 'top', html_class: 'time_ago', exclude_author: false)
+    return if object.last_edited_at == object.created_at || object.last_edited_at.blank?
 
-    content_tag :small, class: "edited-text" do
-      output = content_tag(:span, "Edited ")
-      output << time_ago_with_tooltip(object.updated_at, placement: placement, html_class: html_class)
+    content_tag :small, class: 'edited-text' do
+      output = content_tag(:span, 'Edited ')
+      output << time_ago_with_tooltip(object.last_edited_at, placement: placement, html_class: html_class)
 
-      if include_author && object.updated_by && object.updated_by != object.author
-        output << content_tag(:span, " by ")
-        output << link_to_member(object.project, object.updated_by, avatar: false, author_class: nil)
+      if !exclude_author && object.last_edited_by
+        output << content_tag(:span, ' by ')
+        output << link_to_member(object.project, object.last_edited_by, avatar: false, author_class: nil)
       end
 
       output
diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb
index 1ea60e39386..d889d141101 100644
--- a/app/helpers/system_note_helper.rb
+++ b/app/helpers/system_note_helper.rb
@@ -1,6 +1,7 @@
 module SystemNoteHelper
   ICON_NAMES_BY_ACTION = {
     'commit' => 'icon_commit',
+    'description' => 'icon_edit',
     'merge' => 'icon_merge',
     'merged' => 'icon_merged',
     'opened' => 'icon_status_open',
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 16f04305a43..075ec575f9d 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -27,6 +27,7 @@ module Issuable
 
     belongs_to :author, class_name: "User"
     belongs_to :updated_by, class_name: "User"
+    belongs_to :last_edited_by, class_name: 'User'
     belongs_to :milestone
     has_many :notes, as: :noteable, inverse_of: :noteable, dependent: :destroy do
       def authors_loaded?
diff --git a/app/models/note.rb b/app/models/note.rb
index b06985b4a6f..46d0a4f159f 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -18,6 +18,11 @@ class Note < ActiveRecord::Base
 
   cache_markdown_field :note, pipeline: :note, issuable_state_filter_enabled: true
 
+  # Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with notes.
+  # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10392/diffs#note_28719102
+  alias_attribute :last_edited_at, :updated_at
+  alias_attribute :last_edited_by, :updated_by
+
   # Attribute containing rendered and redacted Markdown as generated by
   # Banzai::ObjectRenderer.
   attr_accessor :redacted_note_html
@@ -38,6 +43,7 @@ class Note < ActiveRecord::Base
   belongs_to :noteable, polymorphic: true, touch: true
   belongs_to :author, class_name: "User"
   belongs_to :updated_by, class_name: "User"
+  belongs_to :last_edited_by, class_name: 'User'
 
   has_many :todos, dependent: :destroy
   has_many :events, as: :target, dependent: :destroy
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index d8860718cb5..abfbefdf9a0 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -12,6 +12,11 @@ class Snippet < ActiveRecord::Base
   cache_markdown_field :title, pipeline: :single_line
   cache_markdown_field :content
 
+  # Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with snippets.
+  # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10392/diffs#note_28719102
+  alias_attribute :last_edited_at, :updated_at
+  alias_attribute :last_edited_by, :updated_by
+
   # If file_name changes, it invalidates content
   alias_method :default_content_html_invalidator, :content_html_invalidated?
   def content_html_invalidated?
diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb
index 1e6fc837a75..b44f4fe000c 100644
--- a/app/models/system_note_metadata.rb
+++ b/app/models/system_note_metadata.rb
@@ -1,6 +1,6 @@
 class SystemNoteMetadata < ActiveRecord::Base
   ICON_TYPES = %w[
-    commit merge confidential visible label assignee cross_reference
+    commit description merge confidential visible label assignee cross_reference
     title time_tracking branch milestone discussion task moved opened closed merged
   ].freeze
 
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 6c2777a8d2d..c1e532b504a 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -19,6 +19,10 @@ class IssuableBaseService < BaseService
       issuable, issuable.project, current_user, old_title)
   end
 
+  def create_description_change_note(issuable)
+    SystemNoteService.change_description(issuable, issuable.project, current_user)
+  end
+
   def create_branch_change_note(issuable, branch_type, old_branch, new_branch)
     SystemNoteService.change_branch(
       issuable, issuable.project, current_user, branch_type,
@@ -211,6 +215,10 @@ class IssuableBaseService < BaseService
     if issuable.changed? || params.present?
       issuable.assign_attributes(params.merge(updated_by: current_user))
 
+      if has_title_or_description_changed?(issuable)
+        issuable.assign_attributes(last_edited_at: Time.now, last_edited_by: current_user)
+      end
+
       before_update(issuable)
 
       if issuable.with_transaction_returning_status { issuable.save }
@@ -239,6 +247,10 @@ class IssuableBaseService < BaseService
     old_label_ids.sort != new_label_ids.sort
   end
 
+  def has_title_or_description_changed?(issuable)
+    issuable.title_changed? || issuable.description_changed?
+  end
+
   def change_state(issuable)
     case params.delete(:state_event)
     when 'reopen'
@@ -294,6 +306,10 @@ class IssuableBaseService < BaseService
       create_title_change_note(issuable, issuable.previous_changes['title'].first)
     end
 
+    if issuable.previous_changes.include?('description')
+      create_description_change_note(issuable)
+    end
+
     if issuable.previous_changes.include?('description') && issuable.tasks?
       create_task_status_note(issuable)
     end
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index fb1f56c9cc6..174e7c6e95b 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -299,6 +299,23 @@ module SystemNoteService
     create_note(NoteSummary.new(noteable, project, author, body, action: 'title'))
   end
 
+  # Called when the description of a Noteable is changed
+  #
+  # noteable  - Noteable object that responds to `description`
+  # project   - Project owning noteable
+  # author    - User performing the change
+  #
+  # Example Note text:
+  #
+  #   "changed the description"
+  #
+  # Returns the created Note object
+  def change_description(noteable, project, author)
+    body = 'changed the description'
+
+    create_note(NoteSummary.new(noteable, project, author, body, action: 'description'))
+  end
+
   # Called when the confidentiality changes
   #
   # issue   - Issue object
diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml
index 071c48fa2e4..5c1156b06fb 100644
--- a/app/views/shared/notes/_note.html.haml
+++ b/app/views/shared/notes/_note.html.haml
@@ -40,7 +40,7 @@
       .note-body{ class: note_editable ? 'js-task-list-container' : '' }
         .note-text.md
           = note.redacted_note_html
-        = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true)
+        = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago')
         - if note_editable
           = render 'shared/notes/edit', note: note
         .note-awards
diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml
index d084f5e9684..501c09d71d5 100644
--- a/app/views/shared/snippets/_header.html.haml
+++ b/app/views/shared/snippets/_header.html.haml
@@ -21,4 +21,4 @@
     = markdown_field(@snippet, :title)
 
   - if @snippet.updated_at != @snippet.created_at
-    = edited_time_ago_with_tooltip(@snippet, placement: 'bottom', html_class: 'snippet-edited-ago')
+    = edited_time_ago_with_tooltip(@snippet, placement: 'bottom', html_class: 'snippet-edited-ago', exclude_author: true)
diff --git a/changelogs/unreleased/add_system_note_for_editing_issuable.yml b/changelogs/unreleased/add_system_note_for_editing_issuable.yml
new file mode 100644
index 00000000000..3cbc7f91bf0
--- /dev/null
+++ b/changelogs/unreleased/add_system_note_for_editing_issuable.yml
@@ -0,0 +1,4 @@
+---
+title: Add system note on description change of issue/merge request
+merge_request: 10392
+author: blackst0ne
diff --git a/db/migrate/20170503021915_add_last_edited_at_and_last_edited_by_id_to_issues.rb b/db/migrate/20170503021915_add_last_edited_at_and_last_edited_by_id_to_issues.rb
new file mode 100644
index 00000000000..6ac10723c82
--- /dev/null
+++ b/db/migrate/20170503021915_add_last_edited_at_and_last_edited_by_id_to_issues.rb
@@ -0,0 +1,14 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddLastEditedAtAndLastEditedByIdToIssues < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    add_column :issues, :last_edited_at, :timestamp
+    add_column :issues, :last_edited_by_id, :integer
+  end
+end
diff --git a/db/migrate/20170503022548_add_last_edited_at_and_last_edited_by_id_to_merge_requests.rb b/db/migrate/20170503022548_add_last_edited_at_and_last_edited_by_id_to_merge_requests.rb
new file mode 100644
index 00000000000..7a1acdcbf69
--- /dev/null
+++ b/db/migrate/20170503022548_add_last_edited_at_and_last_edited_by_id_to_merge_requests.rb
@@ -0,0 +1,14 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddLastEditedAtAndLastEditedByIdToMergeRequests < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    add_column :merge_requests, :last_edited_at, :timestamp
+    add_column :merge_requests, :last_edited_by_id, :integer
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index aad97d5f810..ebe802a3087 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -498,6 +498,8 @@ ActiveRecord::Schema.define(version: 20170504102911) do
     t.integer "relative_position"
     t.datetime "closed_at"
     t.integer "cached_markdown_version"
+    t.datetime "last_edited_at"
+    t.integer "last_edited_by_id"
   end
 
   add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
@@ -684,6 +686,8 @@ ActiveRecord::Schema.define(version: 20170504102911) do
     t.text "description_html"
     t.integer "time_estimate"
     t.integer "cached_markdown_version"
+    t.datetime "last_edited_at"
+    t.integer "last_edited_by_id"
   end
 
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
@@ -1425,4 +1429,4 @@ ActiveRecord::Schema.define(version: 20170504102911) do
   add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade
   add_foreign_key "trending_projects", "projects", on_delete: :cascade
   add_foreign_key "u2f_registrations", "users"
-end
\ No newline at end of file
+end
diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb
index 4a54e7ef2e7..956763fa399 100644
--- a/lib/gitlab/import_export/relation_factory.rb
+++ b/lib/gitlab/import_export/relation_factory.rb
@@ -15,7 +15,7 @@ module Gitlab
                     priorities: :label_priorities,
                     label: :project_label }.freeze
 
-      USER_REFERENCES = %w[author_id assignee_id updated_by_id user_id created_by_id merge_user_id resolved_by_id].freeze
+      USER_REFERENCES = %w[author_id assignee_id updated_by_id user_id created_by_id last_edited_by_id merge_user_id resolved_by_id].freeze
 
       PROJECT_REFERENCES = %w[project_id source_project_id target_project_id].freeze
 
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index cd2fa27bb9f..f7654f1aade 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -9,6 +9,7 @@ issues:
 - notes
 - label_links
 - labels
+- last_edited_by
 - todos
 - user_agent_detail
 - moved_to
@@ -27,6 +28,7 @@ notes:
 - noteable
 - author
 - updated_by
+- last_edited_by
 - resolved_by
 - todos
 - events
@@ -72,6 +74,7 @@ merge_requests:
 - notes
 - label_links
 - labels
+- last_edited_by
 - todos
 - target_project
 - source_project
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 59c8b48a2be..a66086f8b47 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -23,6 +23,8 @@ Issue:
 - weight
 - time_estimate
 - relative_position
+- last_edited_at
+- last_edited_by_id
 Event:
 - id
 - target_type
@@ -154,6 +156,8 @@ MergeRequest:
 - approvals_before_merge
 - rebase_commit_sha
 - time_estimate
+- last_edited_at
+- last_edited_by_id
 MergeRequestDiff:
 - id
 - state
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 6633ac10236..1954d8739f6 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -132,6 +132,17 @@ describe Issues::UpdateService, services: true do
       end
     end
 
+    context 'when description changed' do
+      it 'creates system note about description change' do
+        update_issue(description: 'Changed description')
+
+        note = find_note('changed the description')
+
+        expect(note).not_to be_nil
+        expect(note.note).to eq('changed the description')
+      end
+    end
+
     context 'when issue turns confidential' do
       let(:opts) do
         {
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 694ec3a579f..31487c0f794 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -102,6 +102,13 @@ describe MergeRequests::UpdateService, services: true do
         expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**'
       end
 
+      it 'creates system note about description change' do
+        note = find_note('changed the description')
+
+        expect(note).not_to be_nil
+        expect(note.note).to eq('changed the description')
+      end
+
       it 'creates system note about branch change' do
         note = find_note('changed target')
 
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 68816bf36b8..516566eddef 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -339,6 +339,20 @@ describe SystemNoteService, services: true do
     end
   end
 
+  describe '.change_description' do
+    subject { described_class.change_description(noteable, project, author) }
+
+    context 'when noteable responds to `description`' do
+      it_behaves_like 'a system note' do
+        let(:action) { 'description' }
+      end
+
+      it 'sets the note text' do
+        expect(subject.note).to eq('changed the description')
+      end
+    end
+  end
+
   describe '.change_issue_confidentiality' do
     subject { described_class.change_issue_confidentiality(noteable, project, author) }