From 0325e2af02302f7f3d823b55bfc1c0121c1ba67c Mon Sep 17 00:00:00 2001 From: Rares Mardare Date: Wed, 17 Jul 2024 12:16:27 +0300 Subject: [PATCH] Label error handling tweaks (#4687) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What this PR does These changes make it so that we no longer show an error notification when you first add an empty pair `key/value` due to the undefined key. Other than that, the handling remains the same, with some refactorings as we don't need the try catch clauses. Would be nice in the future to find a way not to duplicate the usage of those 2 functions that fetch the key and the value and share them somehow 🤔 --- .../ColumnsSelectorWrapper/ColumnsModal.tsx | 3 +- .../ColumnsSelectorWrapper.tsx | 3 +- .../IntegrationLabelsForm.tsx | 28 ++++--------------- .../src/containers/Labels/Labels.tsx | 26 ++++------------- .../RouteLabelsDisplay/RouteLabelsDisplay.tsx | 21 ++++---------- .../src/models/alertgroup/alertgroup.ts | 4 +-- grafana-plugin/src/models/label/label.ts | 3 ++ grafana-plugin/src/utils/consts.ts | 1 + 8 files changed, 27 insertions(+), 62 deletions(-) diff --git a/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsModal.tsx b/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsModal.tsx index 7ca7d07d..dead50d3 100644 --- a/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsModal.tsx +++ b/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsModal.tsx @@ -26,6 +26,7 @@ import { ApiSchemas } from 'network/oncall-api/api.types'; import { components } from 'network/oncall-api/autogenerated-api.types'; import { useStore } from 'state/useStore'; import { UserActions } from 'utils/authorization/authorization'; +import { PROCESSING_REQUEST_ERROR } from 'utils/consts'; import { WrapWithGlobalNotification } from 'utils/decorators'; import { useDebouncedCallback, useIsLoading } from 'utils/hooks'; import { pluralize } from 'utils/utils'; @@ -142,7 +143,7 @@ export const ColumnsModal: React.FC = observer( variant="primary" onClick={WrapWithGlobalNotification(onAddNewColumns, { success: 'New column has been added to the list.', - failure: 'There was an error processing your request. Please try again.', + failure: PROCESSING_REQUEST_ERROR, })} > {isLoading ? : 'Add'} diff --git a/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsSelectorWrapper.tsx b/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsSelectorWrapper.tsx index fe7ccc9d..eb346615 100644 --- a/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsSelectorWrapper.tsx +++ b/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsSelectorWrapper.tsx @@ -13,6 +13,7 @@ import { ActionKey } from 'models/loader/action-keys'; import { ApiSchemas } from 'network/oncall-api/api.types'; import { useStore } from 'state/useStore'; import { UserActions } from 'utils/authorization/authorization'; +import { PROCESSING_REQUEST_ERROR } from 'utils/consts'; import { WrapAutoLoadingState, WrapWithGlobalNotification } from 'utils/decorators'; import { useIsLoading } from 'utils/hooks'; @@ -81,7 +82,7 @@ export const ColumnsSelectorWrapper: React.FC = obs onClick={WrapAutoLoadingState( WrapWithGlobalNotification(onColumnRemovalClick, { success: 'Column has been removed from the list.', - failure: 'There was an error processing your request. Please try again', + failure: PROCESSING_REQUEST_ERROR, }), ActionKey.REMOVE_COLUMN_FROM_ALERT_GROUP )} diff --git a/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx b/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx index bee1cb62..e5dd7b26 100644 --- a/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx +++ b/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx @@ -284,32 +284,16 @@ const CustomLabels = (props: CustomLabelsProps) => { }; const onLoadKeys = async (search?: string) => { - let result = undefined; - - try { - result = await labelsStore.loadKeys(search); - } catch (error) { - openErrorNotification('There was an error processing your request. Please try again'); - } - - const groups = splitToGroups(result); - - return groups; + const result = await labelsStore.loadKeys(search); + return splitToGroups(result); }; const onLoadValuesForKey = async (key: string, search?: string) => { - let result = undefined; - - try { - const { values } = await labelsStore.loadValuesForKey(key, search); - result = values; - } catch (error) { - openErrorNotification('There was an error processing your request. Please try again'); + if (!key) { + return []; } - - const groups = splitToGroups(result); - - return groups; + const { values } = await labelsStore.loadValuesForKey(key, search); + return splitToGroups(values); }; return ( diff --git a/grafana-plugin/src/containers/Labels/Labels.tsx b/grafana-plugin/src/containers/Labels/Labels.tsx index d7a76ff8..02625f46 100644 --- a/grafana-plugin/src/containers/Labels/Labels.tsx +++ b/grafana-plugin/src/containers/Labels/Labels.tsx @@ -49,30 +49,16 @@ const _Labels = observer( ); const onLoadKeys = async (search?: string) => { - let result = undefined; - try { - result = await labelsStore.loadKeys(search); - } catch (error) { - openErrorNotification('There was an error processing your request. Please try again'); - } - - const groups = splitToGroups(result); - - return groups; + const result = await labelsStore.loadKeys(search); + return splitToGroups(result); }; const onLoadValuesForKey = async (key: string, search?: string) => { - let result = undefined; - try { - const { values } = await labelsStore.loadValuesForKey(key, search); - result = values; - } catch (error) { - openErrorNotification('There was an error processing your request. Please try again'); + if (!key) { + return []; } - - const groups = splitToGroups(result); - - return groups; + const { values } = await labelsStore.loadValuesForKey(key, search); + return splitToGroups(values); }; const isValid = () => { diff --git a/grafana-plugin/src/containers/RouteLabelsDisplay/RouteLabelsDisplay.tsx b/grafana-plugin/src/containers/RouteLabelsDisplay/RouteLabelsDisplay.tsx index 17743e8e..cd3c64fc 100644 --- a/grafana-plugin/src/containers/RouteLabelsDisplay/RouteLabelsDisplay.tsx +++ b/grafana-plugin/src/containers/RouteLabelsDisplay/RouteLabelsDisplay.tsx @@ -32,28 +32,17 @@ export const RouteLabelsDisplay: React.FC = ({ labels, }; const onLoadKeys = async (search?: string) => { - let result = undefined; - - try { - result = await labelsStore.loadKeys(search); - } catch (error) { - openErrorNotification('There was an error processing your request. Please try again'); - } - + const result = await labelsStore.loadKeys(search); return splitToGroups(result); }; const onLoadValuesForKey = async (key: string, search?: string) => { - let result = undefined; - - try { - const { values } = await labelsStore.loadValuesForKey(key, search); - result = values; - } catch (error) { - openErrorNotification('There was an error processing your request. Please try again'); + if (!key) { + return []; } - return splitToGroups(result); + const { values } = await labelsStore.loadValuesForKey(key, search); + return splitToGroups(values); }; return ( diff --git a/grafana-plugin/src/models/alertgroup/alertgroup.ts b/grafana-plugin/src/models/alertgroup/alertgroup.ts index aefde6e6..ca4c11c5 100644 --- a/grafana-plugin/src/models/alertgroup/alertgroup.ts +++ b/grafana-plugin/src/models/alertgroup/alertgroup.ts @@ -9,7 +9,7 @@ import { onCallApi } from 'network/oncall-api/http-client'; import { RootStore } from 'state/rootStore'; import { SelectOption } from 'state/types'; import { LocationHelper } from 'utils/LocationHelper'; -import { GENERIC_ERROR, PAGE } from 'utils/consts'; +import { GENERIC_ERROR, PAGE, PROCESSING_REQUEST_ERROR } from 'utils/consts'; import { AutoLoadingState, WithGlobalNotification } from 'utils/decorators'; import { AlertGroupHelper } from './alertgroup.helpers'; @@ -136,7 +136,7 @@ export class AlertGroupStore { @AutoLoadingState(ActionKey.REMOVE_COLUMN_FROM_ALERT_GROUP) @WithGlobalNotification({ success: 'Column has been removed from the list.', - failure: 'There was an error processing your request. Please try again', + failure: PROCESSING_REQUEST_ERROR, }) async removeTableColumn( columnToBeRemoved: AlertGroupColumn, diff --git a/grafana-plugin/src/models/label/label.ts b/grafana-plugin/src/models/label/label.ts index 0ca89ce3..7559632a 100644 --- a/grafana-plugin/src/models/label/label.ts +++ b/grafana-plugin/src/models/label/label.ts @@ -6,6 +6,7 @@ import { ApiSchemas } from 'network/oncall-api/api.types'; import { components } from 'network/oncall-api/autogenerated-api.types'; import { onCallApi } from 'network/oncall-api/http-client'; import { RootStore } from 'state/rootStore'; +import { PROCESSING_REQUEST_ERROR } from 'utils/consts'; import { WithGlobalNotification } from 'utils/decorators'; export class LabelStore extends BaseStore { @@ -17,6 +18,7 @@ export class LabelStore extends BaseStore { this.path = '/labels/'; } + @WithGlobalNotification({ failure: PROCESSING_REQUEST_ERROR }) @action.bound public async loadKeys(search = ''): Promise> { const { data } = await onCallApi().GET('/labels/keys/', undefined); @@ -26,6 +28,7 @@ export class LabelStore extends BaseStore { return filtered; } + @WithGlobalNotification({ failure: PROCESSING_REQUEST_ERROR }) @action.bound async loadValuesForKey( key: ApiSchemas['LabelKey']['id'], diff --git a/grafana-plugin/src/utils/consts.ts b/grafana-plugin/src/utils/consts.ts index a091378e..99b6a19a 100644 --- a/grafana-plugin/src/utils/consts.ts +++ b/grafana-plugin/src/utils/consts.ts @@ -97,5 +97,6 @@ export enum OnCallAGStatus { } export const GENERIC_ERROR = 'An error has occurred. Please try again'; +export const PROCESSING_REQUEST_ERROR = 'There was an error processing your request. Please try again'; export const INTEGRATION_SERVICENOW = 'servicenow';