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: Disable saving threshold check if no threshold selected #15348

Merged
merged 6 commits into from
Oct 8, 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
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@

## v2.0.0-alpha.19 [unreleased]

### Features
1. [15313](https://github.com/influxdata/influxdb/pull/15313): Add shortcut for toggling comments in script editor

### Bug Fixes
1. [15295](https://github.com/influxdata/influxdb/pull/15295): Ensures users are created with an active status
2. [15306](https://github.com/influxdata/influxdb/pull/15306): Added missing string values for CacheStatus type
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

## v2.0.0-alpha.18 [2019-09-26]

Expand Down
6 changes: 6 additions & 0 deletions ui/cypress/e2e/checks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ describe('Checks', () => {

cy.getByTestID(`selector-list ${field}`).click()

cy.getByTestID('save-cell--button').should('be.disabled')

cy.getByTestID('checkeo--header alerting-tab').click()

cy.getByTestID('add-threshold-condition-WARN').click()

cy.getByTestID('save-cell--button').should('be.enabled')
})
})
7 changes: 6 additions & 1 deletion ui/src/alerting/components/CheckEOHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ const CheckEOHeader: FC<Props> = ({
}

const saveButtonStatus = () => {
if (!isCheckSaveable(draftQueries, check.type)) {
if (!isCheckSaveable(draftQueries, check)) {
return ComponentStatus.Disabled
}

Expand All @@ -98,6 +98,10 @@ const CheckEOHeader: FC<Props> = ({
}

const {singleField, singleAggregateFunc} = isDraftQueryAlertable(draftQueries)
const oneOrMoreThresholds =
check.type === 'threshold'
? check.thresholds && !!check.thresholds.length
Copy link
Contributor

Choose a reason for hiding this comment

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

do we prefer !! over Boolean()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no strong preference but !! feels more readable?

: false

return (
<Page.Header fullWidth={true}>
Expand Down Expand Up @@ -130,6 +134,7 @@ const CheckEOHeader: FC<Props> = ({
checkType={check.type}
singleField={singleField}
singleAggregateFunc={singleAggregateFunc}
oneOrMoreThresholds={oneOrMoreThresholds}
/>
</Page.Header.Right>
</Page.Header>
Expand Down
18 changes: 13 additions & 5 deletions ui/src/alerting/components/CheckEOSaveButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ interface Props {
checkType: string
singleField: boolean
singleAggregateFunc: boolean
oneOrMoreThresholds: boolean
}

const CheckEOSaveButton: FunctionComponent<Props> = ({
Expand All @@ -31,6 +32,7 @@ const CheckEOSaveButton: FunctionComponent<Props> = ({
checkType,
singleField,
singleAggregateFunc,
oneOrMoreThresholds,
}) => {
return (
<Popover
Expand All @@ -42,14 +44,20 @@ const CheckEOSaveButton: FunctionComponent<Props> = ({
type={PopoverType.Outline}
contents={() => (
<div className="query-checklist--popover">
<p>{`To create a ${checkType} check, your query must include:`}</p>
<p>{`To create a ${checkType} check, you must select:`}</p>
<ul className="query-checklist--list">
<QueryChecklistItem text="One field" selected={singleField} />
{checkType === 'threshold' && (
<QueryChecklistItem
text="One aggregate function"
selected={singleAggregateFunc}
/>
<>
<QueryChecklistItem
text="One aggregate function"
selected={singleAggregateFunc}
/>
<QueryChecklistItem
text="One or more thresholds"
selected={oneOrMoreThresholds}
/>
</>
)}
</ul>
</div>
Expand Down
1 change: 1 addition & 0 deletions ui/src/alerting/components/builder/ThresholdCondition.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ const ThresholdCondition: FC<Props> = ({
color={LEVEL_COMPONENT_COLORS[level]}
size={ComponentSize.Large}
onClick={addLevel}
testID={`add-threshold-condition-${level}`}
/>
)
}
Expand Down
13 changes: 10 additions & 3 deletions ui/src/shared/components/dashed_button/DashedButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,30 @@ import {ComponentColor, ComponentSize} from '@influxdata/clockface'
interface Props {
text: string
onClick: (e: MouseEvent) => void
color: ComponentColor
size: ComponentSize
color?: ComponentColor
size?: ComponentSize
testID?: string
}

const DashedButton: FC<Props> = ({
text,
onClick,
color = ComponentColor.Primary,
size = ComponentSize.Medium,
testID = 'dashed-button',
}) => {
const classname = classnames('dashed-button', {
[`dashed-button__${color}`]: color,
[`dashed-button__${size}`]: size,
})

return (
<button className={classname} onClick={onClick} type="button">
<button
className={classname}
onClick={onClick}
type="button"
data-testid={testID}
>
{text}
</button>
)
Expand Down
15 changes: 11 additions & 4 deletions ui/src/timeMachine/utils/queryBuilder.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {get, isEmpty} from 'lodash'
import {BuilderConfig, DashboardDraftQuery} from 'src/types'
import {BuilderConfig, DashboardDraftQuery, Check} from 'src/types'
import {FUNCTIONS} from 'src/timeMachine/constants/queryBuilder'
import {
TIME_RANGE_START,
Expand Down Expand Up @@ -48,7 +48,7 @@ export const isDraftQueryAlertable = (

export const isCheckSaveable = (
draftQueries: DashboardDraftQuery[],
checkType: string
check: Partial<Check>
): boolean => {
const {
oneQuery,
Expand All @@ -57,11 +57,18 @@ export const isCheckSaveable = (
singleField,
} = isDraftQueryAlertable(draftQueries)

if (checkType === 'deadman') {
if (check.type === 'deadman') {
return oneQuery && builderMode && singleField
}

return oneQuery && builderMode && singleAggregateFunc && singleField
return (
oneQuery &&
builderMode &&
singleAggregateFunc &&
singleField &&
check.thresholds &&
!!check.thresholds.length
)
}

export function buildQuery(builderConfig: BuilderConfig): string {
Expand Down