Merge pull request #52 from razorpay/fix-incident-update-order

Bug Fix - Incident Updates Order

handlers/incident.go

@@ -8,6 +8,7 @@ import (
 	"github.com/razorpay/statping/types/services"
 	"github.com/razorpay/statping/utils"
 	"net/http"
+	"sort"
 	"time"
 )
 
@@ -71,7 +72,7 @@ func getVisibleIncidentsOfService(service *services.Service) []incidents.Inciden
 			visibleIncidentIds = append(visibleIncidentIds, incident.Id)
 		} else if checkResolvedVisibility(incident.Updates) {
 			incidentVar := *incident
-			reverse(incidentVar.Updates)
+			sortUpdates(incidentVar.Updates, "DESC")
 			visibleIncidents = append(visibleIncidents, incidentVar)
 			visibleIncidentIds = append(visibleIncidentIds, incident.Id)
 		}
@@ -80,10 +81,16 @@ func getVisibleIncidentsOfService(service *services.Service) []incidents.Inciden
 	return visibleIncidents
 }
 
-func reverse(incidents []*incidents.IncidentUpdate) {
+func sortUpdates(updates []*incidents.IncidentUpdate, order string) {
-	for i, j := 0, len(incidents)-1; i < j; i, j = i+1, j-1 {
+	sort.Slice(updates, func(i, j int) bool {
-		incidents[i], incidents[j] = incidents[j], incidents[i]
+		switch order {
-	}
+		case "DESC":
+			return updates[i].CreatedAt.Unix() >= updates[j].CreatedAt.Unix()
+		case "ASC":
+			return updates[i].CreatedAt.Unix() <= updates[j].CreatedAt.Unix()
+		}
+		return false
+	})
 }
 
 func hasZeroUpdates(Updates []*incidents.IncidentUpdate) bool {
@@ -93,10 +100,15 @@ func hasZeroUpdates(Updates []*incidents.IncidentUpdate) bool {
 	return false
 }
 
+// NOT [(last update is resolved) AND (last update was more than 2 hrs ago)]
 func checkResolvedVisibility(incidentUpdates []*incidents.IncidentUpdate) bool {
-	if !(incidentUpdates[len(incidentUpdates)-1].Type == resolved && getTimeDiff(incidentUpdates[len(incidentUpdates)-1]) > incidentsTimeoutInMinutes) {
+	if !(incidentUpdates[len(incidentUpdates)-1].Type == resolved &&
+		getTimeDiff(incidentUpdates[len(incidentUpdates)-1]) > incidentsTimeoutInMinutes) {
+		log.Info(fmt.Sprintf("Incident update resolved visibility: %v", true))
 		return true
 	}
+	// This log will also given insight into number of unnecessary incidents fetched from DB
+	log.Info(fmt.Sprintf("Incident update resolved visibility: %v", false))
 	return false
 }
 
@@ -110,6 +122,7 @@ func apiIncidentUpdatesHandler(w http.ResponseWriter, r *http.Request) {
 		sendErrorJson(err, w, r)
 		return
 	}
+	sortUpdates(incid.Updates, "ASC")
 	returnJson(incid.Updates, w, r)
 }
 

handlers/incidents_test.go

@@ -1,8 +1,11 @@
 package handlers
 
 import (
+	"github.com/razorpay/statping/types/incidents"
+	"github.com/razorpay/statping/utils"
 	"github.com/stretchr/testify/assert"
 	"testing"
+	"time"
 )
 
 func TestUnAuthenticatedIncidentRoutes(t *testing.T) {
@@ -153,3 +156,41 @@ func TestIncidentsAPIRoutes(t *testing.T) {
 		})
 	}
 }
+
+func TestIncidentUpdateOrder(t *testing.T) {
+	now := utils.Now()
+	t0 := now.Add(-5 * time.Minute)
+	t1 := now.Add(-5*time.Minute + 1*time.Second)
+	t2 := now.Add(-5*time.Minute + 2*time.Second)
+
+	updates := []*incidents.IncidentUpdate{
+		{
+			Id:         78,
+			IncidentId: 23,
+			Message:    "Update 2",
+			Type:       "update",
+			CreatedAt:  t1,
+			UpdatedAt:  t2,
+		},
+		{
+			Id:         79,
+			IncidentId: 23,
+			Message:    "Resolved 1",
+			Type:       "resolved",
+			CreatedAt:  t2,
+			UpdatedAt:  t2,
+		},
+		{
+			Id:         76,
+			IncidentId: 23,
+			Message:    "Update 1",
+			Type:       "update",
+			CreatedAt:  t0,
+			UpdatedAt:  t2,
+		},
+	}
+	sortUpdates(updates, "DESC")
+	assert.Equal(t, int64(79), updates[0].Id)
+	assert.Equal(t, int64(78), updates[1].Id)
+	assert.Equal(t, int64(76), updates[2].Id)
+}