diff options
author | Luke "Jared" Bennett <lbennett@gitlab.com> | 2017-05-15 09:51:35 +0100 |
---|---|---|
committer | Luke "Jared" Bennett <lbennett@gitlab.com> | 2017-05-17 17:10:08 +0100 |
commit | 6dd88474bed70c2aa70c7fdf7ba6dbdc29dc8998 (patch) | |
tree | 2e5c6b5a453d6c653f5435af055c4f5a1756e408 | |
parent | 436514d20909cf30ef272502f8b14d76f1dda2ac (diff) | |
download | gitlab-ce-6dd88474bed70c2aa70c7fdf7ba6dbdc29dc8998.tar.gz |
Remove unneeded media query hiding the edited timeago
[ci skip] Add edited component for issue_show
Added tests for issue_title_description use of edited and for edited itself
Fix handling is_edited for realtime edit text
Fix tests failures due to whitespace changes
Update edited.vue to include required and default props
-rw-r--r-- | app/assets/javascripts/issue_show/components/edited.vue | 55 | ||||
-rw-r--r-- | app/assets/javascripts/issue_show/index.js | 3 | ||||
-rw-r--r-- | app/assets/javascripts/issue_show/issue_title_description.vue | 27 | ||||
-rw-r--r-- | app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue | 66 | ||||
-rw-r--r-- | app/assets/stylesheets/framework/mobile.scss | 5 | ||||
-rw-r--r-- | app/controllers/projects/issues_controller.rb | 12 | ||||
-rw-r--r-- | app/helpers/application_helper.rb | 4 | ||||
-rw-r--r-- | app/helpers/editable_helper.rb | 5 | ||||
-rw-r--r-- | app/views/projects/issues/show.html.haml | 3 | ||||
-rw-r--r-- | spec/javascripts/issue_show/components/edited_spec.js | 49 | ||||
-rw-r--r-- | spec/javascripts/issue_show/issue_title_description_spec.js | 15 | ||||
-rw-r--r-- | spec/javascripts/issue_show/mock_data.js | 9 |
12 files changed, 233 insertions, 20 deletions
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..f5038e55c09 --- /dev/null +++ b/app/assets/javascripts/issue_show/components/edited.vue @@ -0,0 +1,55 @@ +<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 eb20a597bb5..6dba7b90716 100644 --- a/app/assets/javascripts/issue_show/index.js +++ b/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, }, }), }); diff --git a/app/assets/javascripts/issue_show/issue_title_description.vue b/app/assets/javascripts/issue_show/issue_title_description.vue index dc3ba2550c5..3f77ba22d23 100644 --- a/app/assets/javascripts/issue_show/issue_title_description.vue +++ b/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> diff --git a/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue b/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue new file mode 100644 index 00000000000..934e7e8eacb --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue @@ -0,0 +1,66 @@ +<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> diff --git a/app/assets/stylesheets/framework/mobile.scss b/app/assets/stylesheets/framework/mobile.scss index eb73f7cc794..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 bcd23d61519..5dfe00a35cd 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -5,6 +5,7 @@ class Projects::IssuesController < Projects::ApplicationController include ToggleAwardEmoji include IssuableCollections include SpammableActions + include EditableHelper prepend_before_action :authenticate_user!, only: [:new] @@ -202,15 +203,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 is_edited?(@issue) + 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 6d6bcbaf88a..16734fe4c75 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -2,6 +2,8 @@ require 'digest/md5' require 'uri' module ApplicationHelper + include EditableHelper + # Check if a particular controller is the current one # # args - One or more controller names to check @@ -181,7 +183,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 is_edited?(object) content_tag :small, class: 'edited-text' do output = content_tag(:span, 'Edited ') diff --git a/app/helpers/editable_helper.rb b/app/helpers/editable_helper.rb new file mode 100644 index 00000000000..78d9998b97e --- /dev/null +++ b/app/helpers/editable_helper.rb @@ -0,0 +1,5 @@ +module EditableHelper + def is_edited?(object) + !object.last_edited_at.blank? && object.last_edited_at != object.created_at + end +end diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 9084883eb3e..144233b54bb 100644 --- a/app/views/projects/issues/show.html.haml +++ b/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": is_edited?(@issue), } } .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. 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/issue_title_description_spec.js b/spec/javascripts/issue_show/issue_title_description_spec.js index 1ec4fe58b08..8180e67255c 100644 --- a/spec/javascripts/issue_show/issue_title_description_spec.js +++ b/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(); }); diff --git a/spec/javascripts/issue_show/mock_data.js b/spec/javascripts/issue_show/mock_data.js index ad5a7b63470..a4562449ff1 100644 --- a/spec/javascripts/issue_show/mock_data.js +++ b/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', }, }; |