Graphite: Handle unknown Graphite functions without breaking the visual editor (#32635)

public/app/plugins/datasource/graphite/FunctionEditor.test.tsx

@@ -0,0 +1,49 @@
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+import { FunctionEditor } from './FunctionEditor';
+import { FunctionDescriptor } from './FunctionEditorControls';
+
+function mockFunctionDescriptor(name: string, unknown?: boolean): FunctionDescriptor {
+  return {
+    text: '',
+    params: [],
+    def: {
+      category: 'category',
+      defaultParams: [],
+      fake: false,
+      name: name,
+      params: [],
+      unknown: unknown,
+    },
+  };
+}
+
+describe('FunctionEditor', () => {
+  it('should display a defined function with name and no icon', () => {
+    render(
+      <FunctionEditor
+        func={mockFunctionDescriptor('foo')}
+        onMoveLeft={() => {}}
+        onMoveRight={() => {}}
+        onRemove={() => {}}
+      />
+    );
+
+    expect(screen.getByText('foo')).toBeInTheDocument();
+    expect(screen.queryByTestId('warning-icon')).not.toBeInTheDocument();
+  });
+
+  it('should display an unknown function with name and warning icon', () => {
+    render(
+      <FunctionEditor
+        func={mockFunctionDescriptor('bar', true)}
+        onMoveLeft={jest.fn()}
+        onMoveRight={jest.fn()}
+        onRemove={jest.fn()}
+      />
+    );
+
+    expect(screen.getByText('bar')).toBeInTheDocument();
+    expect(screen.getByTestId('warning-icon')).toBeInTheDocument();
+  });
+});

public/app/plugins/datasource/graphite/FunctionEditor.tsx

@@ -1,54 +1,40 @@
-import React from 'react';
+import React, { useRef } from 'react';
-import { PopoverController, Popover, ClickOutsideWrapper } from '@grafana/ui';
+import { PopoverController, Popover, ClickOutsideWrapper, Icon, Tooltip, useTheme } from '@grafana/ui';
 import { FunctionDescriptor, FunctionEditorControls, FunctionEditorControlsProps } from './FunctionEditorControls';
+import { css } from '@emotion/css';
 
 interface FunctionEditorProps extends FunctionEditorControlsProps {
   func: FunctionDescriptor;
 }
 
-interface FunctionEditorState {
+const FunctionEditor: React.FC<FunctionEditorProps> = ({ onMoveLeft, onMoveRight, func, ...props }) => {
-  showingDescription: boolean;
+  const triggerRef = useRef<HTMLSpanElement>(null);
-}
+  const theme = useTheme();
 
-class FunctionEditor extends React.PureComponent<FunctionEditorProps, FunctionEditorState> {
+  const renderContent = ({ updatePopperPosition }: any) => (
-  private triggerRef = React.createRef<HTMLSpanElement>();
-
-  constructor(props: FunctionEditorProps) {
-    super(props);
-
-    this.state = {
-      showingDescription: false,
-    };
-  }
-
-  renderContent = ({ updatePopperPosition }: any) => {
-    const { onMoveLeft, onMoveRight } = this.props;
-
-    return (
     <FunctionEditorControls
-        {...this.props}
+      {...props}
+      func={func}
       onMoveLeft={() => {
-          onMoveLeft(this.props.func);
+        onMoveLeft(func);
         updatePopperPosition();
       }}
       onMoveRight={() => {
-          onMoveRight(this.props.func);
+        onMoveRight(func);
         updatePopperPosition();
       }}
     />
   );
-  };
 
-  render() {
   return (
-      <PopoverController content={this.renderContent} placement="top" hideAfter={100}>
+    <PopoverController content={renderContent} placement="top" hideAfter={100}>
       {(showPopper, hidePopper, popperProps) => {
         return (
           <>
-              {this.triggerRef.current && (
+            {triggerRef.current && (
               <Popover
                 {...popperProps}
-                  referenceElement={this.triggerRef.current}
+                referenceElement={triggerRef.current}
                 wrapperClassName="popper"
                 className="popper__background"
                 renderArrow={({ arrowProps, placement }) => (
@@ -63,12 +49,20 @@ class FunctionEditor extends React.PureComponent<FunctionEditorProps, FunctionEd
                 }
               }}
             >
-                <span
+              <span ref={triggerRef} onClick={popperProps.show ? hidePopper : showPopper} style={{ cursor: 'pointer' }}>
-                  ref={this.triggerRef}
+                {func.def.unknown && (
-                  onClick={popperProps.show ? hidePopper : showPopper}
+                  <Tooltip content={<TooltipContent />} placement="bottom">
-                  style={{ cursor: 'pointer' }}
+                    <Icon
-                >
+                      data-testid="warning-icon"
-                  {this.props.func.def.name}
+                      name="exclamation-triangle"
+                      size="xs"
+                      className={css`
+                        margin-right: ${theme.spacing.xxs};
+                      `}
+                    />
+                  </Tooltip>
+                )}
+                {func.def.name}
               </span>
             </ClickOutsideWrapper>
           </>
@@ -76,7 +70,24 @@ class FunctionEditor extends React.PureComponent<FunctionEditorProps, FunctionEd
       }}
     </PopoverController>
   );
-  }
+};
-}
+
+const TooltipContent = React.memo(() => {
+  return (
+    <span>
+      This function is not supported. Check your function for typos and{' '}
+      <a
+        target="_blank"
+        className="external-link"
+        rel="noreferrer noopener"
+        href="https://graphite.readthedocs.io/en/latest/functions.html"
+      >
+        read the docs
+      </a>{' '}
+      to see whether you need to upgrade your data source’s version to make this function available.
+    </span>
+  );
+});
+TooltipContent.displayName = 'FunctionEditorTooltipContent';
 
 export { FunctionEditor };

public/app/plugins/datasource/graphite/FunctionEditorControls.tsx

@@ -11,6 +11,10 @@ export interface FunctionDescriptor {
     fake: boolean;
     name: string;
     params: string[];
+    /**
+     * True if the function was not found on the list of available function descriptions.
+     */
+    unknown?: boolean;
   };
 }
 

public/app/plugins/datasource/graphite/func_editor.ts

@@ -166,6 +166,10 @@ export function graphiteFuncEditor($compile: any, templateSrv: TemplateSrv) {
       function addElementsAndCompile() {
         $funcLink.appendTo(elem);
 
+        if (func.def.unknown) {
+          elem.addClass('unknown-function');
+        }
+
         const defParams: any = _.clone(func.def.params);
         const lastParam: any = _.last(func.def.params);
 

public/app/plugins/datasource/graphite/gfunc.test.ts

@@ -0,0 +1,18 @@
+import gfunc from './gfunc';
+
+describe('gfunc', () => {
+  const INDEX = {
+    foo: {
+      name: 'foo',
+      params: [],
+    },
+  };
+
+  it('returns function from the index', () => {
+    expect(gfunc.getFuncDef('foo', INDEX)).toEqual(INDEX.foo);
+  });
+
+  it('marks function as unknown when it is not available in the index', () => {
+    expect(gfunc.getFuncDef('bar', INDEX)).toEqual({ name: 'bar', params: [{ multiple: true }], unknown: true });
+  });
+});

public/app/plugins/datasource/graphite/gfunc.ts

@@ -1083,7 +1083,7 @@ function createFuncInstance(funcDef: any, options?: { withDefaultParams: any },
 
 function getFuncDef(name: string, idx?: any) {
   if (!(idx || index)[name]) {
-    throw { message: 'Method not found ' + name };
+    return { name: name, params: [{ multiple: true }], unknown: true };
   }
   return (idx || index)[name];
 }

public/sass/components/_query_part.scss

@@ -1,6 +1,10 @@
 .query-part {
   background-color: $tight-form-func-bg;
 
+  &.unknown-function {
+    border: 1px solid $error-text-color;
+  }
+
   &.show-function-controls {
     padding-top: 5px;
     min-width: 100px;