refactoring: alert rule query refactoring (#10941)

pkg/api/alerting.go

@@ -52,6 +52,7 @@ func GetAlerts(c *middleware.Context) Response {
 		DashboardId: c.QueryInt64("dashboardId"),
 		DashboardId: c.QueryInt64("dashboardId"),
 		PanelId:     c.QueryInt64("panelId"),
 		PanelId:     c.QueryInt64("panelId"),
 		Limit:       c.QueryInt64("limit"),
 		Limit:       c.QueryInt64("limit"),
+		User:        c.SignedInUser,
 	}
 	}
 
 
 	states := c.QueryStrings("state")
 	states := c.QueryStrings("state")
@@ -63,74 +64,11 @@ func GetAlerts(c *middleware.Context) Response {
 		return ApiError(500, "List alerts failed", err)
 		return ApiError(500, "List alerts failed", err)
 	}
 	}
 
 
-	alertDTOs, resp := transformToDTOs(query.Result, c)
-	if resp != nil {
-		return resp
+	for _, alert := range query.Result {
+		alert.Url = models.GetDashboardUrl(alert.DashboardUid, alert.DashboardSlug)
 	}
 	}
 
 
-	return Json(200, alertDTOs)
-}
-
-func transformToDTOs(alerts []*models.Alert, c *middleware.Context) ([]*dtos.AlertRule, Response) {
-	if len(alerts) == 0 {
-		return []*dtos.AlertRule{}, nil
-	}
-
-	dashboardIds := make([]int64, 0)
-	alertDTOs := make([]*dtos.AlertRule, 0)
-	for _, alert := range alerts {
-		dashboardIds = append(dashboardIds, alert.DashboardId)
-		alertDTOs = append(alertDTOs, &dtos.AlertRule{
-			Id:             alert.Id,
-			DashboardId:    alert.DashboardId,
-			PanelId:        alert.PanelId,
-			Name:           alert.Name,
-			Message:        alert.Message,
-			State:          alert.State,
-			NewStateDate:   alert.NewStateDate,
-			ExecutionError: alert.ExecutionError,
-			EvalData:       alert.EvalData,
-		})
-	}
-
-	dashboardsQuery := models.GetDashboardsQuery{
-		DashboardIds: dashboardIds,
-	}
-
-	if err := bus.Dispatch(&dashboardsQuery); err != nil {
-		return nil, ApiError(500, "List alerts failed", err)
-	}
-
-	//TODO: should be possible to speed this up with lookup table
-	for _, alert := range alertDTOs {
-		for _, dash := range dashboardsQuery.Result {
-			if alert.DashboardId == dash.Id {
-				alert.Url = dash.GenerateUrl()
-				break
-			}
-		}
-	}
-
-	permissionsQuery := models.GetDashboardPermissionsForUserQuery{
-		DashboardIds: dashboardIds,
-		OrgId:        c.OrgId,
-		UserId:       c.SignedInUser.UserId,
-		OrgRole:      c.SignedInUser.OrgRole,
-	}
-
-	if err := bus.Dispatch(&permissionsQuery); err != nil {
-		return nil, ApiError(500, "List alerts failed", err)
-	}
-
-	for _, alert := range alertDTOs {
-		for _, perm := range permissionsQuery.Result {
-			if alert.DashboardId == perm.DashboardId {
-				alert.CanEdit = perm.Permission > 1
-			}
-		}
-	}
-
-	return alertDTOs, nil
+	return Json(200, query.Result)
 }
 }
 
 
 // POST /api/alerts/test
 // POST /api/alerts/test

pkg/models/alert.go

