Просмотр исходного кода

fix(alerts): Send null on unassigned owner and correct default owner rule (#24476)

To be more consistent with other "empty" values on the frontend we send null on an unassigned owner instead of undefined which gets dropped upon reaching the backend.

Additionally I corrected the default owner in a new rule to be the first team that the user is a part of and is also a part of the project. Previously, it would pick any team the user is a member of which could result in that team not being part of the project.

In the below example, none of the teams I am a member of belong to the selected project so unassigned should be the default owner instead of sentry
David Wang 4 лет назад
Родитель
Сommit
a402d0be32

+ 3 - 3
src/sentry/static/sentry/app/components/selectMembers/index.tsx

@@ -45,7 +45,7 @@ const StyledIconUser = styled(IconUser)`
 `;
 
 const unassignedOption = {
-  value: undefined,
+  value: null,
   label: (
     <UnassignedWrapper>
       <StyledIconUser size="20px" color="gray400" />
@@ -53,7 +53,7 @@ const unassignedOption = {
     </UnassignedWrapper>
   ),
   searchKey: 'unassigned',
-  actor: undefined,
+  actor: null,
 };
 type MentionableUnassigned = typeof unassignedOption;
 
@@ -257,7 +257,7 @@ class SelectMembers extends React.Component<Props, State> {
     const {options} = this.state;
 
     // Copy old value
-    const oldValue = [...value];
+    const oldValue = value ? [...value] : {value};
 
     // Optimistic update
     this.props.onChange(this.createMentionableTeam(team));

+ 1 - 1
src/sentry/static/sentry/app/types/alerts.tsx

@@ -70,7 +70,7 @@ export type UnsavedIssueAlertRule = {
   environment?: null | string;
   frequency: number;
   name: string;
-  owner?: string;
+  owner?: string | null;
 };
 
 // Issue-based alert rule

+ 3 - 1
src/sentry/static/sentry/app/views/settings/incidentRules/create.tsx

@@ -46,7 +46,9 @@ class IncidentRulesCreate extends React.Component<Props> {
     const userTeamIds = new Set(userTeamIdArr);
 
     if (props.organization.features.includes('team-alerts-ownership')) {
-      defaultRule.owner = userTeamIdArr[0] && `team:${userTeamIdArr[0]}`;
+      const projectTeamIds = new Set(project.teams.map(({id}) => id));
+      const defaultOwnerId = userTeamIdArr.find(id => projectTeamIds.has(id)) ?? null;
+      defaultRule.owner = defaultOwnerId && `team:${defaultOwnerId}`;
     }
 
     return (

+ 1 - 1
src/sentry/static/sentry/app/views/settings/incidentRules/types.tsx

@@ -62,7 +62,7 @@ export type UnsavedIncidentRule = {
   thresholdType: AlertRuleThresholdType;
   resolveThreshold: number | '' | null;
   eventTypes?: EventTypes[];
-  owner?: string;
+  owner?: string | null;
 };
 
 export type SavedIncidentRule = UnsavedIncidentRule & {

+ 10 - 13
src/sentry/static/sentry/app/views/settings/projectAlerts/issueRuleEditor/index.tsx

@@ -129,7 +129,7 @@ class IssueRuleEditor extends AsyncView<Props, State> {
   }
 
   getDefaultState() {
-    const {organization, teams} = this.props;
+    const {organization, teams, project} = this.props;
     const defaultState = {
       ...super.getDefaultState(),
       configs: null,
@@ -139,8 +139,10 @@ class IssueRuleEditor extends AsyncView<Props, State> {
       uuid: null,
     };
     if (organization.features.includes('team-alerts-ownership')) {
-      const userTeam = teams.find(({isMember}) => !!isMember);
-      defaultState.rule.owner = userTeam ? `team:${userTeam.id}` : undefined;
+      const projectTeamIds = new Set(project.teams.map(({id}) => id));
+      const userTeam =
+        teams.find(({isMember, id}) => !!isMember && projectTeamIds.has(id)) ?? null;
+      defaultState.rule.owner = userTeam && `team:${userTeam.id}`;
     }
     return defaultState;
   }
@@ -452,19 +454,14 @@ class IssueRuleEditor extends AsyncView<Props, State> {
 
   getTeamId = () => {
     const {rule} = this.state;
-    const owner = rule?.owner ?? '';
+    const owner = rule?.owner;
     // ownership follows the format team:<id>, just grab the id
-    return owner.split(':')[1];
+    return owner && owner.split(':')[1];
   };
 
-  handleOwnerChange = ({value}: {value?: string; label: string}) => {
-    if (value) {
-      // currently only supporting teams as owners
-      this.handleChange('owner', `team:${value}`);
-    } else {
-      // allow owner to be set to undefined (unassigned option)
-      this.handleChange('owner', value);
-    }
+  handleOwnerChange = ({value}: {value: string; label: string}) => {
+    const ownerValue = value && `team:${value}`;
+    this.handleChange('owner', ownerValue);
   };
 
   renderLoading() {