From 8eb56c6c010f9a55bebb31ed39c692cc7ff293a3 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Fri, 22 Nov 2024 14:10:22 -0800 Subject: [PATCH] chore(alerts): Remove fe code for activated alerts --- .../rules/metric/ruleConditionsForm.tsx | 189 +----------------- .../alerts/rules/metric/ruleForm.spec.tsx | 90 --------- .../views/alerts/rules/metric/ruleForm.tsx | 67 +------ 3 files changed, 2 insertions(+), 344 deletions(-) diff --git a/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx b/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx index 84cc270d232cc0..91f67d480bfeae 100644 --- a/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx +++ b/static/app/views/alerts/rules/metric/ruleConditionsForm.tsx @@ -35,7 +35,6 @@ import {SearchQueryBuilder} from 'sentry/components/searchQueryBuilder'; import {InvalidReason} from 'sentry/components/searchSyntax/parser'; import {t, tct} from 'sentry/locale'; import {space} from 'sentry/styles/space'; -import {ActivationConditionType, MonitorType} from 'sentry/types/alerts'; import type {SelectValue} from 'sentry/types/core'; import type {Tag, TagCollection} from 'sentry/types/group'; import type {InjectedRouter} from 'sentry/types/legacyReactRouter'; @@ -103,12 +102,6 @@ type Props = { isEditing: boolean; onComparisonDeltaChange: (value: number) => void; onFilterSearch: (query: string, isQueryValid) => void; - onMonitorTypeSelect: (activatedAlertFields: { - activationCondition?: ActivationConditionType | undefined; - monitorType?: MonitorType; - monitorWindowSuffix?: string | undefined; - monitorWindowValue?: number | undefined; - }) => void; onTimeWindowChange: (value: number) => void; organization: Organization; project: Project; @@ -118,7 +111,6 @@ type Props = { thresholdChart: React.ReactNode; timeWindow: number; // optional props - activationCondition?: ActivationConditionType; allowChangeEventTypes?: boolean; comparisonDelta?: number; disableProjectSelector?: boolean; @@ -127,7 +119,6 @@ type Props = { isForLlmMetric?: boolean; isTransactionMigration?: boolean; loadingProjects?: boolean; - monitorType?: number; }; type State = { @@ -483,16 +474,7 @@ class RuleConditionsForm extends PureComponent { } renderInterval() { - const { - organization, - disabled, - alertType, - timeWindow, - onTimeWindowChange, - project, - monitorType, - isForLlmMetric, - } = this.props; + const {organization, disabled, alertType, project, isForLlmMetric} = this.props; return ( @@ -520,118 +502,6 @@ class RuleConditionsForm extends PureComponent { required /> )} - - {monitorType !== MonitorType.ACTIVATED && ( - onTimeWindowChange(value)} - inline={false} - flexibleControlStateSize - /> - )} - - - ); - } - - renderMonitorTypeSelect() { - // TODO: disable select on edit - const { - activationCondition, - isEditing, - monitorType, - onMonitorTypeSelect, - onTimeWindowChange, - timeWindow, - } = this.props; - - return ( - - - -
{t('Select Monitor Type')}
-
-
- - - - isEditing - ? null - : onMonitorTypeSelect({ - monitorType: MonitorType.CONTINUOUS, - }) - } - > - {t('Continuous')} -
{t('Continuously monitor trends for the metrics outlined below')}
-
- - isEditing - ? null - : onMonitorTypeSelect({ - monitorType: MonitorType.ACTIVATED, - }) - } - > - Conditional - {monitorType === MonitorType.ACTIVATED ? ( - - {`${t('Monitor')} `} - - onMonitorTypeSelect({activationCondition: value}) - } - inline={false} - flexibleControlStateSize - size="xs" - /> - {` ${t('for')} `} - onTimeWindowChange(value)} - inline={false} - flexibleControlStateSize - size="xs" - /> - - ) : ( -
- {t('Temporarily monitor specified query given activation condition')} -
- )} -
-
); @@ -654,8 +524,6 @@ class RuleConditionsForm extends PureComponent { } = this.props; const {environments, filterKeys} = this.state; - const hasActivatedAlerts = organization.features.includes('activated-alert-rules'); - const environmentOptions: SelectValue[] = [ { value: null, @@ -692,7 +560,6 @@ class RuleConditionsForm extends PureComponent { )} /> )} - {hasActivatedAlerts && this.renderMonitorTypeSelect()} {!isErrorMigration && this.renderInterval()} {t('Filter events')} @@ -922,58 +789,4 @@ const FormRow = styled('div')<{columns?: number; noMargin?: boolean}>` `} `; -const MonitorSelect = styled('div')` - border-radius: ${p => p.theme.borderRadius}; - border: 1px solid ${p => p.theme.border}; - width: 100%; - display: grid; - grid-template-columns: 1fr 1fr; - height: 5rem; -`; - -type MonitorCardProps = { - isSelected: boolean; - /** - * Adds hover and focus states to the card - */ - position: 'left' | 'right'; - disabled?: boolean; -}; - -const MonitorCard = styled('div')` - padding: ${space(1)} ${space(2)}; - display: flex; - flex-grow: 1; - flex-direction: column; - cursor: ${p => (p.disabled || p.isSelected ? 'default' : 'pointer')}; - justify-content: center; - background-color: ${p => - p.disabled && !p.isSelected ? p.theme.backgroundSecondary : p.theme.background}; - - &:focus, - &:hover { - ${p => - p.disabled || p.isSelected - ? '' - : ` - outline: 1px solid ${p.theme.purple200}; - background-color: ${p.theme.backgroundSecondary}; - `} - } - - border-top-left-radius: ${p => (p.position === 'left' ? p.theme.borderRadius : 0)}; - border-bottom-left-radius: ${p => (p.position === 'left' ? p.theme.borderRadius : 0)}; - border-top-right-radius: ${p => (p.position !== 'left' ? p.theme.borderRadius : 0)}; - border-bottom-right-radius: ${p => (p.position !== 'left' ? p.theme.borderRadius : 0)}; - margin: ${p => - p.isSelected ? (p.position === 'left' ? '1px 2px 1px 0' : '1px 0 1px 2px') : 0}; - outline: ${p => (p.isSelected ? `2px solid ${p.theme.purple400}` : 'none')}; -`; - -const ActivatedAlertFields = styled('div')` - display: flex; - align-items: center; - justify-content: space-between; -`; - export default withApi(withProjects(withTags(RuleConditionsForm))); diff --git a/static/app/views/alerts/rules/metric/ruleForm.spec.tsx b/static/app/views/alerts/rules/metric/ruleForm.spec.tsx index bf85b658b97a09..49f2417cb767dc 100644 --- a/static/app/views/alerts/rules/metric/ruleForm.spec.tsx +++ b/static/app/views/alerts/rules/metric/ruleForm.spec.tsx @@ -9,7 +9,6 @@ import selectEvent from 'sentry-test/selectEvent'; import {addErrorMessage} from 'sentry/actionCreators/indicator'; import type FormModel from 'sentry/components/forms/model'; import ProjectsStore from 'sentry/stores/projectsStore'; -import {ActivationConditionType, MonitorType} from 'sentry/types/alerts'; import {metric} from 'sentry/utils/analytics'; import RuleFormContainer from 'sentry/views/alerts/rules/metric/ruleForm'; import { @@ -157,17 +156,6 @@ describe('Incident Rules Form', () => { expect(await screen.findByText('1 hour interval')).toBeInTheDocument(); }); - - it('renders time window for activated alerts', async () => { - createWrapper({ - rule: { - ...rule, - monitorType: MonitorType.CONTINUOUS, - }, - }); - - expect(await screen.findByText('1 hour interval')).toBeInTheDocument(); - }); }); describe('Creating a new rule', () => { @@ -301,84 +289,6 @@ describe('Incident Rules Form', () => { ); }); - // Activation condition - it('creates a rule with an activation condition', async () => { - organization.features = [ - ...organization.features, - 'mep-rollout-flag', - 'activated-alert-rules', - ]; - const rule = MetricRuleFixture({ - monitorType: MonitorType.ACTIVATED, - activationCondition: ActivationConditionType.RELEASE_CREATION, - }); - createWrapper({ - rule: { - ...rule, - id: undefined, - aggregate: 'count()', - eventTypes: ['transaction'], - dataset: 'transactions', - }, - }); - - expect(await screen.findByTestId('alert-total-events')).toHaveTextContent('Total5'); - - await userEvent.click(screen.getByLabelText('Save Rule')); - - expect(createRule).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ - data: expect.objectContaining({ - name: 'My Incident Rule', - projects: ['project-slug'], - aggregate: 'count()', - eventTypes: ['transaction'], - dataset: 'generic_metrics', - thresholdPeriod: 1, - }), - }) - ); - }); - - it('creates a continuous rule with activated rules enabled', async () => { - organization.features = [ - ...organization.features, - 'mep-rollout-flag', - 'activated-alert-rules', - ]; - const rule = MetricRuleFixture({ - monitorType: MonitorType.CONTINUOUS, - }); - createWrapper({ - rule: { - ...rule, - id: undefined, - aggregate: 'count()', - eventTypes: ['transaction'], - dataset: 'transactions', - }, - }); - - expect(await screen.findByTestId('alert-total-events')).toHaveTextContent('Total5'); - - await userEvent.click(screen.getByLabelText('Save Rule')); - - expect(createRule).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ - data: expect.objectContaining({ - name: 'My Incident Rule', - projects: ['project-slug'], - aggregate: 'count()', - eventTypes: ['transaction'], - dataset: 'generic_metrics', - thresholdPeriod: 1, - }), - }) - ); - }); - it('creates an anomaly detection rule', async () => { organization.features = [ ...organization.features, diff --git a/static/app/views/alerts/rules/metric/ruleForm.tsx b/static/app/views/alerts/rules/metric/ruleForm.tsx index 42a58a4d20e659..6ac29e7e344c5c 100644 --- a/static/app/views/alerts/rules/metric/ruleForm.tsx +++ b/static/app/views/alerts/rules/metric/ruleForm.tsx @@ -25,7 +25,6 @@ import ListItem from 'sentry/components/list/listItem'; import {t, tct} from 'sentry/locale'; import IndicatorStore from 'sentry/stores/indicatorStore'; import {space} from 'sentry/styles/space'; -import {ActivationConditionType, MonitorType} from 'sentry/types/alerts'; import type {PlainRoute, RouteComponentProps} from 'sentry/types/legacyReactRouter'; import type { EventsStats, @@ -148,12 +147,10 @@ type State = { timeWindow: number; triggerErrors: Map; triggers: Trigger[]; - activationCondition?: ActivationConditionType; chartError?: boolean; chartErrorMessage?: string; comparisonDelta?: number; isExtrapolatedChartData?: boolean; - monitorType?: MonitorType; seasonality?: AlertRuleSeasonality; } & DeprecatedAsyncComponent['state']; @@ -202,7 +199,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent { } getDefaultState(): State { - const {rule, location, organization} = this.props; + const {rule, location} = this.props; const triggersClone = [...rule.triggers]; const { aggregate: _aggregate, @@ -227,8 +224,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { ? `is:unresolved ${rule.query ?? ''}` : rule.query ?? ''; - const hasActivatedAlerts = organization.features.includes('activated-alert-rules'); - return { ...super.getDefaultState(), currentData: [], @@ -259,12 +254,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { : AlertRuleComparisonType.COUNT, project: this.props.project, owner: rule.owner, - alertType: getAlertTypeFromAggregateDataset({aggregate, dataset}), - monitorType: hasActivatedAlerts - ? rule.monitorType || MonitorType.CONTINUOUS - : undefined, - activationCondition: - rule.activationCondition || ActivationConditionType.RELEASE_CREATION, }; } @@ -637,24 +626,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { this.setState({query, dataset, isQueryValid}); }; - handleMonitorTypeSelect = (activatedAlertFields: { - activationCondition?: ActivationConditionType | undefined; - monitorType?: MonitorType; - monitorWindowSuffix?: string | undefined; - monitorWindowValue?: number | undefined; - }) => { - const {monitorType} = activatedAlertFields; - let updatedFields = activatedAlertFields; - if (monitorType === MonitorType.CONTINUOUS) { - updatedFields = { - ...updatedFields, - activationCondition: undefined, - monitorWindowValue: undefined, - }; - } - this.setState(updatedFields as State); - }; - validateOnDemandMetricAlert() { if ( !isOnDemandMetricAlert(this.state.dataset, this.state.aggregate, this.state.query) @@ -665,18 +636,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { return !this.state.aggregate.includes(AggregationKey.PERCENTILE); } - validateActivatedAlerts() { - const {organization} = this.props; - const {monitorType, activationCondition, timeWindow} = this.state; - - const hasActivatedAlerts = organization.features.includes('activated-alert-rules'); - return ( - !hasActivatedAlerts || - monitorType !== MonitorType.ACTIVATED || - (activationCondition !== undefined && timeWindow) - ); - } - validateSubmit = model => { if (!this.validateMri()) { addErrorMessage(t('You need to select a metric before you can save the alert')); @@ -689,7 +648,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { const triggerErrors = this.validateTriggers(); const validTriggers = Array.from(triggerErrors).length === 0; const validOnDemandAlert = this.validateOnDemandMetricAlert(); - const validActivatedAlerts = this.validateActivatedAlerts(); if (!validTriggers) { this.setState(state => ({ @@ -715,13 +673,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { return false; } - if (!validActivatedAlerts) { - addErrorMessage( - t('Activation condition and monitor window must be set for activated alerts') - ); - return false; - } - return true; }; @@ -754,8 +705,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { comparisonDelta, timeWindow, eventTypes, - monitorType, - activationCondition, sensitivity, seasonality, comparisonType, @@ -767,7 +716,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { trigger.label !== AlertRuleTriggerType.WARNING || !isEmpty(trigger.alertThreshold) ); - const hasActivatedAlerts = organization.features.includes('activated-alert-rules'); // form model has all form state data, however we use local state to keep // track of the list of triggers (and actions within triggers) const loadingIndicator = IndicatorStore.addMessage( @@ -789,13 +737,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { metric.startSpan({name: 'saveAlertRule'}); - let activatedAlertFields = {}; - if (hasActivatedAlerts) { - activatedAlertFields = { - monitorType, - activationCondition, - }; - } const detectionTypes = new Map([ [AlertRuleComparisonType.COUNT, 'static'], [AlertRuleComparisonType.CHANGE, 'percent'], @@ -812,7 +753,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { { ...rule, // existing rule ...model.getTransformedData(), // form data - ...activatedAlertFields, projects: [project.slug], triggers: sanitizedTriggers, resolveThreshold: @@ -1281,8 +1221,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { alertType, isExtrapolatedChartData, triggersHaveChanged, - activationCondition, - monitorType, } = this.state; const wizardBuilderChart = this.renderTriggerChart(); @@ -1397,7 +1335,6 @@ class RuleFormContainer extends DeprecatedAsyncComponent { > { 'sum(ai.total_cost)', ].includes(aggregate)} isTransactionMigration={isMigration && !showErrorMigrationWarning} - monitorType={monitorType} onComparisonDeltaChange={value => this.handleFieldChange('comparisonDelta', value) } onFilterSearch={this.handleFilterUpdate} - onMonitorTypeSelect={this.handleMonitorTypeSelect} onTimeWindowChange={value => this.handleFieldChange('timeWindow', value)} organization={organization} project={project}