diff --git a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb index 0d7669c5e92..2c7d1d163e5 100644 --- a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb @@ -11,6 +11,7 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController channel: result.channel, tracking: result.tracking, memberships: result.memberships, + load_more_url: result.load_more_url, ), ::Chat::ThreadListSerializer, root: false, diff --git a/plugins/chat/app/models/chat/threads_view.rb b/plugins/chat/app/models/chat/threads_view.rb index 55b9cc22bca..fc98ea8ace9 100644 --- a/plugins/chat/app/models/chat/threads_view.rb +++ b/plugins/chat/app/models/chat/threads_view.rb @@ -2,14 +2,15 @@ module Chat class ThreadsView - attr_reader :user, :channel, :threads, :tracking, :memberships + attr_reader :user, :channel, :threads, :tracking, :memberships, :load_more_url - def initialize(channel:, threads:, user:, tracking:, memberships:) + def initialize(channel:, threads:, user:, tracking:, memberships:, load_more_url:) @channel = channel @threads = threads @user = user @tracking = tracking @memberships = memberships + @load_more_url = load_more_url end end end diff --git a/plugins/chat/app/serializers/chat/thread_list_serializer.rb b/plugins/chat/app/serializers/chat/thread_list_serializer.rb index e2d7373d621..c67b0016963 100644 --- a/plugins/chat/app/serializers/chat/thread_list_serializer.rb +++ b/plugins/chat/app/serializers/chat/thread_list_serializer.rb @@ -21,7 +21,7 @@ module Chat end def meta - { channel_id: object.channel.id } + { channel_id: object.channel.id, load_more_url: object.load_more_url } end end end diff --git a/plugins/chat/app/services/chat/lookup_channel_threads.rb b/plugins/chat/app/services/chat/lookup_channel_threads.rb index 15948a5aba2..93ec6a87486 100644 --- a/plugins/chat/app/services/chat/lookup_channel_threads.rb +++ b/plugins/chat/app/services/chat/lookup_channel_threads.rb @@ -10,41 +10,57 @@ module Chat # of normal or tracking will be returned. # # @example - # Chat::LookupChannelThreads.call(channel_id: 2, guardian: guardian) + # Chat::LookupChannelThreads.call(channel_id: 2, guardian: guardian, limit: 5, offset: 2) # class LookupChannelThreads include Service::Base - MAX_THREADS = 50 + THREADS_LIMIT = 10 - # @!method call(channel_id:, guardian:) + # @!method call(channel_id:, guardian:, limit: nil, offset: nil) # @param [Integer] channel_id # @param [Guardian] guardian + # @param [Integer] limit + # @param [Integer] offset # @return [Service::Base::Context] policy :threaded_discussions_enabled contract + step :set_limit + step :set_offset model :channel policy :threading_enabled_for_channel policy :can_view_channel model :threads step :fetch_tracking step :fetch_memberships + step :build_load_more_url # @!visibility private class Contract attribute :channel_id, :integer validates :channel_id, presence: true + + attribute :limit, :integer + attribute :offset, :integer end private + def set_limit(contract:, **) + context.limit = (contract.limit || THREADS_LIMIT).to_i.clamp(1, THREADS_LIMIT) + end + + def set_offset(contract:, **) + context.offset = [contract.offset || 0, 0].max + end + def threaded_discussions_enabled - SiteSetting.enable_experimental_chat_threaded_discussions + ::SiteSetting.enable_experimental_chat_threaded_discussions end def fetch_channel(contract:, **) - Chat::Channel.find_by(id: contract.channel_id) + ::Chat::Channel.strict_loading.includes(:chatable).find_by(id: contract.channel_id) end def threading_enabled_for_channel(channel:, **) @@ -65,13 +81,16 @@ module Chat OR tracked_threads_subquery.latest_message_id > user_chat_thread_memberships_chat_threads.last_read_message_id SQL .order("tracked_threads_subquery.latest_message_created_at DESC") - .limit(MAX_THREADS) + .limit(context.limit) + .offset(context.offset) .to_a # We do this to avoid having to query additional threads if the user # already has a lot of unread threads. - if unread_threads.length < MAX_THREADS - final_limit = MAX_THREADS - unread_threads.length + if unread_threads.length < context.limit + final_limit = context.limit - unread_threads.length + final_offset = context.offset + unread_threads.length + read_threads = threads_query(guardian, channel) .where(<<~SQL) @@ -79,6 +98,7 @@ module Chat SQL .order("tracked_threads_subquery.latest_message_created_at DESC") .limit(final_limit) + .offset(final_offset) .to_a end @@ -86,7 +106,7 @@ module Chat if threads.present? last_replies = - Chat::Message + ::Chat::Message .strict_loading .includes(:user, :uploads) .from(<<~SQL) @@ -101,7 +121,6 @@ module Chat "INNER JOIN chat_messages ON chat_messages.id = last_replies_subquery.latest_message_id", ) .index_by(&:thread_id) - threads.each { |thread| thread.last_reply = last_replies[thread.id] } end @@ -126,13 +145,14 @@ module Chat end def threads_query(guardian, channel) - Chat::Thread + ::Chat::Thread .strict_loading .includes( :channel, :user_chat_thread_memberships, original_message_user: :user_status, original_message: [ + :uploads, :chat_webhook_event, :chat_channel, chat_mentions: { @@ -154,7 +174,8 @@ module Chat end def tracked_threads_subquery(guardian, channel) - Chat::Thread + ::Chat::Thread + .strict_loading .joins(:chat_messages, :user_chat_thread_memberships) .joins( "LEFT JOIN chat_messages original_messages ON chat_threads.original_message_id = original_messages.id", @@ -167,8 +188,8 @@ module Chat .where( "user_chat_thread_memberships.notification_level IN (?)", [ - Chat::UserChatThreadMembership.notification_levels[:normal], - Chat::UserChatThreadMembership.notification_levels[:tracking], + ::Chat::UserChatThreadMembership.notification_levels[:normal], + ::Chat::UserChatThreadMembership.notification_levels[:tracking], ], ) .where( @@ -180,5 +201,14 @@ module Chat ) .to_sql end + + def build_load_more_url(contract:, **) + load_more_params = { offset: context.offset + context.limit }.to_query + context.load_more_url = + ::URI::HTTP.build( + path: "/chat/api/channels/#{contract.channel_id}/threads", + query: load_more_params, + ).request_uri + end end end diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-browse-view.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-browse-view.hbs index 22d14b9e2cd..6776a23a109 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-browse-view.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-browse-view.hbs @@ -52,7 +52,7 @@ {{#if (and - (not this.channelsCollection.length) (not this.channelsCollection.loading) + this.channelsCollection.fetchedOnce (not this.channelsCollection.length) ) }}
@@ -61,14 +61,14 @@

