From e178eb782a1a5ac24c18ed4e4a1c1d55bee8251f Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Fri, 29 Sep 2017 10:31:10 +0100
Subject: [PATCH] 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 {
       <div class="media-body">
         <mr-widget-author-and-time
           actionText="Closed by"
-          :author="mr.closedBy"
-          :dateTitle="mr.updatedAt"
-          :dateReadable="mr.closedAt"
+          :author="mr.closedEvent.author"
+          :dateTitle="mr.closedEvent.updatedAt"
+          :dateReadable="mr.closedEvent.formattedUpdatedAt"
         />
         <section class="mr-info-list">
           <p>
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 {
         <div class="space-children">
           <mr-widget-author-and-time
             actionText="Merged by"
-            :author="mr.mergedBy"
-            :dateTitle="mr.updatedAt"
-            :dateReadable="mr.mergedAt" />
+            :author="mr.mergedEvent.author"
+            :date-title="mr.mergedEvent.updatedAt"
+            :date-readable="mr.mergedEvent.formattedUpdatedAt" />
           <a
             v-if="mr.canRevertInCurrentMR"
             v-tooltip
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 29464662578..5bd3af3bd8d 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
@@ -37,10 +37,8 @@ export default class MergeRequestStore {
     }
 
     this.updatedAt = data.updated_at;
-    this.mergedAt = MergeRequestStore.getEventDate(data.merge_event);
-    this.closedAt = MergeRequestStore.getEventDate(data.closed_event);
-    this.mergedBy = MergeRequestStore.getAuthorObject(data.merge_event);
-    this.closedBy = MergeRequestStore.getAuthorObject(data.closed_event);
+    this.mergedEvent = MergeRequestStore.getEventObject(data.merge_event);
+    this.closedEvent = MergeRequestStore.getEventObject(data.closed_event);
     this.setToMWPSBy = MergeRequestStore.getAuthorObject({ author: data.merge_user || {} });
     this.mergeUserId = data.merge_user_id;
     this.currentUserId = gon.current_user_id;
@@ -118,6 +116,16 @@ export default class MergeRequestStore {
     }
   }
 
+  static getEventObject(event) {
+    if (!event) return null;
+
+    return {
+      author: MergeRequestStore.getAuthorObject(event),
+      updatedAt: gl.utils.formatDate(event.updated_at),
+      formattedUpdatedAt: MergeRequestStore.getEventDate(event),
+    };
+  }
+
   static getAuthorObject(event) {
     if (!event) {
       return {};
diff --git a/changelogs/unreleased/mr-widget-merged-date-tooltip.yml b/changelogs/unreleased/mr-widget-merged-date-tooltip.yml
new file mode 100644
index 00000000000..ea22993ff52
--- /dev/null
+++ b/changelogs/unreleased/mr-widget-merged-date-tooltip.yml
@@ -0,0 +1,5 @@
+---
+title: Fixed merge request widget merged & closed date tooltip text
+merge_request:
+author:
+type: fixed
diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js
index 47303d1e80f..d23b558f4ea 100644
--- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js
@@ -4,11 +4,15 @@ import closedComponent from '~/vue_merge_request_widget/components/states/mr_wid
 const mr = {
   targetBranch: 'good-branch',
   targetBranchPath: '/good-branch',
-  closedBy: {
-    name: 'Fatih Acet',
-    username: 'fatihacet',
+  closedEvent: {
+    author: {
+      name: 'Fatih Acet',
+      username: 'fatihacet',
+    },
+    updatedAt: 'closedEventUpdatedAt',
+    formattedUpdatedAt: '',
   },
-  updatedAt: '2017-03-23T20:08:08.845Z',
+  updatedAt: 'mrUpdatedAt',
   closedAt: '1 day ago',
 };
 
@@ -18,7 +22,7 @@ const createComponent = () => {
   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');
+    });
   });
 });