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

feat: Add ability to group in Query Builder #16226

Merged
merged 1 commit into from
Dec 27, 2019
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
11 changes: 6 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
### Features

1. [16234](https://github.com/influxdata/influxdb/pull/16234): Add support for notification endpoints to influx templates/pkgs.
2. [16242](https://github.com/influxdata/influxdb/pull/16242): Drop id prefix for secret key requirement for notification endpoints
3. [16259](https://github.com/influxdata/influxdb/pull/16259): Add support for check resource to pkger parser
4. [16262](https://github.com/influxdata/influxdb/pull/16262): Add support for check resource pkger dry run functionality
5. [16275](https://github.com/influxdata/influxdb/pull/16275): Add support for check resource pkger apply functionality
6. [16283](https://github.com/influxdata/influxdb/pull/16283): Add support for check resource pkger export functionality
1. [16242](https://github.com/influxdata/influxdb/pull/16242): Drop id prefix for secret key requirement for notification endpoints
1. [16259](https://github.com/influxdata/influxdb/pull/16259): Add support for check resource to pkger parser
1. [16262](https://github.com/influxdata/influxdb/pull/16262): Add support for check resource pkger dry run functionality
1. [16275](https://github.com/influxdata/influxdb/pull/16275): Add support for check resource pkger apply functionality
1. [16283](https://github.com/influxdata/influxdb/pull/16283): Add support for check resource pkger export functionality
1. [16212](https://github.com/influxdata/influxdb/pull/16212): Add new kv.ForwardCursor interface
1. [16297](https://github.com/influxdata/influxdb/pull/16297): Add support for notification rule to pkger parser
1. [16298](https://github.com/influxdata/influxdb/pull/16298): Add support for notification rule pkger dry run functionality
Expand All @@ -17,6 +17,7 @@
1. [16322](https://github.com/influxdata/influxdb/pull/16322): Add support for tasks to pkger dry run functionality
1. [16323](https://github.com/influxdata/influxdb/pull/16323): Add support for tasks to pkger apply functionality
1. [16324](https://github.com/influxdata/influxdb/pull/16324): Add support for tasks to pkger export functionality
1. [16226](https://github.com/influxdata/influxdb/pull/16226): Add group() to Query Builder

### Bug Fixes

Expand Down
3 changes: 2 additions & 1 deletion ui/cypress/e2e/explorer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,8 @@ describe('DataExplorer', () => {
})
})

describe('raw script editing', () => {
// todo: investigate flakiness of this test: https://github.com/influxdata/influxdb/issues/16330
describe.skip('raw script editing', () => {
beforeEach(() => {
cy.getByTestID('switch-to-script-editor').click()
})
Expand Down
2 changes: 1 addition & 1 deletion ui/src/dataLoaders/reducers/telegrafEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ logfile = ""
hostname = ""
## If set to true, do no set the "host" tag in the telegraf agent.
omit_hostname = false
`
`,
},
{
name: '__default__',
Expand Down
2 changes: 2 additions & 0 deletions ui/src/shared/utils/featureFlag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const OSS_FLAGS = {
monacoEditor: false,
downloadCellCSV: false,
telegrafEditor: false,
queryBuilderGrouping: false,
}

export const CLOUD_FLAGS = {
Expand All @@ -16,6 +17,7 @@ export const CLOUD_FLAGS = {
cloudBilling: CLOUD_BILLING_VISIBLE, // should be visible in dev and acceptance, but not in cloud
downloadCellCSV: false,
telegrafEditor: false,
queryBuilderGrouping: false,
}

export const isFlagEnabled = (flagName: string, equals?: string | boolean) => {
Expand Down
17 changes: 13 additions & 4 deletions ui/src/timeMachine/actions/queryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ import {

// Types
import {Dispatch} from 'redux-thunk'
import {GetState, RemoteDataState} from 'src/types'
import {BuilderAggregateFunctionType} from 'src/client/generatedRoutes'
import {
BuilderAggregateFunctionType,
GetState,
RemoteDataState,
} from 'src/types'
import {BuilderFunctionsType} from '@influxdata/influx'

export type Action =
Expand Down Expand Up @@ -285,15 +288,16 @@ export const selectTagValue = (index: number, value: string) => (
timeMachines: {activeTimeMachineID},
} = state
const tags = getActiveQuery(state).builderConfig.tags
const values = tags[index].values
const currentTag = tags[index]
const values = currentTag.values

let newValues: string[]

if (values.includes(value)) {
newValues = values.filter(v => v !== value)
} else if (
activeTimeMachineID === 'alerting' &&
tags[index].key === '_field'
currentTag.key === '_field'
) {
newValues = [value]
} else {
Expand All @@ -302,6 +306,11 @@ export const selectTagValue = (index: number, value: string) => (

dispatch(setBuilderTagValuesSelection(index, newValues))

// don't add a new tag filter if we're grouping
if (currentTag.aggregateFunctionType === 'group') {
return
}

if (index === tags.length - 1 && newValues.length) {
dispatch(addTagSelector())
} else {
Expand Down
74 changes: 54 additions & 20 deletions ui/src/timeMachine/components/TagSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,22 @@ import {
} from 'src/timeMachine/actions/queryBuilder'

// Utils
import {toComponentStatus} from 'src/shared/utils/toComponentStatus'
import DefaultDebouncer from 'src/shared/utils/debouncer'
import {isFlagEnabled} from 'src/shared/utils/featureFlag'
import {toComponentStatus} from 'src/shared/utils/toComponentStatus'
import {
getActiveQuery,
getActiveTagValues,
getActiveTimeMachine,
getIsInCheckOverlay,
} from 'src/timeMachine/selectors'

// Types
import {AppState, RemoteDataState} from 'src/types'
import {BuilderAggregateFunctionType} from 'src/client'
import {
AppState,
BuilderAggregateFunctionType,
RemoteDataState,
} from 'src/types'

const SEARCH_DEBOUNCE_MS = 500

Expand Down Expand Up @@ -80,8 +85,6 @@ type Props = StateProps & DispatchProps & OwnProps
class TagSelector extends PureComponent<Props> {
private debouncer = new DefaultDebouncer()

// bucky: this will currently always be 'Filter'
// updates to this are imminent
private renderAggregateFunctionType(
aggregateFunctionType: BuilderAggregateFunctionType
) {
Expand All @@ -92,19 +95,32 @@ class TagSelector extends PureComponent<Props> {
}

public render() {
const {aggregateFunctionType, index} = this.props

return (
<BuilderCard>
<BuilderCard.Header
title={this.renderAggregateFunctionType(aggregateFunctionType)}
onDelete={index !== 0 && this.handleRemoveTagSelector}
/>
{this.header}
{this.body}
</BuilderCard>
)
}

private get header() {
const {aggregateFunctionType, index} = this.props

return isFlagEnabled('queryBuilderGrouping') ? (
hoorayimhelping marked this conversation as resolved.
Show resolved Hide resolved
<BuilderCard.DropdownHeader
options={['filter', 'group']}
selectedOption={this.renderAggregateFunctionType(aggregateFunctionType)}
onDelete={index !== 0 && this.handleRemoveTagSelector}
onSelect={this.handleAggregateFunctionSelect}
/>
) : (
<BuilderCard.Header
title={this.renderAggregateFunctionType(aggregateFunctionType)}
onDelete={index !== 0 && this.handleRemoveTagSelector}
/>
)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably be its own component.

Copy link
Contributor Author

@hoorayimhelping hoorayimhelping Dec 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@121watts it could, but I don't think it adds a ton of value. Once this feature flag goes away, the tag selector will have a BuilderCard.DropdownHeader header until we change the interaction, and the BuilderCard.Header will be applied to other parts of the query builder (like the aggregate functions window)

private get body() {
const {
index,
Expand Down Expand Up @@ -265,33 +281,51 @@ class TagSelector extends PureComponent<Props> {

onSearchValues(index)
}

private handleAggregateFunctionSelect = (
option: BuilderAggregateFunctionType
) => {
const {index, onSetBuilderAggregateFunctionType} = this.props
onSetBuilderAggregateFunctionType(option, index)
}
}

const mstp = (state: AppState, ownProps: OwnProps): StateProps => {
const activeQueryBuilder = getActiveTimeMachine(state).queryBuilder

const {
keys,
keysSearchTerm,
keysStatus,
values,
valuesSearchTerm,
valuesStatus,
} = getActiveTimeMachine(state).queryBuilder.tags[ownProps.index]
} = activeQueryBuilder.tags[ownProps.index]

const tags = getActiveQuery(state).builderConfig.tags
const {
key: selectedKey,
values: selectedValues,
aggregateFunctionType,
} = tags[ownProps.index]

let emptyText: string

if (ownProps.index === 0 || !tags[ownProps.index - 1].key) {
const previousTagSelector = tags[ownProps.index - 1]
if (
ownProps.index === 0 ||
!previousTagSelector ||
!previousTagSelector.key
) {
emptyText = ''
} else {
emptyText = `Select a ${tags[ownProps.index - 1].key} value first`
}

const {
key: selectedKey,
values: selectedValues,
aggregateFunctionType,
} = tags[ownProps.index]

hoorayimhelping marked this conversation as resolved.
Show resolved Hide resolved
const values = getActiveTagValues(
activeQueryBuilder.tags,
aggregateFunctionType,
ownProps.index
)
const isInCheckOverlay = getIsInCheckOverlay(state)

return {
Expand Down
2 changes: 2 additions & 0 deletions ui/src/timeMachine/components/builderCard/BuilderCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import classnames from 'classnames'

// Components
import BuilderCardHeader from 'src/timeMachine/components/builderCard/BuilderCardHeader'
import BuilderCardDropdownHeader from 'src/timeMachine/components/builderCard/BuilderCardDropdownHeader'
import BuilderCardMenu from 'src/timeMachine/components/builderCard/BuilderCardMenu'
import BuilderCardBody from 'src/timeMachine/components/builderCard/BuilderCardBody'
import BuilderCardEmpty from 'src/timeMachine/components/builderCard/BuilderCardEmpty'
Expand All @@ -16,6 +17,7 @@ interface Props {

export default class BuilderCard extends PureComponent<Props> {
public static Header = BuilderCardHeader
public static DropdownHeader = BuilderCardDropdownHeader
public static Menu = BuilderCardMenu
public static Body = BuilderCardBody
public static Empty = BuilderCardEmpty
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Libraries
import React, {PureComponent} from 'react'

import {SelectDropdown} from '@influxdata/clockface'
import {BuilderAggregateFunctionType} from 'src/types'

interface Props {
options: string[]
selectedOption: string
testID: string
onSelect?: (option: BuilderAggregateFunctionType) => void
onDelete?: () => void
}

const emptyFunction = () => {}

export default class BuilderCardDropdownHeader extends PureComponent<Props> {
public static defaultProps = {
testID: 'builder-card--header',
}

public render() {
const {children, options, onSelect, selectedOption, testID} = this.props

return (
<div className="builder-card--header" data-testid={testID}>
<SelectDropdown
options={options}
selectedOption={selectedOption}
testID="select-option-dropdown"
onSelect={onSelect ? onSelect : emptyFunction}
/>

{children}
{this.deleteButton}
</div>
)
}

private get deleteButton(): JSX.Element | undefined {
const {onDelete} = this.props

if (onDelete) {
return <div className="builder-card--delete" onClick={onDelete} />
}
}
}
16 changes: 13 additions & 3 deletions ui/src/timeMachine/reducers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -671,11 +671,16 @@ export const timeMachineReducer = (
const {index, builderAggregateFunctionType} = action.payload
const draftQuery = draftState.draftQueries[draftState.activeQueryIndex]

buildActiveQuery(draftState)
if (
draftQuery &&
draftQuery.builderConfig &&
draftQuery.builderConfig.tags[index]
) {
// When switching between filtering and grouping
// we want to clear out any previously selected values
draftQuery.builderConfig.tags[index].values = []

draftQuery.builderConfig.tags[
index
].aggregateFunctionType = builderAggregateFunctionType
Expand Down Expand Up @@ -761,6 +766,8 @@ export const timeMachineReducer = (

draftState.queryBuilder.tags[index].values = values
draftState.queryBuilder.tags[index].valuesStatus = RemoteDataState.Done

buildActiveQuery(draftState)
})
}

Expand Down Expand Up @@ -829,10 +836,13 @@ export const timeMachineReducer = (
const {index} = action.payload
const draftQuery = draftState.draftQueries[draftState.activeQueryIndex]

const selectedValues = draftQuery.builderConfig.tags[index].values
let selectedValues = []
if (draftQuery) {
selectedValues = draftQuery.builderConfig.tags[index].values

draftQuery.builderConfig.tags.splice(index, 1)
draftState.queryBuilder.tags.splice(index, 1)
draftQuery.builderConfig.tags.splice(index, 1)
draftState.queryBuilder.tags.splice(index, 1)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A reducer test for this added logic would be 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@121watts this change i made only adds a check that draftState exists. I'll add tests for this, but in a different ticket since this is kind of big and needs to get out.

#16294


if (selectedValues.length) {
buildActiveQuery(draftState)
Expand Down
Loading