Merge branch '36129-sync-status-popovers' into 'master'

Re-design Geo Replication Popover

Closes #36129

See merge request gitlab-org/gitlab!27033

app/assets/javascripts/vue_shared/components/stacked_progress_bar.vue

@@ -14,15 +14,18 @@ export default {
     },
     successLabel: {
       type: String,
-      required: true,
+      required: false,
+      default: 'successful',
     },
     failureLabel: {
       type: String,
-      required: true,
+      required: false,
+      default: 'failed',
     },
     neutralLabel: {
       type: String,
-      required: true,
+      required: false,
+      default: 'neutral',
     },
     successCount: {
       type: Number,
@@ -36,6 +39,11 @@ export default {
       type: Number,
       required: true,
     },
+    hideTooltips: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   computed: {
     neutralCount() {
@@ -87,7 +95,7 @@ export default {
       return `width: ${percent}%;`;
     },
     getTooltip(label, count) {
-      return `${label}: ${count}`;
+      return this.hideTooltips ? '' : `${label}: ${count}`;
     },
   },
 };

app/assets/stylesheets/utilities.scss

@@ -62,6 +62,14 @@
 .gl-h-32 { height: px-to-rem($grid-size * 4); }
 .gl-h-64 { height: px-to-rem($grid-size * 8); }
 
+.gl-shim-h-2 {
+  height: px-to-rem(4px);
+}
+
+.gl-shim-w-5 {
+  width: px-to-rem(16px);
+}
+
 .gl-text-purple { color: $purple; }
 .gl-text-gray-800 { color: $gray-800; }
 .gl-bg-purple-light { background-color: $purple-light; }

ee/app/assets/javascripts/geo_nodes/components/geo_node_detail_item.vue

 <script>
-import { s__ } from '~/locale';
 import popover from '~/vue_shared/directives/popover';
-import tooltip from '~/vue_shared/directives/tooltip';
-import Icon from '~/vue_shared/components/icon.vue';
-import StackedProgressBar from '~/vue_shared/components/stacked_progress_bar.vue';
 
 import { VALUE_TYPE, CUSTOM_TYPE } from '../constants';
 
 import GeoNodeSyncSettings from './geo_node_sync_settings.vue';
 import GeoNodeEventStatus from './geo_node_event_status.vue';
+import GeoNodeSyncProgress from './geo_node_sync_progress.vue';
 
 export default {
   components: {
-    Icon,
-    StackedProgressBar,
     GeoNodeSyncSettings,
     GeoNodeEventStatus,
+    GeoNodeSyncProgress,
   },
   directives: {
     popover,
-    tooltip,
   },
   props: {
     itemTitle: {
@@ -45,21 +40,6 @@ export default {
       required: false,
       default: '',
     },
-    successLabel: {
-      type: String,
-      required: false,
-      default: s__('GeoNodes|Synced'),
-    },
-    failureLabel: {
-      type: String,
-      required: false,
-      default: s__('GeoNodes|Failed'),
-    },
-    neutralLabel: {
-      type: String,
-      required: false,
-      default: s__('GeoNodes|Out of sync'),
-    },
     itemValueType: {
       type: String,
       required: true,
@@ -79,6 +59,11 @@ export default {
       required: false,
       default: false,
     },
+    detailsPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
   computed: {
     hasHelpInfo() {
@@ -108,25 +93,16 @@ export default {
     <div v-if="isValueTypePlain" :class="cssClass" class="mt-1 node-detail-value">
       {{ itemValue }}
     </div>
-    <div v-if="isValueTypeGraph" :class="{ 'd-flex': itemValueStale }" class="mt-1">
-      <stacked-progress-bar
-        :css-class="itemValueStale ? 'flex-fill' : ''"
-        :success-label="successLabel"
-        :failure-label="failureLabel"
-        :neutral-label="neutralLabel"
-        :success-count="itemValue.successCount"
-        :failure-count="itemValue.failureCount"
-        :total-count="itemValue.totalCount"
-      />
-      <icon
-        v-show="itemValueStale"
-        v-tooltip
-        :title="itemValueStaleTooltip"
-        name="time-out"
-        class="ml-2 text-warning-500"
-        data-container="body"
-      />
-    </div>
+    <geo-node-sync-progress
+      v-if="isValueTypeGraph"
+      :item-title="itemTitle"
+      :item-value="itemValue"
+      :item-value-stale="itemValueStale"
+      :item-value-stale-tooltip="itemValueStaleTooltip"
+      :details-path="detailsPath"
+      :class="{ 'd-flex': itemValueStale }"
+      class="mt-1"
+    />
     <template v-if="isValueTypeCustom">
       <geo-node-sync-settings
         v-if="isCustomTypeSync"

ee/app/assets/javascripts/geo_nodes/components/geo_node_details.vue

@@ -79,7 +79,7 @@ export default {
       :node-removal-allowed="nodeRemovalAllowed"
       :version-mismatch="hasVersionMismatch"
     />
-    <node-details-section-sync v-if="!node.primary" :node-details="nodeDetails" />
+    <node-details-section-sync v-if="!node.primary" :node="node" :node-details="nodeDetails" />
     <node-details-section-verification
       v-if="nodeDetails.repositoryVerificationEnabled"
       :node-details="nodeDetails"

ee/app/assets/javascripts/geo_nodes/components/geo_node_sync_progress.vue

+<script>
+import { GlPopover, GlSprintf, GlLink } from '@gitlab/ui';
+import StackedProgressBar from '~/vue_shared/components/stacked_progress_bar.vue';
+import Icon from '~/vue_shared/components/icon.vue';
+import tooltip from '~/vue_shared/directives/tooltip';
+
+export default {
+  name: 'GeoNodeSyncProgress',
+  directives: {
+    tooltip,
+  },
+  components: {
+    GlPopover,
+    GlSprintf,
+    GlLink,
+    StackedProgressBar,
+    Icon,
+  },
+  props: {
+    itemTitle: {
+      type: String,
+      required: true,
+    },
+    itemValue: {
+      type: Object,
+      required: true,
+      validator: value =>
+        ['totalCount', 'successCount', 'failureCount'].every(key => typeof value[key] === 'number'),
+    },
+    itemValueStale: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    itemValueStaleTooltip: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    detailsPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+  computed: {
+    queuedCount() {
+      return this.itemValue.totalCount - this.itemValue.successCount - this.itemValue.failureCount;
+    },
+  },
+};
+</script>
+
+<template>
+  <div>
+    <stacked-progress-bar
+      :id="`syncProgress-${itemTitle}`"
+      tabindex="0"
+      :css-class="itemValueStale ? 'flex-fill' : ''"
+      :hide-tooltips="true"
+      :success-count="itemValue.successCount"
+      :failure-count="itemValue.failureCount"
+      :total-count="itemValue.totalCount"
+    />
+    <gl-popover
+      :target="`syncProgress-${itemTitle}`"
+      placement="right"
+      triggers="hover focus"
+      :css-classes="['w-100']"
+    >
+      <template #title>
+        <gl-sprintf :message="__('Number of %{itemTitle}')">
+          <template #itemTitle>
+            {{ itemTitle }}
+          </template>
+        </gl-sprintf>
+      </template>
+      <section>
+        <div class="d-flex align-items-center my-1">
+          <div class="mr-2 bg-transparent gl-shim-w-5 gl-shim-h-2"></div>
+          <span class="flex-grow-1 mr-3">{{ __('Total') }}</span>
+          <span class="font-weight-bold">{{ itemValue.totalCount.toLocaleString() }}</span>
+        </div>
+        <div class="d-flex align-items-center my-2">
+          <div class="mr-2 bg-success-500 gl-shim-w-5 gl-shim-h-2"></div>
+          <span class="flex-grow-1 mr-3">{{ __('Synced') }}</span>
+          <span class="font-weight-bold">{{ itemValue.successCount.toLocaleString() }}</span>
+        </div>
+        <div class="d-flex align-items-center my-2">
+          <div class="mr-2 bg-secondary-200 gl-shim-w-5 gl-shim-h-2"></div>
+          <span class="flex-grow-1 mr-3">{{ __('Queued') }}</span>
+          <span class="font-weight-bold">{{ queuedCount.toLocaleString() }}</span>
+        </div>
+        <div class="d-flex align-items-center my-2">
+          <div class="mr-2 bg-danger-500 gl-shim-w-5 gl-shim-h-2"></div>
+          <span class="flex-grow-1 mr-3">{{ __('Failed') }}</span>
+          <span class="font-weight-bold">{{ itemValue.failureCount.toLocaleString() }}</span>
+        </div>
+        <div v-if="detailsPath" class="mt-3">
+          <gl-link class="gl-font-size-small" :href="detailsPath" target="_blank">{{
+            __('More information')
+          }}</gl-link>
+        </div>
+      </section>
+    </gl-popover>
+    <icon
+      v-if="itemValueStale"
+      v-tooltip
+      :title="itemValueStaleTooltip"
+      :aria-label="itemValueStaleTooltip"
+      name="time-out"
+      class="ml-2 text-warning-500"
+      data-container="body"
+    />
+  </div>
+</template>

ee/app/assets/javascripts/geo_nodes/components/node_detail_sections/node_details_section_sync.vue

@@ -16,6 +16,10 @@ export default {
   },
   mixins: [DetailsSectionMixin],
   props: {
+    node: {
+      type: Object,
+      required: true,
+    },
     nodeDetails: {
       type: Object,
       required: true,
@@ -35,6 +39,7 @@ export default {
           itemTitle: s__('GeoNodes|Repositories'),
           itemValue: this.nodeDetails.repositories,
           itemValueType: VALUE_TYPE.GRAPH,
+          detailsPath: `${this.node.url}admin/geo/projects`,
         },
         {
           itemTitle: s__('GeoNodes|Wikis'),
@@ -50,6 +55,7 @@ export default {
           itemTitle: s__('GeoNodes|Attachments'),
           itemValue: this.nodeDetails.attachments,
           itemValueType: VALUE_TYPE.GRAPH,
+          detailsPath: `${this.node.url}admin/geo/uploads`,
         },
         {
           itemTitle: s__('GeoNodes|Job artifacts'),
@@ -66,6 +72,7 @@ export default {
           itemValue: this.nodeDetails.designRepositories,
           itemValueType: VALUE_TYPE.GRAPH,
           featureDisabled: !gon.features.enableGeoDesignSync,
+          detailsPath: `${this.node.url}admin/geo/designs`,
         },
         {
           itemTitle: s__('GeoNodes|Data replication lag'),
@@ -150,6 +157,7 @@ export default {
         :custom-type="nodeDetailItem.customType"
         :event-type-log-status="nodeDetailItem.eventTypeLogStatus"
         :feature-disabled="nodeDetailItem.featureDisabled"
+        :details-path="nodeDetailItem.detailsPath"
       />
     </div>
   </div>

ee/changelogs/unreleased/36129-sync-status-popovers.yml

+---
+title: Create more intuitive Popover information for Geo Node Sync Status
+merge_request: 27033
+author:
+type: changed

ee/spec/frontend/geo_nodes/components/__snapshots__/geo_node_sync_progress_spec.js.snap

+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`GeoNodeSyncProgress template GlPopover renders each row of popover correctly 1`] = `
+<div
+  class="d-flex align-items-center my-1"
+>
+  <div
+    class="mr-2 bg-transparent gl-shim-w-5 gl-shim-h-2"
+  />
+   
+  <span
+    class="flex-grow-1 mr-3"
+  >
+    Total
+  </span>
+   
+  <span
+    class="font-weight-bold"
+  >
+    10
+  </span>
+</div>
+`;
+
+exports[`GeoNodeSyncProgress template GlPopover renders each row of popover correctly 2`] = `
+<div
+  class="mr-2 bg-transparent gl-shim-w-5 gl-shim-h-2"
+/>
+`;
+
+exports[`GeoNodeSyncProgress template GlPopover renders each row of popover correctly 3`] = `
+<div
+  class="d-flex align-items-center my-2"
+>
+  <div
+    class="mr-2 bg-success-500 gl-shim-w-5 gl-shim-h-2"
+  />
+   
+  <span
+    class="flex-grow-1 mr-3"
+  >
+    Synced
+  </span>
+   
+  <span
+    class="font-weight-bold"
+  >
+    5
+  </span>
+</div>
+`;
+
+exports[`GeoNodeSyncProgress template GlPopover renders each row of popover correctly 4`] = `
+<div
+  class="mr-2 bg-success-500 gl-shim-w-5 gl-shim-h-2"
+/>
+`;
+
+exports[`GeoNodeSyncProgress template GlPopover renders each row of popover correctly 5`] = `
+<div
+  class="d-flex align-items-center my-2"
+>
+  <div
+    class="mr-2 bg-secondary-200 gl-shim-w-5 gl-shim-h-2"
+  />
+   
+  <span
+    class="flex-grow-1 mr-3"
+  >
+    Queued
+  </span>
+   
+  <span
+    class="font-weight-bold"
+  >
+    2
+  </span>
+</div>
+`;
+
+exports[`GeoNodeSyncProgress template GlPopover renders each row of popover correctly 6`] = `
+<div
+  class="mr-2 bg-secondary-200 gl-shim-w-5 gl-shim-h-2"
+/>
+`;
+
+exports[`GeoNodeSyncProgress template GlPopover renders each row of popover correctly 7`] = `
+<div
+  class="d-flex align-items-center my-2"
+>
+  <div
+    class="mr-2 bg-danger-500 gl-shim-w-5 gl-shim-h-2"
+  />
+   
+  <span
+    class="flex-grow-1 mr-3"
+  >
+    Failed
+  </span>
+   
+  <span
+    class="font-weight-bold"
+  >
+    3
+  </span>
+</div>
+`;
+
+exports[`GeoNodeSyncProgress template GlPopover renders each row of popover correctly 8`] = `
+<div
+  class="mr-2 bg-danger-500 gl-shim-w-5 gl-shim-h-2"
+/>
+`;

ee/spec/frontend/geo_nodes/components/geo_node_detail_item_spec.js

 import { shallowMount } from '@vue/test-utils';
 
-import StackedProgressBar from '~/vue_shared/components/stacked_progress_bar.vue';
 import GeoNodeDetailItemComponent from 'ee/geo_nodes/components/geo_node_detail_item.vue';
 import GeoNodeSyncSettings from 'ee/geo_nodes/components/geo_node_sync_settings.vue';
 import GeoNodeEventStatus from 'ee/geo_nodes/components/geo_node_event_status.vue';
+import GeoNodeSyncProgress from 'ee/geo_nodes/components/geo_node_sync_progress.vue';
 
 import { VALUE_TYPE, CUSTOM_TYPE } from 'ee/geo_nodes/constants';
 import { rawMockNodeDetails } from '../mock_data';
@@ -40,18 +40,30 @@ describe('GeoNodeDetailItemComponent', () => {
     });
 
     it('renders container elements correctly', () => {
-      expect(wrapper.vm.$el.classList.contains('node-detail-item')).toBeTruthy();
-      expect(wrapper.vm.$el.querySelectorAll('.node-detail-title').length).not.toBe(0);
-      expect(wrapper.vm.$el.querySelector('.node-detail-title').innerText.trim()).toBe(
-        'GitLab version',
-      );
+      expect(wrapper.find('.node-detail-item').exists()).toBeTruthy();
+      expect(wrapper.findAll('.node-detail-title')).not.toHaveLength(0);
+      expect(
+        wrapper
+          .find('.node-detail-title')
+          .text()
+          .trim(),
+      ).toBe('GitLab version');
     });
 
-    it('renders plain item value', () => {
-      expect(wrapper.vm.$el.querySelectorAll('.node-detail-value').length).not.toBe(0);
-      expect(wrapper.vm.$el.querySelector('.node-detail-value').innerText.trim()).toBe(
-        '10.4.0-pre',
-      );
+    describe('when plain text value', () => {
+      it('renders plain item value', () => {
+        expect(wrapper.findAll('.node-detail-value')).not.toHaveLength(0);
+        expect(
+          wrapper
+            .find('.node-detail-value')
+            .text()
+            .trim(),
+        ).toBe('10.4.0-pre');
+      });
+
+      it('does not render graph item', () => {
+        expect(wrapper.find(GeoNodeSyncProgress).exists()).toBeFalsy();
+      });
     });
 
     describe('when graph item value', () => {
@@ -62,29 +74,8 @@ describe('GeoNodeDetailItemComponent', () => {
         });
       });
 
-      it('renders progress bar', () => {
-        expect(wrapper.find(StackedProgressBar).exists()).toBeTruthy();
-      });
-
-      describe('with itemValueStale prop', () => {
-        const itemValueStaleTooltip = 'Data is out of date from 8 hours ago';
-
-        beforeEach(() => {
-          createComponent({
-            itemValueType: VALUE_TYPE.GRAPH,
-            itemValue: { successCount: 5, failureCount: 3, totalCount: 10 },
-            itemValueStale: true,
-            itemValueStaleTooltip,
-          });
-        });
-
-        it('renders stale information icon', () => {
-          const iconEl = wrapper.find('.text-warning-500');
-
-          expect(iconEl).not.toBeNull();
-          expect(iconEl.attributes('data-original-title')).toBe(itemValueStaleTooltip);
-          expect(iconEl.attributes('name')).toBe('time-out');
-        });
+      it('renders graph item', () => {
+        expect(wrapper.find(GeoNodeSyncProgress).exists()).toBeTruthy();
       });
     });
 
@@ -110,6 +101,10 @@ describe('GeoNodeDetailItemComponent', () => {
       it('renders sync settings item value', () => {
         expect(wrapper.find(GeoNodeSyncSettings).exists()).toBeTruthy();
       });
+
+      it('does not render graph item', () => {
+        expect(wrapper.find(GeoNodeSyncProgress).exists()).toBeFalsy();
+      });
     });
 
     describe('when custom type is event', () => {
@@ -127,6 +122,10 @@ describe('GeoNodeDetailItemComponent', () => {
       it('renders event status item value', () => {
         expect(wrapper.find(GeoNodeEventStatus).exists()).toBeTruthy();
       });
+
+      it('does not render graph item', () => {
+        expect(wrapper.find(GeoNodeSyncProgress).exists()).toBeFalsy();
+      });
     });
 
     describe('when featureDisabled is true', () => {

ee/spec/frontend/geo_nodes/components/geo_node_sync_progress_spec.js

+import { shallowMount } from '@vue/test-utils';
+import { GlPopover } from '@gitlab/ui';
+import StackedProgressBar from '~/vue_shared/components/stacked_progress_bar.vue';
+import Icon from '~/vue_shared/components/icon.vue';
+
+import GeoNodeSyncProgress from 'ee/geo_nodes/components/geo_node_sync_progress.vue';
+
+describe('GeoNodeSyncProgress', () => {
+  let wrapper;
+
+  const MOCK_ITEM_VALUE = { successCount: 5, failureCount: 3, totalCount: 10 };
+  MOCK_ITEM_VALUE.queuedCount =
+    MOCK_ITEM_VALUE.totalCount - MOCK_ITEM_VALUE.successCount - MOCK_ITEM_VALUE.failureCount;
+
+  const defaultProps = {
+    itemTitle: 'GitLab version',
+    itemValue: MOCK_ITEM_VALUE,
+  };
+
+  const createComponent = (props = {}) => {
+    wrapper = shallowMount(GeoNodeSyncProgress, {
+      propsData: {
+        ...defaultProps,
+        ...props,
+      },
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+
+  const findStackedProgressBar = () => wrapper.find(StackedProgressBar);
+  const findGlPopover = () => wrapper.find(GlPopover);
+  const findCounts = () => findGlPopover().findAll('div');
+  const findStaleIcon = () => wrapper.find(Icon);
+
+  describe('template', () => {
+    beforeEach(() => {
+      createComponent();
+    });
+
+    it('renders StackedProgressbar always', () => {
+      expect(findStackedProgressBar().exists()).toBeTruthy();
+    });
+
+    describe('GlPopover', () => {
+      it('renders always', () => {
+        expect(findGlPopover().exists()).toBeTruthy();
+      });
+
+      it('renders each row of popover correctly', () => {
+        findCounts().wrappers.forEach(row => {
+          expect(row.element).toMatchSnapshot();
+        });
+      });
+    });
+
+    describe('when itemValueStale is false', () => {
+      it('does not render StaleIcon always', () => {
+        expect(findStaleIcon().exists()).toBeFalsy();
+      });
+    });
+
+    describe('when itemValueStale is true', () => {
+      beforeEach(() => {
+        createComponent({
+          itemValueStale: true,
+          itemValueStaleTooltip: 'Stale',
+        });
+      });
+
+      it('does render StaleIcon always', () => {
+        expect(findStaleIcon().exists()).toBeTruthy();
+      });
+    });
+  });
+
+  describe('computed', () => {
+    beforeEach(() => {
+      createComponent();
+    });
+
+    describe('queuedCount', () => {
+      it('returns total - success - failure', () => {
+        expect(wrapper.vm.queuedCount).toEqual(MOCK_ITEM_VALUE.queuedCount);
+      });
+    });
+  });
+});

locale/gitlab.pot

@@ -9314,9 +9314,6 @@ msgstr ""
 msgid "GeoNodes|Not checksummed"
 msgstr ""
 
-msgid "GeoNodes|Out of sync"
-msgstr ""
-
 msgid "GeoNodes|Pausing replication stops the sync process. Are you sure?"
 msgstr ""
 
@@ -9365,9 +9362,6 @@ msgstr ""
 msgid "GeoNodes|Sync settings"
 msgstr ""
 
-msgid "GeoNodes|Synced"
-msgstr ""
-
 msgid "GeoNodes|Unused slots"
 msgstr ""
 
@@ -13772,6 +13766,9 @@ msgstr ""
 msgid "Now you can access the merge request navigation tabs at the top, where they’re easier to find."
 msgstr ""
 
+msgid "Number of %{itemTitle}"
+msgstr ""
+
 msgid "Number of Elasticsearch replicas"
 msgstr ""
 
@@ -16441,6 +16438,9 @@ msgstr ""
 msgid "Query is valid"
 msgstr ""
 
+msgid "Queued"
+msgstr ""
+
 msgid "Quick actions can be used in the issues description and comment boxes."
 msgstr ""
 
@@ -19568,6 +19568,9 @@ msgstr ""
 msgid "Sync information"
 msgstr ""
 
+msgid "Synced"
+msgstr ""
+
 msgid "System"
 msgstr ""
 

spec/javascripts/vue_shared/components/stacked_progress_bar_spec.js

@@ -68,8 +68,20 @@ describe('StackedProgressBarComponent', () => {
     });
 
     describe('getTooltip', () => {
-      it('returns label string based on label and count provided', () => {
-        expect(vm.getTooltip('Synced', 10)).toBe('Synced: 10');
+      describe('when hideTooltips is false', () => {
+        it('returns label string based on label and count provided', () => {
+          expect(vm.getTooltip('Synced', 10)).toBe('Synced: 10');
+        });
+      });
+
+      describe('when hideTooltips is true', () => {
+        beforeEach(() => {
+          vm = createComponent({ hideTooltips: true });
+        });
+
+        it('returns an empty string', () => {
+          expect(vm.getTooltip('Synced', 10)).toBe('');
+        });
       });
     });
   });