From ce08bcae1b941aa8f39be02ade1ed09d27bf6cb6 Mon Sep 17 00:00:00 2001
From: Marcus Andersson <marcus.andersson@grafana.com>
Date: Tue, 19 Jan 2021 14:43:05 +0100
Subject: [PATCH] Field overrides: skipping overrides for properties no longer
 existing in plugin (#30197)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Safely skipping overrides on missing properties.

* Added test and missing element key.

* added possibility to remove the missing property.

* Minor UI change

* Fix test

* simplify a bit

* Fixed test

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>
Co-authored-by: Torkel Ödegaard <torkel@grafana.com>
---
 .../PanelEditor/OverrideEditor.test.tsx       | 42 ++++++++++++++++++-
 .../components/PanelEditor/OverrideEditor.tsx | 33 ++++++---------
 2 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/public/app/features/dashboard/components/PanelEditor/OverrideEditor.test.tsx b/public/app/features/dashboard/components/PanelEditor/OverrideEditor.test.tsx
index 3df1e34d2a2..a3b51022d29 100644
--- a/public/app/features/dashboard/components/PanelEditor/OverrideEditor.test.tsx
+++ b/public/app/features/dashboard/components/PanelEditor/OverrideEditor.test.tsx
@@ -58,7 +58,47 @@ describe('OverrideEditor', () => {
     expect(selectOptions).toHaveLength(2);
   });
 
-  it('should  not allow override selection that marked as hidden from overrides', () => {
+  it('should be able to handle non registered properties without throwing exceptions', () => {
+    registry.register({
+      id: 'lineStyle',
+      name: 'Line style',
+      path: 'lineStyle',
+      isCustom: true,
+      shouldApply: () => true,
+      process: () => null,
+      override: () => null,
+      editor: () => null,
+      hideFromOverrides: true,
+    });
+
+    render(
+      <OverrideEditor
+        name={'test'}
+        data={[]}
+        override={{
+          matcher: {
+            id: 'byName',
+            options: 'A-series',
+          },
+          properties: [
+            {
+              id: 'lineStyle',
+              value: 'customValue',
+            },
+            {
+              id: 'does.not.exist',
+              value: 'testing',
+            },
+          ],
+        }}
+        registry={registry}
+        onChange={() => {}}
+        onRemove={() => {}}
+      />
+    );
+  });
+
+  it('should not allow override selection that marked as hidden from overrides', () => {
     registry.register({
       id: 'lineStyle',
       name: 'Line style',
diff --git a/public/app/features/dashboard/components/PanelEditor/OverrideEditor.tsx b/public/app/features/dashboard/components/PanelEditor/OverrideEditor.tsx
index 8745763cba6..f50fc884a89 100644
--- a/public/app/features/dashboard/components/PanelEditor/OverrideEditor.tsx
+++ b/public/app/features/dashboard/components/PanelEditor/OverrideEditor.tsx
@@ -9,17 +9,7 @@ import {
   isSystemOverride as isSystemOverrideGuard,
   VariableSuggestionsScope,
 } from '@grafana/data';
-import {
-  Field,
-  fieldMatchersUI,
-  HorizontalGroup,
-  Icon,
-  IconButton,
-  Label,
-  stylesFactory,
-  useTheme,
-  ValuePicker,
-} from '@grafana/ui';
+import { Field, fieldMatchersUI, HorizontalGroup, Icon, IconButton, Label, useStyles, ValuePicker } from '@grafana/ui';
 import { DynamicConfigValueEditor } from './DynamicConfigValueEditor';
 
 import { getDataLinksVariableSuggestions } from '../../../panel/panellinks/link_srv';
@@ -49,9 +39,9 @@ export const OverrideEditor: React.FC<OverrideEditorProps> = ({
   onRemove,
   registry,
 }) => {
-  const theme = useTheme();
   const matcherUi = fieldMatchersUI.get(override.matcher.id);
-  const styles = getStyles(theme);
+  const styles = useStyles(getStyles);
+  const properties = override.properties.map(p => registry.getIfExists(p.id)).filter(prop => !!prop);
 
   const matcherLabel = <Label>{matcherUi.name}</Label>;
 
@@ -114,8 +104,9 @@ export const OverrideEditor: React.FC<OverrideEditorProps> = ({
     });
 
   const renderOverrideTitle = (isExpanded: boolean) => {
-    const overriddenProperites = override.properties.map(p => registry.get(p.id).name).join(', ');
+    const propertyNames = properties.map(p => p?.name).join(', ');
     const matcherOptions = matcherUi.optionsToLabel(override.matcher.options);
+
     return (
       <div>
         <HorizontalGroup justify="space-between">
@@ -127,9 +118,9 @@ export const OverrideEditor: React.FC<OverrideEditorProps> = ({
             <div className={styles.options} title={matcherOptions}>
               {matcherUi.name} <Icon name="angle-right" /> {matcherOptions}
             </div>
-            <div className={styles.options} title={overriddenProperites}>
+            <div className={styles.options} title={propertyNames}>
               Properties overridden <Icon name="angle-right" />
-              {overriddenProperites}
+              {propertyNames}
             </div>
           </div>
         )}
@@ -153,10 +144,9 @@ export const OverrideEditor: React.FC<OverrideEditorProps> = ({
       <>
         {override.properties.map((p, j) => {
           const item = registry.getIfExists(p.id);
-          console.log('item', item);
 
           if (!item) {
-            return <div>Unknown property: {p.id}</div>;
+            return null;
           }
 
           const isCollapsible =
@@ -197,7 +187,7 @@ export const OverrideEditor: React.FC<OverrideEditorProps> = ({
   );
 };
 
-const getStyles = stylesFactory((theme: GrafanaTheme) => {
+const getStyles = (theme: GrafanaTheme) => {
   return {
     matcherUi: css`
       padding: ${theme.spacing.sm};
@@ -214,5 +204,8 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => {
       overflow: hidden;
       padding-right: ${theme.spacing.xl};
     `,
+    unknownLabel: css`
+      margin-bottom: 0;
+    `,
   };
-});
+};