Merge pull request #974 from Retrospring/refactor/inbox

refactor inbox
This commit is contained in:
Georg Gadinger 2023-01-21 13:59:39 +01:00 committed by GitHub
commit 9d7c99fe9b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 300 additions and 49 deletions

View File

@ -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

View File

@ -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 } }

View File

@ -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 } }

View File

@ -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 }

View File

@ -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) }