@@ -166,8 +166,9 @@ type GetAlertsQuery struct {
 	DashboardId int64
 	DashboardId int64
 	PanelId     int64
 	PanelId     int64
 	Limit       int64
 	Limit       int64
+	User        *SignedInUser
 
 
-	Result []*Alert
+	Result []*AlertListItemDTO
 }
 }
 
 
 type GetAllAlertsQuery struct {
 type GetAllAlertsQuery struct {
@@ -187,6 +188,21 @@ type GetAlertStatesForDashboardQuery struct {
 	Result []*AlertStateInfoDTO
 	Result []*AlertStateInfoDTO
 }
 }
 
 
+type AlertListItemDTO struct {
+	Id             int64            `json:"id"`
+	DashboardId    int64            `json:"dashboardId"`
+	DashboardUid   string           `json:"dashboardUid"`
+	DashboardSlug  string           `json:"dashboardSlug"`
+	PanelId        int64            `json:"panelId"`
+	Name           string           `json:"name"`
+	State          AlertStateType   `json:"state"`
+	NewStateDate   time.Time        `json:"newStateDate"`
+	EvalDate       time.Time        `json:"evalDate"`
+	EvalData       *simplejson.Json `json:"evalData"`
+	ExecutionError string           `json:"executionError"`
+	Url            string           `json:"url"`
+}
+
 type AlertStateInfoDTO struct {
 type AlertStateInfoDTO struct {
 	Id           int64          `json:"id"`
 	Id           int64          `json:"id"`
 	DashboardId  int64          `json:"dashboardId"`
 	DashboardId  int64          `json:"dashboardId"`

pkg/services/sqlstore/alert.go

@@ -61,52 +61,61 @@ func deleteAlertByIdInternal(alertId int64, reason string, sess *DBSession) erro
 }
 }
 
 
 func HandleAlertsQuery(query *m.GetAlertsQuery) error {
 func HandleAlertsQuery(query *m.GetAlertsQuery) error {
-	var sql bytes.Buffer
-	params := make([]interface{}, 0)
-
-	sql.WriteString(`SELECT *
-						from alert
-						`)
+	builder := SqlBuilder{}
+
+	builder.Write(`SELECT
+		alert.id,
+		alert.dashboard_id,
+		alert.panel_id,
+		alert.name,
+		alert.state,
+		alert.new_state_date,
+		alert.eval_date,
+		alert.execution_error,
+		dashboard.uid as dashboard_uid,
+		dashboard.slug as dashboard_slug
+		FROM alert
+		INNER JOIN dashboard on dashboard.id = alert.dashboard_id `)
 
 
-	sql.WriteString(`WHERE org_id = ?`)
-	params = append(params, query.OrgId)
+	builder.Write(`WHERE alert.org_id = ?`, query.OrgId)
 
 
 	if query.DashboardId != 0 {
 	if query.DashboardId != 0 {
-		sql.WriteString(` AND dashboard_id = ?`)
-		params = append(params, query.DashboardId)
+		builder.Write(` AND alert.dashboard_id = ?`, query.DashboardId)
 	}
 	}
 
 
 	if query.PanelId != 0 {
 	if query.PanelId != 0 {
-		sql.WriteString(` AND panel_id = ?`)
-		params = append(params, query.PanelId)
+		builder.Write(` AND alert.panel_id = ?`, query.PanelId)
 	}
 	}
 
 
 	if len(query.State) > 0 && query.State[0] != "all" {
 	if len(query.State) > 0 && query.State[0] != "all" {
-		sql.WriteString(` AND (`)
+		builder.Write(` AND (`)
 		for i, v := range query.State {
 		for i, v := range query.State {
 			if i > 0 {
 			if i > 0 {
-				sql.WriteString(" OR ")
+				builder.Write(" OR ")
 			}
 			}
 			if strings.HasPrefix(v, "not_") {
 			if strings.HasPrefix(v, "not_") {
-				sql.WriteString("state <> ? ")
+				builder.Write("state <> ? ")
 				v = strings.TrimPrefix(v, "not_")
 				v = strings.TrimPrefix(v, "not_")
 			} else {
 			} else {
-				sql.WriteString("state = ? ")
+				builder.Write("state = ? ")
 			}
 			}
-			params = append(params, v)
+			builder.AddParams(v)
 		}
 		}
-		sql.WriteString(")")
+		builder.Write(")")
 	}
 	}
 
 
-	sql.WriteString(" ORDER BY name ASC")
+	if query.User.OrgRole != m.ROLE_ADMIN {
+		builder.writeDashboardPermissionFilter(query.User, m.PERMISSION_EDIT)
+	}
+
+	builder.Write(" ORDER BY name ASC")
 
 
 	if query.Limit != 0 {
 	if query.Limit != 0 {
-		sql.WriteString(" LIMIT ?")
-		params = append(params, query.Limit)
+		builder.Write(" LIMIT ?", query.Limit)
 	}
 	}
 
 
-	alerts := make([]*m.Alert, 0)
-	if err := x.SQL(sql.String(), params...).Find(&alerts); err != nil {
+	alerts := make([]*m.AlertListItemDTO, 0)
+	if err := x.SQL(builder.GetSqlString(), builder.params...).Find(&alerts); err != nil {
 		return err
 		return err
 	}
 	}
 
 

pkg/services/sqlstore/alert_test.go

@@ -71,15 +71,13 @@ func TestAlertingDataAccess(t *testing.T) {
 		})
 		})
 
 
 		Convey("Can read properties", func() {
 		Convey("Can read properties", func() {
-			alertQuery := m.GetAlertsQuery{DashboardId: testDash.Id, PanelId: 1, OrgId: 1}
+			alertQuery := m.GetAlertsQuery{DashboardId: testDash.Id, PanelId: 1, OrgId: 1, User: &m.SignedInUser{OrgRole: m.ROLE_ADMIN}}
 			err2 := HandleAlertsQuery(&alertQuery)
 			err2 := HandleAlertsQuery(&alertQuery)
 
 
 			alert := alertQuery.Result[0]
 			alert := alertQuery.Result[0]
 			So(err2, ShouldBeNil)
 			So(err2, ShouldBeNil)
 			So(alert.Name, ShouldEqual, "Alerting title")
 			So(alert.Name, ShouldEqual, "Alerting title")
-			So(alert.Message, ShouldEqual, "Alerting message")
 			So(alert.State, ShouldEqual, "pending")
 			So(alert.State, ShouldEqual, "pending")
-			So(alert.Frequency, ShouldEqual, 1)
 		})
 		})
 
 
 		Convey("Alerts with same dashboard id and panel id should update", func() {
 		Convey("Alerts with same dashboard id and panel id should update", func() {
@@ -100,7 +98,7 @@ func TestAlertingDataAccess(t *testing.T) {
 			})
 			})
 
 
 			Convey("Alerts should be updated", func() {
 			Convey("Alerts should be updated", func() {
-				query := m.GetAlertsQuery{DashboardId: testDash.Id, OrgId: 1}
+				query := m.GetAlertsQuery{DashboardId: testDash.Id, OrgId: 1, User: &m.SignedInUser{OrgRole: m.ROLE_ADMIN}}
 				err2 := HandleAlertsQuery(&query)
 				err2 := HandleAlertsQuery(&query)
 
 
 				So(err2, ShouldBeNil)
 				So(err2, ShouldBeNil)
@@ -149,7 +147,7 @@ func TestAlertingDataAccess(t *testing.T) {
 			Convey("Should save 3 dashboards", func() {
 			Convey("Should save 3 dashboards", func() {
 				So(err, ShouldBeNil)
 				So(err, ShouldBeNil)
 
 
-				queryForDashboard := m.GetAlertsQuery{DashboardId: testDash.Id, OrgId: 1}
+				queryForDashboard := m.GetAlertsQuery{DashboardId: testDash.Id, OrgId: 1, User: &m.SignedInUser{OrgRole: m.ROLE_ADMIN}}
 				err2 := HandleAlertsQuery(&queryForDashboard)
 				err2 := HandleAlertsQuery(&queryForDashboard)
 
 
 				So(err2, ShouldBeNil)
 				So(err2, ShouldBeNil)
@@ -163,7 +161,7 @@ func TestAlertingDataAccess(t *testing.T) {
 				err = SaveAlerts(&cmd)
 				err = SaveAlerts(&cmd)
 
 
 				Convey("should delete the missing alert", func() {
 				Convey("should delete the missing alert", func() {
-					query := m.GetAlertsQuery{DashboardId: testDash.Id, OrgId: 1}
+					query := m.GetAlertsQuery{DashboardId: testDash.Id, OrgId: 1, User: &m.SignedInUser{OrgRole: m.ROLE_ADMIN}}
 					err2 := HandleAlertsQuery(&query)
 					err2 := HandleAlertsQuery(&query)
 					So(err2, ShouldBeNil)
 					So(err2, ShouldBeNil)
 					So(len(query.Result), ShouldEqual, 2)
 					So(len(query.Result), ShouldEqual, 2)
@@ -198,7 +196,7 @@ func TestAlertingDataAccess(t *testing.T) {
 			So(err, ShouldBeNil)
 			So(err, ShouldBeNil)
 
 
 			Convey("Alerts should be removed", func() {
 			Convey("Alerts should be removed", func() {
-				query := m.GetAlertsQuery{DashboardId: testDash.Id, OrgId: 1}
+				query := m.GetAlertsQuery{DashboardId: testDash.Id, OrgId: 1, User: &m.SignedInUser{OrgRole: m.ROLE_ADMIN}}
 				err2 := HandleAlertsQuery(&query)
 				err2 := HandleAlertsQuery(&query)
 
 
 				So(testDash.Id, ShouldEqual, 1)
 				So(testDash.Id, ShouldEqual, 1)

pkg/services/sqlstore/sqlbuilder.go

@@ -12,6 +12,22 @@ type SqlBuilder struct {
 	params []interface{}
 	params []interface{}
 }
 }
 
 
+func (sb *SqlBuilder) Write(sql string, params ...interface{}) {
+	sb.sql.WriteString(sql)
+
+	if len(params) > 0 {
+		sb.params = append(sb.params, params...)
+	}
+}
+
+func (sb *SqlBuilder) GetSqlString() string {
+	return sb.sql.String()
+}
+
+func (sb *SqlBuilder) AddParams(params ...interface{}) {
+	sb.params = append(sb.params, params...)
+}
+
 func (sb *SqlBuilder) writeDashboardPermissionFilter(user *m.SignedInUser, permission m.PermissionType) {
 func (sb *SqlBuilder) writeDashboardPermissionFilter(user *m.SignedInUser, permission m.PermissionType) {
 
 
 	if user.OrgRole == m.ROLE_ADMIN {
 	if user.OrgRole == m.ROLE_ADMIN {

public/app/containers/AlertRuleList/AlertRuleList.tsx

@@ -147,8 +147,7 @@ export class AlertRuleItem extends React.Component<AlertRuleItemProps, any> {
         <div className="alert-rule-item__body">
         <div className="alert-rule-item__body">
           <div className="alert-rule-item__header">
           <div className="alert-rule-item__header">
             <div className="alert-rule-item__name">
             <div className="alert-rule-item__name">
-              {rule.canEdit && <a href={ruleUrl}>{this.renderText(rule.name)}</a>}
-              {!rule.canEdit && <span>{this.renderText(rule.name)}</span>}
+              <a href={ruleUrl}>{this.renderText(rule.name)}</a>
             </div>
             </div>
             <div className="alert-rule-item__text">
             <div className="alert-rule-item__text">
               <span className={`${rule.stateClass}`}>{this.renderText(rule.stateText)}</span>
               <span className={`${rule.stateClass}`}>{this.renderText(rule.stateText)}</span>
@@ -163,24 +162,12 @@ export class AlertRuleItem extends React.Component<AlertRuleItemProps, any> {
             className="btn btn-small btn-inverse alert-list__btn width-2"
             className="btn btn-small btn-inverse alert-list__btn width-2"
             title="Pausing an alert rule prevents it from executing"
             title="Pausing an alert rule prevents it from executing"
             onClick={this.toggleState}
             onClick={this.toggleState}
-            disabled={!rule.canEdit}
           >
           >
             <i className={stateClass} />
             <i className={stateClass} />
           </button>
           </button>
-          {rule.canEdit && (
           <a className="btn btn-small btn-inverse alert-list__btn width-2" href={ruleUrl} title="Edit alert rule">
           <a className="btn btn-small btn-inverse alert-list__btn width-2" href={ruleUrl} title="Edit alert rule">
             <i className="icon-gf icon-gf-settings" />
             <i className="icon-gf icon-gf-settings" />
           </a>
           </a>
-          )}
-          {!rule.canEdit && (
-            <button
-              className="btn btn-small btn-inverse alert-list__btn width-2"
-              title="Edit alert rule"
-              disabled={true}
-            >
-              <i className="icon-gf icon-gf-settings" />
-            </button>
-          )}
         </div>
         </div>
       </li>
       </li>
     );
     );

public/app/containers/AlertRuleList/__snapshots__/AlertRuleList.jest.tsx.snap

@@ -82,7 +82,6 @@ exports[`AlertRuleList should render 1 rule 1`] = `
   >
   >
     <button
     <button
       className="btn btn-small btn-inverse alert-list__btn width-2"
       className="btn btn-small btn-inverse alert-list__btn width-2"
-      disabled={false}
       onClick={[Function]}
       onClick={[Function]}
       title="Pausing an alert rule prevents it from executing"
       title="Pausing an alert rule prevents it from executing"
     >
     >

public/app/stores/AlertListStore/AlertRule.ts

@@ -14,7 +14,6 @@ export const AlertRule = types
     stateAge: types.string,
     stateAge: types.string,
     info: types.optional(types.string, ''),
     info: types.optional(types.string, ''),
     url: types.string,
     url: types.string,
-    canEdit: types.boolean,
   })
   })
   .views(self => ({
   .views(self => ({
     get isPaused() {
     get isPaused() {