Squashed commit of the following:

commit dda167f Author: Nathaniel Cook <NathanielJCook@outlook.com> Date: Thu Sep 28 13:40:51 2023 -0700 mark many alerts as noise from alerts list (#3265) * add noiseReason to updateAlerts * fix type mismatch when setting alert IDs * move slice type cast * add feedback dialog * fix status not being update in list * return alert ids from updating feedback * add submit with snackbar notification * add cypress e2e test for many alerts noise reason * update icon * formatting and column name fix * deprecate setAlertNoiseReason * validate number of alerts * don't allow newStatus with noiseReason * move var decl * update titles for alert list noise feedback * add title test tag --------- Co-authored-by: Nathaniel Caza <mastercactapus@gmail.com>
target · Oct 6, 2023 · 79c160a · 79c160a
1 parent b0474f7
commit 79c160a
Show file tree

Hide file tree

Showing 12 changed files with 341 additions and 26 deletions.
diff --git a/alert/queries.sql b/alert/queries.sql
@@ -38,7 +38,7 @@ SELECT
 FROM
     alert_feedback
 WHERE
-    alert_id = ANY($1::int[]);
+    alert_id = ANY ($1::int[]);
 
 -- name: SetAlertFeedback :exec
 INSERT INTO alert_feedback(alert_id, noise_reason)
@@ -48,3 +48,15 @@ ON CONFLICT (alert_id)
         noise_reason = $2
     WHERE
         alert_feedback.alert_id = $1;
+
+-- name: SetManyAlertFeedback :many
+INSERT INTO alert_feedback(alert_id, noise_reason)
+    VALUES (unnest(@alert_ids::bigint[]), @noise_reason)
+ON CONFLICT (alert_id)
+    DO UPDATE SET
+        noise_reason = excluded.noise_reason
+    WHERE
+        alert_feedback.alert_id = excluded.alert_id
+    RETURNING
+        alert_id;
+
diff --git a/alert/store.go b/alert/store.go
@@ -822,6 +822,44 @@ func (s *Store) Feedback(ctx context.Context, alertIDs []int) ([]Feedback, error
 	return result, nil
 }
 
+func (s Store) UpdateManyAlertFeedback(ctx context.Context, noiseReason string, alertIDs []int) ([]int, error) {
+	err := permission.LimitCheckAny(ctx, permission.User)
+	if err != nil {
+		return nil, err
+	}
+
+	err = validate.Many(
+		validate.Range("AlertIDs", len(alertIDs), 1, maxBatch),
+		validate.Text("NoiseReason", noiseReason, 1, 255),
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	// GraphQL generates type of int[], while sqlc
+	// expects an int64[] as a result of the unnest function
+	ids := make([]int64, len(alertIDs))
+	for i, v := range alertIDs {
+		ids[i] = int64(v)
+	}
+
+	res, err := gadb.New(s.db).SetManyAlertFeedback(ctx, gadb.SetManyAlertFeedbackParams{
+		AlertIds:    ids,
+		NoiseReason: noiseReason,
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	// cast back to []int
+	updatedIDs := make([]int, len(res))
+	for i, v := range res {
+		updatedIDs[i] = int(v)
+	}
+
+	return updatedIDs, nil
+}
+
 func (s Store) UpdateFeedback(ctx context.Context, feedback *Feedback) error {
 	err := permission.LimitCheckAny(ctx, permission.System, permission.User)
 	if err != nil {

diff --git a/gadb/queries.sql.go b/gadb/queries.sql.go
diff --git a/graphql2/generated.go b/graphql2/generated.go
diff --git a/graphql2/graphqlapp/alert.go b/graphql2/graphqlapp/alert.go
@@ -22,6 +22,7 @@ import (
 	"github.com/target/goalert/service"
 	"github.com/target/goalert/util/log"
 	"github.com/target/goalert/util/timeutil"
+	"github.com/target/goalert/validation"
 	"github.com/target/goalert/validation/validate"
 )
 
@@ -500,24 +501,37 @@ func (m *Mutation) EscalateAlerts(ctx context.Context, ids []int) ([]alert.Alert
 }
 
 func (m *Mutation) UpdateAlerts(ctx context.Context, args graphql2.UpdateAlertsInput) ([]alert.Alert, error) {
-	var status alert.Status
-
-	err := validate.OneOf("Status", args.NewStatus, graphql2.AlertStatusStatusAcknowledged, graphql2.AlertStatusStatusClosed)
-	if err != nil {
-		return nil, err
+	if args.NewStatus != nil && args.NoiseReason != nil {
+		return nil, validation.NewGenericError("cannot set both 'newStatus' and 'noiseReason'")
 	}
 
-	switch args.NewStatus {
-	case graphql2.AlertStatusStatusAcknowledged:
-		status = alert.StatusActive
-	case graphql2.AlertStatusStatusClosed:
-		status = alert.StatusClosed
+	var updatedIDs []int
+	if args.NewStatus != nil {
+		err := validate.OneOf("Status", *args.NewStatus, graphql2.AlertStatusStatusAcknowledged, graphql2.AlertStatusStatusClosed)
+		if err != nil {
+			return nil, err
+		}
+
+		var status alert.Status
+		switch *args.NewStatus {
+		case graphql2.AlertStatusStatusAcknowledged:
+			status = alert.StatusActive
+		case graphql2.AlertStatusStatusClosed:
+			status = alert.StatusClosed
+		}
+
+		updatedIDs, err = m.AlertStore.UpdateManyAlertStatus(ctx, status, args.AlertIDs, nil)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	var updatedIDs []int
-	updatedIDs, err = m.AlertStore.UpdateManyAlertStatus(ctx, status, args.AlertIDs, nil)
-	if err != nil {
-		return nil, err
+	if args.NoiseReason != nil {
+		var err error
+		updatedIDs, err = m.AlertStore.UpdateManyAlertFeedback(ctx, *args.NoiseReason, args.AlertIDs)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	return m.AlertStore.FindMany(ctx, updatedIDs)

diff --git a/graphql2/models_gen.go b/graphql2/models_gen.go
diff --git a/graphql2/schema.graphql b/graphql2/schema.graphql
@@ -553,6 +553,7 @@ type Mutation {
 
   createAlert(input: CreateAlertInput!): Alert
   setAlertNoiseReason(input: SetAlertNoiseReasonInput!): Boolean!
+    @deprecated(reason: "Use updateAlerts instead with the noiseReason field.")
 
   createService(input: CreateServiceInput!): Service
   createEscalationPolicy(input: CreateEscalationPolicyInput!): EscalationPolicy
@@ -1006,7 +1007,8 @@ input UpdateAlertsInput {
   # List of alertIDs.
   alertIDs: [Int!]!
 
-  newStatus: AlertStatus!
+  newStatus: AlertStatus
+  noiseReason: String
 }
 
 input UpdateRotationInput {

diff --git a/web/src/app/alerts/AlertsList.tsx b/web/src/app/alerts/AlertsList.tsx
@@ -7,6 +7,7 @@ import {
   ArrowUpward as EscalateIcon,
   Check as AcknowledgeIcon,
   Close as CloseIcon,
+  ThumbDownOffAlt,
 } from '@mui/icons-material'
 
 import AlertsListFilter from './components/AlertsListFilter'
@@ -20,6 +21,7 @@ import { Time } from '../util/Time'
 import { NotificationContext } from '../main/SnackbarNotification'
 import ReactGA from 'react-ga4'
 import { useConfigValue } from '../util/RequireConfig'
+import AlertFeedbackDialog from './components/AlertFeedbackDialog'
 
 interface AlertsListProps {
   serviceID: string
@@ -35,8 +37,9 @@ interface StatusUnacknowledgedVariables {
 }
 
 interface MutationVariablesInput {
-  newStatus: string
   alertIDs: (string | number)[]
+  newStatus: string
+  noiseReason?: string
 }
 
 export const alertsListQuery = gql`
@@ -106,8 +109,14 @@ function getStatusFilter(s: string): string[] {
 export default function AlertsList(props: AlertsListProps): JSX.Element {
   const classes = useStyles()
 
+  // event sent to Google Analytics
   const [event, setEvent] = useState('')
   const [analyticsID] = useConfigValue('General.GoogleAnalyticsID') as [string]
+
+  // stores alertIDs, if length present, feedback dialog is shown
+  const [showFeedbackDialog, setShowFeedbackDialog] = useState<Array<string>>(
+    [],
+  )
   const [selectedCount, setSelectedCount] = useState(0)
   const [checkedCount, setCheckedCount] = useState(0)
 
@@ -148,6 +157,7 @@ export default function AlertsList(props: AlertsListProps): JSX.Element {
 
   const { setNotification } = useContext(NotificationContext)
 
+  // mutation to update alert status to either acknowledge, close, or escalate
   const [mutate] = useMutation(updateMutation, {
     onCompleted: (data) => {
       const numUpdated =
@@ -170,6 +180,7 @@ export default function AlertsList(props: AlertsListProps): JSX.Element {
     },
   })
 
+  // alertIDs passed onClick from ControlledPaginatedList "checkedItems"
   const makeUpdateAlerts =
     (newStatus: string) => (alertIDs: (string | number)[]) => {
       setCheckedCount(alertIDs.length)
@@ -191,9 +202,16 @@ export default function AlertsList(props: AlertsListProps): JSX.Element {
         case 'StatusClosed':
           setEvent('alertlist_closed')
           break
+        case 'noise':
+          setEvent('alertlist_noise')
+          break
       }
 
-      mutate({ mutation, variables })
+      if (newStatus === 'noise') {
+        setShowFeedbackDialog(alertIDs.map((id) => id.toString()))
+      } else {
+        mutate({ mutation, variables })
+      }
     }
 
   /*
@@ -268,6 +286,12 @@ export default function AlertsList(props: AlertsListProps): JSX.Element {
       }
     }
 
+    actions.push({
+      icon: <ThumbDownOffAlt />,
+      label: 'Mark as Noise',
+      onClick: makeUpdateAlerts('noise'),
+    })
+
     return actions
   }
 
@@ -325,6 +349,13 @@ export default function AlertsList(props: AlertsListProps): JSX.Element {
           />
         </Grid>
       </Grid>
+      <AlertFeedbackDialog
+        open={showFeedbackDialog.length > 0}
+        onClose={() => {
+          setShowFeedbackDialog([])
+        }}
+        alertIDs={showFeedbackDialog}
+      />
     </React.Fragment>
   )
 }
diff --git a/web/src/app/alerts/components/AlertFeedback.tsx b/web/src/app/alerts/components/AlertFeedback.tsx
@@ -25,8 +25,11 @@ export const mutation = gql`
 
 interface AlertFeedbackProps {
   alertID: number
+  alertIDs?: Array<number>
 }
 
+export const options = ['False positive', 'Not actionable', 'Poor details']
+
 export default function AlertFeedback(props: AlertFeedbackProps): JSX.Element {
   const { alertID } = props
 
@@ -37,8 +40,6 @@ export default function AlertFeedback(props: AlertFeedbackProps): JSX.Element {
     },
   })
 
-  const options = ['False positive', 'Not actionable', 'Poor details']
-
   const dataNoiseReason = data?.alert?.noiseReason ?? ''
 
   const getDefaults = (): [Array<string>, string] => {