Merge branch '329792-geo-beta-handle-null-better' into 'master'

Geo 2.0 Regression - No Data Errors

See merge request gitlab-org/gitlab!61042

ee/app/assets/javascripts/geo_nodes_beta/components/details/primary_node/geo_node_primary_other_info.vue

@@ -10,6 +10,7 @@ export default {
     otherInformation: __('Other information'),
     replicationSlotWAL: s__('Geo|Replication slot WAL'),
     replicationSlots: s__('Geo|Replication slots'),
+    unknown: __('Unknown'),
   },
   components: {
     GlCard,
@@ -23,7 +24,9 @@ export default {
   },
   computed: {
     replicationSlotWAL() {
-      return numberToHumanSize(this.node.replicationSlotsMaxRetainedWalBytes);
+      return this.node.replicationSlotsMaxRetainedWalBytes
+        ? numberToHumanSize(this.node.replicationSlotsMaxRetainedWalBytes)
+        : this.$options.i18n.unknown;
     },
     replicationSlots() {
       return {
@@ -51,10 +54,7 @@ export default {
         :values="replicationSlots.values"
       />
     </div>
-    <div
-      v-if="node.replicationSlotsMaxRetainedWalBytes"
-      class="gl-display-flex gl-flex-direction-column gl-mb-5"
-    >
+    <div class="gl-display-flex gl-flex-direction-column gl-mb-5">
       <span>{{ $options.i18n.replicationSlotWAL }}</span>
       <span class="gl-font-weight-bold gl-mt-2" data-testid="replication-slot-wal">{{
         replicationSlotWAL

ee/app/assets/javascripts/geo_nodes_beta/components/details/secondary_node/geo_node_secondary_other_info.vue

 <script>
-import { GlCard } from '@gitlab/ui';
+import { GlCard, GlSprintf } from '@gitlab/ui';
 import { parseSeconds, stringifyTime } from '~/lib/utils/datetime_utility';
 import { __, s__ } from '~/locale';
 import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue';
@@ -10,6 +10,7 @@ export default {
     otherInfo: __('Other information'),
     dbReplicationLag: s__('Geo|Data replication lag'),
     lastEventId: s__('Geo|Last event ID from primary'),
+    lastEvent: s__('Geo|%{eventId} (%{timeAgo})'),
     lastCursorEventId: s__('Geo|Last event ID processed by cursor'),
     storageConfig: s__('Geo|Storage config'),
     shardsNotMatched: s__('Geo|Does not match the primary storage configuration'),
@@ -18,6 +19,7 @@ export default {
   },
   components: {
     GlCard,
+    GlSprintf,
     TimeAgo,
   },
   props: {
@@ -73,30 +75,37 @@ export default {
     </div>
     <div class="gl-display-flex gl-flex-direction-column gl-mb-5">
       <span>{{ $options.i18n.lastEventId }}</span>
-      <span class="gl-font-weight-bold gl-mt-2"
-        >{{ node.lastEventId || 0 }} (<time-ago
-          data-testid="last-event"
-          :time="lastEventTimestamp"
-        />)</span
-      >
+      <span class="gl-font-weight-bold gl-mt-2" data-testid="last-event">
+        <gl-sprintf v-if="node.lastEventId" :message="$options.i18n.lastEvent">
+          <template #eventId>
+            {{ node.lastEventId }}
+          </template>
+          <template #timeAgo>
+            <time-ago :time="lastEventTimestamp" />
+          </template>
+        </gl-sprintf>
+        <span v-else>{{ $options.i18n.unknown }}</span>
+      </span>
     </div>
     <div class="gl-display-flex gl-flex-direction-column gl-mb-5">
       <span>{{ $options.i18n.lastCursorEventId }}</span>
-      <span class="gl-font-weight-bold gl-mt-2"
-        >{{ node.cursorLastEventId || 0 }} (<time-ago
-          data-testid="last-cursor-event"
-          :time="lastCursorEventTimestamp"
-        />)</span
-      >
+      <span class="gl-font-weight-bold gl-mt-2" data-testid="last-cursor-event">
+        <gl-sprintf v-if="node.cursorLastEventId" :message="$options.i18n.lastEvent">
+          <template #eventId>
+            {{ node.cursorLastEventId }}
+          </template>
+          <template #timeAgo>
+            <time-ago :time="lastCursorEventTimestamp" />
+          </template>
+        </gl-sprintf>
+        <span v-else>{{ $options.i18n.unknown }}</span>
+      </span>
     </div>
     <div class="gl-display-flex gl-flex-direction-column gl-mb-5">
       <span>{{ $options.i18n.storageConfig }}</span>
-      <span
-        :class="{ 'gl-text-red-500': !node.storageShardsMatch }"
-        class="gl-font-weight-bold gl-mt-2"
-        data-testid="storage-shards"
-        >{{ storageShardsStatus }}</span
-      >
+      <span class="gl-font-weight-bold gl-mt-2" data-testid="storage-shards">{{
+        storageShardsStatus
+      }}</span>
     </div>
   </gl-card>
 </template>

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

@@ -17,8 +17,7 @@ export default {
     node: {
       type: Object,
       required: true,
-      validator: (value) =>
-        ['id', 'name', 'geoNodeId', 'url', 'healthStatus'].every((prop) => value[prop]),
+      validator: (value) => ['id', 'name', 'url'].every((prop) => value[prop]),
     },
   },
   data() {

ee/app/assets/javascripts/geo_nodes_beta/components/header/geo_node_header.vue

@@ -38,7 +38,9 @@ export default {
       return this.collapsed ? this.$options.i18n.expand : this.$options.i18n.collapse;
     },
     statusCheckTimestamp() {
-      return this.node.lastSuccessfulStatusCheckTimestamp * 1000;
+      return this.node.lastSuccessfulStatusCheckTimestamp
+        ? this.node.lastSuccessfulStatusCheckTimestamp * 1000
+        : null;
     },
   },
 };
@@ -68,7 +70,11 @@ export default {
         </div>
         <div class="gl-display-flex gl-align-items-center gl-flex-fill-2">
           <geo-node-health-status :status="node.healthStatus" />
-          <geo-node-last-updated class="gl-ml-2" :status-check-timestamp="statusCheckTimestamp" />
+          <geo-node-last-updated
+            v-if="statusCheckTimestamp"
+            class="gl-ml-2"
+            :status-check-timestamp="statusCheckTimestamp"
+          />
         </div>
       </div>
     </div>

ee/app/assets/javascripts/geo_nodes_beta/components/header/geo_node_health_status.vue

@@ -10,12 +10,13 @@ export default {
   props: {
     status: {
       type: String,
-      required: true,
+      required: false,
+      default: null,
     },
   },
   computed: {
     statusUi() {
-      return HEALTH_STATUS_UI[this.status.toLowerCase()];
+      return this.status ? HEALTH_STATUS_UI[this.status.toLowerCase()] : HEALTH_STATUS_UI.unknown;
     },
   },
 };
@@ -24,6 +25,6 @@ export default {
 <template>
   <gl-badge :variant="statusUi.variant">
     <gl-icon :name="statusUi.icon" />
-    <span class="gl-ml-2 gl-font-weight-bold">{{ status }}</span>
+    <span class="gl-ml-2 gl-font-weight-bold">{{ statusUi.text }}</span>
   </gl-badge>
 </template>

ee/app/assets/javascripts/geo_nodes_beta/constants.js

 import { helpPagePath } from '~/helpers/help_page_helper';
-import { __ } from '~/locale';
+import { __, s__ } from '~/locale';
 
 export const GEO_INFO_URL = helpPagePath('administration/geo/index.md');
 
@@ -31,22 +31,27 @@ export const HEALTH_STATUS_UI = {
   healthy: {
     icon: 'status_success',
     variant: 'success',
+    text: s__('Geo|Healthy'),
   },
   unhealthy: {
     icon: 'status_failed',
     variant: 'danger',
+    text: s__('Geo|Unhealthy'),
   },
   disabled: {
     icon: 'status_canceled',
     variant: 'neutral',
+    text: s__('Geo|Disabled'),
   },
   unknown: {
     icon: 'status_notfound',
     variant: 'neutral',
+    text: s__('Geo|Unknown'),
   },
   offline: {
     icon: 'status_canceled',
     variant: 'neutral',
+    text: s__('Geo|Offline'),
   },
 };
 

ee/spec/frontend/geo_nodes_beta/components/details/primary_node/geo_node_primary_other_info_spec.js

@@ -42,6 +42,10 @@ describe('GeoNodePrimaryOtherInfo', () => {
         expect(findGlCard().exists()).toBe(true);
       });
 
+      it('renders the replication slot WAL section', () => {
+        expect(findReplicationSlotWAL().exists()).toBe(true);
+      });
+
       it('renders the replicationSlots progress bar', () => {
         expect(findGeoNodeProgressBar().exists()).toBe(true);
       });
@@ -53,7 +57,6 @@ describe('GeoNodePrimaryOtherInfo', () => {
       });
 
       it('renders the replicationSlotWAL section correctly', () => {
-        expect(findReplicationSlotWAL().exists()).toBe(true);
         expect(findReplicationSlotWAL().text()).toBe(
           numberToHumanSize(MOCK_NODES[0].replicationSlotsMaxRetainedWalBytes),
         );
@@ -65,8 +68,8 @@ describe('GeoNodePrimaryOtherInfo', () => {
         createComponent({ node: MOCK_NODES[1] });
       });
 
-      it('does not render the replicationSlotWAL section', () => {
-        expect(findReplicationSlotWAL().exists()).toBe(false);
+      it('renders Unknown', () => {
+        expect(findReplicationSlotWAL().text()).toBe('Unknown');
       });
     });
   });

ee/spec/frontend/geo_nodes_beta/components/details/secondary_node/geo_node_secondary_other_info_spec.js

+import { GlSprintf } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import GeoNodeSecondaryOtherInfo from 'ee/geo_nodes_beta/components/details/secondary_node/geo_node_secondary_other_info.vue';
 import { MOCK_NODES } from 'ee_jest/geo_nodes_beta/mock_data';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
+import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue';
+
+// Dates come from the backend in seconds, we mimic that here.
+const MOCK_JUST_NOW = new Date().getTime() / 1000;
 
 describe('GeoNodeSecondaryOtherInfo', () => {
   let wrapper;
@@ -17,6 +22,7 @@ describe('GeoNodeSecondaryOtherInfo', () => {
           ...defaultProps,
           ...props,
         },
+        stubs: { GlSprintf, TimeAgo },
       }),
     );
   };
@@ -40,18 +46,12 @@ describe('GeoNodeSecondaryOtherInfo', () => {
         expect(findDbReplicationLag().exists()).toBe(true);
       });
 
-      it('renders the last event correctly', () => {
+      it('renders the last event', () => {
         expect(findLastEvent().exists()).toBe(true);
-        expect(findLastEvent().props('time')).toBe(
-          new Date(MOCK_NODES[1].lastEventTimestamp * 1000).toString(),
-        );
       });
 
-      it('renders the last cursor event correctly', () => {
+      it('renders the last cursor event', () => {
         expect(findLastCursorEvent().exists()).toBe(true);
-        expect(findLastCursorEvent().props('time')).toBe(
-          new Date(MOCK_NODES[1].cursorLastEventTimestamp * 1000).toString(),
-        );
       });
 
       it('renders the storage shards', () => {
@@ -75,17 +75,45 @@ describe('GeoNodeSecondaryOtherInfo', () => {
       });
 
       describe.each`
-        storageShardsMatch | text                                                  | hasErrorClass
-        ${true}            | ${'OK'}                                               | ${false}
-        ${false}           | ${'Does not match the primary storage configuration'} | ${true}
-      `(`storage shards`, ({ storageShardsMatch, text, hasErrorClass }) => {
+        storageShardsMatch | text
+        ${true}            | ${'OK'}
+        ${false}           | ${'Does not match the primary storage configuration'}
+        ${null}            | ${'Unknown'}
+      `(`storage shards`, ({ storageShardsMatch, text }) => {
         beforeEach(() => {
           createComponent({ node: { storageShardsMatch } });
         });
 
         it(`renders correctly when storageShardsMatch is ${storageShardsMatch}`, () => {
           expect(findStorageShards().text()).toBe(text);
-          expect(findStorageShards().classes('gl-text-red-500')).toBe(hasErrorClass);
+        });
+      });
+
+      describe.each`
+        lastEvent                                                | text
+        ${{ lastEventId: null, lastEventTimestamp: null }}       | ${'Unknown'}
+        ${{ lastEventId: 1, lastEventTimestamp: MOCK_JUST_NOW }} | ${'1 (just now)'}
+      `(`last event`, ({ lastEvent, text }) => {
+        beforeEach(() => {
+          createComponent({ node: { ...lastEvent } });
+        });
+
+        it(`renders correctly when lastEventId is ${lastEvent.lastEventId}`, () => {
+          expect(findLastEvent().text().replace(/\s+/g, ' ')).toBe(text);
+        });
+      });
+
+      describe.each`
+        lastCursorEvent                                                      | text
+        ${{ cursorLastEventId: null, cursorLastEventTimestamp: null }}       | ${'Unknown'}
+        ${{ cursorLastEventId: 1, cursorLastEventTimestamp: MOCK_JUST_NOW }} | ${'1 (just now)'}
+      `(`last cursor event`, ({ lastCursorEvent, text }) => {
+        beforeEach(() => {
+          createComponent({ node: { ...lastCursorEvent } });
+        });
+
+        it(`renders correctly when cursorLastEventId is ${lastCursorEvent.cursorLastEventId}`, () => {
+          expect(findLastCursorEvent().text().replace(/\s+/g, ' ')).toBe(text);
         });
       });
     });

ee/spec/frontend/geo_nodes_beta/components/header/geo_node_header_spec.js

@@ -43,10 +43,6 @@ describe('GeoNodeHeader', () => {
         expect(findGeoNodeHealthStatus().exists()).toBe(true);
       });
 
-      it('renders the Geo Node Last Updated', () => {
-        expect(findGeoNodeLastUpdated().exists()).toBe(true);
-      });
-
       it('renders the Geo Node Actions', () => {
         expect(findGeoNodeActions().exists()).toBe(true);
       });
@@ -107,5 +103,29 @@ describe('GeoNodeHeader', () => {
         });
       });
     });
+
+    describe('Last updated', () => {
+      describe('when lastSuccessfulStatusCheckTimestamp exists', () => {
+        beforeEach(() => {
+          createComponent({
+            node: { ...MOCK_NODES[1], lastSuccessfulStatusCheckTimestamp: new Date().getTime() },
+          });
+        });
+
+        it('renders', () => {
+          expect(findGeoNodeLastUpdated().exists()).toBe(true);
+        });
+      });
+
+      describe('when lastSuccessfulStatusCheckTimestamp does not exist', () => {
+        beforeEach(() => {
+          createComponent();
+        });
+
+        it('renders', () => {
+          expect(findGeoNodeLastUpdated().exists()).toBe(false);
+        });
+      });
+    });
   });
 });

ee/spec/frontend/geo_nodes_beta/components/header/geo_node_health_status_spec.js

@@ -29,6 +29,7 @@ describe('GeoNodeHealthStatus', () => {
 
   describe.each`
     status         | uiData
+    ${undefined}   | ${HEALTH_STATUS_UI.unknown}
     ${'Healthy'}   | ${HEALTH_STATUS_UI.healthy}
     ${'Unhealthy'} | ${HEALTH_STATUS_UI.unhealthy}
     ${'Disabled'}  | ${HEALTH_STATUS_UI.disabled}
@@ -48,8 +49,8 @@ describe('GeoNodeHealthStatus', () => {
         expect(findGeoStatusIcon().attributes('name')).toBe(uiData.icon);
       });
 
-      it(`renders status text to ${status}`, () => {
-        expect(findGeoStatusText().text()).toBe(status);
+      it(`renders status text to ${uiData.text}`, () => {
+        expect(findGeoStatusText().text()).toBe(uiData.text);
       });
     });
   });

locale/gitlab.pot

@@ -14538,6 +14538,9 @@ msgstr ""
 msgid "Geo|%{component} synced"
 msgstr ""
 
+msgid "Geo|%{eventId} (%{timeAgo})"
+msgstr ""
+
 msgid "Geo|%{itemTitle} checksum progress"
 msgstr ""
 
@@ -14616,6 +14619,9 @@ msgstr ""
 msgid "Geo|Data type"
 msgstr ""
 
+msgid "Geo|Disabled"
+msgstr ""
+
 msgid "Geo|Discover GitLab Geo"
 msgstr ""
 
@@ -14643,6 +14649,9 @@ msgstr ""
 msgid "Geo|Go to the primary site"
 msgstr ""
 
+msgid "Geo|Healthy"
+msgstr ""
+
 msgid "Geo|If you want to make changes, you must visit the primary site."
 msgstr ""
 
@@ -14706,6 +14715,9 @@ msgstr ""
 msgid "Geo|Number of %{title}"
 msgstr ""
 
+msgid "Geo|Offline"
+msgstr ""
+
 msgid "Geo|Pending synchronization"
 msgstr ""
 
@@ -14868,6 +14880,12 @@ msgstr ""
 msgid "Geo|Undefined"
 msgstr ""
 
+msgid "Geo|Unhealthy"
+msgstr ""
+
+msgid "Geo|Unknown"
+msgstr ""
+
 msgid "Geo|Unknown state"
 msgstr ""