fix: own comments trigger NEW

mutation_queue.go

@@ -35,6 +35,7 @@ type mutationOperation struct {
 	PRID                 string              `json:"pull_request_id"`
 	ThreadID             string              `json:"thread_id,omitempty"`
 	Body                 string              `json:"body,omitempty"`
+	ReplyID              string              `json:"reply_id,omitempty"`
 	Resolved             bool                `json:"resolved,omitempty"`
 	Viewer               string              `json:"viewer,omitempty"`
 	Original             PullRequestMetadata `json:"original,omitempty"`
@@ -372,9 +373,11 @@ func executeQueuedMutation(
 				reason: "could not checkpoint the reply attempt", err: err,
 			}
 		}
-		if _, err := writer.ReplyToThread(ctx, operation.ThreadID, operation.Body); err != nil {
+		comment, err := writer.ReplyToThread(ctx, operation.ThreadID, operation.Body)
+		if err != nil {
 			return mutationReplayMsg{operation: operation, state: mutationReplayWaiting, details: details, err: err}
 		}
+		operation.ReplyID = comment.ID
 		return verifying()
 	case mutationResolution:
 		if thread == nil {
@@ -477,6 +480,9 @@ func queuedReplyPresent(details PRDetails, operation mutationOperation) bool {
 	}
 	matches := 0
 	for _, comment := range thread.Comments {
+		if operation.ReplyID != "" && comment.ID == operation.ReplyID {
+			return true
+		}
 		if comment.Body == operation.Body && strings.EqualFold(comment.Author, operation.Viewer) &&
 			!comment.CreatedAt.Before(operation.EnqueuedAt.Add(-time.Minute)) {
 			matches++
@@ -546,6 +552,10 @@ func (m App) projectQueuedMutations(details PRDetails) PRDetails {
 			pendingID := "pending:" + operation.ID
 			found := false
 			for index := range thread.Comments {
+				if queuedReplyMatchesComment(thread.Comments[index], operation) {
+					found = true
+					continue
+				}
 				if thread.Comments[index].ID == pendingID {
 					thread.Comments[index].Pending = mutationNeedsAttention(operation)
 					found = true
@@ -573,6 +583,15 @@ func (m App) projectQueuedMutations(details PRDetails) PRDetails {
 	return details
 }
 
+func queuedReplyMatchesComment(comment ReviewComment, operation mutationOperation) bool {
+	if operation.ReplyID != "" && comment.ID == operation.ReplyID {
+		return true
+	}
+	return comment.Body == operation.Body &&
+		strings.EqualFold(comment.Author, operation.Viewer) &&
+		!comment.CreatedAt.Before(operation.EnqueuedAt.Add(-time.Minute))
+}
+
 func mutationNeedsAttention(operation mutationOperation) bool {
 	return operation.Blocked || operation.Unverified
 }

mutation_queue_test.go

@@ -303,6 +303,56 @@ func TestReplayReconcilesReplyBeforeAskingAboutAmbiguousDelivery(t *testing.T) {
 	}
 }
 
+func TestReplyProjectionUsesRemoteCommentWithoutTemporaryDuplicate(t *testing.T) {
+	store := loadMutationQueue("")
+	enqueued := time.Now().Add(-time.Second)
+	operation := mutationOperation{
+		ID: "reply", Kind: mutationReply, Owner: "o", Repository: "r", Number: 1,
+		ThreadID: "thread", Body: "body", Viewer: "me", EnqueuedAt: enqueued,
+	}
+	if err := store.add(operation); err != nil {
+		t.Fatal(err)
+	}
+	m := NewApp(nil, "o", "r", false, 50, time.Minute)
+	m.mutations = store
+	details := PRDetails{
+		PullRequest: PullRequest{Owner: "o", Repository: "r", Number: 1},
+		Threads: []ReviewThread{{ID: "thread", Comments: []ReviewComment{
+			{ID: "remote", Author: "me", Body: "body", CreatedAt: enqueued.Add(time.Second)},
+		}}},
+	}
+
+	projected := m.projectQueuedMutations(details)
+	if len(projected.Threads[0].Comments) != 1 || projected.Threads[0].Comments[0].ID != "remote" {
+		t.Fatalf("single remote reply was duplicated: %#v", projected.Threads[0].Comments)
+	}
+
+	details.Threads[0].Comments = append(details.Threads[0].Comments, ReviewComment{
+		ID: "actual-duplicate", Author: "me", Body: "body", CreatedAt: enqueued.Add(2 * time.Second),
+	})
+	projected = m.projectQueuedMutations(details)
+	if len(projected.Threads[0].Comments) != 2 {
+		t.Fatalf("remote duplicates were not preserved exactly: %#v", projected.Threads[0].Comments)
+	}
+}
+
+func TestSuccessfulReplyCheckpointsRemoteCommentID(t *testing.T) {
+	store := loadMutationQueue("")
+	operation := mutationOperation{
+		ID: "reply", Kind: mutationReply, Owner: "o", Repository: "r", Number: 1,
+		ThreadID: "thread", Body: "body", Viewer: "me", EnqueuedAt: time.Now(),
+	}
+	if err := store.add(operation); err != nil {
+		t.Fatal(err)
+	}
+	details := PRDetails{Threads: []ReviewThread{{ID: "thread", ViewerCanReply: true}}}
+	result := executeQueuedMutation(context.Background(), &recordingService{}, store, operation, details)
+	stored, ok := store.front()
+	if result.state != mutationReplayVerifying || !ok || stored.ReplyID != "new-comment" {
+		t.Fatalf("reply verification identity was not checkpointed: result=%#v stored=%#v", result, stored)
+	}
+}
+
 func TestSuccessfulResolutionWaitsForLiveVerification(t *testing.T) {
 	store := loadMutationQueue("")
 	operation := mutationOperation{

tui.go

@@ -1638,6 +1638,8 @@ func (m *App) trackThreadUpdates(details PRDetails) {
 	state := m.readState.Data[prID]
 	if state.Threads == nil {
 		state.Threads = make(map[string]bool)
+	}
+	if state.Comments == nil {
 		state.Comments = make(map[string]bool)
 	}
 	if !state.Initialized {
@@ -1654,25 +1656,47 @@ func (m *App) trackThreadUpdates(details PRDetails) {
 		}
 		return
 	}
+	readStateChanged := false
 	for _, thread := range details.Threads {
 		threadIsNew := !state.Threads[thread.ID]
 		updated := threadIsNew
-		if threadIsNew {
-			m.newThreads[thread.ID] = true
-		}
+		viewerAuthoredOnly := len(thread.Comments) > 0
 		for _, comment := range thread.Comments {
+			viewerAuthored := details.ViewerLogin != "" &&
+				strings.EqualFold(comment.Author, details.ViewerLogin)
+			if viewerAuthored {
+				if !state.Comments[comment.ID] {
+					state.Comments[comment.ID] = true
+					readStateChanged = true
+				}
+			} else {
+				viewerAuthoredOnly = false
+			}
 			if !state.Comments[comment.ID] {
 				updated = true
 				m.unreadComments[comment.ID] = true
 			}
 			m.knownComments[comment.ID] = true
 		}
+		if threadIsNew && !viewerAuthoredOnly {
+			m.newThreads[thread.ID] = true
+		} else if threadIsNew {
+			state.Threads[thread.ID] = true
+			readStateChanged = true
+			updated = false
+		}
 		m.knownThreads[thread.ID] = true
 		if updated {
 			m.unreadThreads[thread.ID] = true
 			m.updatedThreads[thread.ID] = true
 		}
 	}
+	if readStateChanged {
+		m.readState.Data[prID] = state
+		if err := m.readState.save(); err != nil {
+			m.recordHealth("read state", healthWarning, err.Error())
+		}
+	}
 	m.initializedPRs[prID] = true
 }
 

tui_test.go

@@ -1752,6 +1752,39 @@ func TestPollingMarksNewThreadCommentsUnread(t *testing.T) {
 	}
 }
 
+func TestPollingDoesNotMarkViewerReplyUnread(t *testing.T) {
+	m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
+	m.screen = threadScreen
+	initial := PRDetails{
+		PullRequest: PullRequest{ID: "pr", Owner: "o", Repository: "r", Number: 1},
+		ViewerLogin: "me",
+		Threads: []ReviewThread{{
+			ID: "thread", Comments: []ReviewComment{{ID: "original", Author: "reviewer"}},
+		}},
+	}
+	updated, _ := m.Update(detailsLoadedMsg{owner: "o", repo: "r", number: 1, details: initial})
+	m = updated.(App)
+
+	refreshed := initial
+	refreshed.Threads = []ReviewThread{{
+		ID: "thread", Comments: []ReviewComment{
+			{ID: "original", Author: "reviewer"},
+			{ID: "own-reply", Author: "ME", Body: "sent by me"},
+		},
+	}}
+	updated, _ = m.Update(detailsLoadedMsg{owner: "o", repo: "r", number: 1, details: refreshed})
+	m = updated.(App)
+
+	if m.unreadThreads["thread"] || m.unreadComments["own-reply"] || len(m.updatedThreads) != 0 {
+		t.Fatalf("viewer reply was marked new: threads=%v comments=%v updated=%v",
+			m.unreadThreads, m.unreadComments, m.updatedThreads)
+	}
+	state := m.readState.Data["pr"]
+	if !state.Comments["own-reply"] {
+		t.Fatal("viewer reply was not persisted as read")
+	}
+}
+
 func TestResolvingThreadMarksItRead(t *testing.T) {
 	service := &recordingService{}
 	m := NewApp(service, "o", "r", false, 50, time.Second)