Add merge request popover with details

- Show pipeline status, title, MR Status and project path
- Popover attached to gitlab flavored markdown everywhere, including:
  + MR/Issue Title
  + MR/Issue description
  + MR/Issue comments
  + Rendered markdown files
This commit is contained in:
Sam Bigelow 2019-03-01 16:41:16 -05:00
parent c174fc0cc1
commit 1a14e5230e
18 changed files with 438 additions and 13 deletions

View file

@ -4,6 +4,7 @@ import renderMath from './render_math';
import renderMermaid from './render_mermaid';
import highlightCurrentUser from './highlight_current_user';
import initUserPopovers from '../../user_popovers';
import initMRPopovers from '../../mr_popover';
// Render GitLab flavoured Markdown
//
@ -15,6 +16,7 @@ $.fn.renderGFM = function renderGFM() {
renderMermaid(this.find('.js-render-mermaid'));
highlightCurrentUser(this.find('.gfm-project_member').get());
initUserPopovers(this.find('.gfm-project_member').get());
initMRPopovers(this.find('.gfm-merge_request').get());
return this;
};

View file

@ -0,0 +1,110 @@
<script>
import { GlPopover, GlSkeletonLoading } from '@gitlab/ui';
import Icon from '../../vue_shared/components/icon.vue';
import CiIcon from '../../vue_shared/components/ci_icon.vue';
import timeagoMixin from '../../vue_shared/mixins/timeago';
import query from '../queries/merge_request.graphql';
import { mrStates, humanMRStates } from '../constants';
export default {
name: 'MRPopover',
components: {
GlPopover,
GlSkeletonLoading,
Icon,
CiIcon,
},
mixins: [timeagoMixin],
props: {
target: {
type: HTMLAnchorElement,
required: true,
},
projectPath: {
type: String,
required: true,
},
mergeRequestIID: {
type: String,
required: true,
},
mergeRequestTitle: {
type: String,
required: true,
},
},
data() {
return {
mergeRequest: {},
};
},
computed: {
detailedStatus() {
return this.mergeRequest.headPipeline && this.mergeRequest.headPipeline.detailedStatus;
},
formattedTime() {
return this.timeFormated(this.mergeRequest.createdAt);
},
statusBoxClass() {
switch (this.mergeRequest.state) {
case mrStates.merged:
return 'status-box-mr-merged';
case mrStates.closed:
return 'status-box-closed';
default:
return 'status-box-open';
}
},
stateHumanName() {
switch (this.mergeRequest.state) {
case mrStates.merged:
return humanMRStates.merged;
case mrStates.closed:
return humanMRStates.closed;
default:
return humanMRStates.open;
}
},
showDetails() {
return Object.keys(this.mergeRequest).length > 0;
},
},
apollo: {
mergeRequest: {
query,
update: data => data.project.mergeRequest,
variables() {
const { projectPath, mergeRequestIID } = this;
return {
projectPath,
mergeRequestIID,
};
},
},
},
};
</script>
<template>
<gl-popover :target="target" boundary="viewport" placement="top" show>
<div class="mr-popover">
<div v-if="$apollo.loading">
<gl-skeleton-loading :lines="1" class="animation-container-small mt-1" />
</div>
<div v-else-if="showDetails" class="d-flex align-items-center justify-content-between">
<div class="d-inline-flex align-items-center">
<div :class="`issuable-status-box status-box ${statusBoxClass}`">
{{ stateHumanName }}
</div>
<span class="text-secondary">Opened <time v-text="formattedTime"></time></span>
</div>
<ci-icon v-if="detailedStatus" :status="detailedStatus" />
</div>
<h5 class="my-2">{{ mergeRequestTitle }}</h5>
<div class="text-secondary">
{{ `${projectPath}!${mergeRequestIID}` }}
</div>
</div>
</gl-popover>
</template>

View file

@ -0,0 +1,10 @@
export const mrStates = {
merged: 'merged',
closed: 'closed',
};
export const humanMRStates = {
merged: 'Merged',
closed: 'Closed',
open: 'Open',
};

View file

