Browse Source

disable monitorType on edit & update hover states & modify serializer… (#69541)

… to not allow update

![Screenshot 2024-04-23 at 2 18
28 PM](https://github.com/getsentry/sentry/assets/6186377/7e1d5f72-be0c-4889-91e9-5bf882e1691a)
Nathan Hsieh 10 months ago
parent
commit
5f6826e252

+ 6 - 0
src/sentry/incidents/serializers/alert_rule.py

@@ -503,6 +503,12 @@ class AlertRuleSerializer(CamelSnakeModelSerializer):
         triggers = validated_data.pop("triggers")
         if "id" in validated_data:
             validated_data.pop("id")
+        if "monitor_type" in validated_data:
+            """
+            TODO: enable monitor type editing
+            requires creating/disabling activations accordingly
+            """
+            validated_data.pop("monitor_type")
         with transaction.atomic(router.db_for_write(AlertRule)):
             alert_rule = update_alert_rule(
                 instance,

+ 32 - 14
static/app/views/alerts/rules/metric/ruleConditionsForm.tsx

@@ -68,6 +68,7 @@ type Props = {
   comparisonType: AlertRuleComparisonType;
   dataset: Dataset;
   disabled: boolean;
+  isEditing: boolean;
   onComparisonDeltaChange: (value: number) => void;
   onFilterSearch: (query: string, isQueryValid) => void;
   onMonitorTypeSelect: (activatedAlertFields: {
@@ -402,12 +403,14 @@ class RuleConditionsForm extends PureComponent<Props, State> {
   }
 
   renderMonitorTypeSelect() {
+    // TODO: disable select on edit
     const {
-      onMonitorTypeSelect,
-      monitorType,
       activationCondition,
-      timeWindow,
+      isEditing,
+      monitorType,
+      onMonitorTypeSelect,
       onTimeWindowChange,
+      timeWindow,
     } = this.props;
 
     return (
@@ -420,25 +423,32 @@ class RuleConditionsForm extends PureComponent<Props, State> {
         <FormRow>
           <MonitorSelect>
             <MonitorCard
+              disabled={isEditing}
               position="left"
               isSelected={monitorType === MonitorType.CONTINUOUS}
               onClick={() =>
-                onMonitorTypeSelect({
-                  monitorType: MonitorType.CONTINUOUS,
-                  activationCondition,
-                })
+                isEditing
+                  ? null
+                  : onMonitorTypeSelect({
+                      monitorType: MonitorType.CONTINUOUS,
+                      activationCondition,
+                    })
               }
             >
               <strong>{t('Continuous')}</strong>
               <div>{t('Continuously monitor trends for the metrics outlined below')}</div>
             </MonitorCard>
             <MonitorCard
+              disabled={isEditing}
               position="right"
               isSelected={monitorType === MonitorType.ACTIVATED}
               onClick={() =>
-                onMonitorTypeSelect({
-                  monitorType: MonitorType.ACTIVATED,
-                })
+                isEditing
+                  ? null
+                  : onMonitorTypeSelect({
+                      monitorType: MonitorType.ACTIVATED,
+                      activationCondition,
+                    })
               }
             >
               <strong>Conditional</strong>
@@ -448,6 +458,7 @@ class RuleConditionsForm extends PureComponent<Props, State> {
                   <SelectControl
                     name="activationCondition"
                     styles={this.selectControlStyles}
+                    disabled={isEditing}
                     options={[
                       {
                         value: ActivationConditionType.RELEASE_CREATION,
@@ -780,6 +791,7 @@ type MonitorCardProps = {
    * Adds hover and focus states to the card
    */
   position: 'left' | 'right';
+  disabled?: boolean;
 };
 
 const MonitorCard = styled('div')<MonitorCardProps>`
@@ -787,14 +799,20 @@ const MonitorCard = styled('div')<MonitorCardProps>`
   display: flex;
   flex-grow: 1;
   flex-direction: column;
-  cursor: pointer;
+  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 {
-    outline: 1px solid ${p => p.theme.purple200};
-    background-color: ${p => p.theme.backgroundSecondary};
-    margin: 0;
+    ${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)};

+ 4 - 2
static/app/views/alerts/rules/metric/ruleForm.tsx

@@ -32,7 +32,7 @@ import type {
   Organization,
   Project,
 } from 'sentry/types';
-import {type ActivationConditionType, MonitorType} from 'sentry/types/alerts';
+import {ActivationConditionType, MonitorType} from 'sentry/types/alerts';
 import {defined} from 'sentry/utils';
 import {metric, trackAnalytics} from 'sentry/utils/analytics';
 import type EventView from 'sentry/utils/discover/eventView';
@@ -237,7 +237,8 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
       monitorType: hasActivatedAlerts
         ? rule.monitorType || MonitorType.CONTINUOUS
         : undefined,
-      activationCondition: rule.activationCondition,
+      activationCondition:
+        rule.activationCondition || ActivationConditionType.RELEASE_CREATION,
     };
   }
 
@@ -1183,6 +1184,7 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
               monitorType={monitorType}
               activationCondition={activationCondition}
               onMonitorTypeSelect={this.handleMonitorTypeSelect}
+              isEditing={Boolean(ruleId)}
             />
             <AlertListItem>{t('Set thresholds')}</AlertListItem>
             {thresholdTypeForm(formDisabled)}

+ 1 - 0
tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py

@@ -391,6 +391,7 @@ class AlertRuleDetailsPutEndpointTest(AlertRuleDetailsBase):
         assert resp.data == serialize(alert_rule)
 
         # TODO: determine how to convert activated alert into continuous alert and vice versa (see logic.py)
+        # requires creating/disabling activations accordingly
         # assert resp.data["monitorType"] == AlertRuleMonitorType.ACTIVATED.value
         # assert (
         #     resp.data["activationCondition"]