Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui): fixed regression bug that preventing variable maps from properly assigning values #17072

Merged
merged 6 commits into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
1. [17042](https://github.com/influxdata/influxdb/pull/17042): Fixed issue where tasks are not exported when exporting by org id
1. [17070](https://github.com/influxdata/influxdb/pull/17070): Fixed issue where tasks with imports in query break in pkger
1. [17028](https://github.com/influxdata/influxdb/pull/17028): Fixed issue where selecting an aggregate function in the script editor was not adding the function to a new line
1. [17072](https://github.com/influxdata/influxdb/pull/17072): Fixed issue where creating a variable of type map was piping the incorrect value when map variables were used in queries
1. [17050](https://github.com/influxdata/influxdb/pull/17050): Added missing user names to auth CLI commands

## v2.0.0-beta.5 [2020-02-27]
Expand Down
34 changes: 14 additions & 20 deletions ui/cypress/e2e/dashboardsView.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import {Organization, AppState} from '../../src/types'

const dispatch = action =>
cy
.window()
.its('store')
.invoke('dispatch', action)

describe('Dashboard', () => {
beforeEach(() => {
cy.flush()
Expand Down Expand Up @@ -97,8 +91,7 @@ describe('Dashboard', () => {
variableID: string
) => win => {
const {resources} = win.store.getState() as AppState
return resources.variables.values[contextID].values[variableID]
.selectedValue
return resources.variables.values[contextID].values[variableID].selectedKey
}

it('can manage variable state with a lot of pointing and clicking', () => {
Expand All @@ -109,7 +102,6 @@ describe('Dashboard', () => {
cy.visit(`${orgs}/${orgID}/dashboards/${dashboard.id}`)
})
const [firstKey, secondKey] = Object.keys(variable.arguments.values)

// add cell with variable in its query
cy.getByTestID('add-cell--button').click()
cy.getByTestID('switch-to-script-editor').click()
Expand Down Expand Up @@ -142,27 +134,29 @@ describe('Dashboard', () => {
.pipe(getSelectedVariable('veo', variable.id))
.should('equal', secondKey)

// select 1st value in cell
dispatch({
type: 'SELECT_VARIABLE_VALUE',
contextID: 'veo',
variableID: variable.id,
selectedValue: firstKey,
cy.getByTestID('toolbar-tab').click()
cy.get('.variables-toolbar--label').trigger('mouseover')
// toggle the variable dropdown in the VEO cell dashboard
cy.getByTestID('toolbar-popover--contents').within(() => {
cy.getByTestID('dropdown--button').click()
// select 1st value in cell
cy.getByTestID('dropdown-item')
.first()
.click()
})
// save cell
cy.getByTestID('save-cell--button').click()

// selected value in cell context is 1st value
cy.window()
.pipe(getSelectedVariable('veo', variable.id))
.should('equal', firstKey)

// save cell
cy.getByTestID('save-cell--button').click()

// selected value in dashboard is 1st value
cy.getByTestID('variable-dropdown').should('contain', secondKey)
cy.getByTestID('variable-dropdown').should('contain', firstKey)
cy.window()
.pipe(getSelectedVariable(dashboard.id, variable.id))
.should('equal', secondKey)
.should('equal', firstKey)

// graph tips responds to mouse over
cy.getByTestID('graphtips-question-mark').click()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {AppState, RemoteDataState} from 'src/types'

const values = {
def: 'defbuck',
def2: 'defbuck',
def2: 'defbuck2',
foo: 'foobuck',
goo: 'goobuck',
new: 'newBuck',
Expand Down Expand Up @@ -46,8 +46,9 @@ const setInitialState = (state: AppState): AppState => {
values: {
'03cbdc8a53a63000': {
valueType: 'string',
values: Object.values(values),
values,
selectedValue: 'defbuck',
selectedKey: 'def',
},
},
order: ['03cbdc8a53a63000'],
Expand Down
9 changes: 5 additions & 4 deletions ui/src/dashboards/selectors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const getVariableValuesForDropdown = (
variableID: string,
contextID: string
): DropdownValues => {
const {selectedValue, values} = getValuesForVariable(
const {selectedKey, values} = getValuesForVariable(
state,
variableID,
contextID
Expand All @@ -60,13 +60,14 @@ export const getVariableValuesForDropdown = (
}))

return {
selectedKey: selectedValue,
selectedKey,
list,
}
}
default:
const list = values.map(v => ({name: v, value: v}))
const valueCopy = values as string[]
const list = valueCopy.map(v => ({name: v, value: v}))

return {selectedKey: selectedValue, list}
return {selectedKey, list}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ const setInitialState = (state: AppState) => {
values: {
'04960e76e5afe000': {
valueType: 'string',
values: Object.keys(variableValues),
selectedValue: 'key1',
values: variableValues,
selectedKey: 'key1',
selectedValue: 'value1',
},
},
order: ['04960e76e5afe000'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ const VariableTooltipContents: FunctionComponent<Props> = ({
onAddVariableToTimeMachine,
onSelectVariableValue,
}) => {
const dropdownItems: string[] = get(values, 'values') || []
let dropdownItems = get(values, 'values', [])

if (Object.keys(dropdownItems).length > 0) {
dropdownItems = Object.keys(dropdownItems)
}

const handleMouseEnter = () => {
if (values || valuesStatus === RemoteDataState.Loading) {
Expand All @@ -64,6 +68,9 @@ const VariableTooltipContents: FunctionComponent<Props> = ({
let selectedOption = 'None Selected'
let icon
let status = toComponentStatus(valuesStatus)
// this should set the selectedKey as the key
// set as a ternary in order to get e2e test to pass since values are undefined on the test
const key = values && values.selectedKey ? values.selectedKey : undefined

if (!values) {
selectedOption = 'Failed to Load'
Expand All @@ -73,19 +80,20 @@ const VariableTooltipContents: FunctionComponent<Props> = ({
selectedOption = 'Failed to Load'
icon = IconFont.AlertTriangle
status = ComponentStatus.Disabled
} else if (!values.values.length) {
} else if (key === undefined || values.values[key] === undefined) {
selectedOption = 'No Results'
} else {
selectedOption = get(values, 'selectedValue', 'None Selected')
selectedOption = get(values, 'selectedKey', 'None Selected')
}

return (
<div onMouseEnter={handleMouseEnter}>
<Form.Element label="Value">
<SelectDropdown
buttonIcon={icon}
options={dropdownItems}
options={dropdownItems as string[]}
selectedOption={selectedOption}
testID="variable--tooltip-dropdown"
buttonStatus={status}
style={{width: '200px'}}
onSelect={value => onSelectVariableValue(variableID, value)}
Expand All @@ -98,7 +106,6 @@ const VariableTooltipContents: FunctionComponent<Props> = ({
const mstp = (state: AppState, ownProps: OwnProps) => {
const valuesStatus = getTimeMachineValuesStatus(state)
const values = getTimeMachineValues(state, ownProps.variableID)

return {values, valuesStatus}
}

Expand Down
7 changes: 6 additions & 1 deletion ui/src/types/variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,14 @@ export type FluxColumnType =
| 'dateTime'
| 'duration'

export type mapValue = string
export interface VariableMapObject {
[mapKey: string]: mapValue
}
export interface VariableValues {
values: string[]
values: VariableMapObject | string[]
valueType: FluxColumnType
selectedKey?: string
selectedValue: string
error?: string
}
Expand Down
12 changes: 4 additions & 8 deletions ui/src/variables/utils/ValueFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import {runQuery} from 'src/shared/apis/query'

// Utils
import {resolveSelectedValue} from 'src/variables/utils/resolveSelectedValue'
import {resolveSelectedKey} from 'src/variables/utils/resolveSelectedValue'
import {formatVarsOption} from 'src/variables/utils/formatVarsOption'
import {parseResponse} from 'src/shared/parsing/flux/response'
import {buildVarsOption} from 'src/variables/utils/buildVarsOption'
Expand Down Expand Up @@ -55,11 +55,7 @@ export const extractValues = (
return {
values,
valueType: table.dataTypes._value as FluxColumnType,
selectedValue: resolveSelectedValue(
values,
prevSelection,
defaultSelection
),
selectedValue: resolveSelectedKey(values, prevSelection, defaultSelection),
}
}

Expand Down Expand Up @@ -119,8 +115,8 @@ export class DefaultValueFetcher implements ValueFetcher {

return {
...cachedValues,
selectedValue: resolveSelectedValue(
cachedValues.values,
selectedValue: resolveSelectedKey(
cachedValues.values as string[],
prevSelection,
defaultSelection
),
Expand Down
10 changes: 7 additions & 3 deletions ui/src/variables/utils/hydrateVars.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ describe('hydrate vars', () => {
cancel()
})

test('returns the keys (not the values) of map types', async () => {
test('returns the selected key/value pair of the selected map', async () => {
const firstVariable = createMapVariable('0495e1b2c71fd000', {
key1: 'value1',
key2: 'value2',
Expand All @@ -294,8 +294,12 @@ describe('hydrate vars', () => {
const expected = {
'0495e1b2c71fd000': {
valueType: 'string',
values: ['key1', 'key2'],
selectedValue: 'key2',
values: {
key1: 'value1',
key2: 'value2',
},
selectedKey: 'key2',
selectedValue: 'value2',
},
}

Expand Down
18 changes: 12 additions & 6 deletions ui/src/variables/utils/hydrateVars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
import {valueFetcher, ValueFetcher} from 'src/variables/utils/ValueFetcher'
import Deferred from 'src/utils/Deferred'
import {getVarAssignment} from 'src/variables/utils/getVarAssignment'
import {resolveSelectedValue} from 'src/variables/utils/resolveSelectedValue'
import {
resolveSelectedKey,
resolveSelectedValue,
} from 'src/variables/utils/resolveSelectedValue'

// Constants
import {OPTION_NAME, BOUNDARY_GROUP} from 'src/variables/constants/index'
Expand Down Expand Up @@ -148,6 +151,7 @@ const errorVariableValues = (
message = 'Failed to load values for variable'
): VariableValues => ({
values: null,
selectedKey: null,
selectedValue: null,
valueType: null,
error: message,
Expand All @@ -161,14 +165,15 @@ const mapVariableValues = (
prevSelection: string,
defaultSelection: string
): VariableValues => {
const values: string[] = Object.keys(variable.arguments.values)

const keys: string[] = Object.keys(variable.arguments.values)
const selectedKey = resolveSelectedKey(keys, prevSelection, defaultSelection)
return {
valueType: 'string',
values,
values: variable.arguments.values,
selectedKey,
selectedValue: resolveSelectedValue(
values,
prevSelection,
variable.arguments.values,
selectedKey,
defaultSelection
),
}
Expand Down Expand Up @@ -408,6 +413,7 @@ export const hydrateVars = (

try {
node.values = await hydrateVarsHelper(node, options)
node.variable.selected = [node.values.selectedKey]
node.status = RemoteDataState.Done

return Promise.all(node.parents.filter(readyToResolve).map(resolve))
Expand Down
21 changes: 20 additions & 1 deletion ui/src/variables/utils/resolveSelectedValue.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
export const resolveSelectedValue = (
import {VariableMapObject} from 'src/types'

export const resolveSelectedKey = (
values: string[],
prevSelection?: string,
defaultSelection?: string
Expand All @@ -13,3 +15,20 @@ export const resolveSelectedValue = (

return values[0]
}

export const resolveSelectedValue = (
values: VariableMapObject,
selectedKey: string,
defaultSelection?: string
): string => {
if (selectedKey in values) {
return values[selectedKey]
}

if (defaultSelection in values) {
return values[defaultSelection]
}
// return get first value
const first = Object.keys(values)[0]
return values[first]
}
8 changes: 4 additions & 4 deletions ui/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1016,10 +1016,10 @@
resolved "https://registry.yarnpkg.com/@influxdata/clockface/-/clockface-1.1.5.tgz#dfedc4f59788717d5e92bd00935dda35c0c60fc8"
integrity sha512-5+RDswLCiOBX21rey6e1j4vrwQ6obwMv+qcP6ucH7y0vTRnAaMk9lqKqHH3FGUqUEfBNegtj4Ho8430ywfS6ag==

"@influxdata/flux-lsp-browser@^0.2.2":
version "0.2.2"
resolved "https://registry.yarnpkg.com/@influxdata/flux-lsp-browser/-/flux-lsp-browser-0.2.2.tgz#766ef965da25149ac300df6db1a64ad277b5b50b"
integrity sha512-MlFmu7gVdgJ1pkoC1CRaWmjedi6IuSJa2Bg3vDl0l/1cJyAShtoDs8levYvL0gqcKyvq/i/baXrNXF+SCIM7MQ==
"@influxdata/flux-lsp-browser@0.3.1":
version "0.3.1"
resolved "https://registry.yarnpkg.com/@influxdata/flux-lsp-browser/-/flux-lsp-browser-0.3.1.tgz#472bdd4b82b239955776133c299eea3643237041"
integrity sha512-G813h+ytOnSxjjdNhAjwz+6iL+nZn425eMsCR08fajvMNztbk3GpY5X7LLA7f8JXIsVg1SCczkmSpGkK3lWKSg==

"@influxdata/flux-parser@^0.3.0":
version "0.3.0"
Expand Down