Refactor: Change sqlstore.inTransaction to SQLStore.WithTransactionalDBSession in alert files (#43815)

* Refactor: Change sqlstore.inTransaction to SQLStore.WithTransactionalDBSession in alert files

* Fix: Revert second SaveAlerts back to normal func

* Refactor: Update tests so functions are now SQLStore methods

* Fix: Refactor pauseAlert and pauseAllAlerts to be SQLStore methods

* Fix: Refactor SaveAlerts to be SQLStore method

* Refactor: Update SaveAlerts func signature to have correct arguments

* Refactor: Define sqlStore

* Chore: Delete commented out code

* Chore: Remove unused SaveAlertsCommand

pkg/models/alert.go

@@ -127,14 +127,6 @@ func (a *Alert) GetTagsFromSettings() []*Tag {
 	return tags
 }
 
-type SaveAlertsCommand struct {
-	DashboardId int64
-	UserId      int64
-	OrgId       int64
-
-	Alerts []*Alert
-}
-
 type PauseAlertCommand struct {
 	OrgId       int64
 	AlertIds    []int64

pkg/services/sqlstore/alert.go

@@ -15,14 +15,13 @@ import (
 var timeNow = time.Now
 
 func (ss *SQLStore) addAlertQueryAndCommandHandlers() {
-	bus.AddHandler("sql", SaveAlerts)
 	bus.AddHandler("sql", ss.HandleAlertsQuery)
 	bus.AddHandler("sql", ss.GetAlertById)
 	bus.AddHandler("sql", ss.GetAllAlertQueryHandler)
 	bus.AddHandler("sql", ss.SetAlertState)
 	bus.AddHandler("sql", ss.GetAlertStatesForDashboard)
-	bus.AddHandler("sql", PauseAlert)
+	bus.AddHandler("sql", ss.PauseAlert)
-	bus.AddHandler("sql", PauseAllAlerts)
+	bus.AddHandler("sql", ss.PauseAllAlerts)
 }
 
 func (ss *SQLStore) GetAlertById(ctx context.Context, query *models.GetAlertByIdQuery) error {
@@ -174,7 +173,7 @@ func deleteAlertDefinition(dashboardId int64, sess *DBSession) error {
 }
 
 func (ss *SQLStore) SaveAlerts(ctx context.Context, dashID int64, alerts []*models.Alert) error {
-	return ss.WithTransactionalDbSession(context.Background(), func(sess *DBSession) error {
+	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
 		existingAlerts, err := GetAlertsByDashboardId2(dashID, sess)
 		if err != nil {
 			return err
@@ -192,25 +191,6 @@ func (ss *SQLStore) SaveAlerts(ctx context.Context, dashID int64, alerts []*mode
 	})
 }
 
-func SaveAlerts(ctx context.Context, cmd *models.SaveAlertsCommand) error {
-	return inTransaction(func(sess *DBSession) error {
-		existingAlerts, err := GetAlertsByDashboardId2(cmd.DashboardId, sess)
-		if err != nil {
-			return err
-		}
-
-		if err := updateAlerts(existingAlerts, cmd.Alerts, sess); err != nil {
-			return err
-		}
-
-		if err := deleteMissingAlerts(existingAlerts, cmd.Alerts, sess); err != nil {
-			return err
-		}
-
-		return nil
-	})
-}
-
 func updateAlerts(existingAlerts []*models.Alert, alerts []*models.Alert, sess *DBSession) error {
 	for _, alert := range alerts {
 		update := false
@@ -344,8 +324,8 @@ func (ss *SQLStore) SetAlertState(ctx context.Context, cmd *models.SetAlertState
 	})
 }
 
-func PauseAlert(ctx context.Context, cmd *models.PauseAlertCommand) error {
+func (ss *SQLStore) PauseAlert(ctx context.Context, cmd *models.PauseAlertCommand) error {
-	return inTransaction(func(sess *DBSession) error {
+	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
 		if len(cmd.AlertIds) == 0 {
 			return fmt.Errorf("command contains no alertids")
 		}
@@ -378,8 +358,8 @@ func PauseAlert(ctx context.Context, cmd *models.PauseAlertCommand) error {
 	})
 }
 
-func PauseAllAlerts(ctx context.Context, cmd *models.PauseAllAlertCommand) error {
+func (ss *SQLStore) PauseAllAlerts(ctx context.Context, cmd *models.PauseAllAlertCommand) error {
-	return inTransaction(func(sess *DBSession) error {
+	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
 		var newState string
 		if cmd.Paused {
 			newState = string(models.AlertStatePaused)

pkg/services/sqlstore/alert_test.go

@@ -34,7 +34,6 @@ func TestAlertingDataAccess(t *testing.T) {
 
 	var sqlStore *SQLStore
 	var testDash *models.Dashboard
-	var cmd models.SaveAlertsCommand
 	var items []*models.Alert
 
 	setup := func(t *testing.T) {
@@ -56,14 +55,7 @@ func TestAlertingDataAccess(t *testing.T) {
 			},
 		}
 
-		cmd = models.SaveAlertsCommand{
+		err = sqlStore.SaveAlerts(context.Background(), testDash.Id, items)
-			Alerts:      items,
-			DashboardId: testDash.Id,
-			OrgId:       1,
-			UserId:      1,
-		}
-
-		err = SaveAlerts(context.Background(), &cmd)
 		require.Nil(t, err)
 	}
 
@@ -91,7 +83,7 @@ func TestAlertingDataAccess(t *testing.T) {
 		stateDateBeforePause := alert.NewStateDate
 
 		t.Run("can pause all alerts", func(t *testing.T) {
-			err := pauseAllAlerts(t, true)
+			err := sqlStore.pauseAllAlerts(t, true)
 			require.Nil(t, err)
 
 			t.Run("cannot updated paused alert", func(t *testing.T) {
@@ -117,7 +109,7 @@ func TestAlertingDataAccess(t *testing.T) {
 			})
 
 			t.Run("unpausing alerts should update their NewStateDate again", func(t *testing.T) {
-				err := pauseAllAlerts(t, false)
+				err := sqlStore.pauseAllAlerts(t, false)
 				require.Nil(t, err)
 				alert, _ = getAlertById(t, insertedAlert.Id, sqlStore)
 				stateDateAfterUnpause := alert.NewStateDate
@@ -162,14 +154,7 @@ func TestAlertingDataAccess(t *testing.T) {
 		modifiedItems := items
 		modifiedItems[0].Name = "Name"
 
-		modifiedCmd := models.SaveAlertsCommand{
+		err := sqlStore.SaveAlerts(context.Background(), testDash.Id, items)
-			DashboardId: testDash.Id,
-			OrgId:       1,
-			UserId:      1,
-			Alerts:      modifiedItems,
-		}
-
-		err := SaveAlerts(context.Background(), &modifiedCmd)
 
 		t.Run("Can save alerts with same dashboard and panel id", func(t *testing.T) {
 			require.Nil(t, err)
@@ -189,7 +174,7 @@ func TestAlertingDataAccess(t *testing.T) {
 		})
 
 		t.Run("Updates without changes should be ignored", func(t *testing.T) {
-			err3 := SaveAlerts(context.Background(), &modifiedCmd)
+			err3 := sqlStore.SaveAlerts(context.Background(), testDash.Id, items)
 			require.Nil(t, err3)
 		})
 	})
@@ -220,8 +205,7 @@ func TestAlertingDataAccess(t *testing.T) {
 			},
 		}
 
-		cmd.Alerts = multipleItems
+		err := sqlStore.SaveAlerts(context.Background(), testDash.Id, multipleItems)
-		err := SaveAlerts(context.Background(), &cmd)
 
 		t.Run("Should save 3 dashboards", func(t *testing.T) {
 			require.Nil(t, err)
@@ -236,8 +220,7 @@ func TestAlertingDataAccess(t *testing.T) {
 		t.Run("should updated two dashboards and delete one", func(t *testing.T) {
 			missingOneAlert := multipleItems[:2]
 
-			cmd.Alerts = missingOneAlert
+			err = sqlStore.SaveAlerts(context.Background(), testDash.Id, missingOneAlert)
-			err = SaveAlerts(context.Background(), &cmd)
 
 			t.Run("should delete the missing alert", func(t *testing.T) {
 				query := models.GetAlertsQuery{DashboardIDs: []int64{testDash.Id}, OrgId: 1, User: &models.SignedInUser{OrgRole: models.ROLE_ADMIN}}
@@ -259,14 +242,7 @@ func TestAlertingDataAccess(t *testing.T) {
 			},
 		}
 
-		cmd := models.SaveAlertsCommand{
+		err := sqlStore.SaveAlerts(context.Background(), testDash.Id, items)
-			Alerts:      items,
-			DashboardId: testDash.Id,
-			OrgId:       1,
-			UserId:      1,
-		}
-
-		err := SaveAlerts(context.Background(), &cmd)
 		require.Nil(t, err)
 
 		err = DeleteDashboard(context.Background(), &models.DeleteDashboardCommand{
@@ -293,7 +269,7 @@ func TestPausingAlerts(t *testing.T) {
 		sqlStore := InitTestDB(t)
 
 		testDash := insertTestDashboard(t, sqlStore, "dashboard with alerts", 1, 0, false, "alert")
-		alert, err := insertTestAlert("Alerting title", "Alerting message", testDash.OrgId, testDash.Id, simplejson.New())
+		alert, err := insertTestAlert("Alerting title", "Alerting message", testDash.OrgId, testDash.Id, simplejson.New(), sqlStore)
 		require.Nil(t, err)
 
 		stateDateBeforePause := alert.NewStateDate
@@ -307,7 +283,7 @@ func TestPausingAlerts(t *testing.T) {
 		insertedAlert := alertQuery.Result[0]
 
 		t.Run("when paused", func(t *testing.T) {
-			_, err := pauseAlert(t, testDash.OrgId, insertedAlert.Id, true)
+			_, err := sqlStore.pauseAlert(t, testDash.OrgId, insertedAlert.Id, true)
 			require.Nil(t, err)
 
 			t.Run("the NewStateDate should be updated", func(t *testing.T) {
@@ -320,7 +296,7 @@ func TestPausingAlerts(t *testing.T) {
 		})
 
 		t.Run("when unpaused", func(t *testing.T) {
-			_, err := pauseAlert(t, testDash.OrgId, insertedAlert.Id, false)
+			_, err := sqlStore.pauseAlert(t, testDash.OrgId, insertedAlert.Id, false)
 			require.Nil(t, err)
 
 			t.Run("the NewStateDate should be updated again", func(t *testing.T) {
@@ -333,17 +309,17 @@ func TestPausingAlerts(t *testing.T) {
 		})
 	})
 }
-func pauseAlert(t *testing.T, orgId int64, alertId int64, pauseState bool) (int64, error) {
+func (ss *SQLStore) pauseAlert(t *testing.T, orgId int64, alertId int64, pauseState bool) (int64, error) {
 	cmd := &models.PauseAlertCommand{
 		OrgId:    orgId,
 		AlertIds: []int64{alertId},
 		Paused:   pauseState,
 	}
-	err := PauseAlert(context.Background(), cmd)
+	err := ss.PauseAlert(context.Background(), cmd)
 	require.Nil(t, err)
 	return cmd.ResultCount, err
 }
-func insertTestAlert(title string, message string, orgId int64, dashId int64, settings *simplejson.Json) (*models.Alert, error) {
+func insertTestAlert(title string, message string, orgId int64, dashId int64, settings *simplejson.Json, ss *SQLStore) (*models.Alert, error) {
 	items := []*models.Alert{
 		{
 			PanelId:     1,
@@ -356,15 +332,8 @@ func insertTestAlert(title string, message string, orgId int64, dashId int64, se
 		},
 	}
 
-	cmd := models.SaveAlertsCommand{
+	err := ss.SaveAlerts(context.Background(), dashId, items)
-		Alerts:      items,
+	return items[0], err
-		DashboardId: dashId,
-		OrgId:       orgId,
-		UserId:      1,
-	}
-
-	err := SaveAlerts(context.Background(), &cmd)
-	return cmd.Alerts[0], err
 }
 
 func getAlertById(t *testing.T, id int64, ss *SQLStore) (*models.Alert, error) {
@@ -376,11 +345,11 @@ func getAlertById(t *testing.T, id int64, ss *SQLStore) (*models.Alert, error) {
 	return q.Result, err
 }
 
-func pauseAllAlerts(t *testing.T, pauseState bool) error {
+func (ss *SQLStore) pauseAllAlerts(t *testing.T, pauseState bool) error {
 	cmd := &models.PauseAllAlertCommand{
 		Paused: pauseState,
 	}
-	err := PauseAllAlerts(context.Background(), cmd)
+	err := ss.PauseAllAlerts(context.Background(), cmd)
 	require.Nil(t, err)
 	return err
 }