From 25dd144201e8b0eaa3532a9a169c46bfa9b46c94 Mon Sep 17 00:00:00 2001
From: Elwyn Benson <ebenson@gitlab.com>
Date: Mon, 31 Jul 2023 07:57:35 +0000
Subject: [PATCH] Add error state to panels
Each panel now deals with its own error handling and displays an
internal error state instead of emitting up to parent component.
---
.../customizable_dashboard/constants.js | 11 +++-
.../customizable_dashboard.vue | 13 -----
.../customizable_dashboard/panels_base.vue | 55 ++++++++++++++++---
.../customizable_dashboard_spec.js | 30 ----------
.../panels_base_spec.js | 31 +++++++++--
locale/gitlab.pot | 14 +++--
6 files changed, 94 insertions(+), 60 deletions(-)
diff --git a/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/constants.js b/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/constants.js
index bfd365de30de..263395339e8c 100644
--- a/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/constants.js
+++ b/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/constants.js
@@ -1,4 +1,5 @@
import { s__ } from '~/locale';
+import { helpPagePath } from '~/helpers/help_page_helper';
export const GRIDSTACK_MARGIN = 10;
export const GRIDSTACK_CSS_HANDLE = '.grid-stack-item-handle';
@@ -6,7 +7,15 @@ export const GRIDSTACK_CELL_HEIGHT = '120px';
export const GRIDSTACK_MIN_ROW = 1;
export const I18N_PANEL_EMPTY_STATE_MESSAGE = s__(
- 'Analytics|No results match your query or filter',
+ 'Analytics|No results match your query or filter.',
+);
+export const I18N_PANEL_ERROR_STATE_MESSAGE = s__('Analytics|Something went wrong.');
+export const I18N_PANEL_ERROR_POPOVER_TITLE = s__('Analytics|Failed to fetch data');
+export const I18N_PANEL_ERROR_POPOVER_MESSAGE = s__(
+ 'Analytics|Something went wrong while connecting to your data source. See %{linkStart}troubleshooting documentation%{linkEnd}.',
+);
+export const PANEL_TROUBLESHOOTING_URL = helpPagePath(
+ '/user/analytics/analytics_dashboards#troubleshooting',
);
export const CURSOR_GRABBING_CLASS = 'gl-cursor-grabbing!';
diff --git a/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/customizable_dashboard.vue b/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/customizable_dashboard.vue
index 93c9f0c222ab..5dfcfa3a74f7 100644
--- a/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/customizable_dashboard.vue
+++ b/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/customizable_dashboard.vue
@@ -5,8 +5,6 @@ import cloneDeep from 'lodash/cloneDeep';
import { GlButton, GlFormInput, GlForm } from '@gitlab/ui';
import { loadCSSFile } from '~/lib/utils/css_utils';
import { slugify } from '~/lib/utils/text_utility';
-import { createAlert } from '~/alert';
-import { s__, sprintf } from '~/locale';
import UrlSync, { HISTORY_REPLACE_UPDATE_METHOD } from '~/vue_shared/components/url_sync.vue';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { createNewVisualizationPanel } from 'ee/analytics/analytics_dashboards/utils';
@@ -250,16 +248,6 @@ export default {
updatedPanel.gridAttributes = this.convertToGridAttributes(item);
}
},
- handlePanelError(panelTitle, error) {
- this.alert = createAlert({
- message: sprintf(
- s__('ProductAnalytics|An error occurred while loading the %{panelTitle} panel.'),
- { panelTitle },
- ),
- error,
- captureError: true,
- });
- },
setDateRangeFilter({ dateRangeOption, startDate, endDate }) {
this.filters = {
...this.filters,
@@ -381,7 +369,6 @@ export default {
:visualization="panel.visualization"
:query-overrides="panel.queryOverrides || undefined"
:filters="filters"
- @error="handlePanelError(panel.title, $event)"
/>
</div>
</div>
diff --git a/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/panels_base.vue b/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/panels_base.vue
index 9caa05d74744..14d2e8d59010 100644
--- a/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/panels_base.vue
+++ b/ee/app/assets/javascripts/vue_shared/components/customizable_dashboard/panels_base.vue
@@ -1,14 +1,25 @@
<script>
-import { GlLoadingIcon } from '@gitlab/ui';
+import * as Sentry from '@sentry/browser';
+import { GlIcon, GlLink, GlLoadingIcon, GlPopover, GlSprintf } from '@gitlab/ui';
import dataSources from 'ee/analytics/analytics_dashboards/data_sources';
import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue';
import { isEmptyPanelData } from 'ee/vue_shared/components/customizable_dashboard/utils';
-import { I18N_PANEL_EMPTY_STATE_MESSAGE } from './constants';
+import {
+ I18N_PANEL_EMPTY_STATE_MESSAGE,
+ I18N_PANEL_ERROR_POPOVER_MESSAGE,
+ I18N_PANEL_ERROR_POPOVER_TITLE,
+ I18N_PANEL_ERROR_STATE_MESSAGE,
+ PANEL_TROUBLESHOOTING_URL,
+} from './constants';
export default {
name: 'AnalyticsDashboardPanel',
components: {
+ GlIcon,
+ GlLink,
GlLoadingIcon,
+ GlPopover,
+ GlSprintf,
TooltipOnTruncate,
LineChart: () =>
import('ee/analytics/analytics_dashboards/components/visualizations/line_chart.vue'),
@@ -52,6 +63,9 @@ export default {
showEmptyState() {
return !this.error && isEmptyPanelData(this.visualization.type, this.data);
},
+ showErrorState() {
+ return Boolean(this.error);
+ },
},
watch: {
visualization: {
@@ -80,19 +94,25 @@ export default {
});
} catch (error) {
this.error = error;
- this.$emit('error', error);
+ Sentry.captureException(error);
} finally {
this.loading = false;
}
},
},
I18N_PANEL_EMPTY_STATE_MESSAGE,
+ I18N_PANEL_ERROR_STATE_MESSAGE,
+ I18N_PANEL_ERROR_POPOVER_TITLE,
+ I18N_PANEL_ERROR_POPOVER_MESSAGE,
+ PANEL_TROUBLESHOOTING_URL,
};
</script>
<template>
<div
+ ref="panelWrapper"
class="grid-stack-item-content gl-shadow-sm gl-rounded-base gl-p-4 gl-display-flex gl-flex-direction-column gl-bg-white"
+ :class="{ 'gl-border-t-2 gl-border-t-solid gl-border-red-500': showErrorState }"
>
<tooltip-on-truncate
v-if="title"
@@ -101,18 +121,20 @@ export default {
boundary="viewport"
class="gl-pb-3 gl-text-truncate"
>
+ <gl-icon v-if="showErrorState" name="warning" class="gl-text-red-500" />
<strong>{{ title }}</strong>
</tooltip-on-truncate>
- <div
- class="gl-overflow-y-auto gl-h-full"
- :class="{ 'gl--flex-center': loading || showEmptyState }"
- >
+ <div class="gl-overflow-y-auto gl-h-full" :class="{ 'gl--flex-center': loading }">
<gl-loading-icon v-if="loading" size="lg" />
- <div v-else-if="showEmptyState" class="gl-text-center gl-text-secondary">
+ <div v-else-if="showEmptyState" class="gl-text-secondary">
{{ $options.I18N_PANEL_EMPTY_STATE_MESSAGE }}
</div>
+ <div v-else-if="showErrorState" class="gl-text-secondary">
+ {{ $options.I18N_PANEL_ERROR_STATE_MESSAGE }}
+ </div>
+
<component
:is="visualization.type"
v-else-if="!error"
@@ -120,5 +142,22 @@ export default {
:options="visualization.options"
/>
</div>
+
+ <gl-popover
+ v-if="showErrorState"
+ triggers="hover focus"
+ :title="$options.I18N_PANEL_ERROR_POPOVER_TITLE"
+ :show-close-button="false"
+ placement="top"
+ :target="$refs.panelWrapper"
+ >
+ <gl-sprintf :message="$options.I18N_PANEL_ERROR_POPOVER_MESSAGE">
+ <template #link="{ content }">
+ <gl-link :href="$options.PANEL_TROUBLESHOOTING_URL" class="gl-font-sm">{{
+ content
+ }}</gl-link>
+ </template>
+ </gl-sprintf>
+ </gl-popover>
</div>
</template>
diff --git a/ee/spec/frontend/vue_shared/components/customizable_dashboard/customizable_dashboard_spec.js b/ee/spec/frontend/vue_shared/components/customizable_dashboard/customizable_dashboard_spec.js
index 1918aee78f0b..905645cdbd11 100644
--- a/ee/spec/frontend/vue_shared/components/customizable_dashboard/customizable_dashboard_spec.js
+++ b/ee/spec/frontend/vue_shared/components/customizable_dashboard/customizable_dashboard_spec.js
@@ -15,7 +15,6 @@ import {
NEW_DASHBOARD_SLUG,
} from 'ee/vue_shared/components/customizable_dashboard/constants';
import { loadCSSFile } from '~/lib/utils/css_utils';
-import { createAlert } from '~/alert';
import waitForPromises from 'helpers/wait_for_promises';
import {
filtersToQueryParams,
@@ -27,13 +26,6 @@ import { NEW_DASHBOARD } from 'ee/analytics/analytics_dashboards/constants';
import { TEST_VISUALIZATION } from 'ee_jest/analytics/analytics_dashboards/mock_data';
import { dashboard, builtinDashboard, mockDateRangeFilterChangePayload } from './mock_data';
-const mockAlertDismiss = jest.fn();
-jest.mock('~/alert', () => ({
- createAlert: jest.fn().mockImplementation(() => ({
- dismiss: mockAlertDismiss,
- })),
-}));
-
jest.mock('gridstack', () => ({
GridStack: {
init: jest.fn(() => {
@@ -187,18 +179,6 @@ describe('CustomizableDashboard', () => {
},
);
- it('calls createAlert when a panel emits an error', () => {
- const error = new Error('foo');
-
- findPanels().at(0).vm.$emit('error', error);
-
- expect(createAlert).toHaveBeenCalledWith({
- message: `An error occurred while loading the ${dashboard.panels[0].title} panel.`,
- captureError: true,
- error,
- });
- });
-
it('does not show the Edit Button for a custom dashboard', () => {
expect(findEditButton().exists()).toBe(false);
});
@@ -216,16 +196,6 @@ describe('CustomizableDashboard', () => {
beforeEach(() => {
createWrapper();
});
-
- it('should dismiss the alert', async () => {
- findPanels().at(0).vm.$emit('error', new Error());
-
- wrapper.destroy();
-
- await nextTick();
-
- expect(mockAlertDismiss).toHaveBeenCalled();
- });
});
describe('when builtin Dashboard is loaded', () => {
diff --git a/ee/spec/frontend/vue_shared/components/customizable_dashboard/panels_base_spec.js b/ee/spec/frontend/vue_shared/components/customizable_dashboard/panels_base_spec.js
index 3f46a4609e49..1f0e43a024a7 100644
--- a/ee/spec/frontend/vue_shared/components/customizable_dashboard/panels_base_spec.js
+++ b/ee/spec/frontend/vue_shared/components/customizable_dashboard/panels_base_spec.js
@@ -1,11 +1,16 @@
-import { GlLoadingIcon } from '@gitlab/ui';
+import * as Sentry from '@sentry/browser';
+import { GlLoadingIcon, GlPopover } from '@gitlab/ui';
import LineChart from 'ee/analytics/analytics_dashboards/components/visualizations/line_chart.vue';
import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
import PanelsBase from 'ee/vue_shared/components/customizable_dashboard/panels_base.vue';
import dataSources from 'ee/analytics/analytics_dashboards/data_sources';
import waitForPromises from 'helpers/wait_for_promises';
import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue';
-import { I18N_PANEL_EMPTY_STATE_MESSAGE } from 'ee/vue_shared/components/customizable_dashboard/constants';
+import {
+ I18N_PANEL_EMPTY_STATE_MESSAGE,
+ I18N_PANEL_ERROR_POPOVER_TITLE,
+ I18N_PANEL_ERROR_STATE_MESSAGE,
+} from 'ee/vue_shared/components/customizable_dashboard/constants';
import { dashboard } from './mock_data';
jest.mock('ee/analytics/analytics_dashboards/data_sources', () => ({
@@ -34,6 +39,7 @@ describe('PanelsBase', () => {
const findVisualization = () => wrapper.findComponent(LineChart);
const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
const findPanelTitle = () => wrapper.findComponent(TooltipOnTruncate);
+ const findPanelErrorPopover = () => wrapper.findComponent(GlPopover);
describe('default behaviour', () => {
beforeEach(() => {
@@ -116,13 +122,20 @@ describe('PanelsBase', () => {
describe('when there was an error while fetching the data', () => {
const mockError = new Error('foo');
+ let captureExceptionSpy;
beforeEach(() => {
jest.spyOn(dataSources.cube_analytics(), 'fetch').mockRejectedValue(mockError);
+ captureExceptionSpy = jest.spyOn(Sentry, 'captureException');
+
createWrapper();
return waitForPromises();
});
+ afterEach(() => {
+ captureExceptionSpy.mockRestore();
+ });
+
it('should not render the loading icon', () => {
expect(findLoadingIcon().exists()).toBe(false);
});
@@ -135,8 +148,18 @@ describe('PanelsBase', () => {
expect(findVisualization().exists()).toBe(false);
});
- it('should emit an error event', () => {
- expect(wrapper.emitted('error')[0]).toStrictEqual([mockError]);
+ it('should render the error state', () => {
+ expect(wrapper.text()).toContain(I18N_PANEL_ERROR_STATE_MESSAGE);
+ });
+
+ it('should render a popover with more information on the error', () => {
+ const popover = findPanelErrorPopover();
+ expect(popover.exists()).toBe(true);
+ expect(popover.props('title')).toBe(I18N_PANEL_ERROR_POPOVER_TITLE);
+ });
+
+ it('should log the error to Sentry', () => {
+ expect(captureExceptionSpy).toHaveBeenCalledWith(mockError);
});
});
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index c1a96a97fe23..880da90ded5d 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -5351,6 +5351,9 @@ msgstr ""
msgid "Analytics|Error while saving visualization."
msgstr ""
+msgid "Analytics|Failed to fetch data"
+msgstr ""
+
msgid "Analytics|Host"
msgstr ""
@@ -5369,7 +5372,7 @@ msgstr ""
msgid "Analytics|No dashboard matches the specified URL path."
msgstr ""
-msgid "Analytics|No results match your query or filter"
+msgid "Analytics|No results match your query or filter."
msgstr ""
msgid "Analytics|OS"
@@ -5414,6 +5417,12 @@ msgstr ""
msgid "Analytics|Single Statistic"
msgstr ""
+msgid "Analytics|Something went wrong while connecting to your data source. See %{linkStart}troubleshooting documentation%{linkEnd}."
+msgstr ""
+
+msgid "Analytics|Something went wrong."
+msgstr ""
+
msgid "Analytics|To create your own dashboards, first configure a project to store your dashboards."
msgstr ""
@@ -35165,9 +35174,6 @@ msgstr ""
msgid "ProductAnalytics|An error occurred while fetching data. Refresh the page to try again."
msgstr ""
-msgid "ProductAnalytics|An error occurred while loading the %{panelTitle} panel."
-msgstr ""
-
msgid "ProductAnalytics|Analyze your product with Product Analytics"
msgstr ""
--
GitLab