From 1ffa2e51253a9072c2da154ff4030c35404484b5 Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Tue, 21 Feb 2023 10:12:13 +0100 Subject: [PATCH] filter out non-anon questions from blocked/muted users in questions view --- app/controllers/question_controller.rb | 6 +- app/models/question/answer_methods.rb | 11 +- spec/controllers/question_controller_spec.rb | 21 +++- spec/models/question_spec.rb | 107 +++++++++++++++---- 4 files changed, 117 insertions(+), 28 deletions(-) diff --git a/app/controllers/question_controller.rb b/app/controllers/question_controller.rb index f8a3ffa1..9d9c2891 100644 --- a/app/controllers/question_controller.rb +++ b/app/controllers/question_controller.rb @@ -1,9 +1,11 @@ +# frozen_string_literal: true + class QuestionController < ApplicationController def show @question = Question.find(params[:id]) - @answers = @question.cursored_answers(last_id: params[:last_id]) + @answers = @question.cursored_answers(last_id: params[:last_id], current_user:) @answers_last_id = @answers.map(&:id).min - @more_data_available = !@question.cursored_answers(last_id: @answers_last_id, size: 1).count.zero? + @more_data_available = !@question.cursored_answers(last_id: @answers_last_id, size: 1, current_user:).count.zero? respond_to do |format| format.html diff --git a/app/models/question/answer_methods.rb b/app/models/question/answer_methods.rb index 2de7f3b2..efb94a7b 100644 --- a/app/models/question/answer_methods.rb +++ b/app/models/question/answer_methods.rb @@ -5,8 +5,17 @@ module Question::AnswerMethods define_cursor_paginator :cursored_answers, :ordered_answers - def ordered_answers + def ordered_answers(current_user: nil) answers + .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? + + query + .where.not(answers: { user_id: blocked_and_muted_user_ids }) + end .order(:created_at) .reverse_order end diff --git a/spec/controllers/question_controller_spec.rb b/spec/controllers/question_controller_spec.rb index c8daf340..59baab9d 100644 --- a/spec/controllers/question_controller_spec.rb +++ b/spec/controllers/question_controller_spec.rb @@ -8,11 +8,11 @@ describe QuestionController, type: :controller do before do stub_const("APP_CONFIG", { - "site_name" => "Specspring", - "hostname" => "test.host", - "https" => false, - "items_per_page" => 10, - }) + "site_name" => "Specspring", + "hostname" => "test.host", + "https" => false, + "items_per_page" => 10, + }) end context "question exists" do @@ -56,6 +56,17 @@ describe QuestionController, type: :controller do expect(assigns(:more_data_available)).to eq(true) end end + + context "when signed in" do + before do + sign_in(FactoryBot.create(:user)) + end + + it "is fine" do + # basic test to make sure nothing breaks with an user + expect(subject).to have_http_status(:ok) + end + end end end end diff --git a/spec/models/question_spec.rb b/spec/models/question_spec.rb index 7da9eab0..ab4e6cfc 100644 --- a/spec/models/question_spec.rb +++ b/spec/models/question_spec.rb @@ -1,30 +1,97 @@ -require 'rails_helper' +# frozen_string_literal: true -RSpec.describe Question, :type => :model do - before :each do - @question = Question.new( - content: 'Is this a question?', - user: FactoryBot.create(:user) - ) +require "rails_helper" + +RSpec.describe Question, type: :model do + let(:user) { FactoryBot.create :user } + + let(:question) { Question.new(content: "Is this a question?", user:) } + + subject { question } + + it { is_expected.to respond_to(:content) } + + describe "#content" do + it "returns a string" do + expect(question.content).to match "Is this a question?" + end end - subject { @question } + context "when it has many answers" do + before do + 5.times do |i| + Answer.create( + content: "This is an answer. #{i}", + user: FactoryBot.create(:user), + question: + ) + end + end - it { should respond_to(:content) } + its(:answer_count) { is_expected.to eq 5 } - it '#content returns a string' do - expect(@question.content).to match 'Is this a question?' + it "deletes the answers when deleted" do + first_answer_id = question.answers.first.id + question.destroy + expect { Answer.find(first_answer_id) }.to raise_error(ActiveRecord::RecordNotFound) + end end - it 'has many answers' do - 5.times { |i| Answer.create(content: "This is an answer. #{i}", user: FactoryBot.create(:user), question: @question) } - expect(@question.answer_count).to match 5 - end + describe "#ordered_answers" do + let(:normal_user) { FactoryBot.create(:user) } - it 'also deletes the answers when deleted' do - 5.times { |i| Answer.create(content: "This is an answer. #{i}", user: FactoryBot.create(:user), question: @question) } - first_answer_id = @question.answers.first.id - @question.destroy - expect{Answer.find(first_answer_id)}.to raise_error(ActiveRecord::RecordNotFound) + let(:blocked_user) { FactoryBot.create(:user) } + + let(:muted_user) { FactoryBot.create(:user) } + + let!(:answer_from_normal_user) do + FactoryBot.create( + :answer, + user: normal_user, + content: "answer from a normal user", + question: + ) + end + + let!(:answer_from_blocked_user) do + FactoryBot.create( + :answer, + user: blocked_user, + content: "answer from a blocked user", + question: + ) + end + + let!(:answer_from_muted_user) do + FactoryBot.create( + :answer, + user: muted_user, + content: "answer from a blocked user", + question: + ) + end + + subject { question.ordered_answers } + + it "includes all answers to questions" do + expect(subject).to include(answer_from_normal_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 + before do + user.block blocked_user + user.mute muted_user + end + + subject { question.ordered_answers current_user: user } + + it "only includes answers to questions from users the user doesn't block or mute" do + expect(subject).to include(answer_from_normal_user) + expect(subject).not_to include(answer_from_blocked_user) + expect(subject).not_to include(answer_from_muted_user) + end + end end end