FIX: allows listing messages of any thread (#27259)
Before this fix we could only list messages of a thread if it was part of a `threading_enabled` channel or if the thread was set to `force`. Due to our design of also using a thread id when this is just a chain of replies so we can switch from threading enabled to disabled at any time, we will allow `Chat:: ListChannelThreadMessages` to list the messages of any thread, the only important requirements are: - having a thread id - being able to access this thread To allow this, this commit simply removes the check on `threading_enabled` or `force`.
This commit is contained in:
parent
222a5f4677
commit
5aefda1dee
|
@ -13,7 +13,6 @@ class Chat::Api::ChannelThreadMessagesController < Chat::ApiController
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
on_failure { render(json: failed_json, status: 422) }
|
on_failure { render(json: failed_json, status: 422) }
|
||||||
on_failed_policy(:ensure_thread_enabled) { raise Discourse::NotFound }
|
|
||||||
on_failed_policy(:target_message_exists) { raise Discourse::NotFound }
|
on_failed_policy(:target_message_exists) { raise Discourse::NotFound }
|
||||||
on_failed_policy(:can_view_thread) { raise Discourse::InvalidAccess }
|
on_failed_policy(:can_view_thread) { raise Discourse::InvalidAccess }
|
||||||
on_model_not_found(:thread) { raise Discourse::NotFound }
|
on_model_not_found(:thread) { raise Discourse::NotFound }
|
||||||
|
|
|
@ -18,7 +18,6 @@ module Chat
|
||||||
contract
|
contract
|
||||||
|
|
||||||
model :thread
|
model :thread
|
||||||
policy :ensure_thread_enabled
|
|
||||||
policy :can_view_thread
|
policy :can_view_thread
|
||||||
step :fetch_optional_membership
|
step :fetch_optional_membership
|
||||||
step :determine_target_message_id
|
step :determine_target_message_id
|
||||||
|
@ -62,10 +61,6 @@ module Chat
|
||||||
::Chat::Thread.strict_loading.includes(channel: :chatable).find_by(id: contract.thread_id)
|
::Chat::Thread.strict_loading.includes(channel: :chatable).find_by(id: contract.thread_id)
|
||||||
end
|
end
|
||||||
|
|
||||||
def ensure_thread_enabled(thread:)
|
|
||||||
thread.channel.threading_enabled || thread.force
|
|
||||||
end
|
|
||||||
|
|
||||||
def can_view_thread(guardian:, thread:)
|
def can_view_thread(guardian:, thread:)
|
||||||
guardian.user == Discourse.system_user || guardian.can_preview_chat_channel?(thread.channel)
|
guardian.user == Discourse.system_user || guardian.can_preview_chat_channel?(thread.channel)
|
||||||
end
|
end
|
||||||
|
|
|
@ -86,9 +86,6 @@ module ChatSDK
|
||||||
on_success { result.messages }
|
on_success { result.messages }
|
||||||
on_failed_policy(:can_view_thread) { raise "Guardian can't view thread" }
|
on_failed_policy(:can_view_thread) { raise "Guardian can't view thread" }
|
||||||
on_failed_policy(:target_message_exists) { raise "Target message doesn't exist" }
|
on_failed_policy(:target_message_exists) { raise "Target message doesn't exist" }
|
||||||
on_failed_policy(:ensure_thread_enabled) do
|
|
||||||
raise "Threading is not enabled for this channel"
|
|
||||||
end
|
|
||||||
on_failure { raise "Unexpected error" }
|
on_failure { raise "Unexpected error" }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -60,18 +60,6 @@ RSpec.describe Chat::Api::ChannelThreadMessagesController do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when channel disabled threading" do
|
|
||||||
fab!(:thread) do
|
|
||||||
Fabricate(:chat_thread, channel: Fabricate(:chat_channel, threading_enabled: false))
|
|
||||||
end
|
|
||||||
|
|
||||||
it "returns a 404" do
|
|
||||||
get "/chat/api/channels/#{thread.channel.id}/threads/#{thread.id}/messages"
|
|
||||||
|
|
||||||
expect(response.status).to eq(404)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
context "when params are invalid" do
|
context "when params are invalid" do
|
||||||
it "returns a 400" do
|
it "returns a 400" do
|
||||||
get "/chat/api/channels/#{thread.channel.id}/threads/#{thread.id}/messages?page_size=9999"
|
get "/chat/api/channels/#{thread.channel.id}/threads/#{thread.id}/messages?page_size=9999"
|
||||||
|
|
|
@ -4,9 +4,7 @@ RSpec.describe Chat::ListChannelThreadMessages do
|
||||||
subject(:result) { described_class.call(params) }
|
subject(:result) { described_class.call(params) }
|
||||||
|
|
||||||
fab!(:user)
|
fab!(:user)
|
||||||
fab!(:thread) do
|
fab!(:thread) { Fabricate(:chat_thread, channel: Fabricate(:chat_channel)) }
|
||||||
Fabricate(:chat_thread, channel: Fabricate(:chat_channel, threading_enabled: true))
|
|
||||||
end
|
|
||||||
|
|
||||||
let(:guardian) { Guardian.new(user) }
|
let(:guardian) { Guardian.new(user) }
|
||||||
let(:thread_id) { thread.id }
|
let(:thread_id) { thread.id }
|
||||||
|
@ -37,34 +35,9 @@ RSpec.describe Chat::ListChannelThreadMessages do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when ensure_thread_enabled?" do
|
|
||||||
context "when channel threading is disabled" do
|
|
||||||
before { thread.channel.update!(threading_enabled: false) }
|
|
||||||
|
|
||||||
it { is_expected.to fail_a_policy(:ensure_thread_enabled) }
|
|
||||||
|
|
||||||
context "when the thread is forced" do
|
|
||||||
before { thread.update!(force: true) }
|
|
||||||
|
|
||||||
it { is_expected.to be_a_success }
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
context "when channel and site setting are enabling threading" do
|
|
||||||
before { thread.channel.update!(threading_enabled: true) }
|
|
||||||
|
|
||||||
it { is_expected.to be_a_success }
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
context "when can_view_thread" do
|
context "when can_view_thread" do
|
||||||
context "when channel is private" do
|
context "when channel is private" do
|
||||||
fab!(:thread) do
|
fab!(:thread) { Fabricate(:chat_thread, channel: Fabricate(:private_category_channel)) }
|
||||||
Fabricate(
|
|
||||||
:chat_thread,
|
|
||||||
channel: Fabricate(:private_category_channel, threading_enabled: true),
|
|
||||||
)
|
|
||||||
end
|
|
||||||
|
|
||||||
it { is_expected.to fail_a_policy(:can_view_thread) }
|
it { is_expected.to fail_a_policy(:can_view_thread) }
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue