Merge branch 'fix-realtime-edited-text-for-issues' into '9-2-stable'

Fix realtime edited text for issues

See merge request !11375

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

@@ -4,7 +4,7 @@ import '../vue_shared/vue_resource_interceptor';
 (() => {
   const issueTitleData = document.querySelector('.issue-title-data').dataset;
-  const { canUpdateTasksClass, endpoint } = issueTitleData;
+  const { canUpdateTasksClass, endpoint, isEdited } = issueTitleData;
   const vm = new Vue({
     el: '.issue-title-entrypoint',
@@ -12,6 +12,7 @@ import '../vue_shared/vue_resource_interceptor';
       props: {
         canUpdateTasksClass,
         endpoint,
+        isEdited,
       },
     }),
   });

app/assets/javascripts/issue_show/issue_title_description.vue

@@ -3,6 +3,7 @@ import Visibility from 'visibilityjs';
 import Poll from './../lib/utils/poll';
 import Service from './services/index';
 import tasks from './actions/tasks';
+import edited from './components/edited.vue';
 export default {
   props: {
@@ -14,6 +15,11 @@ export default {
       required: true,
       type: String,
     },
+    isEdited: {
+      type: Boolean,
+      default: false,
+      required: false,
+    },
   },
   data() {
     const resource = new Service(this.$http, this.endpoint);
@@ -46,10 +52,13 @@ export default {
         pre: true,
         pulse: false,
       },
-      timeAgoEl: $('.issue_edited_ago'),
       titleEl: document.querySelector('title'),
+      hasBeenEdited: this.isEdited,
     };
   },
