diff options
author | Phil Hughes <me@iamphill.com> | 2017-06-05 13:42:33 +0300 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2017-06-05 13:42:33 +0300 |
commit | c34107608ecc5c36e80a748eb4c9b88d2b1157cf (patch) | |
tree | b1a67e41a2b6740f2a7d6c2759a872fcdd87b23a | |
parent | 65581fad5e26fdf2612c098a7fbc48a53aae5e28 (diff) | |
parent | b2d577a7a293ac6c82a8bc64f5b134558460df5b (diff) |
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
20 files changed, 237 insertions, 30 deletions
diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index 800bb9f1fe8..e14414d3f68 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/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> diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue index 3281ec6b172..5ae617356e0 100644 --- a/app/assets/javascripts/issue_show/components/description.vue +++ b/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(); }); }, diff --git a/app/assets/javascripts/issue_show/components/edited.vue b/app/assets/javascripts/issue_show/components/edited.vue new file mode 100644 index 00000000000..d59e6d11032 --- /dev/null +++ b/app/assets/javascripts/issue_show/components/edited.vue @@ -0,0 +1,56 @@ +<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> + diff --git a/app/assets/javascripts/issue_show/index.js b/app/assets/javascripts/issue_show/index.js index faf79471946..14b2a1e18e9 100644 --- a/app/assets/javascripts/issue_show/index.js +++ b/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, }, }); }, diff --git a/app/assets/javascripts/issue_show/stores/index.js b/app/assets/javascripts/issue_show/stores/index.js index 4a16c3cb4dc..27c2d349f52 100644 --- a/app/assets/javascripts/issue_show/stores/index.js +++ b/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) { diff --git a/app/assets/stylesheets/framework/mobile.scss b/app/assets/stylesheets/framework/mobile.scss index 678af978edd..0140dcf19c3 100644 --- a/app/assets/stylesheets/framework/mobile.scss +++ b/app/assets/stylesheets/framework/mobile.scss @@ -112,11 +112,6 @@ } } - .issue-edited-ago, - .note_edited_ago { - display: none; - } - aside:not(.right-sidebar) { display: none; } diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 59df1e7b86a..8b1efd0c572 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/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 diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f422c48329c..71154da7ec5 100644 --- a/app/helpers/application_helper.rb +++ b/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 ') diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index c380a10c82d..5e8f0849969 100644 --- a/app/helpers/issuables_helper.rb +++ b/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 diff --git a/app/models/concerns/editable.rb b/app/models/concerns/editable.rb new file mode 100644 index 00000000000..c62c7e1e936 --- /dev/null +++ b/app/models/concerns/editable.rb @@ -0,0 +1,7 @@ +module Editable + extend ActiveSupport::Concern + + def is_edited? + last_edited_at.present? && last_edited_at != created_at + end +end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 075ec575f9d..ea10d004c9c 100644 --- a/app/models/concerns/issuable.rb +++ b/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 diff --git a/app/models/note.rb b/app/models/note.rb index 832c68243fb..563af47f314 100644 --- a/app/models/note.rb +++ b/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 diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 882e2fa0594..6c3358685fe 100644 --- a/app/models/snippet.rb +++ b/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 diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 7bf271c2fc5..d909b0bfbbd 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -63,7 +63,7 @@ .wiki= markdown_field(@issue, :description) %textarea.hidden.js-task-list-field= @issue.description - = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue-edited-ago js-issue-edited-ago') + = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue-edited-ago js-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. diff --git a/db/schema.rb b/db/schema.rb index 7966c732080..0496ce2ced3 100644 --- a/db/schema.rb +++ b/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 diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index c1ecb46aece..8fcf7f5fa15 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/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 diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index 0030a953119..59c006aa0af 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/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(); }); diff --git a/spec/javascripts/issue_show/components/edited_spec.js b/spec/javascripts/issue_show/components/edited_spec.js new file mode 100644 index 00000000000..a0d0750ae34 --- /dev/null +++ b/spec/javascripts/issue_show/components/edited_spec.js @@ -0,0 +1,49 @@ +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(); + }); +}); diff --git a/spec/javascripts/issue_show/mock_data.js b/spec/javascripts/issue_show/mock_data.js index 6683d581bc5..eb3111412a7 100644 --- a/spec/javascripts/issue_show/mock_data.js +++ b/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', }, }; diff --git a/spec/models/concerns/editable_spec.rb b/spec/models/concerns/editable_spec.rb new file mode 100644 index 00000000000..cd73af3b480 --- /dev/null +++ b/spec/models/concerns/editable_spec.rb @@ -0,0 +1,11 @@ +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 |