diff --git a/app/controllers/timeline_controller.rb b/app/controllers/timeline_controller.rb index 14a27a5c..30c3fc48 100644 --- a/app/controllers/timeline_controller.rb +++ b/app/controllers/timeline_controller.rb @@ -11,7 +11,7 @@ class TimelineController < ApplicationController def list @title = list_title(@list) - paginate_timeline { |args| @list.cursored_timeline(**args) } + paginate_timeline { |args| @list.cursored_timeline(**args, current_user:) } end def public diff --git a/app/models/list/timeline_methods.rb b/app/models/list/timeline_methods.rb index 3cf68e5e..7909d544 100644 --- a/app/models/list/timeline_methods.rb +++ b/app/models/list/timeline_methods.rb @@ -6,7 +6,21 @@ module List::TimelineMethods define_cursor_paginator :cursored_timeline, :timeline # @return [Array] the lists' timeline - def timeline - Answer.where('user_id in (?)', members.pluck(:user_id)).order(:created_at).reverse_order + def timeline(current_user: nil) + Answer + .then do |query| + next query unless current_user + + blocked_and_muted_user_ids = current_user.blocked_user_ids_cached + current_user.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) + .where.not(answers: { user_id: blocked_and_muted_user_ids }) + end + .where(answers: { user_id: members.pluck(:user_id) }).order(:created_at).reverse_order end end diff --git a/spec/models/list_spec.rb b/spec/models/list_spec.rb index 4732d873..8e5068dd 100644 --- a/spec/models/list_spec.rb +++ b/spec/models/list_spec.rb @@ -1,24 +1,24 @@ # frozen_string_literal: true -require 'rails_helper' +require "rails_helper" RSpec.describe(List, type: :model) do - let(:user) { FactoryBot.build(:user) } + let(:user) { FactoryBot.create(:user) } - describe 'name mangling' do + describe "name mangling" do subject do - List.new(user: user, display_name: display_name).tap(&:validate) + List.new(user:, display_name:).tap(&:validate) end { - 'great list' => 'great-list', - 'followers' => '-followers-', - ' followers ' => '-followers-', - " the game \t\nyes" => 'the-game-yes', + "great list" => "great-list", + "followers" => "-followers-", + " followers " => "-followers-", + " the game \t\nyes" => "the-game-yes", # not nice, but this is just the way it is: - "\u{1f98a} :3" => '3', - "\u{1f98a}" => '' + "\u{1f98a} :3" => "3", + "\u{1f98a}" => "", }.each do |display_name, expected_name| context "when display name is #{display_name.inspect}" do let(:display_name) { display_name } @@ -28,31 +28,31 @@ RSpec.describe(List, type: :model) do end end - describe 'validations' do + describe "validations" do subject do - List.new(user: user, display_name: display_name).validate + List.new(user:, display_name:).validate end context "when display name is 'great list' (valid)" do - let(:display_name) { 'great list' } + let(:display_name) { "great list" } it { is_expected.to be true } end context "when display name is '1' (valid)" do - let(:display_name) { '1' } + let(:display_name) { "1" } it { is_expected.to be true } end - context 'when display name is the letter E 621 times (invalid, too long)' do - let(:display_name) { 'E' * 621 } + context "when display name is the letter E 621 times (invalid, too long)" do + let(:display_name) { "E" * 621 } it { is_expected.to be false } end - context 'when display name is an empty string (invalid, as `name` would be empty)' do - let(:display_name) { '' } + context "when display name is an empty string (invalid, as `name` would be empty)" do + let(:display_name) { "" } it { is_expected.to be false } end @@ -63,4 +63,153 @@ RSpec.describe(List, type: :model) do it { is_expected.to be false } end end + + describe "#timeline" do + let(:list) { List.create(user:, display_name: "test list") } + let(:user1) { FactoryBot.create(:user) } + let(:user2) { FactoryBot.create(:user) } + + let(:blocked_user) { FactoryBot.create(:user) } + let(:muted_user) { 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_from_blocked_user) do + FactoryBot.create( + :answer, + user: blocked_user, + content: "answer from a blocked user", + question: FactoryBot.create(:question) + ) + 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_from_muted_user) do + FactoryBot.create( + :answer, + user: muted_user, + content: "answer from a muted user", + question: FactoryBot.create(:question) + ) + 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 + list.add_member user1 + list.add_member user2 + list.add_member blocked_user + list.add_member muted_user + + # block it here already, to test behaviour without a `current_user` passed in + user.block blocked_user + user.mute muted_user + end + + subject { list.timeline } + + it "includes all answers to questions from users in the list" 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) + expect(subject).to include(answer_from_blocked_user) + expect(subject).to include(answer_from_muted_user) + end + + context "when given a current user who blocks and mutes some users" do + subject { list.timeline current_user: user } + + 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_from_blocked_user) + expect(subject).not_to include(answer_to_muted_user) + expect(subject).not_to include(answer_from_muted_user) + end + end + end end