+  components: {
+    edited,
+  },
   methods: {
     updateFlag(key, toggle) {
       this[key].pre = toggle;
@@ -57,6 +66,9 @@ export default {
     },
     renderResponse(res) {
       this.apiData = res.json();
+
+      if (this.apiData.updated_at) this.hasBeenEdited = true;
+
       this.triggerAnimation();
     },
     updateTaskHTML() {
@@ -110,11 +122,6 @@ export default {
       this.elementsToVisualize(noTitleChange, noDescriptionChange);
       this.animate(title, description);
     },
-    updateEditedTimeAgo() {
-      const toolTipTime = gl.utils.formatDate(this.apiData.updated_at);
-      this.timeAgoEl.attr('datetime', this.apiData.updated_at);
-      this.timeAgoEl.attr('title', toolTipTime).tooltip('fixTitle');
-    },
   },
   created() {
     if (!Visibility.hidden()) {
@@ -132,8 +139,6 @@ export default {
   updated() {
     // if new html is injected (description changed) - bind TaskList and call renderGFM
     if (this.descriptionChange) {
-      this.updateEditedTimeAgo();
-
       $(this.$refs['issue-content-container-gfm-entry']).renderGFM();
       const tl = new gl.TaskList({
@@ -176,5 +181,11 @@ export default {
         v-if="descriptionText"
       >{{descriptionText}}</textarea>
     </div>
+    <edited
+      v-if="hasBeenEdited"
+      :updated-at="apiData.updated_at"
+      :updated-by-name="apiData.updated_by_name"
+      :updated-by-path="apiData.updated_by_path"
+    />
   </div>
 </template>

app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue

+<script>
+import tooltipMixin from '../mixins/tooltip';
+import '../../lib/utils/datetime_utility';
+
+/**
+ * Port of ruby helper time_ago_with_tooltip
+ */
+
+export default {
+  props: {
+    time: {
+      type: String,
+      required: true,
+    },
+
+    tooltipPlacement: {
+      type: String,
+      required: false,
+      default: 'top',
+    },
+
+    shortFormat: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+
+    htmlClass: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+
+  mixins: [tooltipMixin],
+
+  computed: {
+    cssClass() {
+      return this.shortFormat ? 'js-short-timeago' : 'js-timeago';
+    },
+
+    tooltipTitle() {
+      return gl.utils.formatDate(this.time);
+    },
+
+    timeFormated() {
+      const timeago = gl.utils.getTimeago();
+
+      return timeago.format(this.time);
+    },
+  },
+};
+</script>
+
+<template>
+  <time
+    :class="[cssClass, htmlClass]"
+    class="js-timeago js-timeago-render"
+    :title="tooltipTitle"
+    :data-placement="tooltipPlacement"
+    data-container="body"
+    ref="tooltip"
+  >
+    {{timeFormated}}
+  </time>
+</template>

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

@@ -202,15 +202,22 @@ class Projects::IssuesController < Projects::ApplicationController
   def rendered_title
     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,
       issue_number: @issue.iid,
-      updated_at: @issue.updated_at,
     }
+
+    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/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

app/views/projects/issues/show.html.haml

@@ -53,11 +53,10 @@
   .detail-page-description.content-block
     .issue-title-data.hidden{ "data" => { "endpoint" => rendered_title_namespace_project_issue_path(@project.namespace, @project, @issue),
       "can-update-tasks-class" => can?(current_user, :update_issue, @issue) ? 'js-task-list-container' : '',
+      "is-edited": @issue.is_edited?,
     } }
     .issue-title-entrypoint
-    = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue_edited_ago')
-
     #merge-requests{ data: { url: referenced_merge_requests_namespace_project_issue_url(@project.namespace, @project, @issue) } }
       // This element is filled in using JavaScript.

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/issue_title_description_spec.js

@@ -7,6 +7,10 @@ import issueShowData from './mock_data';
 window.$ = $;
+function formatText(text) {
+  return text.trim().replace(/\s\s+/g, ' ');
+}
+
 const issueShowInterceptor = data => (request, next) => {
   next(request.respondWith(JSON.stringify(data), {
     status: 200,
@@ -29,7 +33,7 @@ describe('Issue Title', () => {
     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) => {
     Vue.http.interceptors.push(issueShowInterceptor(issueShowData.initialRequest));
     const issueShowComponent = new IssueTitleDescriptionComponent({
@@ -40,10 +44,15 @@ describe('Issue Title', () => {
     }).$mount();
     setTimeout(() => {
+      const editedText = issueShowComponent.$el.querySelector('.edited-text');
+
       expect(document.querySelector('title').innerText).toContain('this is a title (#1)');
       expect(issueShowComponent.$el.querySelector('.title').innerHTML).toContain('<p>this is a title</p>');
       expect(issueShowComponent.$el.querySelector('.wiki').innerHTML).toContain('<p>this is a description!</p>');
       expect(issueShowComponent.$el.querySelector('.js-task-list-field').innerText).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));
@@ -52,6 +61,10 @@ describe('Issue Title', () => {
         expect(issueShowComponent.$el.querySelector('.title').innerHTML).toContain('<p>2</p>');
         expect(issueShowComponent.$el.querySelector('.wiki').innerHTML).toContain('<p>42</p>');
         expect(issueShowComponent.$el.querySelector('.js-task-list-field').innerText).toContain('42');
+        expect(issueShowComponent.$el.querySelector('.edited-text')).toBeTruthy();
+        expect(formatText(issueShowComponent.$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/mock_data.js

@@ -6,6 +6,9 @@ export default {
     description_text: 'this is a description',
     issue_number: 1,
     task_status: '2 of 4 completed',
+    updated_at: '2015-05-15T12:31:04.428Z',
+    updated_by_name: 'Some User',
+    updated_by_path: '/some_user',
   },
   secondRequest: {
     title: '<p>2</p>',
@@ -14,6 +17,9 @@ export default {
     description_text: '42',
     issue_number: 1,
     task_status: '0 of 0 completed',
+    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>',
@@ -22,5 +28,8 @@ export default {
     description_text: '- [ ] Task List Item',
     issue_number: 1,
     task_status: '0 of 1 completed',
+    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