[bugfix] Fix no notification if mention edited into status (#4102)

This pull request adds mention notifications if a mention was edited into a status after its initial publication.

Closes https://codeberg.org/superseriousbusiness/gotosocial/issues/3869

Reviewed-on: https://codeberg.org/superseriousbusiness/gotosocial/pulls/4102
Co-authored-by: tobi <tobi.smethurst@protonmail.com>
Co-committed-by: tobi <tobi.smethurst@protonmail.com>

4 files changed, 100 insertions, 39 deletions

diff --git a/internal/processing/workers/fromclientapi.go b/internal/processing/workers/fromclientapi.go
index 0e27eea0e..dbbeff220 100644
--- a/internal/processing/workers/fromclientapi.go
+++ b/internal/processing/workers/fromclientapi.go
@@ -729,6 +729,25 @@ func (p *clientAPI) UpdateStatus(ctx context.Context, cMsg *messages.FromClientA
 		}
 	}
 
+	// Notify any *new* mentions added
+	// to this status by the editor.
+	for _, mention := range status.Mentions {
+		// Check if we've seen
+		// this mention already.
+		if !mention.IsNew {
+			// Already seen
+			// it, skip.
+			continue
+		}
+
+		// Haven't seen this mention
+		// yet, notify it if necessary.
+		mention.Status = status
+		if err := p.surface.notifyMention(ctx, mention); err != nil {
+			log.Errorf(ctx, "error notifying mention: %v", err)
+		}
+	}
+
 	// Push message that the status has been edited to streams.
 	if err := p.surface.timelineStatusUpdate(ctx, status); err != nil {
 		log.Errorf(ctx, "error streaming status edit: %v", err)
diff --git a/internal/processing/workers/fromclientapi_test.go b/internal/processing/workers/fromclientapi_test.go
index 552db75da..c643e0c70 100644
--- a/internal/processing/workers/fromclientapi_test.go
+++ b/internal/processing/workers/fromclientapi_test.go
@@ -89,6 +89,7 @@ func (suite *FromClientAPITestSuite) newStatus(
 			OriginAccountID:  account.ID,
 			OriginAccountURI: account.URI,
 			TargetAccountID:  replyToStatus.AccountID,
+			IsNew:            true,
 		}
 
 		if err := state.DB.PutMention(ctx, mention); err != nil {
@@ -117,6 +118,7 @@ func (suite *FromClientAPITestSuite) newStatus(
 			TargetAccountID:  mentionedAccount.ID,
 			TargetAccount:    mentionedAccount,
 			Silent:           util.Ptr(false),
+			IsNew:            true,
 		}
 
 		newStatus.Mentions = append(newStatus.Mentions, newMention)
diff --git a/internal/processing/workers/fromfediapi.go b/internal/processing/workers/fromfediapi.go
index 93dd5b00e..93b12d89c 100644
--- a/internal/processing/workers/fromfediapi.go
+++ b/internal/processing/workers/fromfediapi.go
@@ -996,7 +996,26 @@ func (p *fediAPI) UpdateStatus(ctx context.Context, fMsg *messages.FromFediAPI)
 		log.Errorf(ctx, "error streaming status edit: %v", err)
 	}
 
-	// Status representation was refetched, uncache from timelines.
+	// Notify any *new* mentions added
+	// to this status by the editor.
+	for _, mention := range status.Mentions {
+		// Check if we've seen
+		// this mention already.
+		if !mention.IsNew {
+			// Already seen
+			// it, skip.
+			continue
+		}
+
+		// Haven't seen this mention
+		// yet, notify it if necessary.
+		mention.Status = status
+		if err := p.surface.notifyMention(ctx, mention); err != nil {
+			log.Errorf(ctx, "error notifying mention: %v", err)
+		}
+	}
+
+	// Status representation changed, uncache from timelines.
 	p.surface.invalidateStatusFromTimelines(status.ID)
 
 	return nil
diff --git a/internal/processing/workers/surfacenotify.go b/internal/processing/workers/surfacenotify.go
index 6dddae359..8e034c23a 100644
--- a/internal/processing/workers/surfacenotify.go
+++ b/internal/processing/workers/surfacenotify.go
@@ -99,54 +99,75 @@ func (s *Surface) notifyMentions(
 
 	for _, mention := range status.Mentions {
 		// Set status on the mention (stops
-		// the below function populating it).
+		// notifyMention having to populate it).
 		mention.Status = status
 
-		// Beforehand, ensure the passed mention is fully populated.
-		if err := s.State.DB.PopulateMention(ctx, mention); err != nil {
-			errs.Appendf("error populating mention %s: %w", mention.ID, err)
-			continue
+		// Do the thing.
+		if err := s.notifyMention(ctx, mention); err != nil {
+			errs = append(errs, err)
 		}
+	}
 
-		if mention.TargetAccount.IsRemote() {
-			// no need to notify
-			// remote accounts.
-			continue
-		}
+	return errs.Combine()
+}
 
-		// Ensure thread not muted
-		// by mentioned account.
-		muted, err := s.State.DB.IsThreadMutedByAccount(
-			ctx,
-			status.ThreadID,
-			mention.TargetAccountID,
+// notifyMention notifies the target
+// of the given mention that they've
+// been mentioned in a status.
+func (s *Surface) notifyMention(
+	ctx context.Context,
+	mention *gtsmodel.Mention,
+) error {
+	// Beforehand, ensure the passed mention is fully populated.
+	if err := s.State.DB.PopulateMention(ctx, mention); err != nil {
+		return gtserror.Newf(
+			"error populating mention %s: %w",
+			mention.ID, err,
 		)
-		if err != nil {
-			errs.Appendf("error checking status thread mute %s: %w", status.ThreadID, err)
-			continue
-		}
+	}
 
-		if muted {
-			// This mentioned account
-			// has muted the thread.
-			// Don't pester them.
-			continue
-		}
+	if mention.TargetAccount.IsRemote() {
+		// no need to notify
+		// remote accounts.
+		return nil
+	}
 
-		// notify mentioned
-		// by status author.
-		if err := s.Notify(ctx,
-			gtsmodel.NotificationMention,
-			mention.TargetAccount,
-			mention.OriginAccount,
-			mention.StatusID,
-		); err != nil {
-			errs.Appendf("error notifying mention target %s: %w", mention.TargetAccountID, err)
-			continue
-		}
+	// Ensure thread not muted
+	// by mentioned account.
+	muted, err := s.State.DB.IsThreadMutedByAccount(
+		ctx,
+		mention.Status.ThreadID,
+		mention.TargetAccountID,
+	)
+	if err != nil {
+		return gtserror.Newf(
+			"error checking status thread mute %s: %w",
+			mention.Status.ThreadID, err,
+		)
 	}
 
-	return errs.Combine()
+	if muted {
+		// This mentioned account
+		// has muted the thread.
+		// Don't pester them.
+		return nil
+	}
+
+	// Notify mentioned
+	// by status author.
+	if err := s.Notify(ctx,
+		gtsmodel.NotificationMention,
+		mention.TargetAccount,
+		mention.OriginAccount,
+		mention.StatusID,
+	); err != nil {
+		return gtserror.Newf(
+			"error notifying mention target %s: %w",
+			mention.TargetAccountID, err,
+		)
+	}
+
+	return nil
 }
 
 // notifyFollowRequest notifies the target of the given