feat: don't sent the same notif more then once (#293)

Author: ErikKalkoken

internal/app/characterservice/communications.go

@@ -47,7 +47,7 @@ func (s *CharacterService) NotifyCommunications(ctx context.Context, characterID
 			}
 			title, content := s.RenderNotificationSummary(n)
 			notify(title, content)
-			if err := s.st.UpdateCharacterNotificationSetProcessed(ctx, n.ID); err != nil {
+			if err := s.st.UpdateCharacterNotificationsSetProcessed(ctx, n.NotificationID); err != nil {
 				return nil, err
 			}
 		}

internal/app/storage/characternotification.go

@@ -496,6 +496,9 @@ func (st *Storage) ListCharacterNotificationsUnread(ctx context.Context, charact
 	return ee, nil
 }
 
+// ListCharacterNotificationsUnprocessed returns all unprocessed notifications for character characterID.
+// Notifications older then earliest are ignored or which have no body or title are ignored.
+// Notifications which are duplicates of already processed ones are ignored too.
 func (st *Storage) ListCharacterNotificationsUnprocessed(ctx context.Context, characterID int32, earliest time.Time) ([]*app.CharacterNotification, error) {
 	arg := queries.ListCharacterNotificationsUnprocessedParams{
 		CharacterID: int64(characterID),
@@ -573,9 +576,10 @@ func (st *Storage) UpdateCharacterNotification(ctx context.Context, arg UpdateCh
 	return nil
 }
 
-func (st *Storage) UpdateCharacterNotificationSetProcessed(ctx context.Context, id int64) error {
-	if err := st.qRW.UpdateCharacterNotificationSetProcessed(ctx, id); err != nil {
-		return fmt.Errorf("update notification set processed for id %d: %w", id, err)
+// UpdateCharacterNotificationsSetProcessed marks all notifications with the same notificationID as processed.
+func (st *Storage) UpdateCharacterNotificationsSetProcessed(ctx context.Context, notificationID int64) error {
+	if err := st.qRW.UpdateCharacterNotificationsSetProcessed(ctx, notificationID); err != nil {
+		return fmt.Errorf("UpdateCharacterNotificationsSetProcessed for notification ID %d: %w", notificationID, err)
 	}
 	return nil
 }

internal/app/storage/characternotification_test.go

@@ -188,20 +188,37 @@ func TestCharacterNotification(t *testing.T) {
 			assert.Equal(t, "body", o.Body.ValueOrZero())
 		}
 	})
-	t.Run("can set processed", func(t *testing.T) {
+	t.Run("can mark notifs as processed", func(t *testing.T) {
 		// given
 		testutil.TruncateTables(db)
-		n := factory.CreateCharacterNotification(storage.CreateCharacterNotificationParams{})
+		c1 := factory.CreateCharacter()
+		n1 := factory.CreateCharacterNotification(storage.CreateCharacterNotificationParams{
+			CharacterID: c1.ID,
+			Body:        optional.New("Body"),
+			Title:       optional.New("Title"),
+		})
+		factory.CreateCharacterNotification(storage.CreateCharacterNotificationParams{
+			CharacterID:    c1.ID,
+			NotificationID: 42,
+		})
+		factory.CreateCharacterNotification(storage.CreateCharacterNotificationParams{
+			NotificationID: 42,
+		})
 		// when
-		err := st.UpdateCharacterNotificationSetProcessed(ctx, n.ID)
+		err := st.UpdateCharacterNotificationsSetProcessed(ctx, 42)
 		// then
 		if !assert.NoError(t, err) {
 			t.Fatal(err)
 		}
-		o, err := st.GetCharacterNotification(ctx, n.CharacterID, n.ID)
-		if assert.NoError(t, err) {
-			assert.True(t, o.IsProcessed)
+		ee, err := st.ListCharacterNotificationsUnprocessed(ctx, c1.ID, time.Now().Add(-24*time.Hour))
+		if !assert.NoError(t, err) {
+			t.Fatal(err)
 		}
+		got := set.Collect(xiter.MapSlice(ee, func(x *app.CharacterNotification) int64 {
+			return x.ID
+		}))
+		want := set.Of(n1.ID)
+		assert.True(t, got.Equal(want), "got %q, wanted %q", got, want)
 	})
 	t.Run("can calculate counts", func(t *testing.T) {
 		// given
@@ -417,6 +434,46 @@ func TestCharacterNotification_ListUnprocessed(t *testing.T) {
 		want := set.Of(n1.ID)
 		assert.True(t, got.Equal(want), "got %q, wanted %q", got, want)
 	})
+	t.Run("should not return duplicates of processed notifs", func(t *testing.T) {
+		// given
+		testutil.TruncateTables(db)
+		c := factory.CreateCharacter()
+		now := time.Now().UTC()
+		n1 := factory.CreateCharacterNotification(storage.CreateCharacterNotificationParams{
+			Body:        optional.New("body"),
+			CharacterID: c.ID,
+			Type:        "bravo",
+			Timestamp:   now,
+			Title:       optional.New("title"),
+		})
+		factory.CreateCharacterNotification(storage.CreateCharacterNotificationParams{
+			Body:           optional.New("body"),
+			CharacterID:    c.ID,
+			NotificationID: 42,
+			Type:           "bravo",
+			Timestamp:      now,
+			Title:          optional.New("title"),
+		})
+		factory.CreateCharacterNotification(storage.CreateCharacterNotificationParams{
+			Body:           optional.New("body"),
+			NotificationID: 42,
+			IsProcessed:    true,
+			Type:           "bravo",
+			Timestamp:      now,
+			Title:          optional.New("title"),
+		})
+		// when
+		ee, err := st.ListCharacterNotificationsUnprocessed(ctx, c.ID, now.Add(-24*time.Hour))
+		// then
+		if !assert.NoError(t, err) {
+			t.Fatal(err)
+		}
+		got := set.Collect(xiter.MapSlice(ee, func(x *app.CharacterNotification) int64 {
+			return x.ID
+		}))
+		want := set.Of(n1.ID)
+		assert.True(t, got.Equal(want), "got %q, wanted %q", got, want)
+	})
 }
 
 func TestNotificationType(t *testing.T) {

internal/app/storage/queries/character_notifications.sql

@@ -23,7 +23,7 @@ INSERT INTO
         notification_id,
         recipient_id,
         sender_id,
-        text,
+        TEXT,
         timestamp,
         title,
         type_id
@@ -121,37 +121,46 @@ FROM
     JOIN notification_types nt ON nt.id = cn.type_id
     LEFT JOIN eve_entities recipient ON recipient.id = cn.recipient_id
 WHERE
-    character_id = ?
+    cn.character_id = ?
     AND cn.is_processed IS FALSE
-    AND title IS NOT NULL
-    AND body IS NOT NULL
-    AND timestamp > ?
+    AND cn.title IS NOT NULL
+    AND cn.body IS NOT NULL
+    AND cn.timestamp > ?
+    AND notification_id NOT IN (
+        SELECT
+            cn2.notification_id
+        FROM
+            character_notifications cn2
+        WHERE
+            cn2.is_processed IS TRUE
+            AND cn2.timestamp > ?
+    )
 ORDER BY
     timestamp;
 
 -- name: UpdateCharacterNotification :exec
-UPDATE
-    character_notifications
+UPDATE character_notifications
 SET
     body = ?2,
     is_read = ?3,
     title = ?4
 WHERE
     id = ?1;
 
--- name: UpdateCharacterNotificationSetProcessed :exec
-UPDATE
-    character_notifications
+-- name: UpdateCharacterNotificationsSetProcessed :exec
+UPDATE character_notifications
 SET
     is_processed = TRUE
 WHERE
-    id = ?1;
+    notification_id = ?;
 
 -- name: CreateNotificationType :one
 INSERT INTO
     notification_types (name)
 VALUES
-    (?) RETURNING id;
+    (?)
+RETURNING
+    id;
 
 -- name: GetNotificationTypeID :one
 SELECT