@ -0,0 +1,62 @@
import Vue from 'vue';
import VueApollo from 'vue-apollo';
import MRPopover from './components/mr_popover.vue';
import createDefaultClient from '~/lib/graphql';
let renderedPopover;
let renderFn;
const handleUserPopoverMouseOut = ({ target }) => {
target.removeEventListener('mouseleave', handleUserPopoverMouseOut);
if (renderFn) {
clearTimeout(renderFn);
}
if (renderedPopover) {
renderedPopover.$destroy();
renderedPopover = null;
}
};
/**
* Adds a MergeRequestPopover component to the body, hands over as much data as the target element has in data attributes.
* loads based on data-project-path and data-iid more data about an MR from the API and sets it on the popover
*/
const handleMRPopoverMount = apolloProvider => ({ target }) => {
// Add listener to actually remove it again
target.addEventListener('mouseleave', handleUserPopoverMouseOut);
const { projectPath, mrTitle, iid } = target.dataset;
const mergeRequest = {};
renderFn = setTimeout(() => {
const MRPopoverComponent = Vue.extend(MRPopover);
renderedPopover = new MRPopoverComponent({
propsData: {
target,
projectPath,
mergeRequestIID: iid,
mergeRequest,
mergeRequestTitle: mrTitle,
},
apolloProvider,
});
renderedPopover.$mount();
}, 200); // 200ms delay so not every mouseover triggers Popover + API Call
};
export default elements => {
const mrLinks = elements || [...document.querySelectorAll('.gfm-merge_request')];
if (mrLinks.length > 0) {
Vue.use(VueApollo);
const apolloProvider = new VueApollo({
defaultClient: createDefaultClient(),
});
mrLinks.forEach(el => {
el.addEventListener('mouseenter', handleMRPopoverMount(apolloProvider));
});
}
};

View file

@ -0,0 +1,14 @@
query mergeRequest($projectPath: ID!, $mergeRequestIID: ID!) {
project(fullPath: $projectPath) {
mergeRequest(iid: $mergeRequestIID) {
createdAt
state
headPipeline {
detailedStatus {
icon
group
}
}
}
}
}

View file

@ -22,6 +22,7 @@ import Icon from '../../vue_shared/components/icon.vue';
* - Jobs show view header
* - Jobs show view sidebar
* - Linked pipelines
* - Extended MR Popover
*/
const validSizes = [8, 12, 16, 18, 24, 32, 48, 72];

View file

@ -7,3 +7,10 @@
line-height: $gl-line-height;
}
}
.mr-popover {
.text-secondary {
font-size: 12px;
line-height: 1.33;
}
}

View file

@ -0,0 +1,17 @@
# frozen_string_literal: true
module Types
module Ci
class DetailedStatusType < BaseObject
graphql_name 'DetailedStatus'
field :group, GraphQL::STRING_TYPE, null: false
field :icon, GraphQL::STRING_TYPE, null: false
field :favicon, GraphQL::STRING_TYPE, null: false
field :details_path, GraphQL::STRING_TYPE, null: false
field :has_details, GraphQL::BOOLEAN_TYPE, null: false, method: :has_details?
field :label, GraphQL::STRING_TYPE, null: false
field :text, GraphQL::STRING_TYPE, null: false
field :tooltip, GraphQL::STRING_TYPE, null: false, method: :status_tooltip
end
end
end

View file

@ -13,6 +13,10 @@ module Types
field :sha, GraphQL::STRING_TYPE, null: false
field :before_sha, GraphQL::STRING_TYPE, null: true
field :status, PipelineStatusEnum, null: false
field :detailed_status,
Types::Ci::DetailedStatusType,
null: false,
resolve: -> (obj, _args, ctx) { obj.detailed_status(ctx[:current_user]) }
field :duration,
GraphQL::INT_TYPE,
null: true,

View file

@ -14,7 +14,7 @@ module CacheMarkdownField
# Increment this number every time the renderer changes its output
CACHE_COMMONMARK_VERSION_START = 10
CACHE_COMMONMARK_VERSION = 14
CACHE_COMMONMARK_VERSION = 15
# changes to these attributes cause the cache to be invalidates
INVALIDATED_BY = %w[author project].freeze

View file

@ -0,0 +1,5 @@
---
title: Add extended merge request tooltip
merge_request: !25221
author:
type: added

View file

@ -181,9 +181,10 @@ module Banzai
title = object_link_title(object, matches)
klass = reference_class(object_sym)
data = data_attributes_for(link_content || match, parent, object,
data_attributes = data_attributes_for(link_content || match, parent, object,
link_content: !!link_content,
link_reference: link_reference)
data = data_attribute(data_attributes)
url =
if matches.names.include?("url") && matches[:url]
@ -206,13 +207,13 @@ module Banzai
def data_attributes_for(text, parent, object, link_content: false, link_reference: false)
object_parent_type = parent.is_a?(Group) ? :group : :project
data_attribute(
{
original: text,
link: link_content,
link_reference: link_reference,
object_parent_type => parent.id,
object_sym => object.id
)
}
end
def object_link_text_extras(object, matches)

View file

@ -20,7 +20,9 @@ module Banzai
end
def object_link_title(object, matches)
object_link_commit_title(object, matches) || super
# The method will return `nil` if object is not a commit
# allowing for properly handling the extended MR Tooltip
object_link_commit_title(object, matches)
end
def object_link_text_extras(object, matches)
@ -53,6 +55,14 @@ module Banzai
.includes(target_project: :namespace)
end
def reference_class(object_sym, options = {})
super(object_sym, tooltip: false)
end
def data_attributes_for(text, parent, object, data = {})
super.merge(project_path: parent.full_path, iid: object.iid, mr_title: object.title)
end
private
def object_link_commit_title(object, matches)

View file

@ -0,0 +1,93 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`MR Popover loaded state matches the snapshot 1`] = `
<glpopover-stub
boundary="viewport"
placement="top"
show=""
target=""
>
<div
class="mr-popover"
>
<div
class="d-flex align-items-center justify-content-between"
>
<div
class="d-inline-flex align-items-center"
>
<div
class="issuable-status-box status-box status-box-open"
>
Open
</div>
<span
class="text-secondary"
>
Opened
<time>
just now
</time>
</span>
</div>
<ciicon-stub
cssclasses=""
size="16"
status="[object Object]"
/>
</div>
<h5
class="my-2"
>
MR Title
</h5>
<div
class="text-secondary"
>
foo/bar!1
</div>
</div>
</glpopover-stub>
`;
exports[`MR Popover shows skeleton-loader while apollo is loading 1`] = `
<glpopover-stub
boundary="viewport"
placement="top"
show=""
target=""
>
<div
class="mr-popover"
>
<div>
<glskeletonloading-stub
class="animation-container-small mt-1"
lines="1"
/>
</div>
<h5
class="my-2"
>
MR Title
</h5>
<div
class="text-secondary"
>
foo/bar!1
</div>
</div>
</glpopover-stub>
`;

