Merge branch '260388-changed-metrics-above-new-metrics' into 'master'

Sort metrics in metrics report MR widget

See merge request gitlab-org/gitlab!55217

doc/ci/metrics_reports.md

@@ -30,10 +30,10 @@ Metrics for a branch are read from the latest metrics report artifact (default f
 
 For an MR, the values of these metrics from the feature branch are compared to the values from the target branch. Then they are displayed in the MR widget in this order:
 
-- Metrics that have been added by the MR. Marked with a **New** badge.
 - Existing metrics with changed values.
-- Existing metrics with unchanged values.
+- Metrics that have been added by the MR. Marked with a **New** badge.
 - Metrics that have been removed by the MR. Marked with a **Removed** badge.
+- Existing metrics with unchanged values.
 
 ## How to set it up
 

ee/app/assets/javascripts/vue_shared/metrics_reports/store/getters.js

@@ -13,7 +13,8 @@ export const summaryStatus = (state) => {
 };
 
 export const metrics = (state) => [
+  ...state.changedMetrics,
   ...state.newMetrics.map((metric) => ({ ...metric, isNew: true })),
-  ...state.existingMetrics,
   ...state.removedMetrics.map((metric) => ({ ...metric, wasRemoved: true })),
+  ...state.unchangedMetrics,
 ];

ee/app/assets/javascripts/vue_shared/metrics_reports/store/mutations.js

@@ -12,25 +12,24 @@ export default {
     state.hasError = false;
     state.isLoading = false;
 
+    state.changedMetrics =
+      response.existing_metrics?.filter((metric) => metric?.previous_value) || [];
     state.newMetrics = response.new_metrics || [];
-    state.existingMetrics = [
-      ...(response.existing_metrics?.filter((metric) => metric?.previous_value) || []),
-      ...(response.existing_metrics?.filter((metric) => !metric?.previous_value) || []),
-    ];
     state.removedMetrics = response.removed_metrics || [];
+    state.unchangedMetrics =
+      response.existing_metrics?.filter((metric) => !metric?.previous_value) || [];
 
     state.numberOfChanges =
-      state.existingMetrics.filter((metric) => metric?.previous_value !== undefined).length +
-      state.newMetrics.length +
-      state.removedMetrics.length;
+      state.changedMetrics.length + state.newMetrics.length + state.removedMetrics.length;
   },
   [types.RECEIVE_METRICS_ERROR](state) {
     state.isLoading = false;
     state.hasError = true;
 
+    state.changedMetrics = [];
     state.newMetrics = [];
-    state.existingMetrics = [];
     state.removedMetrics = [];
+    state.unchangedMetrics = [];
 
     state.numberOfChanges = 0;
   },

ee/app/assets/javascripts/vue_shared/metrics_reports/store/state.js

@@ -12,9 +12,10 @@ export default () => ({
    *    previous_value: {String}
    * }
    */
+  changedMetrics: [],
   newMetrics: [],
-  existingMetrics: [],
   removedMetrics: [],
+  unchangedMetrics: [],
 
   numberOfChanges: 0,
 });

ee/changelogs/unreleased/260388-changed-metrics-above-new-metrics.yml

+---
+title: Sort metrics report MR widget - changed, new, removed, unchanged
+merge_request: 55217
+author:
+type: changed

ee/spec/frontend/vue_shared/metrics_reports/grouped_metrics_reports_app_spec.js

