mirror of
https://github.com/discourse/discourse.git
synced 2026-08-17 16:35:03 -05:00
WIP: threads list pagination (#22502)
This implementation will need more work in the future. For simplification of tracking and other events (new thread, delete/restore OM...) we used the threads from `threadsManager` which makes pagination more complicated as we already have some results when we start. Note this commit also simplify `Collection` to only have one `load` method which can be called repeatedly.
This commit is contained in:
Joffrey JAFFEUX
@@ -1,131 +1,247 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
RSpec.describe Chat::LookupChannelThreads do
|
||||
describe Chat::LookupChannelThreads::Contract, type: :model do
|
||||
it { is_expected.to validate_presence_of :channel_id }
|
||||
end
|
||||
RSpec.describe ::Chat::LookupChannelThreads::Contract, type: :model do
|
||||
it { is_expected.to validate_presence_of :channel_id }
|
||||
end
|
||||
|
||||
describe ".call" do
|
||||
subject(:result) { described_class.call(params) }
|
||||
RSpec.describe ::Chat::LookupChannelThreads do
|
||||
subject(:result) { described_class.call(params) }
|
||||
|
||||
fab!(:current_user) { Fabricate(:user) }
|
||||
fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) }
|
||||
fab!(:channel_with_no_threads) { Fabricate(:chat_channel, threading_enabled: true) }
|
||||
fab!(:thread_1) { Fabricate(:chat_thread, channel: channel) }
|
||||
fab!(:thread_2) { Fabricate(:chat_thread, channel: channel) }
|
||||
fab!(:thread_3) { Fabricate(:chat_thread, channel: channel) }
|
||||
fab!(:current_user) { Fabricate(:user) }
|
||||
|
||||
let(:guardian) { Guardian.new(current_user) }
|
||||
let(:params) { { guardian: guardian, channel_id: thread_1.channel_id } }
|
||||
let(:guardian) { Guardian.new(current_user) }
|
||||
let(:channel_id) { nil }
|
||||
let(:limit) { 10 }
|
||||
let(:offset) { 0 }
|
||||
let(:params) { { guardian: guardian, channel_id: channel_id, limit: limit, offset: offset } }
|
||||
|
||||
context "when enable_experimental_chat_threaded_discussions is disabled" do
|
||||
before { SiteSetting.enable_experimental_chat_threaded_discussions = true }
|
||||
|
||||
describe "policy - threaded_discussions_enabled" do
|
||||
context "when disabled" do
|
||||
before { SiteSetting.enable_experimental_chat_threaded_discussions = false }
|
||||
|
||||
it { is_expected.to fail_a_policy(:threaded_discussions_enabled) }
|
||||
end
|
||||
end
|
||||
|
||||
context "when enable_experimental_chat_threaded_discussions is enabled" do
|
||||
before do
|
||||
SiteSetting.enable_experimental_chat_threaded_discussions = true
|
||||
[thread_1, thread_2, thread_3].each do |t|
|
||||
t.original_message.update!(created_at: 1.week.ago)
|
||||
t.add(current_user)
|
||||
end
|
||||
describe "step - set_limit" do
|
||||
fab!(:channel_1) { Fabricate(:chat_channel) }
|
||||
let(:channel_id) { channel_1.id }
|
||||
|
||||
context "when limit is not set" do
|
||||
let(:limit) { nil }
|
||||
|
||||
it "defaults to a max value" do
|
||||
expect(result.limit).to eq(described_class::THREADS_LIMIT)
|
||||
end
|
||||
end
|
||||
|
||||
it "does not return any threads when a channel has no threads" do
|
||||
expect(
|
||||
described_class.call(channel_id: channel_with_no_threads.id, guardian:).threads,
|
||||
).to eq([])
|
||||
context "when limit is over max" do
|
||||
let(:limit) { described_class::THREADS_LIMIT + 1 }
|
||||
|
||||
it "defaults to a max value" do
|
||||
expect(result.limit).to eq(described_class::THREADS_LIMIT)
|
||||
end
|
||||
end
|
||||
|
||||
context "when all steps pass" do
|
||||
before do
|
||||
msg_1 =
|
||||
Fabricate(:chat_message, user: current_user, chat_channel: channel, thread: thread_1)
|
||||
msg_1.update!(created_at: 10.minutes.ago)
|
||||
msg_2 =
|
||||
Fabricate(:chat_message, user: current_user, chat_channel: channel, thread: thread_2)
|
||||
msg_2.update!(created_at: 1.day.ago)
|
||||
msg_3 =
|
||||
Fabricate(:chat_message, user: current_user, chat_channel: channel, thread: thread_3)
|
||||
msg_3.update!(created_at: 2.seconds.ago)
|
||||
end
|
||||
context "when limit is under min" do
|
||||
let(:limit) { 0 }
|
||||
|
||||
it "sets the service result as successful" do
|
||||
expect(result).to be_a_success
|
||||
end
|
||||
it "defaults to a max value" do
|
||||
expect(result.limit).to eq(1)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it "returns the threads ordered by the last reply created_at date and time for the thread" do
|
||||
expect(result.threads.map(&:id)).to eq([thread_3.id, thread_1.id, thread_2.id])
|
||||
end
|
||||
describe "step - set_offset" do
|
||||
fab!(:channel_1) { Fabricate(:chat_channel) }
|
||||
let(:channel_id) { channel_1.id }
|
||||
|
||||
it "orders threads with unread messages at the top even if their last reply created_at date and time is older" do
|
||||
unread_message = Fabricate(:chat_message, chat_channel: channel, thread: thread_2)
|
||||
unread_message.update!(created_at: 2.days.ago)
|
||||
expect(result.threads.map(&:id)).to eq([thread_2.id, thread_3.id, thread_1.id])
|
||||
end
|
||||
context "when offset is not set" do
|
||||
let(:offset) { nil }
|
||||
|
||||
it "does not return threads where the original message is trashed" do
|
||||
thread_1.original_message.trash!
|
||||
expect(result.threads.map(&:id)).to eq([thread_3.id, thread_2.id])
|
||||
end
|
||||
it "defaults to zero" do
|
||||
expect(result.offset).to eq(0)
|
||||
end
|
||||
end
|
||||
|
||||
it "does not return threads where the original message is deleted" do
|
||||
thread_1.original_message.destroy
|
||||
expect(result.threads.map(&:id)).to eq([thread_3.id, thread_2.id])
|
||||
end
|
||||
context "when offset is under min" do
|
||||
let(:offset) { -99 }
|
||||
|
||||
it "does not count deleted messages for sort order" do
|
||||
Chat::Message.where(thread: thread_3).each(&:trash!)
|
||||
expect(result.threads.map(&:id)).to eq([thread_1.id, thread_2.id])
|
||||
end
|
||||
it "defaults to a min value" do
|
||||
expect(result.offset).to eq(0)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it "only returns threads where the user has their thread notification level as tracking or regular" do
|
||||
new_thread_1 = Fabricate(:chat_thread, channel: channel)
|
||||
new_thread_2 = Fabricate(:chat_thread, channel: channel)
|
||||
new_thread_1.add(current_user)
|
||||
new_thread_1.membership_for(current_user).update!(
|
||||
notification_level: Chat::UserChatThreadMembership.notification_levels[:muted],
|
||||
)
|
||||
describe "model - channel" do
|
||||
context "when channel doesn’t exist" do
|
||||
let(:channel_id) { -999 }
|
||||
|
||||
expect(result.threads.map(&:id)).to eq([thread_3.id, thread_1.id, thread_2.id])
|
||||
end
|
||||
it { is_expected.to fail_to_find_a_model(:channel) }
|
||||
end
|
||||
end
|
||||
|
||||
it "does not return threads from another channel" do
|
||||
thread_4 = Fabricate(:chat_thread)
|
||||
describe "policy - threading_enabled_for_channel" do
|
||||
context "when channel threading is disabled" do
|
||||
fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: false) }
|
||||
let(:channel_id) { channel_1.id }
|
||||
|
||||
it { is_expected.to fail_a_policy(:threading_enabled_for_channel) }
|
||||
end
|
||||
end
|
||||
|
||||
describe "policy - can_view_channel" do
|
||||
context "when channel threading is disabled" do
|
||||
fab!(:channel_1) { Fabricate(:private_category_channel, threading_enabled: true) }
|
||||
let(:channel_id) { channel_1.id }
|
||||
|
||||
it { is_expected.to fail_a_policy(:can_view_channel) }
|
||||
end
|
||||
end
|
||||
|
||||
context "when channel has no threads" do
|
||||
fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) }
|
||||
let(:channel_id) { channel_1.id }
|
||||
|
||||
describe "model - threads" do
|
||||
it "returns an empty list of threads" do
|
||||
expect(result.threads).to eq([])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context "when channel has threads" do
|
||||
fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) }
|
||||
fab!(:thread_1) { Fabricate(:chat_thread, channel: channel_1) }
|
||||
fab!(:thread_2) { Fabricate(:chat_thread, channel: channel_1) }
|
||||
fab!(:thread_3) { Fabricate(:chat_thread, channel: channel_1) }
|
||||
|
||||
let(:channel_id) { channel_1.id }
|
||||
|
||||
before do
|
||||
[thread_1, thread_2, thread_3].each.with_index do |t, index|
|
||||
t.original_message.update!(created_at: (index + 1).weeks.ago)
|
||||
t.add(current_user)
|
||||
end
|
||||
end
|
||||
|
||||
describe "model - threads" do
|
||||
it { is_expected.to be_a_success }
|
||||
|
||||
it "orders threads by the last reply created_at timestamp" do
|
||||
[
|
||||
[thread_1, 10.minutes.ago],
|
||||
[thread_2, 1.day.ago],
|
||||
[thread_3, 2.seconds.ago],
|
||||
].each do |thread, created_at|
|
||||
Fabricate(
|
||||
:chat_message,
|
||||
user: current_user,
|
||||
thread: thread_4,
|
||||
chat_channel: thread_4.channel,
|
||||
created_at: 2.seconds.ago,
|
||||
chat_channel: channel_1,
|
||||
thread: thread,
|
||||
created_at: created_at,
|
||||
)
|
||||
expect(result.threads.map(&:id)).to eq([thread_3.id, thread_1.id, thread_2.id])
|
||||
end
|
||||
|
||||
expect(result.threads.map(&:id)).to eq([thread_3.id, thread_1.id, thread_2.id])
|
||||
end
|
||||
|
||||
it "sorts by unread over recency" do
|
||||
unread_message = Fabricate(:chat_message, chat_channel: channel_1, thread: thread_2)
|
||||
unread_message.update!(created_at: 2.days.ago)
|
||||
|
||||
expect(result.threads.map(&:id)).to eq([thread_2.id, thread_1.id, thread_3.id])
|
||||
end
|
||||
|
||||
it "does not return threads where the original message is trashed" do
|
||||
thread_1.original_message.trash!
|
||||
|
||||
expect(result.threads.map(&:id)).to eq([thread_2.id, thread_3.id])
|
||||
end
|
||||
|
||||
it "does not return threads where the original message is deleted" do
|
||||
thread_1.original_message.destroy
|
||||
|
||||
expect(result.threads.map(&:id)).to eq([thread_2.id, thread_3.id])
|
||||
end
|
||||
|
||||
it "does not return threads from another channel" do
|
||||
thread_4 = Fabricate(:chat_thread)
|
||||
Fabricate(
|
||||
:chat_message,
|
||||
user: current_user,
|
||||
thread: thread_4,
|
||||
chat_channel: thread_4.channel,
|
||||
created_at: 2.seconds.ago,
|
||||
)
|
||||
|
||||
expect(result.threads.map(&:id)).to eq([thread_1.id, thread_2.id, thread_3.id])
|
||||
end
|
||||
|
||||
it "only returns threads where the user has their thread notification level as tracking or regular" do
|
||||
thread_4 = Fabricate(:chat_thread, channel: channel_1)
|
||||
thread_4.add(current_user)
|
||||
thread_4.membership_for(current_user).update!(
|
||||
notification_level: ::Chat::UserChatThreadMembership.notification_levels[:muted],
|
||||
)
|
||||
thread_5 = Fabricate(:chat_thread, channel: channel_1)
|
||||
|
||||
expect(result.threads.map(&:id)).to eq([thread_1.id, thread_2.id, thread_3.id])
|
||||
end
|
||||
|
||||
it "does not count deleted messages for sort order" do
|
||||
unread_message = Fabricate(:chat_message, chat_channel: channel_1, thread: thread_3)
|
||||
unread_message.update!(created_at: 2.days.ago)
|
||||
unread_message.trash!
|
||||
|
||||
expect(result.threads.map(&:id)).to eq([thread_1.id, thread_2.id, thread_3.id])
|
||||
end
|
||||
|
||||
context "when limit param is set" do
|
||||
let(:limit) { 1 }
|
||||
|
||||
it "limits the number of threads returned" do
|
||||
expect(result.threads).to contain_exactly(thread_1)
|
||||
end
|
||||
end
|
||||
|
||||
context "when params are not valid" do
|
||||
before { params.delete(:channel_id) }
|
||||
context "when offset param is set" do
|
||||
let(:offset) { 1 }
|
||||
|
||||
it { is_expected.to fail_a_contract }
|
||||
end
|
||||
|
||||
context "when user cannot see channel" do
|
||||
fab!(:private_channel) { Fabricate(:private_category_channel, group: Fabricate(:group)) }
|
||||
|
||||
before do
|
||||
thread_1.update!(channel: private_channel)
|
||||
private_channel.update!(threading_enabled: true)
|
||||
it "returns results from the offset the number of threads returned" do
|
||||
expect(result.threads).to eq([thread_2, thread_3])
|
||||
end
|
||||
|
||||
it { is_expected.to fail_a_policy(:can_view_channel) }
|
||||
end
|
||||
end
|
||||
|
||||
context "when threading is not enabled for the channel" do
|
||||
before { channel.update!(threading_enabled: false) }
|
||||
describe "step - fetch_tracking" do
|
||||
it "returns correct threads tracking" do
|
||||
expect(result.tracking).to eq(
|
||||
::Chat::TrackingStateReportQuery.call(
|
||||
guardian: guardian,
|
||||
thread_ids: [thread_1, thread_2, thread_3].map(&:id),
|
||||
include_threads: true,
|
||||
).thread_tracking,
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
it { is_expected.to fail_a_policy(:threading_enabled_for_channel) }
|
||||
describe "step - fetch_memberships" do
|
||||
it "returns correct memberships" do
|
||||
expect(result.memberships).to eq(
|
||||
::Chat::UserChatThreadMembership.where(
|
||||
thread_id: [thread_1, thread_2, thread_3].map(&:id),
|
||||
user_id: current_user.id,
|
||||
),
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
describe "step - build_load_more_url" do
|
||||
it "returns a url with the correct params" do
|
||||
expect(result.load_more_url).to eq("/chat/api/channels/#{channel_1.id}/threads?offset=10")
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -23,12 +23,16 @@ module PageObjects
|
||||
item_by_id(thread.id)
|
||||
end
|
||||
|
||||
def has_threads?(count:)
|
||||
component.has_css?(".chat-thread-list-item", count: count)
|
||||
end
|
||||
|
||||
def has_no_thread?(thread)
|
||||
component.has_no_css?(item_by_id_selector(thread.id))
|
||||
end
|
||||
|
||||
def item_by_id(id)
|
||||
component.find(item_by_id_selector(id))
|
||||
component.find(item_by_id_selector(id), visible: :all)
|
||||
end
|
||||
|
||||
def avatar_selector(user)
|
||||
|
||||
Reference in New Issue
Block a user