feat(sync): reset Meta.Done on newer remote notification (#137)

nobe4 · Aug 10, 2024 · 7215d87 · 7215d87
1 parent b348efc
commit 7215d87
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 27 deletions.
diff --git a/internal/gh/enrichments.go b/internal/gh/enrichments.go
@@ -23,7 +23,7 @@ func (c *Client) Enrich(n *notifications.Notification) error {
 		return err
 	}
 
-	slog.Debug("enriching", "notifications", n)
+	slog.Debug("enriching", "id", n.Id)
 
 	n.Author = extra.User
 	n.Subject.State = extra.State

diff --git a/internal/notifications/sync.go b/internal/notifications/sync.go
@@ -7,20 +7,19 @@ Sync merges the local and remote notifications.
 
 It applies the following rules:
 
-	| remote \ local | Missing   | Exist     | Done      |
-	| ---            | ---       | ---       | ---       |
-	| Exist          | (1)Insert | (2)Update | (2)Update |
-	| Missing        | (3)Keep   | (3)Keep   | (4)Drop   |
+	| remote \ local | Missing    | Exist      | Done       |
+	| ---            | ---        | ---        | ---        |
+	| Exist          | (1) Insert | (2) Update | (2) Update |
+	| Missing        | (3) Keep   | (3) Keep   | (4) Drop   |
 
-	1. Insert: Add the notification ass is.
-	2. Update: Update the local notification with the remote data, keep the Meta
-	   unchanged.
-	3. Keep: Keep the local notification unchanged.
-	4. Drop: Remove the notification from the local list.
+	(1) Insert: Add the notification ass is.
+	(2) Update: Update the local notification with the remote data, keep the Meta
+	    unchanged.
+	(3) Keep: Keep the local notification unchanged.
+	(4) Drop: Remove the notification from the local list.
 
-Notes on 'Update': Updating the notification will also reset the `Meta.Done`
-state if the notification on the API is newer than the cached one.
-TODO: https://github.com/nobe4/gh-not/issues/126
+Notes on (2) Update: Updating the notification will also reset the `Meta.Done`
+state if the remote notification is newer than the local one.
 
 TODO: refactor this to `func (n Notifications) Sync(remote Notifications) {}`
 */
@@ -33,8 +32,8 @@ func Sync(local, remote Notifications) Notifications {
 	// Add any new notifications to the list
 	for remoteId, remote := range remoteMap {
 		if _, ok := localMap[remoteId]; !ok {
-			// (1)Insert
-			slog.Debug("sync", "action", "insert", "notification", remote)
+			// (1) Insert
+			slog.Debug("sync", "action", "insert", "id", remote.Id)
 
 			remote.Meta.RemoteExists = true
 			n = append(n, remote)
@@ -47,20 +46,25 @@ func Sync(local, remote Notifications) Notifications {
 		local.Meta.RemoteExists = remoteExist
 
 		if remoteExist {
-			// (2)Update
-			slog.Debug("sync", "action", "update", "notification", remote)
+			// (2) Update
+			slog.Debug("sync", "action", "update", "id", remote.Id)
+
+			if local.Meta.Done && remote.UpdatedAt.After(local.UpdatedAt) {
+				slog.Debug("sync", "action", "reseting done", "id", local.Id)
+				local.Meta.Done = false
+			}
 
 			remote.Meta = local.Meta
 			n = append(n, remote)
 		} else {
 			if local.Meta.Done {
-				// (4)Drop
-				slog.Debug("sync", "action", "drop", "notification", local)
+				// (4) Drop
+				slog.Debug("sync", "action", "drop", "id", local.Id)
 				continue
 			}
 
-			// (3)Keep
-			slog.Debug("sync", "action", "keep", "notification", local)
+			// (3) Keep
+			slog.Debug("sync", "action", "keep", "id", local.Id)
 			n = append(n, local)
 		}
 	}

diff --git a/internal/notifications/sync_test.go b/internal/notifications/sync_test.go
@@ -1,7 +1,6 @@
 package notifications
 
 import (
-	"fmt"
 	"testing"
 	"time"
 )
@@ -86,7 +85,7 @@ func TestSync(t *testing.T) {
 		})
 	}
 
-	t.Run("update remoteExist", func(t *testing.T) {
+	t.Run("update Meta.RemoteExist", func(t *testing.T) {
 		got := Sync(
 			Notifications{
 				&Notification{Id: "0", UpdatedAt: time.Unix(0, 2), Meta: Meta{RemoteExists: false}},
@@ -98,10 +97,6 @@ func TestSync(t *testing.T) {
 			},
 		)
 
-		fmt.Printf("%#v\n", got[0])
-		fmt.Printf("%#v\n", got[1])
-		fmt.Printf("%#v\n", got[2])
-
 		if !got[0].Meta.RemoteExists {
 			t.Fatalf("expected RemoteExists to be true but got false")
 		}
@@ -112,4 +107,46 @@ func TestSync(t *testing.T) {
 			t.Fatalf("expected RemoteExists to be true but got false")
 		}
 	})
+
+	t.Run("update Meta.Done", func(t *testing.T) {
+		tests := []struct {
+			name         string
+			local        *Notification
+			remote       *Notification
+			expectedDone bool
+		}{
+			{
+				name:         "Not done && Not updated",
+				local:        &Notification{UpdatedAt: time.Unix(0, 0), Meta: Meta{Done: false}},
+				remote:       &Notification{UpdatedAt: time.Unix(0, 0)},
+				expectedDone: false,
+			},
+			{
+				name:         "Not done && updated",
+				local:        &Notification{UpdatedAt: time.Unix(0, 0), Meta: Meta{Done: false}},
+				remote:       &Notification{UpdatedAt: time.Unix(0, 1)},
+				expectedDone: false,
+			},
+			{
+				name:         "Done && Not updated",
+				local:        &Notification{UpdatedAt: time.Unix(0, 0), Meta: Meta{Done: true}},
+				remote:       &Notification{UpdatedAt: time.Unix(0, 0)},
+				expectedDone: true,
+			},
+			{
+				name:         "Done && updated",
+				local:        &Notification{UpdatedAt: time.Unix(0, 0), Meta: Meta{Done: true}},
+				remote:       &Notification{UpdatedAt: time.Unix(0, 1)},
+				expectedDone: false,
+			},
+		}
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				got := Sync(Notifications{test.local}, Notifications{test.remote})
+				if got[0].Meta.Done != test.expectedDone {
+					t.Fatalf("expected Done to be %v but got %v", test.expectedDone, got[0].Meta.Done)
+				}
+			})
+		}
+	})
 }