From b6b8f82c874ed555f67439a4e012d1003a8c5255 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 29 Sep 2023 10:39:35 +0200 Subject: [PATCH] [bugfix] Move follow.show_reblogs check further up to avoid showing unwanted reblogs in home timeline (#2234) --- .../processing/workers/surfacetimeline.go | 21 ++++-------- internal/visibility/home_timeline.go | 27 ++++++++-------- internal/visibility/home_timeline_test.go | 32 +++++++++++++++++++ 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 @@ 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 { - log.Trace(ctx, "ignoring visible status from unfollowed author") + if follow == nil { + log.Trace(ctx, "ignoring status from unfollowed author") + return false, nil + } + + 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 := >smodel.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"]