From a76ed4599d65d781fc46fed0545465ab7f53d7e8 Mon Sep 17 00:00:00 2001
From: Martin Wortschack <mwortschack@gitlab.com>
Date: Mon, 16 Sep 2019 12:56:36 +0000
Subject: [PATCH] Add metric column component
- This ensures that we render numeric values only
The component renders a dash for non-numeric and null values
---
.../components/metric_column.vue | 49 +++++++++
.../components/mr_table_row.vue | 22 ++--
.../__snapshots__/metric_column_spec.js.snap | 24 +++++
.../__snapshots__/mr_table_row_spec.js.snap | 50 ++-------
.../components/metric_column_spec.js | 101 ++++++++++++++++++
.../components/mr_table_row_spec.js | 7 +-
6 files changed, 196 insertions(+), 57 deletions(-)
create mode 100644 ee/app/assets/javascripts/analytics/productivity_analytics/components/metric_column.vue
create mode 100644 ee/spec/frontend/analytics/productivity_analytics/components/__snapshots__/metric_column_spec.js.snap
create mode 100644 ee/spec/frontend/analytics/productivity_analytics/components/metric_column_spec.js
diff --git a/ee/app/assets/javascripts/analytics/productivity_analytics/components/metric_column.vue b/ee/app/assets/javascripts/analytics/productivity_analytics/components/metric_column.vue
new file mode 100644
index 0000000000000..a4da2dc7d8147
--- /dev/null
+++ b/ee/app/assets/javascripts/analytics/productivity_analytics/components/metric_column.vue
@@ -0,0 +1,49 @@
+<script>
+import { n__ } from '~/locale';
+
+export default {
+ props: {
+ type: {
+ type: String,
+ required: true,
+ },
+ /**
+ * With default null we will render a "-" in the last column as opposed to a numeric value
+ */
+ value: {
+ type: Number,
+ required: false,
+ default: null,
+ },
+ label: {
+ type: String,
+ required: false,
+ default: '',
+ },
+ },
+ computed: {
+ isNumericValue() {
+ return this.value !== null && !Number.isNaN(Number(this.value));
+ },
+ unit() {
+ return this.type === 'days_to_merge'
+ ? n__('day', 'days', this.value)
+ : n__('Time|hr', 'Time|hrs', this.value);
+ },
+ },
+};
+</script>
+<template>
+ <div class="metric-col">
+ <span class="time">
+ <template v-if="isNumericValue">
+ {{ value }}
+ <span> {{ unit }} </span>
+ </template>
+ <template v-else>
+ –
+ </template>
+ </span>
+ <span v-if="label" class="d-flex d-md-none text-secondary metric-label">{{ label }}</span>
+ </div>
+</template>
diff --git a/ee/app/assets/javascripts/analytics/productivity_analytics/components/mr_table_row.vue b/ee/app/assets/javascripts/analytics/productivity_analytics/components/mr_table_row.vue
index b5e85e363fba1..8dcbbd50b8168 100644
--- a/ee/app/assets/javascripts/analytics/productivity_analytics/components/mr_table_row.vue
+++ b/ee/app/assets/javascripts/analytics/productivity_analytics/components/mr_table_row.vue
@@ -1,11 +1,13 @@
<script>
import { sprintf, __, n__ } from '~/locale';
import { GlLink, GlAvatar } from '@gitlab/ui';
+import MetricColumn from './metric_column.vue';
export default {
components: {
GlLink,
GlAvatar,
+ MetricColumn,
},
props: {
mergeRequest: {
@@ -68,20 +70,12 @@ export default {
</div>
</div>
<div class="table-section section-50 d-flex flex-row align-items-start qa-mr-metrics">
- <div class="metric-col">
- <span class="time">
- {{ mergeRequest.time_to_merge }}
- <span> {{ n__('Time|hr', 'Time|hrs', mergeRequest.time_to_merge) }} </span>
- </span>
- <span class="d-flex d-md-none text-secondary metric-label">{{ __('Time to merge') }}</span>
- </div>
- <div class="metric-col">
- <span class="time">
- {{ selectedMetric }}
- <span> {{ metricTimeUnit }} </span>
- </span>
- <span class="d-flex d-md-none text-secondary metric-label">{{ metricLabel }}</span>
- </div>
+ <metric-column
+ type="time_to_merge"
+ :value="mergeRequest.time_to_merge"
+ :label="__('Time to merge')"
+ />
+ <metric-column :type="metricType" :value="selectedMetric" :label="metricLabel" />
</div>
</div>
</template>
diff --git a/ee/spec/frontend/analytics/productivity_analytics/components/__snapshots__/metric_column_spec.js.snap b/ee/spec/frontend/analytics/productivity_analytics/components/__snapshots__/metric_column_spec.js.snap
new file mode 100644
index 0000000000000..275864512c6e1
--- /dev/null
+++ b/ee/spec/frontend/analytics/productivity_analytics/components/__snapshots__/metric_column_spec.js.snap
@@ -0,0 +1,24 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`MetricColumn component on creation matches the snapshot 1`] = `
+<div
+ class="metric-col"
+>
+ <span
+ class="time"
+ >
+
+ 10
+
+ <span>
+ hrs
+ </span>
+ </span>
+
+ <span
+ class="d-flex d-md-none text-secondary metric-label"
+ >
+ Time from first comment to last commit
+ </span>
+</div>
+`;
diff --git a/ee/spec/frontend/analytics/productivity_analytics/components/__snapshots__/mr_table_row_spec.js.snap b/ee/spec/frontend/analytics/productivity_analytics/components/__snapshots__/mr_table_row_spec.js.snap
index bf774b3bd815e..f1f73f73f4a58 100644
--- a/ee/spec/frontend/analytics/productivity_analytics/components/__snapshots__/mr_table_row_spec.js.snap
+++ b/ee/spec/frontend/analytics/productivity_analytics/components/__snapshots__/mr_table_row_spec.js.snap
@@ -58,47 +58,17 @@ exports[`MergeRequestTableRow component on creation matches the snapshot 1`] = `
<div
class="table-section section-50 d-flex flex-row align-items-start qa-mr-metrics"
>
- <div
- class="metric-col"
- >
- <span
- class="time"
- >
-
- 0
-
- <span>
- hrs
- </span>
- </span>
-
- <span
- class="d-flex d-md-none text-secondary metric-label"
- >
- Time to merge
- </span>
- </div>
+ <metriccolumn-stub
+ label="Time to merge"
+ type="time_to_merge"
+ value="0"
+ />
- <div
- class="metric-col"
- >
- <span
- class="time"
- >
-
- 0
-
- <span>
- hrs
- </span>
- </span>
-
- <span
- class="d-flex d-md-none text-secondary metric-label"
- >
- Time from first comment to last commit
- </span>
- </div>
+ <metriccolumn-stub
+ label="Time from first comment to last commit"
+ type="time_to_last_commit"
+ value="0"
+ />
</div>
</div>
`;
diff --git a/ee/spec/frontend/analytics/productivity_analytics/components/metric_column_spec.js b/ee/spec/frontend/analytics/productivity_analytics/components/metric_column_spec.js
new file mode 100644
index 0000000000000..e4a962891e432
--- /dev/null
+++ b/ee/spec/frontend/analytics/productivity_analytics/components/metric_column_spec.js
@@ -0,0 +1,101 @@
+import { createLocalVue, shallowMount } from '@vue/test-utils';
+import MetricColumn from 'ee/analytics/productivity_analytics/components/metric_column.vue';
+
+describe('MetricColumn component', () => {
+ let wrapper;
+
+ const defaultProps = {
+ type: 'time_to_last_commit',
+ value: 10,
+ label: 'Time from first comment to last commit',
+ };
+
+ const factory = (props = defaultProps) => {
+ const localVue = createLocalVue();
+
+ wrapper = shallowMount(localVue.extend(MetricColumn), {
+ localVue,
+ sync: false,
+ propsData: { ...props },
+ });
+ };
+
+ afterEach(() => {
+ wrapper.destroy();
+ });
+
+ const findTimeContainer = () => wrapper.find('.time');
+
+ describe('on creation', () => {
+ beforeEach(() => {
+ factory();
+ });
+
+ it('matches the snapshot', () => {
+ expect(wrapper.element).toMatchSnapshot();
+ });
+ });
+
+ describe('template', () => {
+ describe('when metric has value', () => {
+ beforeEach(() => {
+ factory();
+ });
+
+ it('renders the value and unit', () => {
+ const unit = 'hrs';
+ expect(findTimeContainer().text()).toContain(`${defaultProps.value}`);
+ expect(findTimeContainer().text()).toContain(unit);
+ });
+ });
+
+ describe('when metric has no value', () => {
+ beforeEach(() => {
+ factory({
+ ...defaultProps,
+ value: null,
+ });
+ });
+
+ it('renders a dash', () => {
+ expect(findTimeContainer().text()).toContain('–');
+ });
+ });
+ });
+
+ describe('computed', () => {
+ describe('isNumericValue', () => {
+ it('returns true when the value is neither null nor undefined', () => {
+ factory();
+ expect(wrapper.vm.isNumericValue).toBe(true);
+ });
+
+ it('returns false when the value is null', () => {
+ factory({
+ ...defaultProps,
+ value: null,
+ });
+ expect(wrapper.vm.isNumericValue).toBe(false);
+ });
+ });
+
+ describe('unit', () => {
+ it('returns "days" for the "days_to_merge" metric', () => {
+ factory({
+ ...defaultProps,
+ type: 'days_to_merge',
+ });
+
+ expect(wrapper.vm.unit).toBe('days');
+ });
+
+ it('returns "hrs" for the any other metric', () => {
+ factory({
+ ...defaultProps,
+ });
+
+ expect(wrapper.vm.unit).toBe('hrs');
+ });
+ });
+ });
+});
diff --git a/ee/spec/frontend/analytics/productivity_analytics/components/mr_table_row_spec.js b/ee/spec/frontend/analytics/productivity_analytics/components/mr_table_row_spec.js
index b8dd73911dac7..0daff7aaa5444 100644
--- a/ee/spec/frontend/analytics/productivity_analytics/components/mr_table_row_spec.js
+++ b/ee/spec/frontend/analytics/productivity_analytics/components/mr_table_row_spec.js
@@ -1,5 +1,6 @@
import { createLocalVue, shallowMount } from '@vue/test-utils';
import MergeRequestTableRow from 'ee/analytics/productivity_analytics/components/mr_table_row.vue';
+import MetricColumn from 'ee/analytics/productivity_analytics/components/metric_column.vue';
import { GlAvatar } from '@gitlab/ui';
import { mockMergeRequests } from '../mock_data';
@@ -24,7 +25,7 @@ describe('MergeRequestTableRow component', () => {
const findMrDetails = () => wrapper.find('.qa-mr-details');
const findMrMetrics = () => wrapper.find('.qa-mr-metrics');
- const findMetricColumns = () => findMrMetrics().findAll('.metric-col');
+ const findMetricColumns = () => findMrMetrics().findAll(MetricColumn);
afterEach(() => {
wrapper.destroy();
@@ -68,8 +69,8 @@ describe('MergeRequestTableRow component', () => {
expect(
findMetricColumns()
.at(0)
- .text(),
- ).toContain(defaultProps.mergeRequest.time_to_merge);
+ .props('value'),
+ ).toBe(defaultProps.mergeRequest.time_to_merge);
});
});
});
--
GitLab