From 1dd00b628fcbdefb0b05b96306c330c0cdd52acb Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Fri, 6 Jun 2025 10:06:15 -0700 Subject: [PATCH] feat(aci): Add above/below controls to monitor builder Adds threshold above/below configuration. Syncs the selection into the prioritize section. We have a meeting today on how to better configure or type the form model so that we don't have weird long strings everywhere. --- .../form/control/index.stories.tsx | 2 +- .../form/control/priorityControl.spec.tsx | 77 ++++++++-- .../form/control/priorityControl.tsx | 141 ++++++++---------- .../detectors/components/forms/metric.tsx | 106 ++++++++----- 4 files changed, 202 insertions(+), 124 deletions(-) diff --git a/static/app/components/workflowEngine/form/control/index.stories.tsx b/static/app/components/workflowEngine/form/control/index.stories.tsx index ae4cb78ad16d2c..2c4febfcb1c9ec 100644 --- a/static/app/components/workflowEngine/form/control/index.stories.tsx +++ b/static/app/components/workflowEngine/form/control/index.stories.tsx @@ -16,7 +16,7 @@ export default Storybook.story('Form Controls', story => {
- +
diff --git a/static/app/components/workflowEngine/form/control/priorityControl.spec.tsx b/static/app/components/workflowEngine/form/control/priorityControl.spec.tsx index 40124e7fccdead..43e232f8470c03 100644 --- a/static/app/components/workflowEngine/form/control/priorityControl.spec.tsx +++ b/static/app/components/workflowEngine/form/control/priorityControl.spec.tsx @@ -1,36 +1,89 @@ import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import Form from 'sentry/components/forms/form'; +import FormModel from 'sentry/components/forms/model'; import PriorityControl from 'sentry/components/workflowEngine/form/control/priorityControl'; import {PriorityLevel} from 'sentry/types/group'; describe('PriorityControl', function () { it('renders children', async function () { - render(); + const formModel = new FormModel({ + initialData: { + 'conditionGroup.conditions.0.type': 'above', + 'conditionGroup.conditions.0.comparison': '0', + 'conditionGroup.conditions.0.conditionResult': PriorityLevel.LOW, + }, + }); + render( +
+ + + ); - expect(await screen.findByText('Issue created')).toBeInTheDocument(); + expect(await screen.findByText('Above 0s')).toBeInTheDocument(); expect(await screen.findByTestId('priority-control-medium')).toBeInTheDocument(); expect(await screen.findByTestId('priority-control-high')).toBeInTheDocument(); }); + it('allows configuring priority', async function () { - const mock = jest.fn(); - render(); - await userEvent.click(await screen.findByRole('button')); + const formModel = new FormModel({ + initialData: { + 'conditionGroup.conditions.0.type': 'above', + 'conditionGroup.conditions.0.comparison': '0', + 'conditionGroup.conditions.0.conditionResult': PriorityLevel.LOW, + }, + }); + render( +
+ + + ); + expect(await screen.findByRole('button', {name: 'Low'})).toBeInTheDocument(); + expect(screen.getByText('Med')).toBeInTheDocument(); + expect(screen.getByText('High')).toBeInTheDocument(); + + await userEvent.click(screen.getByRole('button', {name: 'Low'})); await userEvent.click(await screen.findByRole('option', {name: 'High'})); - expect(mock).toHaveBeenCalledWith(PriorityLevel.HIGH); + expect(formModel.getValue('conditionGroup.conditions.0.conditionResult')).toBe( + PriorityLevel.HIGH + ); + // Check that the medium threshold is not visible + expect(screen.getAllByRole('button')).toHaveLength(1); }); + it('allows configuring medium threshold', async function () { - const mock = jest.fn(); - render(); + const formModel = new FormModel({ + initialData: { + 'conditionGroup.conditions.0.type': 'above', + 'conditionGroup.conditions.0.comparison': '0', + 'conditionGroup.conditions.0.conditionResult': PriorityLevel.LOW, + }, + }); + render( +
+ + + ); const medium = await screen.findByTestId('priority-control-medium'); await userEvent.type(medium, '12'); - expect(mock).toHaveBeenCalledWith(PriorityLevel.MEDIUM, 12); + expect(formModel.getValue('conditionGroup.conditions.1.comparison')).toBe('12'); }); it('allows configuring high value', async function () { - const mock = jest.fn(); - render(); + const formModel = new FormModel({ + initialData: { + 'conditionGroup.conditions.0.type': 'above', + 'conditionGroup.conditions.0.comparison': '0', + 'conditionGroup.conditions.0.conditionResult': PriorityLevel.LOW, + }, + }); + render( +
+ + + ); const high = await screen.findByTestId('priority-control-high'); await userEvent.type(high, '12'); - expect(mock).toHaveBeenCalledWith(PriorityLevel.HIGH, 12); + expect(formModel.getValue('conditionGroup.conditions.2.comparison')).toBe('12'); }); }); diff --git a/static/app/components/workflowEngine/form/control/priorityControl.tsx b/static/app/components/workflowEngine/form/control/priorityControl.tsx index 4004f0af6e4106..0d1718fbb46af9 100644 --- a/static/app/components/workflowEngine/form/control/priorityControl.tsx +++ b/static/app/components/workflowEngine/form/control/priorityControl.tsx @@ -1,72 +1,72 @@ -import {useCallback, useState} from 'react'; +import {useContext} from 'react'; import styled from '@emotion/styled'; import {GroupPriorityBadge} from 'sentry/components/badge/groupPriority'; import {Flex} from 'sentry/components/container/flex'; -import {CompactSelect, type SelectOption} from 'sentry/components/core/compactSelect'; +import {CompactSelect} from 'sentry/components/core/compactSelect'; import {FieldWrapper} from 'sentry/components/forms/fieldGroup/fieldWrapper'; import NumberField from 'sentry/components/forms/fields/numberField'; +import FormContext from 'sentry/components/forms/formContext'; import InteractionStateLayer from 'sentry/components/interactionStateLayer'; +import {useFormField} from 'sentry/components/workflowEngine/form/hooks'; import {IconArrow, IconChevron} from 'sentry/icons'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import {PriorityLevel} from 'sentry/types/group'; -interface PriorityControlGridProps { - name: string; - onPriorityChange?: (value: PriorityLevel) => void; - onThresholdChange?: (level: PriorityLevel, threshold: number) => void; - priority?: PriorityLevel; - thresholds?: PriorityThresholds; +function ThresholdPriority() { + const lowThresholdDirection = useFormField('conditionGroup.conditions.0.type')!; + const lowThreshold = useFormField('conditionGroup.conditions.0.comparison')!; + return ( +
+ {lowThresholdDirection === '' + ? t('Above') + : lowThresholdDirection === 'above' + ? t('Above') + : t('Below')}{' '} + {lowThreshold === '' ? '0s' : lowThreshold + 's'} +
+ ); } -interface PriorityThresholds { - high?: number; - medium?: number; +function ChangePriority() { + const lowThresholdDirection = useFormField('conditionGroup.conditions.0.type')!; + const lowThreshold = useFormField('conditionGroup.conditions.0.comparison')!; + return ( +
+ {lowThreshold === '' ? '0' : lowThreshold}%{' '} + {lowThresholdDirection === '' + ? t('higher') + : lowThresholdDirection === 'higher' + ? t('higher') + : t('lower')} +
+ ); } -export default function PriorityControl({ - name, - priority: initialPriority, - onPriorityChange, - thresholds: initialThresholds, - onThresholdChange, -}: PriorityControlGridProps) { - const [priority, setPriority] = useState( - initialPriority ?? PriorityLevel.LOW - ); - const [thresholds, setThresholds] = useState( - initialThresholds ?? {} - ); - const setCreatedPriority = useCallback( - (level: PriorityLevel) => { - setPriority(level); - onPriorityChange?.(level); - }, - [setPriority, onPriorityChange] - ); - const setMediumThreshold = useCallback( - (threshold: number) => { - setThresholds(v => ({...v, [PriorityLevel.MEDIUM]: threshold})); - onThresholdChange?.(PriorityLevel.MEDIUM, threshold); - }, - [setThresholds, onThresholdChange] - ); - const setHighThreshold = useCallback( - (threshold: number) => { - setThresholds(v => ({...v, [PriorityLevel.HIGH]: threshold})); - onThresholdChange?.(PriorityLevel.HIGH, threshold); - }, - [setThresholds, onThresholdChange] - ); +export default function PriorityControl() { + // TODO: kind type not yet available from detector types + const detectorKind = useFormField('kind')!; + const conditionResult = + useFormField('conditionGroup.conditions.0.conditionResult') || + PriorityLevel.LOW; return ( {t('Issue created')}} - right={} + left={ + + {!detectorKind || detectorKind === 'threshold' ? ( + + ) : ( + + )} + ({t('issue created')}) + + } + right={} /> - {priorityIsConfigurable(priority, PriorityLevel.MEDIUM) && ( + {priorityIsConfigurable(conditionResult, PriorityLevel.MEDIUM) && ( setMediumThreshold(Number(threshold))} - name={`${name}-medium`} + name={`conditionGroup.conditions.1.comparison`} data-test-id="priority-control-medium" /> } right={} /> )} - {priorityIsConfigurable(priority, PriorityLevel.HIGH) && ( + {priorityIsConfigurable(conditionResult, PriorityLevel.HIGH) && ( setHighThreshold(Number(threshold))} - name={`${name}-high`} + name={`conditionGroup.conditions.2.comparison`} data-test-id="priority-control-high" /> } @@ -143,21 +137,11 @@ function PrioritizeRow({left, right}: {left: React.ReactNode; right: React.React const priorities = [PriorityLevel.LOW, PriorityLevel.MEDIUM, PriorityLevel.HIGH]; -function PrioritySelect({ - value: initialValue, - onChange = () => {}, -}: { - onChange?: (value: PriorityLevel) => void; - value?: PriorityLevel; -}) { - const [value, setValue] = useState(initialValue ?? PriorityLevel.HIGH); - const handleChange = useCallback( - (select: SelectOption) => { - onChange(select.value); - setValue(select.value); - }, - [onChange, setValue] - ); +function PrioritySelect() { + const formContext = useContext(FormContext); + const conditionResult = + useFormField('conditionGroup.conditions.0.conditionResult') || + PriorityLevel.LOW; return ( { return ( - + @@ -177,8 +161,10 @@ function PrioritySelect({ value: priority, textValue: priority, }))} - value={value} - onChange={handleChange} + value={conditionResult} + onChange={({value}) => { + formContext.form?.setValue('conditionGroup.conditions.0.conditionResult', value); + }} /> ); } @@ -212,3 +198,8 @@ const Cell = styled(Flex)` width: 5rem; } `; + +const SecondaryLabel = styled('div')` + font-size: ${p => p.theme.fontSizeSmall}; + color: ${p => p.theme.subText}; +`; diff --git a/static/app/views/detectors/components/forms/metric.tsx b/static/app/views/detectors/components/forms/metric.tsx index 5e64a18bf71dbe..db7ef0c6f4a453 100644 --- a/static/app/views/detectors/components/forms/metric.tsx +++ b/static/app/views/detectors/components/forms/metric.tsx @@ -1,4 +1,4 @@ -import {useMemo} from 'react'; +import {useContext, useMemo} from 'react'; import styled from '@emotion/styled'; import {Flex} from 'sentry/components/container/flex'; @@ -8,6 +8,7 @@ import SegmentedRadioField from 'sentry/components/forms/fields/segmentedRadioFi import SelectField from 'sentry/components/forms/fields/selectField'; import SentryMemberTeamSelectorField from 'sentry/components/forms/fields/sentryMemberTeamSelectorField'; import Form from 'sentry/components/forms/form'; +import FormContext from 'sentry/components/forms/formContext'; import type FormModel from 'sentry/components/forms/model'; import Spinner from 'sentry/components/forms/spinner'; import {SearchQueryBuilder} from 'sentry/components/searchQueryBuilder'; @@ -42,15 +43,56 @@ export function MetricDetectorForm({model}: {model: FormModel}) { - + ); } +function MonitorKind() { + const formContext = useContext(FormContext); + + /** + * Reset other fields when kind changes + */ + function handleChangeKind(kind: MetricDetectorKind) { + if (kind === 'threshold') { + formContext.form?.setValue('conditionGroup.conditions.0.type', 'above'); + } else if (kind === 'change') { + formContext.form?.setValue('conditionGroup.conditions.0.type', 'higher'); + } else if (kind === 'dynamic') { + formContext.form?.setValue('conditionGroup.conditions.0.type', 'above'); + } + } + + return ( + + ); +} + function ResolveSection() { const kind = useFormField('kind')!; @@ -120,7 +162,7 @@ function PrioritizeSection() { : t('Update issue priority when the following thresholds are met:') } > - {kind !== 'dynamic' && } + {kind !== 'dynamic' && } ); @@ -163,40 +205,32 @@ function DetectSection() { - + {(!kind || kind === 'threshold') && ( - - {t('An issue will be created when query value exceeds:')} - - + {t('An issue will be created when query value is:')} + + + + )} {kind === 'change' && ( @@ -204,14 +238,14 @@ function DetectSection() { {t('An issue will be created when query value is:')} {t('percent')}