Add more generic implementation of inbox filtering
This commit is contained in:
parent
cb0229b12b
commit
1ac767902b
|
@ -4,16 +4,8 @@ class InboxController < ApplicationController
|
||||||
before_action :authenticate_user!
|
before_action :authenticate_user!
|
||||||
|
|
||||||
def show
|
def show
|
||||||
find_author
|
|
||||||
find_inbox_entries
|
find_inbox_entries
|
||||||
|
|
||||||
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])
|
|
||||||
return
|
|
||||||
end
|
|
||||||
|
|
||||||
@delete_id = find_delete_id
|
@delete_id = find_delete_id
|
||||||
@disabled = true if @inbox.empty?
|
@disabled = true if @inbox.empty?
|
||||||
|
|
||||||
|
@ -47,36 +39,24 @@ class InboxController < ApplicationController
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def find_author
|
def filter_params
|
||||||
return if params[:author].blank?
|
params.slice(*InboxFilter::KEYS).permit(*InboxFilter::KEYS)
|
||||||
|
|
||||||
@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
|
end
|
||||||
|
|
||||||
def find_inbox_entries
|
def find_inbox_entries
|
||||||
@inbox = current_user.cursored_inbox(last_id: params[:last_id]).then(&method(:filter_author_chain))
|
filter = InboxFilter.new(current_user, filter_params)
|
||||||
|
@inbox = filter.cursored_results(last_id: params[:last_id])
|
||||||
@inbox_last_id = @inbox.map(&:id).min
|
@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?
|
@more_data_available = filter.cursored_results(last_id: @inbox_last_id, size: 1).count.positive?
|
||||||
@inbox_count = current_user.inboxes.then(&method(:filter_author_chain)).count
|
@inbox_count = filter.results.count
|
||||||
end
|
end
|
||||||
|
|
||||||
def find_delete_id
|
def find_delete_id
|
||||||
return "ib-delete-all-author" if @author_user && @inbox_count.positive?
|
return "ib-delete-all-author" if params[:author].present? && @inbox_count.positive?
|
||||||
|
|
||||||
"ib-delete-all"
|
"ib-delete-all"
|
||||||
end
|
end
|
||||||
|
|
||||||
def filter_author_chain(query)
|
|
||||||
return query unless @author_user
|
|
||||||
|
|
||||||
query
|
|
||||||
.joins(:question)
|
|
||||||
.where(questions: { user: @author_user, author_is_anonymous: false })
|
|
||||||
end
|
|
||||||
|
|
||||||
# rubocop:disable Rails/SkipsModelValidations
|
# rubocop:disable Rails/SkipsModelValidations
|
||||||
def mark_inbox_entries_as_read
|
def mark_inbox_entries_as_read
|
||||||
# using .dup to not modify @inbox -- useful in tests
|
# using .dup to not modify @inbox -- useful in tests
|
||||||
|
|
|
@ -0,0 +1,45 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
class InboxFilter
|
||||||
|
include CursorPaginatable
|
||||||
|
|
||||||
|
define_cursor_paginator :cursored_results, :results
|
||||||
|
|
||||||
|
KEYS = %i(
|
||||||
|
author
|
||||||
|
anonymous
|
||||||
|
).freeze
|
||||||
|
|
||||||
|
attr_reader :params, :user
|
||||||
|
|
||||||
|
def initialize(user, params)
|
||||||
|
@user = user
|
||||||
|
@params = params
|
||||||
|
end
|
||||||
|
|
||||||
|
def results
|
||||||
|
scope = @user.inboxes
|
||||||
|
.includes(:question, user: :profile)
|
||||||
|
.order(:created_at)
|
||||||
|
.reverse_order
|
||||||
|
|
||||||
|
params.each do |key, value|
|
||||||
|
scope.merge!(scope_for(key, value)) if value.present?
|
||||||
|
end
|
||||||
|
|
||||||
|
scope
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def scope_for(key, value)
|
||||||
|
case key.to_s
|
||||||
|
when "author"
|
||||||
|
@user.inboxes.joins(question: [:user])
|
||||||
|
.where(questions: { users: { screen_name: value }, author_is_anonymous: false })
|
||||||
|
when "anonymous"
|
||||||
|
@user.inboxes.joins(:question)
|
||||||
|
.where(questions: { author_is_anonymous: true })
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -144,39 +144,10 @@ describe InboxController, type: :controller do
|
||||||
|
|
||||||
subject { get :show, params: { author: author_param } }
|
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
|
context "with an existing screen name" do
|
||||||
let(:author_param) { other_user.screen_name }
|
let(:author_param) { other_user.screen_name }
|
||||||
|
|
||||||
context "with no questions from the other user in the inbox" do
|
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
|
include_examples "sets the expected ivars" do
|
||||||
# these are the ivars set before the redirect happened
|
# these are the ivars set before the redirect happened
|
||||||
let(:expected_assigns) do
|
let(:expected_assigns) do
|
||||||
|
@ -202,13 +173,6 @@ describe InboxController, type: :controller do
|
||||||
)
|
)
|
||||||
end
|
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
|
include_examples "sets the expected ivars" do
|
||||||
# these are the ivars set before the redirect happened
|
# these are the ivars set before the redirect happened
|
||||||
let(:expected_assigns) do
|
let(:expected_assigns) do
|
||||||
|
@ -259,6 +223,37 @@ describe InboxController, type: :controller do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context "when passed the anonymous param" do
|
||||||
|
let!(:other_user) { FactoryBot.create(:user) }
|
||||||
|
let!(:generic_inbox_entry) do
|
||||||
|
Inbox.create(
|
||||||
|
user:,
|
||||||
|
question: FactoryBot.create(
|
||||||
|
:question,
|
||||||
|
user: other_user,
|
||||||
|
author_is_anonymous: false
|
||||||
|
)
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
let!(:inbox_entry_fillers) do
|
||||||
|
# 9 times => 1 entry less than default page size
|
||||||
|
9.times.map { Inbox.create(user:, question: FactoryBot.create(:question, author_is_anonymous: true)) }
|
||||||
|
end
|
||||||
|
|
||||||
|
subject { get :show, params: { anonymous: true } }
|
||||||
|
|
||||||
|
include_examples "sets the expected ivars" do
|
||||||
|
let(:expected_assigns) do
|
||||||
|
{
|
||||||
|
inbox: [*inbox_entry_fillers.reverse],
|
||||||
|
more_data_available: false,
|
||||||
|
inbox_count: 9
|
||||||
|
}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue