From 78244ce6dec0ed7f5f7be70394d939f3a3d2088a Mon Sep 17 00:00:00 2001 From: Bucky Schwarz Date: Mon, 7 Oct 2019 16:37:48 -0700 Subject: [PATCH] fix: Map type Variables selector shows keys, not values add test for hydrateVars dashboard variable dropdown test: inspect values, not just array length add RTL test for variable dropdown changes lint fix: Disable saving threshold check if no threshold selected (#15348) * Prevent check saving if no thresholds * Add tests * Add changes to changelog * make optional props optional * use false instead of null for boolean changelog fix(ui): ignore false change events in VariableForm (#15317) closes #15059 the issue is to persist user data across variable type selection interfaces within the variable editor. this commit pushes all of the variable editor information down to redux to allow persistence outside of the view state until the user clicks "cancel" or "create" in the interface. --- CHANGELOG.md | 1 + .../VariableDropdown.test.tsx | 6 +- .../variablesControlBar/VariableDropdown.tsx | 9 +- ui/src/dashboards/selectors/index.ts | 4 +- .../VariableTooltipContents.test.tsx | 83 +++++++++++++++++++ ui/src/variables/mocks/index.ts | 16 ++++ ui/src/variables/utils/hydrateVars.test.ts | 33 +++++++- ui/src/variables/utils/hydrateVars.ts | 6 +- 8 files changed, 144 insertions(+), 14 deletions(-) create mode 100644 ui/src/timeMachine/components/variableToolbar/VariableTooltipContents.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 7218ae1ada0..e99e0175763 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ 1. [15295](https://github.com/influxdata/influxdb/pull/15295): Ensures users are created with an active status 1. [15306](https://github.com/influxdata/influxdb/pull/15306): Added missing string values for CacheStatus type 1. [15348](https://github.com/influxdata/influxdb/pull/15348): Disable saving for threshold check if no threshold selected +1. [15354](https://github.com/influxdata/influxdb/pull/15354): Query variable selector shows variable keys, not values ## v2.0.0-alpha.18 [2019-09-26] diff --git a/ui/src/dashboards/components/variablesControlBar/VariableDropdown.test.tsx b/ui/src/dashboards/components/variablesControlBar/VariableDropdown.test.tsx index 1b8b9e01a96..a45716b7d7c 100644 --- a/ui/src/dashboards/components/variablesControlBar/VariableDropdown.test.tsx +++ b/ui/src/dashboards/components/variablesControlBar/VariableDropdown.test.tsx @@ -74,9 +74,11 @@ describe('Dashboards.Components.VariablesControlBar.VariableDropdown', () => { const dropdownButton = getByTestId('variable-dropdown--button') fireEvent.click(dropdownButton) - const dropdownItems = getAllByTestId('variable-dropdown--item') + const dropdownItems = getAllByTestId('variable-dropdown--item').map( + node => node.id + ) - expect(dropdownItems.length).toBe(Object.keys(values).length) + expect(dropdownItems).toEqual(Object.keys(values)) }) }) }) diff --git a/ui/src/dashboards/components/variablesControlBar/VariableDropdown.tsx b/ui/src/dashboards/components/variablesControlBar/VariableDropdown.tsx index 969537cbcb4..d3b3f6a8173 100644 --- a/ui/src/dashboards/components/variablesControlBar/VariableDropdown.tsx +++ b/ui/src/dashboards/components/variablesControlBar/VariableDropdown.tsx @@ -73,7 +73,7 @@ class VariableDropdown extends PureComponent { > {dropdownValues.map(({name}) => ( /* - Use key as value since they are unique otherwise + Use key as value since they are unique otherwise multiple selection appear in the dropdown */ { } private handleSelect = (selectedKey: string) => { - const {dashboardID, variableID, onSelectValue, values} = this.props + const {dashboardID, variableID, onSelectValue} = this.props - const selection = values.find(v => v.name === selectedKey) - const selectedValue = !!selection ? selection.value : '' - - onSelectValue(dashboardID, variableID, selectedValue) + onSelectValue(dashboardID, variableID, selectedKey) } } diff --git a/ui/src/dashboards/selectors/index.ts b/ui/src/dashboards/selectors/index.ts index 8b969acdf8f..6cffa4a80f7 100644 --- a/ui/src/dashboards/selectors/index.ts +++ b/ui/src/dashboards/selectors/index.ts @@ -72,14 +72,14 @@ export const getVariableValuesForDropdown = ( const mapValues = getArgumentValuesForVariable(state, variableID) as { [key: string]: string } + const list = Object.entries(mapValues).map(([name, value]) => ({ name, value, })) - const selection = list.find(({value}) => value === selectedValue) return { - selectedKey: get(selection, 'name', ''), + selectedKey: selectedValue, list, } } diff --git a/ui/src/timeMachine/components/variableToolbar/VariableTooltipContents.test.tsx b/ui/src/timeMachine/components/variableToolbar/VariableTooltipContents.test.tsx new file mode 100644 index 00000000000..090b36c6b5d --- /dev/null +++ b/ui/src/timeMachine/components/variableToolbar/VariableTooltipContents.test.tsx @@ -0,0 +1,83 @@ +// Libraries +import React from 'react' +import {fireEvent} from 'react-testing-library' + +// Components +import VariableTooltipContents from 'src/timeMachine/components/variableToolbar/VariableTooltipContents' + +// Utils +import {renderWithRedux} from 'src/mockState' +import {AppState} from 'src/types' + +const variableValues = { + key1: 'value1', + key2: 'value2', +} + +const setInitialState = (state: AppState) => { + return { + ...state, + orgs: [ + { + id: '2e9f65b990c28374', + }, + ], + variables: { + status: 'Done', + variables: { + '04960e76e5afe000': { + variable: { + id: '04960e76e5afe000', + orgID: '2e9f65b990c28374', + name: 'example_map', + description: '', + selected: null, + arguments: { + type: 'map', + values: variableValues, + }, + createdAt: '2019-10-07T14:59:58.102045-07:00', + updatedAt: '2019-10-07T14:59:58.102045-07:00', + labels: [], + links: { + self: '/api/v2/variables/04960e76e5afe000', + labels: '/api/v2/variables/04960e76e5afe000/labels', + org: '/api/v2/orgs/2e9f65b990c28374', + }, + }, + status: 'Done', + }, + }, + values: { + de: { + status: 'Done', + values: { + '04960e76e5afe000': { + valueType: 'string', + values: Object.keys(variableValues), + selectedValue: 'key1', + }, + }, + order: ['04960e76e5afe000'], + }, + }, + }, + } +} + +describe("Time Machine's variable dropdown", () => { + describe('rendering map type variables', () => { + it("renders the variables' keys, rather than their values", async () => { + const {getByTestId, getByLabelText, getByText} = renderWithRedux( + , + setInitialState + ) + + fireEvent.click(getByLabelText('Value')) + fireEvent.click(getByTestId('dropdown--button')) + Object.keys(variableValues).forEach(variableKey => { + expect(getByText(variableKey)).toBeTruthy() + }) + }) + }) +}) diff --git a/ui/src/variables/mocks/index.ts b/ui/src/variables/mocks/index.ts index d4d801df501..12f2db9e227 100644 --- a/ui/src/variables/mocks/index.ts +++ b/ui/src/variables/mocks/index.ts @@ -19,3 +19,19 @@ export const createVariable = ( }, }, }) + +export const createMapVariable = ( + name: string, + map: {[key: string]: string} = {}, + selected?: string +): Variable => ({ + name, + id: name, + orgID: 'howdy', + selected: selected ? [selected] : [], + labels: [], + arguments: { + type: 'map', + values: {...map}, + }, +}) diff --git a/ui/src/variables/utils/hydrateVars.test.ts b/ui/src/variables/utils/hydrateVars.test.ts index 81c9ab8c39b..6e90a473761 100644 --- a/ui/src/variables/utils/hydrateVars.test.ts +++ b/ui/src/variables/utils/hydrateVars.test.ts @@ -3,7 +3,7 @@ import {ValueFetcher} from 'src/variables/utils/ValueFetcher' import {hydrateVars} from 'src/variables/utils/hydrateVars' // Mocks -import {createVariable} from 'src/variables/mocks' +import {createMapVariable, createVariable} from 'src/variables/mocks' // Types import {CancellationError} from 'src/types/promises' @@ -278,4 +278,35 @@ describe('hydrate vars', () => { cancel() }) + + test('returns the keys (not the values) of map types', async () => { + const firstVariable = createMapVariable('0495e1b2c71fd000', { + key1: 'value1', + key2: 'value2', + }) + const secondVariable = createMapVariable('04960a2028efe000', { + key3: 'value3', + key4: 'value4', + }) + + const expected = { + '0495e1b2c71fd000': { + valueType: 'string', + values: ['key1', 'key2'], + selectedValue: 'key2', + }, + } + + const actual = await hydrateVars( + [firstVariable], + [firstVariable, secondVariable], + { + url: '', + orgID: '', + selections: {'0495e1b2c71fd000': 'key2'}, + } + ).promise + + expect(actual).toEqual(expected) + }) }) diff --git a/ui/src/variables/utils/hydrateVars.ts b/ui/src/variables/utils/hydrateVars.ts index 058bfaf1618..59e28778b84 100644 --- a/ui/src/variables/utils/hydrateVars.ts +++ b/ui/src/variables/utils/hydrateVars.ts @@ -161,7 +161,7 @@ const mapVariableValues = ( prevSelection: string, defaultSelection: string ): VariableValues => { - const values: string[] = Object.values(variable.arguments.values) + const values: string[] = Object.keys(variable.arguments.values) return { valueType: 'string', @@ -219,7 +219,7 @@ export const collectDescendants = ( /* Hydrate the values of a single node in the graph. - This assumes that every descendant of this node has already been hydrated. + This assumes that every descendant of this node has already been hydrated. */ const hydrateVarsHelper = async ( node: VariableNode, @@ -360,7 +360,7 @@ const extractResult = (graph: VariableNode[]): VariableValuesByID => { 1. Construct a graph that represents the dependency structure between variables. A variable `a` depends on variable `b` if the query for `a` uses the variable `b`. - + Each node in the graph has a hydration status: it it either `NotStarted`, `Loading` (values are being fetched), `Error` (values failed to fetch or cannot be fetched), or `Done` (values have been fetched successfully).