@@ -73,7 +73,7 @@ describe('Grouped metrics reports app', () => {
     describe('when user expands to view metrics', () => {
       beforeEach(() => {
         mockStore.state.numberOfChanges = 0;
-        mockStore.state.existingMetrics = [
+        mockStore.state.unchangedMetrics = [
           {
             name: 'name',
             value: 'value',
@@ -110,7 +110,7 @@ describe('Grouped metrics reports app', () => {
     describe('with no changes', () => {
       beforeEach(() => {
         mockStore.state.numberOfChanges = 0;
-        mockStore.state.existingMetrics = [
+        mockStore.state.unchangedMetrics = [
           {
             name: 'name',
             value: 'value',
@@ -129,7 +129,7 @@ describe('Grouped metrics reports app', () => {
     describe('with one change', () => {
       beforeEach(() => {
         mockStore.state.numberOfChanges = 1;
-        mockStore.state.existingMetrics = [
+        mockStore.state.changedMetrics = [
           {
             name: 'name',
             value: 'value',
@@ -149,7 +149,7 @@ describe('Grouped metrics reports app', () => {
     describe('with multiple changes', () => {
       beforeEach(() => {
         mockStore.state.numberOfChanges = 2;
-        mockStore.state.existingMetrics = [
+        mockStore.state.changedMetrics = [
           {
             name: 'name',
             value: 'value',
@@ -212,7 +212,7 @@ describe('Grouped metrics reports app', () => {
     describe('when has metrics', () => {
       beforeEach(() => {
         mockStore.state.numberOfChanges = 1;
-        mockStore.state.existingMetrics = [
+        mockStore.state.changedMetrics = [
           {
             name: 'name',
             value: 'value',

ee/spec/frontend/vue_shared/metrics_reports/store/getters_spec.js

@@ -56,10 +56,10 @@ describe('metrics reports getters', () => {
       });
     });
 
-    describe('when state has existing metrics', () => {
-      it('returns array with existing metrics', () => {
+    describe('when state has changed metrics', () => {
+      it('returns array with changed metrics', () => {
         const mockState = state();
-        mockState.existingMetrics = [{ name: 'name', value: 'value', previous_value: 'prev' }];
+        mockState.changedMetrics = [{ name: 'name', value: 'value', previous_value: 'prev' }];
         const metricsResult = metrics(mockState);
 
         expect(metricsResult.length).toEqual(1);
@@ -69,6 +69,18 @@ describe('metrics reports getters', () => {
       });
     });
 
+    describe('when state has unchanged metrics', () => {
+      it('returns array with unchanged metrics', () => {
+        const mockState = state();
+        mockState.unchangedMetrics = [{ name: 'name', value: 'value' }];
+        const metricsResult = metrics(mockState);
+
+        expect(metricsResult.length).toEqual(1);
+        expect(metricsResult[0].name).toEqual('name');
+        expect(metricsResult[0].value).toEqual('value');
+      });
+    });
+
     describe('when state has removed metrics', () => {
       it('returns array with removed metrics', () => {
         const mockState = state();
@@ -82,23 +94,31 @@ describe('metrics reports getters', () => {
       });
     });
 
-    describe('when state has new, existing, and removed metrics', () => {
-      it('returns array with new, existing, and removed metrics combined', () => {
+    describe('when state has new, changed, unchanged, and removed metrics', () => {
+      it('returns array with changed, new, removed, and unchanged metrics combined', () => {
         const mockState = state();
-        mockState.newMetrics = [{ name: 'name1', value: 'value1' }];
-        mockState.existingMetrics = [{ name: 'name2', value: 'value2', previous_value: 'prev' }];
+        mockState.changedMetrics = [{ name: 'name1', value: 'value1', previous_value: 'prev' }];
+        mockState.newMetrics = [{ name: 'name2', value: 'value2' }];
         mockState.removedMetrics = [{ name: 'name3', value: 'value3' }];
+        mockState.unchangedMetrics = [{ name: 'name4', value: 'value4' }];
         const metricsResult = metrics(mockState);
 
-        expect(metricsResult.length).toEqual(3);
+        expect(metricsResult.length).toEqual(4);
+
         expect(metricsResult[0].name).toEqual('name1');
         expect(metricsResult[0].value).toEqual('value1');
-        expect(metricsResult[0].isNew).toEqual(true);
+        expect(metricsResult[0].previous_value).toEqual('prev');
+
         expect(metricsResult[1].name).toEqual('name2');
         expect(metricsResult[1].value).toEqual('value2');
+        expect(metricsResult[1].isNew).toEqual(true);
+
         expect(metricsResult[2].name).toEqual('name3');
         expect(metricsResult[2].value).toEqual('value3');
         expect(metricsResult[2].wasRemoved).toEqual(true);
+
+        expect(metricsResult[3].name).toEqual('name4');
+        expect(metricsResult[3].value).toEqual('value4');
       });
     });
 

ee/spec/frontend/vue_shared/metrics_reports/store/mutations_spec.js

@@ -37,7 +37,7 @@ describe('metrics reports mutations', () => {
       };
       mutations[types.RECEIVE_METRICS_SUCCESS](mockState, data);
 
-      expect(mockState.existingMetrics).toEqual(data.existing_metrics);
+      expect(mockState.unchangedMetrics).toEqual(data.existing_metrics);
       expect(mockState.numberOfChanges).toEqual(0);
       expect(mockState.isLoading).toEqual(false);
     });
@@ -70,38 +70,10 @@ describe('metrics reports mutations', () => {
       };
       mutations[types.RECEIVE_METRICS_SUCCESS](mockState, data);
 
-      expect(mockState.existingMetrics).toEqual(data.existing_metrics);
+      expect(mockState.changedMetrics).toEqual(data.existing_metrics);
       expect(mockState.numberOfChanges).toEqual(1);
       expect(mockState.isLoading).toEqual(false);
     });
-
-    it('should put changed metrics before unchanged metrics', () => {
-      const unchangedMetrics = [
-        {
-          name: 'an unchanged metric',
-          value: 'one',
-        },
-        {
-          name: 'another unchanged metric metric',
-          value: 'four',
-        },
-      ];
-      const changedMetric = {
-        name: 'changed metric',
-        value: 'two',
-        previous_value: 'three',
-      };
-      const data = {
-        existing_metrics: [unchangedMetrics[0], changedMetric, unchangedMetrics[1]],
-      };
-      mutations[types.RECEIVE_METRICS_SUCCESS](mockState, data);
-
-      expect(mockState.existingMetrics).toEqual([
-        changedMetric,
-        unchangedMetrics[0],
-        unchangedMetrics[1],
-      ]);
-    });
   });
 
   describe('RECEIVE_METRICS_ERROR', () => {