View file

@ -0,0 +1,61 @@
import MRPopover from '~/mr_popover/components/mr_popover';
import { shallowMount } from '@vue/test-utils';
describe('MR Popover', () => {
let wrapper;
beforeEach(() => {
wrapper = shallowMount(MRPopover, {
propsData: {
target: document.createElement('a'),
projectPath: 'foo/bar',
mergeRequestIID: '1',
mergeRequestTitle: 'MR Title',
},
mocks: {
$apollo: {
loading: false,
},
},
});
});
it('shows skeleton-loader while apollo is loading', () => {
wrapper.vm.$apollo.loading = true;
expect(wrapper.element).toMatchSnapshot();
});
describe('loaded state', () => {
it('matches the snapshot', () => {
wrapper.setData({
mergeRequest: {
state: 'opened',
createdAt: new Date(),
headPipeline: {
detailedStatus: {
group: 'success',
status: 'status_success',
},
},
},
});
expect(wrapper.element).toMatchSnapshot();
});
it('does not show CI Icon if there is no pipeline data', () => {
wrapper.setData({
mergeRequest: {
state: 'opened',
headPipeline: null,
stateHumanName: 'Open',
title: 'Merge Request Title',
createdAt: new Date(),
},
});
expect(wrapper.contains('ciicon-stub')).toBe(false);
});
});
});

View file

@ -0,0 +1,11 @@
require 'spec_helper'
describe Types::Ci::DetailedStatusType do
it { expect(described_class.graphql_name).to eq('DetailedStatus') }
it "has all fields" do
expect(described_class).to have_graphql_fields(:group, :icon, :favicon,
:details_path, :has_details,
:label, :text, :tooltip)
end
end

View file

@ -30,6 +30,23 @@ describe Banzai::Filter::MergeRequestReferenceFilter do
end
end
describe 'all references' do
let(:doc) { reference_filter(merge.to_reference) }
let(:tag_el) { doc.css('a').first }
it 'adds merge request iid' do
expect(tag_el["data-iid"]).to eq(merge.iid.to_s)
end
it 'adds project data attribute with project id' do
expect(tag_el["data-project-path"]).to eq(project.full_path)
end
it 'does not add `has-tooltip` class' do
expect(tag_el["class"]).not_to include('has-tooltip')
end
end
context 'internal reference' do
let(:reference) { merge.to_reference }
@ -57,9 +74,9 @@ describe Banzai::Filter::MergeRequestReferenceFilter do
expect(reference_filter(act).to_html).to eq exp
end
it 'includes a title attribute' do
it 'has no title' do
doc = reference_filter("Merge #{reference}")
expect(doc.css('a').first.attr('title')).to eq merge.title
expect(doc.css('a').first.attr('title')).to eq ""
end
it 'escapes the title attribute' do
@ -69,9 +86,9 @@ describe Banzai::Filter::MergeRequestReferenceFilter do
expect(doc.text).to eq "Merge #{reference}"
end
it 'includes default classes' do
it 'includes default classes, without tooltip' do
doc = reference_filter("Merge #{reference}")
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-merge_request has-tooltip'
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-merge_request'
end
it 'includes a data-project attribute' do

View file

@ -70,13 +70,13 @@ describe 'getting merge request information nested in a project' do
context 'when there are pipelines' do
before do
pipeline = create(
create(
:ci_pipeline,
project: merge_request.source_project,
ref: merge_request.source_branch,
sha: merge_request.diff_head_sha
)
merge_request.update!(head_pipeline: pipeline)
merge_request.update_head_pipeline
end
it 'has a head pipeline' do