Show who last edited a comment if it wasn't the original author

16 files changed, 53 insertions, 67 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 7ca450d423c..a2afd9fe8e6 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -17,6 +17,7 @@ v 7.14.0 (unreleased)
   - Add fetch command to the MR page
   - Fix bug causing Bitbucket importer to crash when OAuth application had been removed.
   - Add fetch command to the MR page.
+  - Show who last edited a comment if it wasn't the original author
 
 v 7.13.2
   - Fix randomly failed spec
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index c4a87e9dbd8..0f5d82ce133 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -30,13 +30,10 @@ class Projects::NotesController < Projects::ApplicationController
   end
 
   def update
-    if note.editable?
-      note.update_attributes(note_params)
-      note.reset_events_cache
-    end
+    @note = Notes::UpdateService.new(project, current_user, note_params).execute(note)
 
     respond_to do |format|
-      format.json { render_note_json(note) }
+      format.json { render_note_json(@note) }
       format.html { redirect_to :back }
     end
   end
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index d4c345fe431..6ddb37cd0dc 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -43,21 +43,6 @@ module IssuesHelper
     end
   end
 
-  def issue_timestamp(issue)
-    # Shows the created at time and the updated at time if different
-    ts = time_ago_with_tooltip(issue.created_at, placement: 'bottom', html_class: 'note_created_ago')
-    if issue.updated_at != issue.created_at
-      ts << capture_haml do
-        haml_tag :span do
-          haml_concat '&middot;'
-          haml_concat icon('edit', title: 'edited')
-          haml_concat time_ago_with_tooltip(issue.updated_at, placement: 'bottom', html_class: 'issue_edited_ago')
-        end
-      end
-    end
-    ts.html_safe
-  end
-
   def bulk_update_milestone_options
     options_for_select([['None (backlog)', -1]]) +
         options_from_collection_for_select(project_active_milestones, 'id',
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index dda9b17d61d..5f0c921413a 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -23,21 +23,6 @@ module NotesHelper
     end
   end
 
-  def note_timestamp(note)
-    # Shows the created at time and the updated at time if different
-    ts = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note_created_ago')
-    if note.updated_at != note.created_at
-      ts << capture_haml do
-        haml_tag :span do
-          haml_concat '&middot;'
-          haml_concat icon('edit', title: 'edited')
-          haml_concat time_ago_with_tooltip(note.updated_at, placement: 'bottom', html_class: 'note_edited_ago')
-        end
-      end
-    end
-    ts.html_safe
-  end
-
   def noteable_json(noteable)
     {
       id: noteable.id,
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index a675a292432..9f65e3d4a05 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -21,7 +21,7 @@ module ProjectsHelper
   end
 
   def link_to_member(project, author, opts = {})
-    default_opts = { avatar: true, name: true, size: 16 }
+    default_opts = { avatar: true, name: true, size: 16, author_class: 'author' }
     opts = default_opts.merge(opts)
 
     return "(deleted)" unless author
@@ -32,7 +32,7 @@ module ProjectsHelper
     author_html << image_tag(avatar_icon(author.try(:email), opts[:size]), width: opts[:size], class: "avatar avatar-inline #{"s#{opts[:size]}" if opts[:size]}", alt:'') if opts[:avatar]
 
     # Build name span tag
-    author_html << content_tag(:span, sanitize(author.name), class: 'author') if opts[:name]
+    author_html << content_tag(:span, sanitize(author.name), class: opts[:author_class]) if opts[:name]
 
     author_html = author_html.html_safe
 
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index c21e7fd0e3b..40642dc63ba 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -12,6 +12,7 @@ module Issuable
   included do
     belongs_to :author, class_name: "User"
     belongs_to :assignee, class_name: "User"
+    belongs_to :updated_by, class_name: "User"
     belongs_to :milestone
     has_many :notes, as: :noteable, dependent: :destroy
     has_many :label_links, as: :target, dependent: :destroy
diff --git a/app/models/note.rb b/app/models/note.rb
index 62567f471dc..f7ca609f516 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -33,6 +33,7 @@ class Note < ActiveRecord::Base
   belongs_to :project
   belongs_to :noteable, polymorphic: true, touch: true
   belongs_to :author, class_name: "User"
+  belongs_to :updated_by, class_name: "User"
 
   delegate :name, to: :project, prefix: true
   delegate :name, :email, to: :author, prefix: true
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index eabab65c9b0..2fc6ef7f356 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -14,7 +14,7 @@ module Issues
       filter_params
       old_labels = issue.labels.to_a
 
-      if params.present? && issue.update_attributes(params)
+      if params.present? && issue.update_attributes(params.merge(updated_by: current_user))
         issue.reset_events_cache
 
         if issue.labels != old_labels
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index 589fad16165..25d79e22e39 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -24,7 +24,7 @@ module MergeRequests
       filter_params
       old_labels = merge_request.labels.to_a
 
-      if params.present? && merge_request.update_attributes(params)
+      if params.present? && merge_request.update_attributes(params.merge(updated_by: current_user))
         merge_request.reset_events_cache
 
         if merge_request.labels != old_labels
diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb
index b5611d46257..897716cf6a5 100644
--- a/app/services/notes/update_service.rb
+++ b/app/services/notes/update_service.rb
@@ -1,22 +1,11 @@
 module Notes
   class UpdateService < BaseService
-    def execute
-      note = project.notes.find(params[:note_id])
-      note.note = params[:note]
-      if note.save
-        notification_service.new_note(note)
+    def execute(note)
+      return note unless note.editable?
 
-        # Skip system notes, like status changes and cross-references.
-        unless note.system
-          event_service.leave_note(note, note.author)
+      note.update_attributes(params.merge(updated_by: current_user))
 
-          # Create a cross-reference note if this Note contains GFM that
-          # names an issue, merge request, or commit.
-          note.references.each do |mentioned|
-            SystemNoteService.cross_reference(mentioned, note.noteable, note.author)
-          end
-        end
-      end
+      project.reset_events_cache
 
       note
     end
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 54d33a5ddd1..e7b14e7582c 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -9,7 +9,13 @@
           Open
       Issue ##{@issue.iid}
       %small.creator
-        &middot; created by #{link_to_member(@project, @issue.author)} #{issue_timestamp(@issue)}
+        &middot; created by #{link_to_member(@project, @issue.author)}
+        = time_ago_with_tooltip(@issue.created_at, placement: 'bottom', html_class: 'issue_created_ago')
+        - if @issue.updated_at != @issue.created_at
+          %span
+            &middot;
+            = icon('edit', title: 'edited')
+            = time_ago_with_tooltip(@issue.updated_at, placement: 'bottom', html_class: 'issue_edited_ago')
 
       .pull-right
         - if can?(current_user, :create_issue, @project)
diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml
index 4e8144b4de2..9a1eb36fc88 100644
--- a/app/views/projects/merge_requests/show/_mr_title.html.haml
+++ b/app/views/projects/merge_requests/show/_mr_title.html.haml
@@ -1,10 +1,16 @@
 %h4.page-title
   .issue-box{ class: issue_box_class(@merge_request) }
     = @merge_request.state_human_name
-  = "Merge Request ##{@merge_request.iid}"
+  Merge Request ##{@merge_request.iid}
   %small.creator
     &middot;
-    created by #{link_to_member(@project, @merge_request.author)} #{time_ago_with_tooltip(@merge_request.created_at)}
+    created by #{link_to_member(@project, @merge_request.author)}
+    = time_ago_with_tooltip(@merge_request.created_at)
+    - if @merge_request.updated_at != @merge_request.created_at
+      %span
+        &middot;
+        = icon('edit', title: 'edited')
+        = time_ago_with_tooltip(@merge_request.updated_at, placement: 'bottom')
 
   .issue-btn-group.pull-right
     - if can?(current_user, :update_merge_request, @merge_request)
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index 4a1009686c6..de75d44fc41 100644
--- a/app/views/projects/notes/_note.html.haml
+++ b/app/views/projects/notes/_note.html.haml
@@ -33,7 +33,14 @@
 
         %span.note-last-update
           = link_to "##{dom_id(note)}", name: dom_id(note), title: "Link here" do
-            = note_timestamp(note)
+            = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note_created_ago')
+          - if note.updated_at != note.created_at
+            %span
+              &middot;
+              = icon('edit', title: 'edited')
+              = time_ago_with_tooltip(note.updated_at, placement: 'bottom', html_class: 'note_edited_ago')
+              - if note.updated_by && note.updated_by != note.author
+                by #{link_to_member(note.project, note.updated_by, avatar: false, author_class: nil)}
 
         - if note.superceded?(@notes)
           - if note.upvote?
diff --git a/db/migrate/20150730122406_add_updated_by_to_issuables_and_notes.rb b/db/migrate/20150730122406_add_updated_by_to_issuables_and_notes.rb
new file mode 100644
index 00000000000..78d45c7f96b
--- /dev/null
+++ b/db/migrate/20150730122406_add_updated_by_to_issuables_and_notes.rb
@@ -0,0 +1,7 @@
+class AddUpdatedByToIssuablesAndNotes < ActiveRecord::Migration
+  def change
+    add_column :notes, :updated_by_id, :integer
+    add_column :issues, :updated_by_id, :integer
+    add_column :merge_requests, :updated_by_id, :integer
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index a63c2d05821..36b500c5420 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150717130904) do
+ActiveRecord::Schema.define(version: 20150730122406) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -128,12 +128,13 @@ ActiveRecord::Schema.define(version: 20150717130904) do
     t.integer  "project_id"
     t.datetime "created_at"
     t.datetime "updated_at"
-    t.integer  "position",     default: 0
+    t.integer  "position",      default: 0
     t.string   "branch_name"
     t.text     "description"
     t.integer  "milestone_id"
     t.string   "state"
     t.integer  "iid"
+    t.integer  "updated_by_id"
   end
 
   add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
@@ -230,6 +231,7 @@ ActiveRecord::Schema.define(version: 20150717130904) do
     t.text     "description"
     t.integer  "position",          default: 0
     t.datetime "locked_at"
+    t.integer  "updated_by_id"
   end
 
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
@@ -289,6 +291,7 @@ ActiveRecord::Schema.define(version: 20150717130904) do
     t.integer  "noteable_id"
     t.boolean  "system",        default: false, null: false
     t.text     "st_diff"
+    t.integer  "updated_by_id"
   end
 
   add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 3726be7c537..3efdfe2d46e 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -78,17 +78,15 @@ module API
         put ":id/#{noteables_str}/:#{noteable_id_str}/notes/:note_id" do
           required_attributes! [:body]
 
-          authorize! :admin_note, user_project.notes.find(params[:note_id])
+          note = user_project.notes.find(params[:note_id])
+
+          authorize! :admin_note, note
 
           opts = {
-            note: params[:body],
-            note_id: params[:note_id],
-            noteable_type: noteables_str.classify,
-            noteable_id: params[noteable_id_str]
+            note: params[:body]
           }
 
-          @note = ::Notes::UpdateService.new(user_project, current_user,
-                                             opts).execute
+          @note = ::Notes::UpdateService.new(user_project, current_user, opts).execute(note)
 
           if @note.valid?
             present @note, with: Entities::Note