Commit e178eb78 authored by Phil Hughes's avatar Phil Hughes

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
parent a481337b
...@@ -16,9 +16,9 @@ export default { ...@@ -16,9 +16,9 @@ export default {
<div class="media-body"> <div class="media-body">
<mr-widget-author-and-time <mr-widget-author-and-time
actionText="Closed by" actionText="Closed by"
:author="mr.closedBy" :author="mr.closedEvent.author"
:dateTitle="mr.updatedAt" :dateTitle="mr.closedEvent.updatedAt"
:dateReadable="mr.closedAt" :dateReadable="mr.closedEvent.formattedUpdatedAt"
/> />
<section class="mr-info-list"> <section class="mr-info-list">
<p> <p>
......
...@@ -69,9 +69,9 @@ export default { ...@@ -69,9 +69,9 @@ export default {
<div class="space-children"> <div class="space-children">
<mr-widget-author-and-time <mr-widget-author-and-time
actionText="Merged by" actionText="Merged by"
:author="mr.mergedBy" :author="mr.mergedEvent.author"
:dateTitle="mr.updatedAt" :date-title="mr.mergedEvent.updatedAt"
:dateReadable="mr.mergedAt" /> :date-readable="mr.mergedEvent.formattedUpdatedAt" />
<a <a
v-if="mr.canRevertInCurrentMR" v-if="mr.canRevertInCurrentMR"
v-tooltip v-tooltip
......
...@@ -37,10 +37,8 @@ export default class MergeRequestStore { ...@@ -37,10 +37,8 @@ export default class MergeRequestStore {
} }
this.updatedAt = data.updated_at; this.updatedAt = data.updated_at;
this.mergedAt = MergeRequestStore.getEventDate(data.merge_event); this.mergedEvent = MergeRequestStore.getEventObject(data.merge_event);
this.closedAt = MergeRequestStore.getEventDate(data.closed_event); this.closedEvent = MergeRequestStore.getEventObject(data.closed_event);
this.mergedBy = MergeRequestStore.getAuthorObject(data.merge_event);
this.closedBy = MergeRequestStore.getAuthorObject(data.closed_event);
this.setToMWPSBy = MergeRequestStore.getAuthorObject({ author: data.merge_user || {} }); this.setToMWPSBy = MergeRequestStore.getAuthorObject({ author: data.merge_user || {} });
this.mergeUserId = data.merge_user_id; this.mergeUserId = data.merge_user_id;
this.currentUserId = gon.current_user_id; this.currentUserId = gon.current_user_id;
...@@ -118,6 +116,16 @@ export default class MergeRequestStore { ...@@ -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) { static getAuthorObject(event) {
if (!event) { if (!event) {
return {}; return {};
......
---
title: Fixed merge request widget merged & closed date tooltip text
merge_request:
author:
type: fixed
...@@ -4,11 +4,15 @@ import closedComponent from '~/vue_merge_request_widget/components/states/mr_wid ...@@ -4,11 +4,15 @@ import closedComponent from '~/vue_merge_request_widget/components/states/mr_wid
const mr = { const mr = {
targetBranch: 'good-branch', targetBranch: 'good-branch',
targetBranchPath: '/good-branch', targetBranchPath: '/good-branch',
closedBy: { closedEvent: {
name: 'Fatih Acet', author: {
username: 'fatihacet', name: 'Fatih Acet',
username: 'fatihacet',
},
updatedAt: 'closedEventUpdatedAt',
formattedUpdatedAt: '',
}, },
updatedAt: '2017-03-23T20:08:08.845Z', updatedAt: 'mrUpdatedAt',
closedAt: '1 day ago', closedAt: '1 day ago',
}; };
...@@ -18,7 +22,7 @@ const createComponent = () => { ...@@ -18,7 +22,7 @@ const createComponent = () => {
return new Component({ return new Component({
el: document.createElement('div'), el: document.createElement('div'),
propsData: { mr }, propsData: { mr },
}).$el; });
}; };
describe('MRWidgetClosed', () => { describe('MRWidgetClosed', () => {
...@@ -38,14 +42,30 @@ describe('MRWidgetClosed', () => { ...@@ -38,14 +42,30 @@ describe('MRWidgetClosed', () => {
}); });
describe('template', () => { describe('template', () => {
it('should have correct elements', () => { let vm;
const el = createComponent(); 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('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.textContent).toContain('The changes were not merged into');
expect(el.querySelector('.label-branch').getAttribute('href')).toEqual(mr.targetBranchPath); expect(el.querySelector('.label-branch').getAttribute('href')).toEqual(mr.targetBranchPath);
expect(el.querySelector('.label-branch').textContent).toContain(mr.targetBranch); 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');
});
}); });
}); });
...@@ -14,9 +14,12 @@ const createComponent = () => { ...@@ -14,9 +14,12 @@ const createComponent = () => {
canRevertInCurrentMR: true, canRevertInCurrentMR: true,
canRemoveSourceBranch: true, canRemoveSourceBranch: true,
sourceBranchRemoved: true, sourceBranchRemoved: true,
mergedBy: {}, mergedEvent: {
mergedAt: '', author: {},
updatedAt: '', updatedAt: 'mergedUpdatedAt',
formattedUpdatedAt: '',
},
updatedAt: 'mrUpdatedAt',
targetBranch, targetBranch,
}; };
...@@ -170,5 +173,11 @@ describe('MRWidgetMerged', () => { ...@@ -170,5 +173,11 @@ describe('MRWidgetMerged', () => {
done(); done();
}); });
}); });
it('should use mergedEvent updatedAt as tooltip title', () => {
expect(
el.querySelector('time').getAttribute('title'),
).toBe('mergedUpdatedAt');
});
}); });
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment