From e178eb782a1a5ac24c18ed4e4a1c1d55bee8251f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 29 Sep 2017 10:31:10 +0100 Subject: [PATCH 1/2] Fixes merge request widget date tooltip inconsistencies Previously the merge request widget would use the `updated_at` date as the tooltip text for both closed & merged states. This is incorrect as the `updated_at` date is actually changed when a user updates merge request through commenting, description changes or anything else. The widget states for merged & closed events now use their own event object which holds their own `updated_at` date string. Also this text has been correctly formatted through our date utilities to correctly display the right timezone data in a user friendly way. Closes #38545 --- .../components/states/mr_widget_closed.js | 6 ++-- .../components/states/mr_widget_merged.js | 6 ++-- .../stores/mr_widget_store.js | 16 ++++++--- .../mr-widget-merged-date-tooltip.yml | 5 +++ .../states/mr_widget_closed_spec.js | 36 ++++++++++++++----- .../states/mr_widget_merged_spec.js | 15 ++++++-- 6 files changed, 63 insertions(+), 21 deletions(-) create mode 100644 changelogs/unreleased/mr-widget-merged-date-tooltip.yml diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js index 4078aad7f83..b25cc3443ef 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js @@ -16,9 +16,9 @@ export default {

diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js index e452260a4d0..74fc52796a0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js @@ -69,9 +69,9 @@ export default {

+ :author="mr.mergedEvent.author" + :date-title="mr.mergedEvent.updatedAt" + :date-readable="mr.mergedEvent.formattedUpdatedAt" /> { return new Component({ el: document.createElement('div'), propsData: { mr }, - }).$el; + }); }; describe('MRWidgetClosed', () => { @@ -38,14 +42,30 @@ describe('MRWidgetClosed', () => { }); describe('template', () => { - it('should have correct elements', () => { - const el = createComponent(); + let vm; + let el; + beforeEach(() => { + vm = createComponent(); + el = vm.$el; + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should have correct elements', () => { expect(el.querySelector('h4').textContent).toContain('Closed by'); - expect(el.querySelector('h4').textContent).toContain(mr.closedBy.name); + expect(el.querySelector('h4').textContent).toContain(mr.closedEvent.author.name); expect(el.textContent).toContain('The changes were not merged into'); expect(el.querySelector('.label-branch').getAttribute('href')).toEqual(mr.targetBranchPath); expect(el.querySelector('.label-branch').textContent).toContain(mr.targetBranch); }); + + it('should use closedEvent updatedAt as tooltip title', () => { + expect( + el.querySelector('time').getAttribute('title'), + ).toBe('closedEventUpdatedAt'); + }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js index afaa750199a..2714e8294fa 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js @@ -14,9 +14,12 @@ const createComponent = () => { canRevertInCurrentMR: true, canRemoveSourceBranch: true, sourceBranchRemoved: true, - mergedBy: {}, - mergedAt: '', - updatedAt: '', + mergedEvent: { + author: {}, + updatedAt: 'mergedUpdatedAt', + formattedUpdatedAt: '', + }, + updatedAt: 'mrUpdatedAt', targetBranch, }; @@ -170,5 +173,11 @@ describe('MRWidgetMerged', () => { done(); }); }); + + it('should use mergedEvent updatedAt as tooltip title', () => { + expect( + el.querySelector('time').getAttribute('title'), + ).toBe('mergedUpdatedAt'); + }); }); }); From 775c9f68708a1a7290f5a950da9a953908645a65 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 29 Sep 2017 12:49:14 +0100 Subject: [PATCH 2/2] fixed specs when merged event was empty --- .../stores/mr_widget_store.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 5bd3af3bd8d..2f1c0a4c654 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -117,11 +117,9 @@ export default class MergeRequestStore { } static getEventObject(event) { - if (!event) return null; - return { author: MergeRequestStore.getAuthorObject(event), - updatedAt: gl.utils.formatDate(event.updated_at), + updatedAt: gl.utils.formatDate(MergeRequestStore.getEventUpdatedAtDate(event)), formattedUpdatedAt: MergeRequestStore.getEventDate(event), }; } @@ -139,6 +137,14 @@ export default class MergeRequestStore { }; } + static getEventUpdatedAtDate(event) { + if (!event) { + return ''; + } + + return event.updated_at; + } + static getEventDate(event) { const timeagoInstance = new Timeago(); @@ -146,7 +152,7 @@ export default class MergeRequestStore { return ''; } - return timeagoInstance.format(event.updated_at); + return timeagoInstance.format(MergeRequestStore.getEventUpdatedAtDate(event)); } }