{{i18n "chat.empty_state.direct_message"}}

{{else if this.channelsCollection.length}}
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-browse-view.js b/plugins/chat/assets/javascripts/discourse/components/chat-browse-view.js index 70c584587a1..ac4a09ffc73 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-browse-view.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-browse-view.js @@ -50,7 +50,7 @@ export default class ChatBrowseView extends Component { onScroll() { discourseDebounce( this, - this.channelsCollection.loadMore, + this.channelsCollection.load, { filter: this.filter, status: this.status }, INPUT_DELAY ); @@ -58,6 +58,8 @@ export default class ChatBrowseView extends Component { @action debouncedFiltering(event) { + this.set("channelsCollection", this.chatApi.channels()); + discourseDebounce( this, this.channelsCollection.load, diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-members-view.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-channel-members-view.hbs index 56be4a2a42d..54f52fe97b1 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-members-view.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel-members-view.hbs @@ -1,8 +1,5 @@ {{#if (gt this.channel.membershipsCount 0)}} - +
{{else}} - {{#unless this.isFetchingMembers}} - {{i18n "chat.channel.no_memberships_found"}} - {{/unless}} + {{#if this.members.fetchedOnce}} +
+ {{i18n "chat.channel.no_memberships_found"}} +
+ {{/if}} {{/each}}
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-members-view.js b/plugins/chat/assets/javascripts/discourse/components/chat-channel-members-view.js index 6c56520e2d3..6063524a3ff 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-members-view.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel-members-view.js @@ -43,6 +43,7 @@ export default class ChatChannelMembersView extends Component { @action onFilterMembers(username) { this.set("filter", username); + this.set("members", this.chatApi.listChannelMemberships(this.channel.id)); discourseDebounce( this, @@ -53,8 +54,8 @@ export default class ChatChannelMembersView extends Component { } @action - loadMore() { - discourseDebounce(this, this.members.loadMore, INPUT_DELAY); + load() { + discourseDebounce(this, this.members.load, INPUT_DELAY); } _focusSearch() { diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel.js b/plugins/chat/assets/javascripts/discourse/components/chat-channel.js index e746dde30fc..60a80674b9a 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel.js @@ -216,7 +216,7 @@ export default class ChatLivePane extends Component { if (result.threads) { result.threads.forEach((thread) => { - const storedThread = this.args.channel.threadsManager.store( + const storedThread = this.args.channel.threadsManager.add( this.args.channel, thread, { replace: true } @@ -332,7 +332,7 @@ export default class ChatLivePane extends Component { if (result.threads) { result.threads.forEach((thread) => { - const storedThread = this.args.channel.threadsManager.store( + const storedThread = this.args.channel.threadsManager.add( this.args.channel, thread, { replace: true } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.hbs index ef81a25b158..ed91b635bc3 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.hbs @@ -12,17 +12,27 @@ {{/if}}
- {{#if this.loading}} - {{loading-spinner size="medium"}} + {{#each this.sortedThreads as |thread|}} + {{else}} - {{#each this.sortedThreads as |thread|}} - - {{else}} + {{#if this.threadsCollection.fetchedOnce}}
{{i18n "chat.threads.none"}}
- {{/each}} - {{/if}} + {{/if}} + {{/each}} +
{{/if}} \ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.js b/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.js index bbc0b8c84f8..0989b0ba656 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.js @@ -1,24 +1,25 @@ import Component from "@glimmer/component"; import { bind } from "discourse-common/utils/decorators"; -import { tracked } from "@glimmer/tracking"; +import { cached } from "@glimmer/tracking"; import { action } from "@ember/object"; import { inject as service } from "@ember/service"; export default class ChatThreadList extends Component { @service chat; + @service chatApi; @service messageBus; + @service chatTrackingStateManager; - @tracked loading = true; + get threadsManager() { + return this.args.channel.threadsManager; + } // NOTE: This replicates sort logic from the server. We need this because // the thread unread count + last reply date + time update when new messages // are sent to the thread, and we want the list to react in realtime to this. get sortedThreads() { - if (!this.args.channel.threadsManager.threads) { - return []; - } - - return this.args.channel.threadsManager.threads + return this.threadsManager.threads + .filter((thread) => !thread.originalMessage.deletedAt) .sort((threadA, threadB) => { // If both are unread we just want to sort by last reply date + time descending. if (threadA.tracking.unreadCount && threadB.tracking.unreadCount) { @@ -50,14 +51,18 @@ export default class ChatThreadList extends Component { } else { return 1; } - }) - .filter((thread) => !thread.originalMessage.deletedAt); + }); } get shouldRender() { return !!this.args.channel; } + @action + loadThreads() { + return this.threadsCollection.load({ limit: 10 }); + } + @action subscribe() { this.#unsubscribe(); @@ -82,11 +87,10 @@ export default class ChatThreadList extends Component { } handleDeleteMessage(data) { - const deletedOriginalMessageThread = - this.args.channel.threadsManager.threads.findBy( - "originalMessage.id", - data.deleted_id - ); + const deletedOriginalMessageThread = this.threadsManager.threads.findBy( + "originalMessage.id", + data.deleted_id + ); if (!deletedOriginalMessageThread) { return; @@ -96,11 +100,10 @@ export default class ChatThreadList extends Component { } handleRestoreMessage(data) { - const restoredOriginalMessageThread = - this.args.channel.threadsManager.threads.findBy( - "originalMessage.id", - data.chat_message.id - ); + const restoredOriginalMessageThread = this.threadsManager.threads.findBy( + "originalMessage.id", + data.chat_message.id + ); if (!restoredOriginalMessageThread) { return; @@ -109,17 +112,29 @@ export default class ChatThreadList extends Component { restoredOriginalMessageThread.originalMessage.deletedAt = null; } - @action - loadThreads() { - this.loading = true; - this.args.channel.threadsManager.index(this.args.channel.id).finally(() => { - this.loading = false; + @cached + get threadsCollection() { + return this.chatApi.threads(this.args.channel.id, this.handleLoadedThreads); + } + + @bind + handleLoadedThreads(result) { + return result.threads.map((thread) => { + const threadModel = this.threadsManager.add(this.args.channel, thread, { + replace: true, + }); + + this.chatTrackingStateManager.setupChannelThreadState( + this.args.channel, + result.tracking + ); + + return threadModel; }); } @action teardown() { - this.loading = true; this.#unsubscribe(); } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread-list/item.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/thread-list/item.hbs index 0f8799c2efc..09e5177fd37 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/thread-list/item.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread-list/item.hbs @@ -4,6 +4,7 @@ (if (gt @thread.tracking.unreadCount 0) "-is-unread") }} data-thread-id={{@thread.id}} + ...attributes >
{ - return this.#loadIndex(channelId).then((result) => { - const threads = result.threads.map((thread) => { - return channel.threadsManager.store(channel, thread, { - replace: true, - }); - }); - - this.chatTrackingStateManager.setupChannelThreadState( - channel, - result.tracking - ); - - return { threads, meta: result.meta }; - }); - }); - } - - get threads() { - return Object.values(this._cached); - } - - store(channel, threadObject, options = {}) { + add(channel, threadObject, options = {}) { let model; if (!options.replace) { - model = this.#findStale(threadObject.id); + model = this.#getFromCache(threadObject.id); } if (!model) { @@ -88,23 +68,19 @@ export default class ChatThreadsManager { return model; } - async #find(channelId, threadId) { - return this.chatApi.thread(channelId, threadId).then((result) => { - return this.chatChannelsManager.find(channelId).then((channel) => { - return channel.threadsManager.store(channel, result.thread); - }); - }); - } - #cache(thread) { this._cached[thread.id] = thread; } - #findStale(id) { + #getFromCache(id) { return this._cached[id]; } - async #loadIndex(channelId) { - return this.chatApi.threads(channelId); + async #fetchFromServer(channelId, threadId) { + return this.chatApi.thread(channelId, threadId).then((result) => { + return this.chatChannelsManager.find(channelId).then((channel) => { + return channel.threadsManager.add(channel, result.thread); + }); + }); } } diff --git a/plugins/chat/assets/javascripts/discourse/lib/collection.js b/plugins/chat/assets/javascripts/discourse/lib/collection.js index 8fea367f766..e472d45bffe 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/collection.js +++ b/plugins/chat/assets/javascripts/discourse/lib/collection.js @@ -10,6 +10,7 @@ export default class Collection { @tracked items = []; @tracked meta = {}; @tracked loading = false; + @tracked fetchedOnce = false; constructor(resourceURL, handler) { this._resourceURL = resourceURL; @@ -18,15 +19,15 @@ export default class Collection { } get loadMoreURL() { - return this.meta.load_more_url; + return this.meta?.load_more_url; } get totalRows() { - return this.meta.total_rows; + return this.meta?.total_rows; } get length() { - return this.items.length; + return this.items?.length; } // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols @@ -35,7 +36,7 @@ export default class Collection { return { next: () => { - if (index < this.items.length) { + if (index < this.length) { return { value: this.items[index++], done: false }; } else { return { done: true }; @@ -50,69 +51,48 @@ export default class Collection { */ @bind load(params = {}) { - this._fetchedAll = false; - - if (this.loading) { + if ( + this.loading || + this._fetchedAll || + (this.totalRows && this.items.length >= this.totalRows) + ) { return Promise.resolve(); } this.loading = true; - const filteredQueryParams = Object.entries(params).filter( - ([, v]) => v !== undefined - ); - const queryString = new URLSearchParams(filteredQueryParams).toString(); + let endpoint; + if (this.loadMoreURL) { + endpoint = this.loadMoreURL; + } else { + const filteredQueryParams = Object.entries(params).filter( + ([, v]) => v !== undefined + ); + + const queryString = new URLSearchParams(filteredQueryParams).toString(); + endpoint = this._resourceURL + (queryString ? `?${queryString}` : ""); + } - const endpoint = this._resourceURL + (queryString ? `?${queryString}` : ""); return this.#fetch(endpoint) .then((result) => { - this.items = this._handler(result); + const items = this._handler(result); + + if (items.length) { + this.items = (this.items ?? []).concat(items); + } + + if (!items.length || items.length < params.limit) { + this._fetchedAll = true; + } + this.meta = result.meta; + this.fetchedOnce = true; }) .finally(() => { this.loading = false; }); } - /** - * Attempts to load more results - * @returns {Promise} - */ - @bind - loadMore() { - let promise = Promise.resolve(); - - if (this.loading) { - return promise; - } - - if ( - this._fetchedAll || - (this.totalRows && this.items.length >= this.totalRows) - ) { - return promise; - } - - this.loading = true; - - if (this.loadMoreURL) { - promise = this.#fetch(this.loadMoreURL).then((result) => { - const newItems = this._handler(result); - - if (newItems.length) { - this.items = this.items.concat(newItems); - } else { - this._fetchedAll = true; - } - this.meta = result.meta; - }); - } - - return promise.finally(() => { - this.loading = false; - }); - } - #fetch(url) { return ajax(url, { type: "GET" }); } diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js index e1e2a51dc71..a580b2fc9ef 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js @@ -269,7 +269,7 @@ export default class ChatChannel { }); clonedMessage.thread = thread; - this.threadsManager.store(this, thread); + this.threadsManager.add(this, thread); thread.messagesManager.addMessages([clonedMessage]); return thread; diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js index 54d86607e81..6e123503e9f 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js @@ -81,8 +81,11 @@ export default class ChatApi extends Service { * @param {number} channelId - The ID of the channel. * @returns {Promise} */ - threads(channelId) { - return this.#getRequest(`/channels/${channelId}/threads`); + threads(channelId, handler) { + return new Collection( + `${this.#basePath}/channels/${channelId}/threads`, + handler + ); } /** diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-tracking-state-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-tracking-state-manager.js index 9964a158b9d..208eac437b6 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-tracking-state-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-tracking-state-manager.js @@ -34,8 +34,9 @@ export default class ChatTrackingStateManager extends Service { setupChannelThreadState(channel, threadTracking) { channel.threadsManager.threads.forEach((thread) => { - if (threadTracking[thread.id.toString()]) { - this.#setState(thread, threadTracking[thread.id.toString()]); + const tracking = threadTracking[thread.id.toString()]; + if (tracking) { + this.#setState(thread, tracking); } }); } diff --git a/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb b/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb index f724565e1c3..824d31452ee 100644 --- a/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb +++ b/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb @@ -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 diff --git a/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb b/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb index 89cfb06a179..0f24ea3b335 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb @@ -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)