From 765397d8130ce3177cf99b84f487c2d4808d07e3 Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Mon, 20 Feb 2023 21:06:10 +0100 Subject: [PATCH] filter out non-anon questions from blocked/muted users in home timeline --- app/models/user/relationship/block.rb | 20 +++- app/models/user/relationship/mute.rb | 26 +++++- app/models/user/timeline_methods.rb | 12 ++- spec/models/user_spec.rb | 126 +++++++++++++++++++++++++- 4 files changed, 177 insertions(+), 7 deletions(-) diff --git a/app/models/user/relationship/block.rb b/app/models/user/relationship/block.rb index 674fdb46..01cd25ed 100644 --- a/app/models/user/relationship/block.rb +++ b/app/models/user/relationship/block.rb @@ -21,12 +21,16 @@ class User raise Errors::BlockingSelf if target_user == self unfollow_and_remove(target_user) - create_relationship(active_block_relationships, target_user) + create_relationship(active_block_relationships, target_user).tap do + expire_blocked_user_ids_cache + end end # Unblock an user def unblock(target_user) - destroy_relationship(active_block_relationships, target_user) + destroy_relationship(active_block_relationships, target_user).tap do + expire_blocked_user_ids_cache + end end # Is self blocking target_user? @@ -34,6 +38,16 @@ class User relationship_active?(blocked_users, target_user) end + # Expire the blocked user ids cache + def expire_blocked_user_ids_cache = Rails.cache.delete(cache_key_blocked_user_ids) + + # Cached ids of the blocked users + def blocked_user_ids_cached + Rails.cache.fetch(cache_key_blocked_user_ids, expires_in: 1.hour) do + blocked_user_ids + end + end + private def unfollow_and_remove(target_user) @@ -43,6 +57,8 @@ class User inboxes.joins(:question).where(questions: { user_id: target_user.id, author_is_anonymous: false }).destroy_all ListMember.joins(:list).where(list: { user_id: target_user.id }, user_id: id).destroy_all end + + def cache_key_blocked_user_ids = "#{cache_key}/blocked_user_ids" end end end diff --git a/app/models/user/relationship/mute.rb b/app/models/user/relationship/mute.rb index 08fe286e..58e2b0a1 100644 --- a/app/models/user/relationship/mute.rb +++ b/app/models/user/relationship/mute.rb @@ -16,19 +16,41 @@ class User has_many :muted_by_users, through: :passive_mute_relationships, source: :source end + # Mute an user def mute(target_user) raise Errors::MutingSelf if target_user == self - create_relationship(active_mute_relationships, target_user) + create_relationship(active_mute_relationships, target_user).tap do + expire_muted_user_ids_cache + end end + # Unmute an user def unmute(target_user) - destroy_relationship(active_mute_relationships, target_user) + destroy_relationship(active_mute_relationships, target_user).tap do + expire_muted_user_ids_cache + end end + # Is self muting target_user? def muting?(target_user) relationship_active?(muted_users, target_user) end + + # Expires the muted user ids cache + def expire_muted_user_ids_cache = Rails.cache.delete(cache_key_muted_user_ids) + + # Cached ids of the muted users + def muted_user_ids_cached + Rails.cache.fetch(cache_key_muted_user_ids, expires_in: 1.hour) do + muted_user_ids + end + end + + private + + # Cache key for the muted_user_ids + def cache_key_muted_user_ids = "#{cache_key}/muted_user_ids" end end end diff --git a/app/models/user/timeline_methods.rb b/app/models/user/timeline_methods.rb index a0fa8c55..822f9d8f 100644 --- a/app/models/user/timeline_methods.rb +++ b/app/models/user/timeline_methods.rb @@ -8,7 +8,17 @@ module User::TimelineMethods # @return [ActiveRecord::Relation] the user's timeline def timeline Answer - .where("user_id in (?) OR user_id = ?", following_ids, id) + .then do |query| + blocked_and_muted_user_ids = blocked_user_ids_cached + muted_user_ids_cached + next query if blocked_and_muted_user_ids.empty? + + # build a more complex query if we block or mute someone + # otherwise the query ends up as "anon OR (NOT anon AND user_id NOT IN (NULL))" which will only return anonymous questions + query + .joins(:question) + .where("questions.author_is_anonymous OR (NOT questions.author_is_anonymous AND questions.user_id NOT IN (?))", blocked_and_muted_user_ids) + end + .where("answers.user_id in (?) OR answers.user_id = ?", following_ids, id) .order(:created_at) .reverse_order .includes(comments: %i[user smiles], question: { user: :profile }, user: [:profile], smiles: [:user]) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2981df53..4097c4a2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -69,7 +69,7 @@ RSpec.describe User, type: :model do describe "email validation" do subject do - FactoryBot.build(:user, email: email).tap(&:validate).errors[:email] + FactoryBot.build(:user, email:).tap(&:validate).errors[:email] end shared_examples_for "valid email" do |example_email| @@ -211,12 +211,134 @@ RSpec.describe User, type: :model do expect(subject).to eq(expected) end end + + context "user follows users with answers to questions from blocked or muted users" do + let(:blocked_user) { FactoryBot.create(:user) } + let(:muted_user) { FactoryBot.create(:user) } + let(:user1) { FactoryBot.create(:user) } + let(:user2) { FactoryBot.create(:user) } + let!(:answer_to_anonymous) do + FactoryBot.create( + :answer, + user: user1, + content: "answer to a true anonymous coward", + question: FactoryBot.create( + :question, + author_is_anonymous: true + ) + ) + end + let!(:answer_to_normal_user) do + FactoryBot.create( + :answer, + user: user2, + content: "answer to a normal user", + question: FactoryBot.create( + :question, + user: user1, + author_is_anonymous: false + ) + ) + end + let!(:answer_to_normal_user_anonymous) do + FactoryBot.create( + :answer, + user: user2, + content: "answer to a cowardly user", + question: FactoryBot.create( + :question, + user: user1, + author_is_anonymous: true + ) + ) + end + let!(:answer_to_blocked_user) do + FactoryBot.create( + :answer, + user: user1, + content: "answer to a blocked user", + question: FactoryBot.create( + :question, + user: blocked_user, + author_is_anonymous: false + ) + ) + end + let!(:answer_to_blocked_user_anonymous) do + FactoryBot.create( + :answer, + user: user1, + content: "answer to a blocked user who's a coward", + question: FactoryBot.create( + :question, + user: blocked_user, + author_is_anonymous: true + ) + ) + end + let!(:answer_to_muted_user) do + FactoryBot.create( + :answer, + user: user2, + content: "answer to a muted user", + question: FactoryBot.create( + :question, + user: muted_user, + author_is_anonymous: false + ) + ) + end + let!(:answer_to_muted_user_anonymous) do + FactoryBot.create( + :answer, + user: user2, + content: "answer to a muted user who's a coward", + question: FactoryBot.create( + :question, + user: muted_user, + author_is_anonymous: true + ) + ) + end + + before do + me.follow user1 + me.follow user2 + end + + it "includes all answers to questions the user follows" do + expect(subject).to include(answer_to_anonymous) + expect(subject).to include(answer_to_normal_user) + expect(subject).to include(answer_to_normal_user_anonymous) + expect(subject).to include(answer_to_blocked_user_anonymous) + expect(subject).to include(answer_to_muted_user_anonymous) + expect(subject).to include(answer_to_blocked_user) + expect(subject).to include(answer_to_muted_user) + end + + context "when blocking and muting some users" do + before do + me.block blocked_user + me.mute muted_user + end + + it "only includes answers to questions from users the user doesn't block or mute" do + expect(subject).to include(answer_to_anonymous) + expect(subject).to include(answer_to_normal_user) + expect(subject).to include(answer_to_normal_user_anonymous) + expect(subject).to include(answer_to_blocked_user_anonymous) + expect(subject).to include(answer_to_muted_user_anonymous) + expect(subject).not_to include(answer_to_blocked_user) + expect(subject).not_to include(answer_to_muted_user) + end + end + end end describe "#cursored_timeline" do let(:last_id) { nil } - subject { me.cursored_timeline(last_id: last_id, size: 3) } + subject { me.cursored_timeline(last_id:, size: 3) } context "user answered nothing and is not following anyone" do include_examples "result is blank"