Merge branch 'fix-realtime-edited-text-for-issues-9-3' into 'master'

Port fix-realtime-edited-text-for-issues 9-2-stable fix to master.

See merge request !11478

app/assets/javascripts/issue_show/components/app.vue

@@ -7,6 +7,7 @@ import Service from '../services/index';
 import Store from '../stores';
 import titleComponent from './title.vue';
 import descriptionComponent from './description.vue';
+import editedComponent from './edited.vue';
 import formComponent from './form.vue';
 import '../../lib/utils/url_utility';
 
@@ -50,6 +51,21 @@ export default {
       required: false,
       default: '',
     },
+    updatedAt: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    updatedByName: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    updatedByPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
     issuableTemplates: {
       type: Array,
       required: false,
@@ -86,6 +102,9 @@ export default {
       titleText: this.initialTitleText,
       descriptionHtml: this.initialDescriptionHtml,
       descriptionText: this.initialDescriptionText,
+      updatedAt: this.updatedAt,
+      updatedByName: this.updatedByName,
+      updatedByPath: this.updatedByPath,
     });
 
     return {
@@ -98,10 +117,14 @@ export default {
     formState() {
       return this.store.formState;
     },
+    hasUpdated() {
+      return !!this.state.updatedAt;
+    },
   },
   components: {
     descriptionComponent,
     titleComponent,
+    editedComponent,
     formComponent,
   },
   methods: {
@@ -240,6 +263,11 @@ export default {
         :description-text="state.descriptionText"
         :updated-at="state.updatedAt"
         :task-status="state.taskStatus" />
+      <edited-component
+        v-if="hasUpdated"
+        :updated-at="state.updatedAt"
+        :updated-by-name="state.updatedByName"
+        :updated-by-path="state.updatedByPath" />
     </div>
   </div>
 </template>

app/assets/javascripts/issue_show/components/description.vue

@@ -16,11 +16,6 @@
         type: String,
         required: true,
       },
-      updatedAt: {
-        type: String,
-        required: false,
-        default: '',
-      },
       taskStatus: {
         type: String,
         required: false,
@@ -31,7 +26,6 @@
       return {
         preAnimation: false,
         pulseAnimation: false,
-        timeAgoEl: $('.js-issue-edited-ago'),
       };
     },
     watch: {
@@ -39,12 +33,6 @@
         this.animateChange();
 
         this.$nextTick(() => {
-          const toolTipTime = gl.utils.formatDate(this.updatedAt);
-
-          this.timeAgoEl.attr('datetime', this.updatedAt)
-            .attr('title', toolTipTime)
-            .tooltip('fixTitle');
-
           this.renderGFM();
         });
       },

app/assets/javascripts/issue_show/components/edited.vue

+<script>
+import timeAgoTooltip from '../../vue_shared/components/time_ago_tooltip.vue';
+
+export default {
+  props: {
+    updatedAt: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    updatedByName: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    updatedByPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+  components: {
+    timeAgoTooltip,
+  },
+  computed: {
+    hasUpdatedBy() {
+      return this.updatedByName && this.updatedByPath;
+    },
+  },
+};
+</script>
+
+<template>
+  <small
+    class="edited-text"
+  >
+    Edited
+    <time-ago-tooltip
+      v-if="updatedAt"
+      placement="bottom"
+      :time="updatedAt"
+    />
+    <span
+      v-if="hasUpdatedBy"
+    >
+      by
+      <a
+        class="author_link"
+        :href="updatedByPath"
+      >
+        <span>{{updatedByName}}</span>
+      </a>
+    </span>
+  </small>
+</template>
+

app/assets/javascripts/issue_show/index.js

@@ -42,6 +42,9 @@ document.addEventListener('DOMContentLoaded', () => {
           projectPath: this.projectPath,
           projectNamespace: this.projectNamespace,
           projectsAutocompleteUrl: this.projectsAutocompleteUrl,
+          updatedAt: this.updatedAt,
+          updatedByName: this.updatedByName,
+          updatedByPath: this.updatedByPath,
         },
       });
     },

app/assets/javascripts/issue_show/stores/index.js

@@ -4,6 +4,9 @@ export default class Store {
     titleText,
     descriptionHtml,
     descriptionText,
+    updatedAt,
+    updatedByName,
+    updatedByPath,
   }) {
     this.state = {
       titleHtml,
@@ -11,7 +14,9 @@ export default class Store {
       descriptionHtml,
       descriptionText,
       taskStatus: '',
-      updatedAt: '',
+      updatedAt,
+      updatedByName,
+      updatedByPath,
     };
     this.formState = {
       title: '',
@@ -30,6 +35,8 @@ export default class Store {
     this.state.descriptionText = data.description_text;
     this.state.taskStatus = data.task_status;
     this.state.updatedAt = data.updated_at;
+    this.state.updatedByName = data.updated_by_name;
+    this.state.updatedByPath = data.updated_by_path;
   }
 
   stateShouldUpdate(data) {

app/assets/stylesheets/framework/mobile.scss

@@ -112,11 +112,6 @@
     }
   }
 
-  .issue-edited-ago,
-  .note_edited_ago {
-    display: none;
-  }
-
   aside:not(.right-sidebar) {
     display: none;
   }

app/controllers/projects/issues_controller.rb

@@ -199,14 +199,21 @@ class Projects::IssuesController < Projects::ApplicationController
   def realtime_changes
     Gitlab::PollingInterval.set_header(response, interval: 3_000)
 
-    render json: {
+    response = {
       title: view_context.markdown_field(@issue, :title),
       title_text: @issue.title,
       description: view_context.markdown_field(@issue, :description),
       description_text: @issue.description,
-      task_status: @issue.task_status,
-      updated_at: @issue.updated_at
+      task_status: @issue.task_status
     }
+
+    if @issue.is_edited?
+      response[:updated_at] = @issue.updated_at
+      response[:updated_by_name] = @issue.last_edited_by.name
+      response[:updated_by_path] = user_path(@issue.last_edited_by)
+    end
+
+    render json: response
   end
 
   def create_merge_request

app/helpers/application_helper.rb

@@ -181,7 +181,7 @@ module ApplicationHelper
   end
 
   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?
+    return unless object.is_edited?
 
     content_tag :small, class: 'edited-text' do
       output = content_tag(:span, 'Edited ')

app/helpers/issuables_helper.rb

@@ -200,7 +200,7 @@ module IssuablesHelper
   end
 
   def issuable_initial_data(issuable)
-    {
+    data = {
       endpoint: namespace_project_issue_path(@project.namespace, @project, issuable),
       canUpdate: can?(current_user, :update_issue, issuable),
       canDestroy: can?(current_user, :destroy_issue, issuable),
@@ -217,7 +217,23 @@ module IssuablesHelper
       initialTitleText: issuable.title,
       initialDescriptionHtml: markdown_field(issuable, :description),
       initialDescriptionText: issuable.description
-    }.to_json
+    }
+
+    data.merge!(updated_at_by(issuable))
+
+    data.to_json
+  end
+
+  def updated_at_by(issuable)
+    return {} unless issuable.is_edited?
+
+    {
+      updatedAt: issuable.updated_at.to_time.iso8601,
+      updatedBy: {
+        name: issuable.last_edited_by.name,
+        path: user_path(issuable.last_edited_by)
+      }
+    }
   end
 
   private

app/models/concerns/editable.rb

+module Editable
+  extend ActiveSupport::Concern
+
+  def is_edited?
+    last_edited_at.present? && last_edited_at != created_at
+  end
+end

app/models/concerns/issuable.rb

@@ -15,6 +15,7 @@ module Issuable
   include Taskable
   include TimeTrackable
   include Importable
+  include Editable
 
   # This object is used to gather issuable meta data for displaying
   # upvotes, downvotes, notes and closing merge requests count for issues and merge requests

app/models/note.rb

@@ -13,6 +13,7 @@ class Note < ActiveRecord::Base
   include AfterCommitQueue
   include ResolvableNote
   include IgnorableColumn
+  include Editable
 
   ignore_column :original_discussion_id
 

app/models/snippet.rb

@@ -8,6 +8,7 @@ class Snippet < ActiveRecord::Base
   include Awardable
   include Mentionable
   include Spammable
+  include Editable
 
   cache_markdown_field :title, pipeline: :single_line
   cache_markdown_field :content

db/schema.rb

@@ -1471,8 +1471,8 @@ ActiveRecord::Schema.define(version: 20170525174156) do
     t.string "token"
     t.boolean "pipeline_events", default: false, null: false
     t.boolean "confidential_issues_events", default: false, null: false
-    t.boolean "job_events", default: false, null: false
     t.boolean "repository_update_events", default: false, null: false
+    t.boolean "job_events", default: false, null: false
   end
 
   add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree

spec/helpers/issuables_helper_spec.rb

@@ -192,4 +192,22 @@ describe IssuablesHelper do
       expect(helper.issuable_filter_present?).to be_falsey
     end
   end
+
+  describe '#updated_at_by' do
+    let(:user) { create(:user) }
+    let(:unedited_issuable) { create(:issue) }
+    let(:edited_issuable) { create(:issue, last_edited_by: user, created_at: 3.days.ago, updated_at: 2.days.ago, last_edited_at: 2.days.ago) }
+    let(:edited_updated_at_by) do
+      {
+        updatedAt: edited_issuable.updated_at.to_time.iso8601,
+        updatedBy: {
+          name: user.name,
+          path: user_path(user)
+        }
+      }
+    end
+
+    it { expect(helper.updated_at_by(unedited_issuable)).to eq({}) }
+    it { expect(helper.updated_at_by(edited_issuable)).to eq(edited_updated_at_by) }
+  end
 end

spec/javascripts/issue_show/components/app_spec.js

@@ -14,6 +14,10 @@ const issueShowInterceptor = data => (request, next) => {
   }));
 };
 
