Minor review feedback

Update gitlab.pot file

Generate values in the time_summary fixture

Adds the prepare_cycle_time_data method
to generate cycle time values

ee/app/assets/javascripts/analytics/cycle_analytics/components/base.vue

@@ -18,6 +18,7 @@ import TimeMetricsCard from './time_metrics_card.vue';
 import StageTableNav from './stage_table_nav.vue';
 import CustomStageForm from './custom_stage_form.vue';
 import PathNavigation from './path_navigation.vue';
+import MetricCard from '../../shared/components/metric_card.vue';
 
 export default {
   name: 'CycleAnalytics',
@@ -35,6 +36,7 @@ export default {
     CustomStageForm,
     StageTableNav,
     PathNavigation,
+    MetricCard,
   },
   mixins: [glFeatureFlagsMixin(), UrlSyncMixin],
   props: {
@@ -267,12 +269,15 @@ export default {
         "
       />
       <div v-else-if="!errorCode">
-        <div class="js-recent-activity mt-3 gl-display-flex">
+        <div class="js-recent-activity gl-mt-3 gl-display-flex">
           <div class="gl-flex-fill-1 gl-pr-2">
             <time-metrics-card
+              #default="{ metrics, loading }"
               :group-path="currentGroupPath"
               :additional-params="cycleAnalyticsRequestParams"
-            />
+            >
+              <metric-card :title="__('Time')" :metrics="metrics" :is-loading="loading" />
+            </time-metrics-card>
           </div>
           <div class="gl-flex-fill-1 gl-pl-2">
             <recent-activity-card

ee/app/assets/javascripts/analytics/cycle_analytics/components/time_metrics_card.vue

@@ -19,7 +19,7 @@ export default {
     additionalParams: {
       type: Object,
       required: false,
-      default: null,
+      default: () => ({}),
     },
   },
   data() {
@@ -40,13 +40,10 @@ export default {
     fetchData() {
       removeFlash();
       this.loading = true;
-      return Api.cycleAnalyticsTimeSummaryData(
-        this.groupPath,
-        this.additionalParams ? this.additionalParams : {},
-      )
+      return Api.cycleAnalyticsTimeSummaryData(this.groupPath, this.additionalParams)
         .then(({ data }) => {
-          this.data = data.map(({ title: label, value }) => ({
-            value: value || '-',
+          this.data = data.map(({ title: label, ...rest }) => ({
+            ...rest,
             label,
             key: slugify(label),
           }));

ee/app/assets/javascripts/analytics/shared/components/metric_card.vue

@@ -26,12 +26,11 @@ export default {
     valueText(metric) {
       const { value = null, unit = null } = metric;
       if (!value) return '-';
-      return unit ? `${value} ${unit}` : value;
+      return unit && value ? `${value} ${unit}` : value;
     },
   },
 };
 </script>
-
 <template>
   <gl-card>
     <template #header>

ee/spec/features/analytics/cycle_analytics/cycle_analytics_spec.rb

@@ -289,14 +289,14 @@ describe 'Group Value Stream Analytics', :js do
     it 'displays the lead time' do
       lead_time = page.all(card_metric_selector).first
 
-      expect(lead_time).to have_content(n_('Lead time', 'Lead time', 10))
-      expect(lead_time).to have_content('10')
+      expect(lead_time).to have_content(_('Lead Time'))
+      expect(lead_time).to have_content('-')
     end
 
     it 'displays the cycle time' do
       cycle_time = page.all(card_metric_selector)[1]
 
-      expect(cycle_time).to have_content(n_('Cycle time', 'Cycle time', 0))
+      expect(cycle_time).to have_content(_('Cycle Time'))
       expect(cycle_time).to have_content('-')
     end
   end

ee/spec/frontend/analytics/cycle_analytics/components/__snapshots__/recent_activity_card_spec.js.snap

@@ -27,7 +27,7 @@ exports[`RecentActivityCard matches the snapshot 1`] = `
         <h3
           class="my-2"
         >
-          3
+          4
         </h3>
          
         <p

ee/spec/frontend/analytics/cycle_analytics/components/__snapshots__/time_metrics_card_spec.js.snap

-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`TimeMetricsCard matches the snapshot 1`] = `
-<div
-  class="card"
->
-  <!---->
-  <div
-    class="card-header"
-  >
-    <strong>
-      Time
-    </strong>
-  </div>
-  <div
-    class="card-body"
-  >
-    <!---->
-    <!---->
-     
-    <div
-      class="d-flex"
-    >
-      <div
-        class="js-metric-card-item flex-grow text-center"
-      >
-        <h3
-          class="my-2"
-        >
-          -
-        </h3>
-         
-        <p
-          class="text-secondary gl-font-sm mb-2"
-        >
-          Lead Time
-        </p>
-      </div>
-      <div
-        class="js-metric-card-item flex-grow text-center"
-      >
-        <h3
-          class="my-2"
-        >
-          -
-        </h3>
-         
-        <p
-          class="text-secondary gl-font-sm mb-2"
-        >
-          Cycle Time
-        </p>
-      </div>
-    </div>
-     
-  </div>
-  <!---->
-  <!---->
-</div>
-`;

ee/spec/frontend/analytics/cycle_analytics/components/base_spec.js

@@ -8,6 +8,7 @@ import MockAdapter from 'axios-mock-adapter';
 import GroupsDropdownFilter from 'ee/analytics/shared/components/groups_dropdown_filter.vue';
 import ProjectsDropdownFilter from 'ee/analytics/shared/components/projects_dropdown_filter.vue';
 import RecentActivityCard from 'ee/analytics/cycle_analytics/components/recent_activity_card.vue';
+import TimeMetricsCard from 'ee/analytics/cycle_analytics/components/time_metrics_card.vue';
 import PathNavigation from 'ee/analytics/cycle_analytics/components/path_navigation.vue';
 import StageTable from 'ee/analytics/cycle_analytics/components/stage_table.vue';
 import 'bootstrap';
@@ -123,6 +124,10 @@ describe('Cycle Analytics component', () => {
     expect(wrapper.find(RecentActivityCard).exists()).toBe(flag);
   };
 
+  const displaysTimeMetricsCard = flag => {
+    expect(wrapper.find(TimeMetricsCard).exists()).toBe(flag);
+  };
+
   const displaysStageTable = flag => {
     expect(wrapper.find(StageTable).exists()).toBe(flag);
   };
@@ -179,10 +184,14 @@ describe('Cycle Analytics component', () => {
         displaysDateRangePicker(false);
       });
 
-      it('does not display the recent activity table', () => {
+      it('does not display the recent activity card', () => {
         displaysRecentActivityCard(false);
       });
 
+      it('does not display the time metrics card', () => {
+        displaysTimeMetricsCard(false);
+      });
+
       it('does not display the stage table', () => {
         displaysStageTable(false);
       });
@@ -243,10 +252,14 @@ describe('Cycle Analytics component', () => {
           displaysDateRangePicker(true);
         });
 
-        it('displays the recent activity table', () => {
+        it('displays the recent activity card', () => {
           displaysRecentActivityCard(true);
         });
 
+        it('displays the time metrics card', () => {
+          displaysTimeMetricsCard(true);
+        });
+
         it('displays the stage table', () => {
           displaysStageTable(true);
         });
@@ -352,10 +365,14 @@ describe('Cycle Analytics component', () => {
           displaysDateRangePicker(false);
         });
 
-        it('does not display the recent activity table', () => {
+        it('does not display the recent activity card', () => {
           displaysRecentActivityCard(false);
         });
 
+        it('does not display the time metrics card', () => {
+          displaysTimeMetricsCard(false);
+        });
+
         it('does not display the stage table', () => {
           displaysStageTable(false);
         });

ee/spec/frontend/analytics/cycle_analytics/components/time_metrics_card_spec.js

-import { mount } from '@vue/test-utils';
+import { shallowMount } from '@vue/test-utils';
 import createFlash from '~/flash';
 import TimeMetricsCard from 'ee/analytics/cycle_analytics/components/time_metrics_card.vue';
 import { group, timeMetricsData } from '../mock_data';
@@ -10,17 +10,18 @@ describe('TimeMetricsCard', () => {
   const { full_path: groupPath } = group;
   let wrapper;
 
-  const createComponent = (additionalParams = {}) => {
-    return mount(TimeMetricsCard, {
+  const createComponent = ({ additionalParams = {} } = {}) => {
+    return shallowMount(TimeMetricsCard, {
       propsData: {
         groupPath,
         additionalParams,
       },
+      slots: {
+        default: 'mockMetricCard',
+      },
     });
   };
 
-  const findMetricCards = () => wrapper.findAll('.js-metric-card-item');
-
   beforeEach(() => {
     jest.spyOn(Api, 'cycleAnalyticsTimeSummaryData').mockResolvedValue({ data: timeMetricsData });
 
@@ -31,27 +32,10 @@ describe('TimeMetricsCard', () => {
     wrapper.destroy();
     wrapper = null;
   });
-
-  it('matches the snapshot', () => {
-    expect(wrapper.element).toMatchSnapshot();
-  });
-
   it('fetches the time metrics data', () => {
     expect(Api.cycleAnalyticsTimeSummaryData).toHaveBeenCalledWith(groupPath, {});
   });
 
-  describe('with data', () => {
-    it.each`
-      metric          | value  | index
-      ${'Lead Time'}  | ${'-'} | ${0}
-      ${'Cycle Time'} | ${'-'} | ${1}
-    `('Renders the $metric', ({ metric, value, index }) => {
-      const card = findMetricCards().at(index);
-      expect(card.html()).toContain(metric);
-      expect(card.html()).toContain(value);
-    });
-  });
-
   describe('with a failing request', () => {
     beforeEach(() => {
       jest.spyOn(Api, 'cycleAnalyticsTimeSummaryData').mockRejectedValue();
@@ -69,9 +53,11 @@ describe('TimeMetricsCard', () => {
   describe('with additional params', () => {
     beforeEach(() => {
       wrapper = createComponent({
-        'project_ids[]': [1],
-        created_after: '2020-01-01',
-        created_before: '2020-02-01',
+        additionalParams: {
+          'project_ids[]': [1],
+          created_after: '2020-01-01',
+          created_before: '2020-02-01',
+        },
       });
     });
 

ee/spec/frontend/fixtures/analytics.rb

@@ -7,9 +7,7 @@ describe 'Analytics (JavaScript fixtures)', :sidekiq_inline do
   let(:group) { create(:group)}
   let(:project) { create(:project, :repository, namespace: group) }
   let(:user) { create(:user, :admin) }
-  # let(:issue) { create(:issue, project: project, created_at: 4.days.ago) }
   let(:milestone) { create(:milestone, project: project) }
-  # let(:mr) { create_merge_request_closing_issue(user, project, issue, commit_message: "References #{issue.to_reference}") }
 
   let(:issue) { create(:issue, project: project, created_at: 4.days.ago) }
   let(:issue_1) { create(:issue, project: project, created_at: 5.days.ago) }
@@ -189,11 +187,22 @@ describe 'Analytics (JavaScript fixtures)', :sidekiq_inline do
 
     let(:params) { { created_after: 3.months.ago, created_before: Time.now, group_id: group.full_path } }
 
+    def prepare_cycle_time_data
+      issue.update!(created_at: 5.days.ago)
+      issue.metrics.update!(first_mentioned_in_commit_at: 4.days.ago)
+      issue.update!(closed_at: 3.days.ago)
+
+      issue_1.update!(created_at: 8.days.ago)
+      issue_1.metrics.update!(first_mentioned_in_commit_at: 6.days.ago)
+      issue_1.update!(closed_at: 1.day.ago)
+    end
+
     before do
       stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
       stub_licensed_features(cycle_analytics_for_groups: true)
 
       prepare_cycle_analytics_data
+      prepare_cycle_time_data
 
       sign_in(user)
     end
@@ -203,24 +212,9 @@ describe 'Analytics (JavaScript fixtures)', :sidekiq_inline do
 
       expect(response).to be_successful
     end
-  end
-
-  describe Analytics::CycleAnalytics::TimeSummaryController, type: :controller do
-    render_views
-
-    let(:params) { { created_after: 3.months.ago, created_before: Time.now, group_id: group.full_path } }
-
-    before do
-      stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
-      stub_licensed_features(cycle_analytics_for_groups: true)
-
-      prepare_cycle_analytics_data
-
-      sign_in(user)
-    end
 
     it 'analytics/value_stream_analytics/time_summary.json' do
-      get(:show, params: params, format: :json)
+      get(:time_summary, params: params, format: :json)
 
       expect(response).to be_successful
     end

locale/gitlab.pot

@@ -21680,6 +21680,9 @@ msgstr ""
 msgid "There was an error while fetching value stream analytics recent activity data."
 msgstr ""
 
+msgid "There was an error while fetching value stream analytics time summary data."
+msgstr ""
+
 msgid "There was an error with the reCAPTCHA. Please solve the reCAPTCHA again."
 msgstr ""