Skip to content

Commit 791e95d

Browse files
authored
FIX: Exclude muted and ignored users from FBFF calculation (#36610)
Previously, a user's Forum Best Friend Forever (FBFF) could be someone they had muted or ignored, which is a poor user experience - you wouldn't want someone you've explicitly blocked from your feed to be presented as your "best friend". This adds NOT EXISTS subqueries to both post_query and like_query methods to filter out any users that the current user has in their muted_users or ignored_users lists. The like_query method now accepts a user parameter to enable this filtering.
1 parent d69cc29 commit 791e95d

File tree

2 files changed

+71
-15
lines changed

2 files changed

+71
-15
lines changed

plugins/discourse-rewind/app/services/discourse_rewind/action/fbff.rb

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def call
3131
return FakeData if should_use_fake_data?
3232

3333
most_liked_users =
34-
like_query(date)
34+
like_query(user, date)
3535
.where(acting_user_id: user.id)
3636
.group(:user_id)
3737
.order("COUNT(*) DESC")
@@ -41,7 +41,7 @@ def call
4141
.reduce({}, :merge)
4242

4343
most_liked_by_users =
44-
like_query(date)
44+
like_query(user, date)
4545
.where(user: user)
4646
.group(:acting_user_id)
4747
.order("COUNT(*) DESC")
@@ -120,16 +120,40 @@ def post_query(user, date)
120120
.where("replies.created_at BETWEEN ? AND ?", date.first, date.last)
121121
.where("posts.created_at BETWEEN ? AND ?", date.first, date.last)
122122
.where("replies.user_id <> posts.user_id")
123+
.where(<<~SQL, user_id: user.id)
124+
NOT EXISTS (
125+
SELECT 1 FROM muted_users
126+
WHERE muted_users.user_id = :user_id
127+
AND muted_users.muted_user_id IN (replies.user_id, posts.user_id)
128+
)
129+
AND NOT EXISTS (
130+
SELECT 1 FROM ignored_users
131+
WHERE ignored_users.user_id = :user_id
132+
AND ignored_users.ignored_user_id IN (replies.user_id, posts.user_id)
133+
)
134+
SQL
123135
end
124136

125-
def like_query(date)
137+
def like_query(user, date)
126138
UserAction
127139
.with(eligible_users: User.real.activated.not_suspended.select(:id))
128140
.joins(:target_topic, :target_post)
129141
.joins("INNER JOIN eligible_users eu ON eu.id = user_actions.user_id")
130142
.joins("INNER JOIN eligible_users eu2 ON eu2.id = user_actions.acting_user_id")
131143
.where(created_at: date)
132144
.where(action_type: UserAction::WAS_LIKED)
145+
.where(<<~SQL, user_id: user.id)
146+
NOT EXISTS (
147+
SELECT 1 FROM muted_users
148+
WHERE muted_users.user_id = :user_id
149+
AND muted_users.muted_user_id IN (user_actions.user_id, user_actions.acting_user_id)
150+
)
151+
AND NOT EXISTS (
152+
SELECT 1 FROM ignored_users
153+
WHERE ignored_users.user_id = :user_id
154+
AND ignored_users.ignored_user_id IN (user_actions.user_id, user_actions.acting_user_id)
155+
)
156+
SQL
133157
end
134158

135159
def apply_score(users, score)

plugins/discourse-rewind/spec/actions/fbff_spec.rb

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,22 @@
7575
described_class.new(user: user, date: date).post_query(user, date).map(&:id),
7676
).to be_empty
7777
end
78+
79+
it "excludes posts by muted users" do
80+
MutedUser.create!(user: user, muted_user: other_user)
81+
82+
expect(
83+
described_class.new(user: user, date: date).post_query(user, date).map(&:id),
84+
).to be_empty
85+
end
86+
87+
it "excludes posts by ignored users" do
88+
IgnoredUser.create!(user: user, ignored_user: other_user, expiring_at: 1.year.from_now)
89+
90+
expect(
91+
described_class.new(user: user, date: date).post_query(user, date).map(&:id),
92+
).to be_empty
93+
end
7894
end
7995

8096
describe ".like_query" do
@@ -102,25 +118,25 @@
102118
end
103119

104120
it "succesfully returns likes for the time period" do
105-
expect(described_class.new(user: user, date: date).like_query(date).map(&:id)).to match_array(
106-
[like_1.id, like_2.id],
107-
)
121+
expect(
122+
described_class.new(user: user, date: date).like_query(user, date).map(&:id),
123+
).to match_array([like_1.id, like_2.id])
108124
end
109125

110126
it "does not return likes outside the time period" do
111127
like_1.update!(created_at: 2.years.ago)
112128

113-
expect(described_class.new(user: user, date: date).like_query(date).map(&:id)).not_to include(
114-
like_1.id,
115-
)
129+
expect(
130+
described_class.new(user: user, date: date).like_query(user, date).map(&:id),
131+
).not_to include(like_1.id)
116132
end
117133

118134
describe "user_id" do
119135
it "excludes likes from deactivated users" do
120136
other_user.deactivate(Discourse.system_user)
121137

122138
expect(
123-
described_class.new(user: user, date: date).like_query(date).map(&:id),
139+
described_class.new(user: user, date: date).like_query(user, date).map(&:id),
124140
).not_to include(like_1.id)
125141
end
126142

@@ -130,7 +146,7 @@
130146
other_user.save!
131147

132148
expect(
133-
described_class.new(user: user, date: date).like_query(date).map(&:id),
149+
described_class.new(user: user, date: date).like_query(user, date).map(&:id),
134150
).not_to include(like_1.id)
135151
end
136152

@@ -139,7 +155,7 @@
139155
like_1.update!(acting_user: bot)
140156

141157
expect(
142-
described_class.new(user: user, date: date).like_query(date).map(&:id),
158+
described_class.new(user: user, date: date).like_query(user, date).map(&:id),
143159
).not_to include(like_1.id)
144160
end
145161
end
@@ -149,7 +165,7 @@
149165
user.deactivate(Discourse.system_user)
150166

151167
expect(
152-
described_class.new(user: user, date: date).like_query(date).map(&:id),
168+
described_class.new(user: user, date: date).like_query(user, date).map(&:id),
153169
).not_to include(like_2.id)
154170
end
155171

@@ -159,7 +175,7 @@
159175
user.save!
160176

161177
expect(
162-
described_class.new(user: user, date: date).like_query(date).map(&:id),
178+
described_class.new(user: user, date: date).like_query(user, date).map(&:id),
163179
).not_to include(like_2.id)
164180
end
165181

@@ -168,9 +184,25 @@
168184
like_2.update!(acting_user: bot)
169185

170186
expect(
171-
described_class.new(user: user, date: date).like_query(date).map(&:id),
187+
described_class.new(user: user, date: date).like_query(user, date).map(&:id),
172188
).not_to include(like_2.id)
173189
end
174190
end
191+
192+
it "excludes likes involving muted users" do
193+
MutedUser.create!(user: user, muted_user: other_user)
194+
195+
expect(
196+
described_class.new(user: user, date: date).like_query(user, date).map(&:id),
197+
).to be_empty
198+
end
199+
200+
it "excludes likes involving ignored users" do
201+
IgnoredUser.create!(user: user, ignored_user: other_user, expiring_at: 1.year.from_now)
202+
203+
expect(
204+
described_class.new(user: user, date: date).like_query(user, date).map(&:id),
205+
).to be_empty
206+
end
175207
end
176208
end

0 commit comments

Comments
 (0)