From d8967b1d6018bddf9ccc2c5c7bec8b2662cccfb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= Date: Fri, 9 Apr 2021 23:21:53 +0200 Subject: [PATCH] Graphite: Handle unknown Graphite functions without breaking the visual editor (#32635) --- .../graphite/FunctionEditor.test.tsx | 49 ++++++ .../datasource/graphite/FunctionEditor.tsx | 153 ++++++++++-------- .../graphite/FunctionEditorControls.tsx | 4 + .../datasource/graphite/func_editor.ts | 4 + .../plugins/datasource/graphite/gfunc.test.ts | 18 +++ .../app/plugins/datasource/graphite/gfunc.ts | 2 +- public/sass/components/_query_part.scss | 4 + 7 files changed, 162 insertions(+), 72 deletions(-) create mode 100644 public/app/plugins/datasource/graphite/FunctionEditor.test.tsx create mode 100644 public/app/plugins/datasource/graphite/gfunc.test.ts diff --git a/public/app/plugins/datasource/graphite/FunctionEditor.test.tsx b/public/app/plugins/datasource/graphite/FunctionEditor.test.tsx new file mode 100644 index 00000000000..bde1a75a729 --- /dev/null +++ b/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( + {}} + 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( + + ); + + expect(screen.getByText('bar')).toBeInTheDocument(); + expect(screen.getByTestId('warning-icon')).toBeInTheDocument(); + }); +}); diff --git a/public/app/plugins/datasource/graphite/FunctionEditor.tsx b/public/app/plugins/datasource/graphite/FunctionEditor.tsx index ffdbd5dddfe..239dec0fab2 100644 --- a/public/app/plugins/datasource/graphite/FunctionEditor.tsx +++ b/public/app/plugins/datasource/graphite/FunctionEditor.tsx @@ -1,82 +1,93 @@ -import React from 'react'; -import { PopoverController, Popover, ClickOutsideWrapper } from '@grafana/ui'; +import React, { useRef } from 'react'; +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 { - showingDescription: boolean; -} +const FunctionEditor: React.FC = ({ onMoveLeft, onMoveRight, func, ...props }) => { + const triggerRef = useRef(null); + const theme = useTheme(); -class FunctionEditor extends React.PureComponent { - private triggerRef = React.createRef(); + const renderContent = ({ updatePopperPosition }: any) => ( + { + onMoveLeft(func); + updatePopperPosition(); + }} + onMoveRight={() => { + onMoveRight(func); + updatePopperPosition(); + }} + /> + ); - constructor(props: FunctionEditorProps) { - super(props); + return ( + + {(showPopper, hidePopper, popperProps) => { + return ( + <> + {triggerRef.current && ( + ( +
+ )} + /> + )} + { + if (popperProps.show) { + hidePopper(); + } + }} + > + + {func.def.unknown && ( + } placement="bottom"> + + + )} + {func.def.name} + + + + ); + }} + + ); +}; - this.state = { - showingDescription: false, - }; - } - - renderContent = ({ updatePopperPosition }: any) => { - const { onMoveLeft, onMoveRight } = this.props; - - return ( - { - onMoveLeft(this.props.func); - updatePopperPosition(); - }} - onMoveRight={() => { - onMoveRight(this.props.func); - updatePopperPosition(); - }} - /> - ); - }; - - render() { - return ( - - {(showPopper, hidePopper, popperProps) => { - return ( - <> - {this.triggerRef.current && ( - ( -
- )} - /> - )} - { - if (popperProps.show) { - hidePopper(); - } - }} - > - - {this.props.func.def.name} - - - - ); - }} - - ); - } -} +const TooltipContent = React.memo(() => { + return ( + + This function is not supported. Check your function for typos and{' '} + + read the docs + {' '} + to see whether you need to upgrade your data source’s version to make this function available. + + ); +}); +TooltipContent.displayName = 'FunctionEditorTooltipContent'; export { FunctionEditor }; diff --git a/public/app/plugins/datasource/graphite/FunctionEditorControls.tsx b/public/app/plugins/datasource/graphite/FunctionEditorControls.tsx index cd9a18b2c78..cefb52bbd48 100644 --- a/public/app/plugins/datasource/graphite/FunctionEditorControls.tsx +++ b/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; }; } diff --git a/public/app/plugins/datasource/graphite/func_editor.ts b/public/app/plugins/datasource/graphite/func_editor.ts index f91f4069092..884c7bae0fc 100644 --- a/public/app/plugins/datasource/graphite/func_editor.ts +++ b/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); diff --git a/public/app/plugins/datasource/graphite/gfunc.test.ts b/public/app/plugins/datasource/graphite/gfunc.test.ts new file mode 100644 index 00000000000..0176b59a5e4 --- /dev/null +++ b/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 }); + }); +}); diff --git a/public/app/plugins/datasource/graphite/gfunc.ts b/public/app/plugins/datasource/graphite/gfunc.ts index 399b4c76816..58eebbfeb21 100644 --- a/public/app/plugins/datasource/graphite/gfunc.ts +++ b/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]; } diff --git a/public/sass/components/_query_part.scss b/public/sass/components/_query_part.scss index 6ec484e841a..893221b2837 100644 --- a/public/sass/components/_query_part.scss +++ b/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;