Merge pull request #230 from COS301-SE-2024/fix/backend/notification-…

…scheduling-logic

chore: Store the location in the context during TimezoneMiddleware

occupi-backend/pkg/database/database.go

@@ -994,7 +994,7 @@ func GetUsersPushTokens(ctx *gin.Context, appsession *models.AppSession, emails
 	return results, nil
 }
 
-func AddScheduledNotification(ctx *gin.Context, appsession *models.AppSession, notification models.ScheduledNotification, pushNotification bool) (bool, error) {
+func AddNotification(ctx *gin.Context, appsession *models.AppSession, notification models.ScheduledNotification, pushNotification bool) (bool, error) {
 	// check if database is nil
 	if appsession.DB == nil {
 		logrus.Error("Database is nil")
@@ -1026,27 +1026,52 @@ func AddScheduledNotification(ctx *gin.Context, appsession *models.AppSession, n
 	return true, nil
 }
 
-func DeleteExpoPushTokensFromScheduledNotification(ctx context.Context, appsession *models.AppSession, notification models.ScheduledNotification) (bool, error) {
+func GetScheduledNotifications(ctx context.Context, appsession *models.AppSession) ([]models.ScheduledNotification, error) {
 	// check if database is nil
 	if appsession.DB == nil {
 		logrus.Error("Database is nil")
-		return false, errors.New("database is nil")
+		return nil, errors.New("database is nil")
 	}
 
 	collection := appsession.DB.Database(configs.GetMongoDBName()).Collection("Notifications")
 
-	filter := bson.M{"_id": notification.ID}
+	// filter where sent flag is false
+	filter := bson.M{"sent": false}
 
-	// only delete the expo push tokens not in the provided list
-	update := bson.M{"$pull": bson.M{"unsentExpoPushTokens": bson.M{"$in": notification.UnsentExpoPushTokens}}}
+	cursor, err := collection.Find(ctx, filter)
+	if err != nil {
+		logrus.Error(err)
+		return nil, err
+	}
+
+	var notifications []models.ScheduledNotification
+	if err = cursor.All(ctx, &notifications); err != nil {
+		return nil, err
+	}
+
+	return notifications, nil
+}
+
+func MarkNotificationAsSent(ctx context.Context, appsession *models.AppSession, notificationID string) error {
+	// check if database is nil
+	if appsession.DB == nil {
+		logrus.Error("Database is nil")
+		return errors.New("database is nil")
+	}
+
+	collection := appsession.DB.Database(configs.GetMongoDBName()).Collection("Notifications")
+
+	// update the notification to sent
+	filter := bson.M{"_id": notificationID}
+	update := bson.M{"$set": bson.M{"sent": true}}
 
 	_, err := collection.UpdateOne(ctx, filter, update)
 	if err != nil {
 		logrus.Error(err)
-		return false, err
+		return err
 	}
 
-	return true, nil
+	return nil
 }
 
 func ReadNotifications(ctx *gin.Context, appsession *models.AppSession, email string) error {
@@ -1072,7 +1097,7 @@ func ReadNotifications(ctx *gin.Context, appsession *models.AppSession, email st
 	return nil
 }
 
-func UpdateSecuritySettings(ctx context.Context, appsession *models.AppSession, securitySettings models.SecuritySettingsRequest) error {
+func UpdateSecuritySettings(ctx *gin.Context, appsession *models.AppSession, securitySettings models.SecuritySettingsRequest) error {
 	collection := appsession.DB.Database(configs.GetMongoDBName()).Collection("Users")
 
 	filter := bson.M{"email": securitySettings.Email}

occupi-backend/pkg/handlers/api_handlers.go

@@ -103,13 +103,14 @@ func BookRoom(ctx *gin.Context, appsession *models.AppSession) {
 	scheduledNotification := models.ScheduledNotification{
 		Title:                "Booking Starting Soon",
 		Message:              utils.ConstructBookingStartingInScheduledString(utils.PrependEmailtoSlice(booking.Emails, booking.Creator), "3 mins"),
+		Sent:                 false,
 		SendTime:             booking.Start.Add(-3 * time.Minute),
 		Emails:               booking.Emails,
 		UnsentExpoPushTokens: utils.ConvertToStringArray(tokens),
 		UnreadEmails:         booking.Emails,
 	}
 
-	success, errv := database.AddScheduledNotification(ctx, appsession, scheduledNotification, true)
+	success, errv := database.AddNotification(ctx, appsession, scheduledNotification, true)
 
 	if errv != nil {
 		ctx.JSON(http.StatusInternalServerError, utils.ErrorResponse(http.StatusInternalServerError, "Failed to schedule notification", constants.InternalServerErrorCode, "Failed to schedule notification", nil))
@@ -124,13 +125,14 @@ func BookRoom(ctx *gin.Context, appsession *models.AppSession) {
 	notification := models.ScheduledNotification{
 		Title:                "Booking Invitation",
 		Message:              utils.ConstructBookingScheduledString(utils.PrependEmailtoSlice(booking.Emails, booking.Creator)),
-		SendTime:             time.Now(),
+		Sent:                 true,
+		SendTime:             utils.GetClientTime(ctx),
 		Emails:               booking.Emails,
 		UnsentExpoPushTokens: utils.ConvertToStringArray(tokens),
 		UnreadEmails:         booking.Emails,
 	}
 
-	success, errv = database.AddScheduledNotification(ctx, appsession, notification, false)
+	success, errv = database.AddNotification(ctx, appsession, notification, false)
 
 	if errv != nil {
 		ctx.JSON(http.StatusInternalServerError, utils.ErrorResponse(http.StatusInternalServerError, "Failed to schedule notification", constants.InternalServerErrorCode, "Failed to schedule notification", nil))

occupi-backend/pkg/middleware/middleware.go

@@ -172,7 +172,8 @@ func TimezoneMiddleware() gin.HandlerFunc {
 			return
 		}
 
-		time.Local = loc
+		// Store the location in the context
+		ctx.Set("timezone", loc)
 		ctx.Next()
 	}
 }

occupi-backend/pkg/models/database.go

@@ -139,6 +139,7 @@ type ScheduledNotification struct {
 	ID                   string    `json:"_id" bson:"_id,omitempty"`
 	Title                string    `json:"title" bson:"title"`
 	Message              string    `json:"message" bson:"message"`
+	Sent                 bool      `json:"sent" bson:"sent"`
 	SendTime             time.Time `json:"send_time" bson:"send_time"`
 	UnsentExpoPushTokens []string  `json:"unsentExpoPushTokens" bson:"unsentExpoPushTokens"`
 	Emails               []string  `json:"emails" bson:"emails"`

occupi-backend/pkg/receiver/recieve.go

@@ -2,7 +2,6 @@ package receiver
 
 import (
 	"context"
-	"errors"
 	"strings"
 	"time"
 
@@ -29,6 +28,19 @@ func StartConsumeMessage(appsession *models.AppSession) {
 		return
 	}
 
+	// check if there are any unsent notifications in the database
+	notifications, err := database.GetScheduledNotifications(context.Background(), appsession)
+
+	if err != nil {
+		logrus.Error("Failed to get notifications: ", err)
+	}
+
+	go func() {
+		for _, notification := range notifications {
+			notificationSendingLogic(notification, appsession)
+		}
+	}()
+
 	go func() {
 		for d := range msgs {
 			parts := strings.Split(string(d.Body), "|")
@@ -50,32 +62,40 @@ func StartConsumeMessage(appsession *models.AppSession) {
 				UnreadEmails:         utils.ConvertCommaDelimitedStringToArray(unreadEmails),
 			}
 
-			// to account for discrepancies in time, we should allow for a range of 5 seconds before and after the scheduled time
-			// whereby we can send the notification, after that, we should discard the notification, else if there
-			// is still more than 5 seconds before the scheduled time, we should wait until the time is right
-			now := time.Now()
-
-			switch {
-			case now.After(sendTime.Add(-5*time.Second)) && now.Before(sendTime.Add(5*time.Second)):
-				err := SendPushNotification(notification, appsession)
-				if err != nil {
-					logrus.Error("Failed to send push notification: ", err)
-				}
-			case now.Before(sendTime.Add(-5 * time.Second)):
-				// wait until the time is right
-				time.Sleep(time.Until(sendTime))
-				err := SendPushNotification(notification, appsession)
-				if err != nil {
-					logrus.Error("Failed to send push notification: ", err)
-				}
-			default:
-				logrus.Error("Failed to send push notification: ", "notification time has passed")
-			}
+			notificationSendingLogic(notification, appsession)
 		}
 	}()
 }
 
-func SendPushNotification(notification models.ScheduledNotification, appsession *models.AppSession) error {
+func notificationSendingLogic(notification models.ScheduledNotification, appsession *models.AppSession) {
+	// to account for discrepancies in time, we should allow for a range of 5 seconds before and after the scheduled time
+	// whereby we can send the notification, after that, we should discard the notification, else if there
+	// is still more than 5 seconds before the scheduled time, we should wait until the time is right
+	now := time.Now()
+
+	switch {
+	case now.After(notification.SendTime.Add(-5*time.Second)) && now.Before(notification.SendTime.Add(5*time.Second)):
+		err := sendPushNotification(notification, appsession)
+		if err != nil {
+			logrus.Error("Failed to send push notification: ", err)
+		}
+	case now.Before(notification.SendTime.Add(-5 * time.Second)):
+		// wait until the time is right
+		time.Sleep(time.Until(notification.SendTime))
+		err := sendPushNotification(notification, appsession)
+		if err != nil {
+			logrus.Error("Failed to send push notification: ", err)
+		}
+	default:
+		// just mark the notification as sent
+		err := database.MarkNotificationAsSent(context.Background(), appsession, notification.ID)
+		if err != nil {
+			logrus.Error("Failed to update notification: ", err)
+		}
+	}
+}
+
+func sendPushNotification(notification models.ScheduledNotification, appsession *models.AppSession) error {
 	for _, token := range notification.UnsentExpoPushTokens {
 		// To check the token is valid
 		pushToken, err := expo.NewExponentPushToken(token)
@@ -112,17 +132,12 @@ func SendPushNotification(notification models.ScheduledNotification, appsession
 	}
 
 	// update notification in database
-	success, err := database.DeleteExpoPushTokensFromScheduledNotification(context.Background(), appsession, notification)
+	err := database.MarkNotificationAsSent(context.Background(), appsession, notification.ID)
 
 	if err != nil {
 		logrus.Error("Failed to update notification: ", err)
 		return err
 	}
 
-	if !success {
-		logrus.Error("Failed to update notification: ", err)
-		return errors.New("failed to update notification")
-	}
-
 	return nil
 }

occupi-backend/pkg/utils/utils.go

@@ -562,3 +562,12 @@ func GetClientIP(ctx *gin.Context) string {
 	}
 	return ctx.ClientIP()
 }
+
+func GetClientTime(ctx *gin.Context) time.Time {
+	loc, exists := ctx.Get("timezone")
+	if !exists {
+		return time.Now()
+	}
+
+	return time.Now().In(loc.(*time.Location))
+}

occupi-backend/tests/middleware_test.go

@@ -1,6 +1,7 @@
 package tests
 
 import (
+	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -895,69 +896,55 @@ func TestAttachOTPRateLimitMiddleware(t *testing.T) {
 	}
 }
 
-func TestTimezoneMiddleware_DefaultTimezone(t *testing.T) {
+func TestTimezoneMiddleware(t *testing.T) {
 	// set gin run mode
 	gin.SetMode(configs.GetGinRunMode())
 	router := gin.Default()
 	router.Use(middleware.TimezoneMiddleware())
 	router.GET("/time", func(c *gin.Context) {
-		currentTime := time.Now().Format(time.RFC1123)
-		c.JSON(http.StatusOK, gin.H{
-			"current_time": currentTime,
-		})
-	})
-
-	req, _ := http.NewRequest("GET", "/time", nil)
-	w := httptest.NewRecorder()
-	router.ServeHTTP(w, req)
+		loc, exists := c.Get("timezone")
+		if !exists {
+			loc = time.UTC
+		}
 
-	assert.Equal(t, 200, w.Code)
-	assert.Contains(t, w.Body.String(), time.Now().UTC().Format(time.RFC1123))
-}
+		currentTime := time.Now().In(loc.(*time.Location))
 
-func TestTimezoneMiddleware_ValidTimezone(t *testing.T) {
-	// set gin run mode
-	gin.SetMode(configs.GetGinRunMode())
-	router := gin.Default()
-	router.Use(middleware.TimezoneMiddleware())
-	router.GET("/time", func(c *gin.Context) {
-		currentTime := time.Now().Format(time.RFC1123)
-		c.JSON(http.StatusOK, gin.H{
-			"current_time": currentTime,
+		c.JSON(200, gin.H{
+			"current_time": currentTime.Format(time.RFC3339),
 		})
 	})
 
-	req, _ := http.NewRequest("GET", "/time", nil)
-	req.Header.Set("X-Timezone", "Africa/Johannesburg")
-	w := httptest.NewRecorder()
-	router.ServeHTTP(w, req)
-
-	assert.Equal(t, 200, w.Code)
+	tests := []struct {
+		header     string
+		timezone   string
+		statusCode int
+	}{
+		{"X-Timezone", "America/New_York", 200},
+		{"X-Timezone", "Asia/Kolkata", 200},
+		{"X-Timezone", "Invalid/Timezone", 400},
+	}
 
-	// Load the expected timezone
-	loc, _ := time.LoadLocation("Africa/Johannesburg")
-	expectedTime := time.Now().In(loc).Format(time.RFC1123)
-	assert.Contains(t, w.Body.String(), expectedTime)
-}
+	for _, tt := range tests {
+		t.Run(tt.timezone, func(t *testing.T) {
+			req, _ := http.NewRequest("GET", "/time", nil)
+			req.Header.Set(tt.header, tt.timezone)
+			w := httptest.NewRecorder()
+			router.ServeHTTP(w, req)
 
-func TestTimezoneMiddleware_InvalidTimezone(t *testing.T) {
-	// set gin run mode
-	gin.SetMode(configs.GetGinRunMode())
-	router := gin.Default()
-	router.Use(middleware.TimezoneMiddleware())
-	router.GET("/time", func(c *gin.Context) {
-		currentTime := time.Now().Format(time.RFC1123)
-		c.JSON(http.StatusOK, gin.H{
-			"current_time": currentTime,
-		})
-	})
+			assert.Equal(t, tt.statusCode, w.Code)
+			if tt.statusCode == 200 {
+				var response map[string]string
+				err := json.Unmarshal(w.Body.Bytes(), &response)
+				assert.NoError(t, err)
 
-	req, _ := http.NewRequest("GET", "/time", nil)
-	req.Header.Set("X-Timezone", "Invalid/Timezone")
-	w := httptest.NewRecorder()
-	router.ServeHTTP(w, req)
+				loc, err := time.LoadLocation(tt.timezone)
+				assert.NoError(t, err)
 
-	assert.Equal(t, 400, w.Code)
+				expectedTime := time.Now().In(loc).Format(time.RFC3339)
+				assert.Contains(t, response["current_time"], expectedTime[:19]) // Compare only date and time part
+			}
+		})
+	}
 }
 
 func TestRealIPMiddleware_CFConnectingIP(t *testing.T) {