mirror of
https://github.com/chatwoot/chatwoot.git
synced 2026-06-04 21:02:35 +08:00
035d2858f5
\`GET /platform/api/v1/agent_bots\` returns 500 when any \`AgentBot\` that was previously registered with a Platform App has since been deleted. The bug was introduced by a missing \`dependent: :destroy\` on the \`AgentBot\` model — deleting a bot left orphaned rows in \`platform_app_permissibles\`, which the index action later iterated over and crashed rendering with a \`NoMethodError\` on \`nil\`. Closes #13407 ## Root cause The index action loads all \`platform_app_permissibles\` for the platform app and passes each \`resource.permissible\` (the associated \`AgentBot\`) to a Jbuilder partial. When the \`AgentBot\` no longer exists, \`resource.permissible\` returns \`nil\` and the partial crashes calling \`.id\`, \`.name\`, etc. on it. Every other \`AgentBot\` association (\`agent_bot_inboxes\`, \`messages\`, \`assigned_conversations\`) had a \`dependent:\` option — \`platform_app_permissibles\` was the only one missing it. There was also an N+1 query: the index fired a separate SQL query per permissible to load each bot. ## What changed **1. Model — prevent orphans at deletion time** \`\`\`ruby has_many :platform_app_permissibles, as: :permissible, dependent: :destroy \`\`\` **2. Controller — eager-load to eliminate N+1** \`\`\`ruby @resources = @platform_app.platform_app_permissibles .where(permissible_type: 'AgentBot') .includes(:permissible) \`\`\` **3. Jbuilder — defensive nil guard for pre-existing orphans** \`\`\`ruby bot = resource.permissible next if bot.nil? json.partial! '...', resource: bot \`\`\` ## Trade-offs considered | Option | Decision | |---|---| | Rescue \`NoMethodError\` in jbuilder | Hides the failure rather than fixing it. Rejected. | | Only add the nil guard, skip the model fix | Leaves the data integrity gap open — future deletions continue creating orphans. Rejected. | | Both layers (chosen) | Model fix prevents new orphans; nil guard is defence-in-depth for any orphans that survived before deployment. | | \`dependent: :nullify\` | Doesn't apply — a nullified permissible would still cause the same nil dereference. Rejected. | ## How to reproduce 1. Create an AgentBot via the Platform API 2. Delete the AgentBot via any path (admin UI, API, or direct model call) 3. Call \`GET /platform/api/v1/agent_bots\` with a Platform App token 4. Observe 500 After this fix, the endpoint returns 200 with an empty array. Co-authored-by: Ramalau Debeila <rdebeila@datacentrix.co.za>
72 lines
1.7 KiB
Ruby
72 lines
1.7 KiB
Ruby
# == Schema Information
|
|
#
|
|
# Table name: agent_bots
|
|
#
|
|
# id :bigint not null, primary key
|
|
# bot_config :jsonb
|
|
# bot_type :integer default("webhook")
|
|
# description :string
|
|
# name :string
|
|
# outgoing_url :string
|
|
# secret :string
|
|
# created_at :datetime not null
|
|
# updated_at :datetime not null
|
|
# account_id :bigint
|
|
#
|
|
# Indexes
|
|
#
|
|
# index_agent_bots_on_account_id (account_id)
|
|
#
|
|
|
|
class AgentBot < ApplicationRecord
|
|
include AccessTokenable
|
|
include Avatarable
|
|
|
|
include WebhookSecretable
|
|
|
|
scope :accessible_to, lambda { |account|
|
|
account_id = account&.id
|
|
where(account_id: [nil, account_id])
|
|
}
|
|
|
|
has_many :agent_bot_inboxes, dependent: :destroy_async
|
|
has_many :inboxes, through: :agent_bot_inboxes
|
|
has_many :messages, as: :sender, dependent: :nullify
|
|
has_many :platform_app_permissibles, as: :permissible, dependent: :destroy
|
|
has_many :assigned_conversations, class_name: 'Conversation',
|
|
foreign_key: :assignee_agent_bot_id,
|
|
dependent: :nullify,
|
|
inverse_of: :assignee_agent_bot
|
|
belongs_to :account, optional: true
|
|
enum bot_type: { webhook: 0 }
|
|
|
|
validates :outgoing_url, length: { maximum: Limits::URL_LENGTH_LIMIT }
|
|
|
|
def available_name
|
|
name
|
|
end
|
|
|
|
def push_event_data(inbox = nil)
|
|
{
|
|
id: id,
|
|
name: name,
|
|
avatar_url: avatar_url || inbox&.avatar_url,
|
|
type: 'agent_bot'
|
|
}
|
|
end
|
|
|
|
def webhook_data
|
|
{
|
|
id: id,
|
|
name: name,
|
|
type: 'agent_bot'
|
|
}
|
|
end
|
|
|
|
def system_bot?
|
|
account.nil?
|
|
end
|
|
end
|
|
|
|
AgentBot.include_mod_with('Audit::AgentBot')
|