diff --git a/app/controllers/inbox_controller.rb b/app/controllers/inbox_controller.rb index 6954b5a1..19c6a70d 100644 --- a/app/controllers/inbox_controller.rb +++ b/app/controllers/inbox_controller.rb @@ -1,50 +1,19 @@ +# frozen_string_literal: true + class InboxController < ApplicationController before_action :authenticate_user! def show - @inbox = current_user.cursored_inbox(last_id: params[:last_id]) - @inbox_last_id = @inbox.map(&:id).min - @more_data_available = !current_user.cursored_inbox(last_id: @inbox_last_id, size: 1).count.zero? - @inbox_count = current_user.inboxes.count + find_author + find_inbox_entries - if params[:author].present? - begin - @author = true - @target_user = User.where('LOWER(screen_name) = ?', params[:author].downcase).first! - @inbox_author = @inbox.joins(:question) - .where(questions: { user_id: @target_user.id, author_is_anonymous: false }) - @inbox_author_count = current_user.inboxes - .joins(:question) - .where(questions: { user_id: @target_user.id, author_is_anonymous: false }) - .count - - if @inbox_author.empty? - @empty = true - flash.now[:info] = t(".author.info", author: params[:author]) - else - @inbox = @inbox_author - @inbox_count = @inbox_author_count - @inbox_last_id = @inbox.map(&:id).min - @more_data_available = !current_user.cursored_inbox(last_id: @inbox_last_id, size: 1) - .joins(:question) - .where(questions: { user_id: @target_user.id, author_is_anonymous: false }) - .count - .zero? - end - rescue => e - Sentry.capture_exception(e) - flash.now[:error] = t(".author.error", author: params[:author]) - @not_found = true - end + if @author_user && @inbox_count.zero? + # rubocop disabled because of a false positive + flash[:info] = t(".author.info", author: @author) # rubocop:disable Rails/ActionControllerFlashBeforeRender + redirect_to inbox_path(last_id: params[:last_id]) end - if @empty or @not_found - @delete_id = "ib-delete-all" - elsif @author - @delete_id = "ib-delete-all-author" - else - @delete_id = "ib-delete-all" - end + @delete_id = find_delete_id @disabled = true if @inbox.empty? respond_to do |format| @@ -52,7 +21,8 @@ class InboxController < ApplicationController format.turbo_stream do render "show", layout: false, status: :see_other - @inbox.update_all(new: false) + # rubocop disabled as just flipping a flag doesn't need to have validations to be run + @inbox.update_all(new: false) # rubocop:disable Rails/SkipsModelValidations end end end @@ -75,4 +45,36 @@ class InboxController < ApplicationController format.html { redirect_to inbox_path } end end + + private + + def find_author + return if params[:author].blank? + + @author = params[:author] + + @author_user = User.where("LOWER(screen_name) = ?", @author.downcase).first + flash.now[:error] = t(".author.error", author: @author) unless @author_user + end + + def find_inbox_entries + @inbox = current_user.cursored_inbox(last_id: params[:last_id]).then(&method(:filter_author_chain)) + @inbox_last_id = @inbox.map(&:id).min + @more_data_available = current_user.cursored_inbox(last_id: @inbox_last_id, size: 1).then(&method(:filter_author_chain)).count.positive? + @inbox_count = current_user.inboxes.then(&method(:filter_author_chain)).count + end + + def find_delete_id + return "ib-delete-all-author" if @author_user && @inbox_count.positive? + + "ib-delete-all" + end + + def filter_author_chain(query) + return query unless @author_user + + query + .joins(:question) + .where(questions: { user: @author_user, author_is_anonymous: false }) + end end diff --git a/app/views/inbox/show.html.haml b/app/views/inbox/show.html.haml index f829ea7e..5c5de4e4 100644 --- a/app/views/inbox/show.html.haml +++ b/app/views/inbox/show.html.haml @@ -1,11 +1,14 @@ #entries - @inbox.each do |i| - = render "inbox/entry", i: i + = render "inbox/entry", i: - if @inbox.empty? %p.empty= t(".empty") - if @more_data_available .d-flex.justify-content-center#paginator - = button_to inbox_path(last_id: @inbox_last_id), class: "btn btn-light" do - = t("voc.load") + = button_to t("voc.load"), inbox_path, + class: "btn btn-light", + method: :get, + params: { last_id: @inbox_last_id, author: @author }.compact, + form: { data: { turbo_stream: true } } diff --git a/app/views/inbox/show.turbo_stream.haml b/app/views/inbox/show.turbo_stream.haml index 8dd7cd2e..fffc48fc 100644 --- a/app/views/inbox/show.turbo_stream.haml +++ b/app/views/inbox/show.turbo_stream.haml @@ -1,8 +1,11 @@ = turbo_stream.append "entries" do - @inbox.each do |i| - = render "inbox/entry", i: i + = render "inbox/entry", i: = turbo_stream.update "paginator" do - if @more_data_available - = button_to t("voc.load"), inbox_path(last_id: @inbox_last_id), class: "btn btn-light" - + = button_to t("voc.load"), inbox_path, + class: "btn btn-light", + method: :get, + params: { last_id: @inbox_last_id, author: @author }.compact, + form: { data: { turbo_stream: true } } diff --git a/config/routes.rb b/config/routes.rb index 90d380ca..c68933ef 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -149,7 +149,7 @@ Rails.application.routes.draw do match "/notifications(/:type)", to: "notifications#index", via: [:get, :post], as: :notifications, defaults: { type: "new" } post "/inbox/create", to: "inbox#create", as: :inbox_create - match "/inbox(/:author)", via: [:get, :post], to: "inbox#show", as: :inbox + get "/inbox", to: "inbox#show", as: :inbox match "/user/:username(/p/:page)", to: "user#show", via: [:get, :post], defaults: { page: 1 } match "/@:username(/p/:page)", to: "user#show", via: [:get, :post], as: :user, defaults: { page: 1 } diff --git a/spec/controllers/inbox_controller_spec.rb b/spec/controllers/inbox_controller_spec.rb index b7426e7b..6cb2e128 100644 --- a/spec/controllers/inbox_controller_spec.rb +++ b/spec/controllers/inbox_controller_spec.rb @@ -6,21 +6,264 @@ describe InboxController, type: :controller do let(:user) { FactoryBot.create(:user) } describe "#show" do + shared_examples_for "sets the expected ivars" do + let(:expected_assigns) { {} } + + it "sets the expected ivars" do + subject + + expected_assigns.each do |name, value| + expect(assigns[name]).to eq(value) + end + end + end + subject { get :show } + it "redirects to the login page when not signed in" do + expect(subject).to redirect_to(new_user_session_path) + end + context "when user is signed in" do before(:each) { sign_in(user) } - it "shows the inbox" do + it "renders the correct template" do subject expect(response).to render_template("show") end + + context "when inbox is empty" do + include_examples "sets the expected ivars", + inbox: [], + inbox_last_id: nil, + more_data_available: false, + inbox_count: 0, + delete_id: "ib-delete-all", + disabled: true + end + + context "when inbox has an amount of questions less than page size" do + let!(:inbox_entry) { Inbox.create(user:, new: true, question: FactoryBot.create(:question)) } + + include_examples "sets the expected ivars" do + let(:expected_assigns) do + { + inbox: [inbox_entry], + inbox_last_id: inbox_entry.id, + more_data_available: false, + inbox_count: 1, + delete_id: "ib-delete-all", + disabled: nil + } + end + end + + context "when requested the turbo stream format" do + subject { get :show, format: :turbo_stream } + + it "updates the inbox entry status" do + expect { subject }.to change { inbox_entry.reload.new? }.from(true).to(false) + end + end + end + + context "when inbox has an amount of questions more than page size" do + let(:inbox_entry_fillers_page1) do + # 9 times => 1 entry less than default page size + 9.times.map { Inbox.create(user:, question: FactoryBot.create(:question)) } + end + let(:last_inbox_entry_page1) { Inbox.create(user:, question: FactoryBot.create(:question)) } + let(:inbox_entry_fillers_page2) do + 5.times.map { Inbox.create(user:, question: FactoryBot.create(:question)) } + end + let(:last_inbox_entry_page2) { Inbox.create(user:, question: FactoryBot.create(:question)) } + + before do + # create inbox entries in reverse so pagination works as expected + last_inbox_entry_page2 + inbox_entry_fillers_page2 + last_inbox_entry_page1 + inbox_entry_fillers_page1 + end + + include_examples "sets the expected ivars" do + let(:expected_assigns) do + { + inbox: [*inbox_entry_fillers_page1.reverse, last_inbox_entry_page1], + inbox_last_id: last_inbox_entry_page1.id, + more_data_available: true, + inbox_count: 16, + delete_id: "ib-delete-all", + disabled: nil + } + end + end + + context "when passed the last_id param" do + subject { get :show, params: { last_id: last_inbox_entry_page1.id } } + + include_examples "sets the expected ivars" do + let(:expected_assigns) do + { + inbox: [*inbox_entry_fillers_page2.reverse, last_inbox_entry_page2], + inbox_last_id: last_inbox_entry_page2.id, + more_data_available: false, + inbox_count: 16, + delete_id: "ib-delete-all", + disabled: nil + } + end + end + end + end + + context "when passed the author param" do + let!(:other_user) { FactoryBot.create(:user) } + let!(:unrelated_user) { FactoryBot.create(:user) } + + let!(:generic_inbox_entry1) do + Inbox.create( + user:, + question: FactoryBot.create( + :question, + user: unrelated_user, + author_is_anonymous: false + ) + ) + end + let!(:generic_inbox_entry2) { Inbox.create(user:, question: FactoryBot.create(:question)) } + + subject { get :show, params: { author: author_param } } + + context "with a nonexisting screen name" do + let(:author_param) { "xXx420MegaGamer2003xXx" } + + it "sets the error flash" do + subject + expect(flash[:error]).to eq "No user with the name @xXx420MegaGamer2003xXx found, showing entries from all users instead!" + end + + include_examples "sets the expected ivars" do + let(:expected_assigns) do + { + inbox: [generic_inbox_entry2, generic_inbox_entry1], + inbox_last_id: generic_inbox_entry1.id, + more_data_available: false, + inbox_count: 2, + delete_id: "ib-delete-all", + disabled: nil + } + end + end + end + + context "with an existing screen name" do + let(:author_param) { other_user.screen_name } + + context "with no questions from the other user in the inbox" do + it { is_expected.to redirect_to inbox_path } + + it "sets the info flash" do + subject + expect(flash[:info]).to eq "No questions from @#{other_user.screen_name} found, showing entries from all users instead!" + end + + include_examples "sets the expected ivars" do + # these are the ivars set before the redirect happened + let(:expected_assigns) do + { + inbox: [], + inbox_last_id: nil, + more_data_available: false, + inbox_count: 0, + delete_id: "ib-delete-all", + disabled: true + } + end + end + end + + context "with no non-anonymous questions from the other user in the inbox" do + let!(:anonymous_inbox_entry) do + Inbox.create( + user:, + question: FactoryBot.create( + :question, + user: other_user, + author_is_anonymous: true + ) + ) + end + + it { is_expected.to redirect_to inbox_path } + + it "sets the info flash" do + subject + expect(flash[:info]).to eq "No questions from @#{other_user.screen_name} found, showing entries from all users instead!" + end + + include_examples "sets the expected ivars" do + # these are the ivars set before the redirect happened + let(:expected_assigns) do + { + inbox: [], + inbox_last_id: nil, + more_data_available: false, + inbox_count: 0, + delete_id: "ib-delete-all", + disabled: true + } + end + end + end + + context "with both non-anonymous and anonymous questions from the other user in the inbox" do + let!(:non_anonymous_inbox_entry) do + Inbox.create( + user:, + question: FactoryBot.create( + :question, + user: other_user, + author_is_anonymous: false + ) + ) + end + let!(:anonymous_inbox_entry) do + Inbox.create( + user:, + question: FactoryBot.create( + :question, + user: other_user, + author_is_anonymous: true + ) + ) + end + + include_examples "sets the expected ivars" do + let(:expected_assigns) do + { + inbox: [non_anonymous_inbox_entry], + inbox_last_id: non_anonymous_inbox_entry.id, + more_data_available: false, + inbox_count: 1, + delete_id: "ib-delete-all-author", + disabled: nil + } + end + end + end + end + end end end describe "#create" do subject { post :create } + it "redirects to the login page when not signed in" do + expect(subject).to redirect_to(new_user_session_path) + end + context "when user is signed in" do before(:each) { sign_in(user) }