[bugfix] Move follow.show_reblogs check further up to avoid showing unwanted reblogs in home timeline (#2234)

3 files changed, 53 insertions, 27 deletions

diff --git a/internal/processing/workers/surfacetimeline.go b/internal/processing/workers/surfacetimeline.go
index a13e6bc70..a45c83188 100644
--- a/internal/processing/workers/surfacetimeline.go
+++ b/internal/processing/workers/surfacetimeline.go
@@ -91,9 +91,13 @@ func (s *surface) timelineAndNotifyStatusForFollowers(
 	)
 
 	for _, follow := range follows {
-		// Do an initial rough-grained check to see if the
-		// status is timelineable for this follower at all
-		// based on its visibility and who it replies to etc.
+		// Check to see if the status is timelineable for this follower,
+		// taking account of its visibility, who it replies to, and, if
+		// it's a reblog, whether follower account wants to see reblogs.
+		//
+		// If it's not timelineable, we can just stop early, since lists
+		// are prettymuch subsets of the home timeline, so if it shouldn't
+		// appear there, it shouldn't appear in lists either.
 		timelineable, err := s.filter.StatusHomeTimelineable(
 			ctx, follow.Account, status,
 		)
@@ -107,17 +111,6 @@ func (s *surface) timelineAndNotifyStatusForFollowers(
 			continue
 		}
 
-		if boost && !*follow.ShowReblogs {
-			// Status is a boost, but the owner of
-			// this follow doesn't want to see boosts
-			// from this account. We can safely skip
-			// everything, then, because we also know
-			// that the follow owner won't want to be
-			// have the status put in any list timelines,
-			// or be notified about the status either.
-			continue
-		}
-
 		// Add status to any relevant lists
 		// for this follow, if applicable.
 		s.listTimelineStatusForFollow(
diff --git a/internal/visibility/home_timeline.go b/internal/visibility/home_timeline.go
index d8bbc3979..56290e836 100644
--- a/internal/visibility/home_timeline.go
+++ b/internal/visibility/home_timeline.go
@@ -19,10 +19,12 @@ package visibility
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
 	"github.com/superseriousbusiness/gotosocial/internal/cache"
+	"github.com/superseriousbusiness/gotosocial/internal/db"
 	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
@@ -169,24 +171,23 @@ func (f *Filter) isStatusHomeTimelineable(ctx context.Context, owner *gtsmodel.A
 	// a status thread *including* the owner, or a conversation thread between
 	// accounts the timeline owner follows.
 
-	if status.Visibility == gtsmodel.VisibilityFollowersOnly ||
-		status.Visibility == gtsmodel.VisibilityMutualsOnly {
-		// Followers/mutuals only post that already passed the status
-		// visibility check, (i.e. we follow / mutuals with author).
-		return true, nil
-	}
-
-	// Ensure owner follows author of public/unlocked status.
-	follow, err := f.state.DB.IsFollowing(ctx,
+	// Ensure owner follows author.
+	follow, err := f.state.DB.GetFollow(ctx,
 		owner.ID,
 		status.AccountID,
 	)
-	if err != nil {
-		return false, gtserror.Newf("error checking follow %s->%s: %w", owner.ID, status.AccountID, err)
+	if err != nil && !errors.Is(err, db.ErrNoEntries) {
+		return false, gtserror.Newf("error retrieving follow %s->%s: %w", owner.ID, status.AccountID, err)
+	}
+
+	if follow == nil {
+		log.Trace(ctx, "ignoring status from unfollowed author")
+		return false, nil
 	}
 
-	if !follow {
-		log.Trace(ctx, "ignoring visible status from unfollowed author")
+	if status.BoostOfID != "" && !*follow.ShowReblogs {
+		// Status is a boost, but the owner of this follow
+		// doesn't want to see boosts from this account.
 		return false, nil
 	}
 
diff --git a/internal/visibility/home_timeline_test.go b/internal/visibility/home_timeline_test.go
index bc64c6425..d8211c8dd 100644
--- a/internal/visibility/home_timeline_test.go
+++ b/internal/visibility/home_timeline_test.go
@@ -55,6 +55,38 @@ func (suite *StatusStatusHomeTimelineableTestSuite) TestFollowingStatusHomeTimel
 	suite.True(timelineable)
 }
 
+func (suite *StatusStatusHomeTimelineableTestSuite) TestFollowingBoostedStatusHomeTimelineable() {
+	ctx := context.Background()
+
+	testStatus := suite.testStatuses["admin_account_status_4"]
+	testAccount := suite.testAccounts["local_account_1"]
+	timelineable, err := suite.filter.StatusHomeTimelineable(ctx, testAccount, testStatus)
+	suite.NoError(err)
+
+	suite.True(timelineable)
+}
+
+func (suite *StatusStatusHomeTimelineableTestSuite) TestFollowingBoostedStatusHomeTimelineableNoReblogs() {
+	ctx := context.Background()
+
+	// Update follow to indicate that local_account_1
+	// doesn't want to see reblogs by admin_account.
+	follow := &gtsmodel.Follow{}
+	*follow = *suite.testFollows["local_account_1_admin_account"]
+	follow.ShowReblogs = util.Ptr(false)
+
+	if err := suite.db.UpdateFollow(ctx, follow, "show_reblogs"); err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	testStatus := suite.testStatuses["admin_account_status_4"]
+	testAccount := suite.testAccounts["local_account_1"]
+	timelineable, err := suite.filter.StatusHomeTimelineable(ctx, testAccount, testStatus)
+	suite.NoError(err)
+
+	suite.False(timelineable)
+}
+
 func (suite *StatusStatusHomeTimelineableTestSuite) TestNotFollowingStatusHomeTimelineable() {
 	testStatus := suite.testStatuses["remote_account_1_status_1"]
 	testAccount := suite.testAccounts["local_account_1"]