+function formatText(text) {
+  return text.trim().replace(/\s\s+/g, ' ');
+}
+
 describe('Issuable output', () => {
   document.body.innerHTML = '<span id="task_status"></span>';
 
@@ -50,12 +54,17 @@ describe('Issuable output', () => {
     Vue.http.interceptors = _.without(Vue.http.interceptors, issueShowInterceptor);
   });
 
-  it('should render a title/description and update title/description on update', (done) => {
+  it('should render a title/description/edited and update title/description/edited on update', (done) => {
     setTimeout(() => {
+      const editedText = vm.$el.querySelector('.edited-text');
+
       expect(document.querySelector('title').innerText).toContain('this is a title (#1)');
       expect(vm.$el.querySelector('.title').innerHTML).toContain('<p>this is a title</p>');
       expect(vm.$el.querySelector('.wiki').innerHTML).toContain('<p>this is a description!</p>');
       expect(vm.$el.querySelector('.js-task-list-field').value).toContain('this is a description');
+      expect(formatText(editedText.innerText)).toMatch(/Edited[\s\S]+?by Some User/);
+      expect(editedText.querySelector('.author_link').href).toMatch(/\/some_user$/);
+      expect(editedText.querySelector('time')).toBeTruthy();
 
       Vue.http.interceptors.push(issueShowInterceptor(issueShowData.secondRequest));
 
@@ -64,6 +73,10 @@ describe('Issuable output', () => {
         expect(vm.$el.querySelector('.title').innerHTML).toContain('<p>2</p>');
         expect(vm.$el.querySelector('.wiki').innerHTML).toContain('<p>42</p>');
         expect(vm.$el.querySelector('.js-task-list-field').value).toContain('42');
+        expect(vm.$el.querySelector('.edited-text')).toBeTruthy();
+        expect(formatText(vm.$el.querySelector('.edited-text').innerText)).toMatch(/Edited[\s\S]+?by Other User/);
+        expect(editedText.querySelector('.author_link').href).toMatch(/\/other_user$/);
+        expect(editedText.querySelector('time')).toBeTruthy();
 
         done();
       });

spec/javascripts/issue_show/components/edited_spec.js

+import Vue from 'vue';
+import edited from '~/issue_show/components/edited.vue';
+
+function formatText(text) {
+  return text.trim().replace(/\s\s+/g, ' ');
+}
+
+describe('edited', () => {
+  const EditedComponent = Vue.extend(edited);
+
+  it('should render an edited at+by string', () => {
+    const editedComponent = new EditedComponent({
+      propsData: {
+        updatedAt: '2017-05-15T12:31:04.428Z',
+        updatedByName: 'Some User',
+        updatedByPath: '/some_user',
+      },
+    }).$mount();
+
+    expect(formatText(editedComponent.$el.innerText)).toMatch(/Edited[\s\S]+?by Some User/);
+    expect(editedComponent.$el.querySelector('.author_link').href).toMatch(/\/some_user$/);
+    expect(editedComponent.$el.querySelector('time')).toBeTruthy();
+  });
+
+  it('if no updatedAt is provided, no time element will be rendered', () => {
+    const editedComponent = new EditedComponent({
+      propsData: {
+        updatedByName: 'Some User',
+        updatedByPath: '/some_user',
+      },
+    }).$mount();
+
+    expect(formatText(editedComponent.$el.innerText)).toMatch(/Edited by Some User/);
+    expect(editedComponent.$el.querySelector('.author_link').href).toMatch(/\/some_user$/);
+    expect(editedComponent.$el.querySelector('time')).toBeFalsy();
+  });
+
+  it('if no updatedByName and updatedByPath is provided, no user element will be rendered', () => {
+    const editedComponent = new EditedComponent({
+      propsData: {
+        updatedAt: '2017-05-15T12:31:04.428Z',
+      },
+    }).$mount();
+
+    expect(formatText(editedComponent.$el.innerText)).not.toMatch(/by Some User/);
+    expect(editedComponent.$el.querySelector('.author_link')).toBeFalsy();
+    expect(editedComponent.$el.querySelector('time')).toBeTruthy();
+  });
+});

spec/javascripts/issue_show/mock_data.js

@@ -5,7 +5,9 @@ export default {
     description: '<p>this is a description!</p>',
     description_text: 'this is a description',
     task_status: '2 of 4 completed',
-    updated_at: new Date().toString(),
+    updated_at: '2015-05-15T12:31:04.428Z',
+    updated_by_name: 'Some User',
+    updated_by_path: '/some_user',
   },
   secondRequest: {
     title: '<p>2</p>',
@@ -13,7 +15,9 @@ export default {
     description: '<p>42</p>',
     description_text: '42',
     task_status: '0 of 0 completed',
-    updated_at: new Date().toString(),
+    updated_at: '2016-05-15T12:31:04.428Z',
+    updated_by_name: 'Other User',
+    updated_by_path: '/other_user',
   },
   issueSpecRequest: {
     title: '<p>this is a title</p>',
@@ -21,6 +25,8 @@ export default {
     description: '<li class="task-list-item enabled"><input type="checkbox" class="task-list-item-checkbox">Task List Item</li>',
     description_text: '- [ ] Task List Item',
     task_status: '0 of 1 completed',
-    updated_at: new Date().toString(),
+    updated_at: '2017-05-15T12:31:04.428Z',
+    updated_by_name: 'Last User',
+    updated_by_path: '/last_user',
   },
 };

spec/models/concerns/editable_spec.rb

+require 'spec_helper'
+
+describe Editable do
+  describe '#is_edited?' do
+    let(:issue) { create(:issue, last_edited_at: nil) }
+    let(:edited_issue) { create(:issue, created_at: 3.days.ago, last_edited_at: 2.days.ago) }
+
+    it { expect(issue.is_edited?).to eq(false) }
+    it { expect(edited_issue.is_edited?).to eq(true) }
+  end
+end