fix: Handle Telegram callback queries and acknowledge inline callbacks (#13678)
## Description This PR fixes Telegram callback query handling for standard and Business chats. ### What was wrong - Inline button callbacks were not acknowledged, so Telegram clients could keep showing a loading spinner. - Business callbacks could be attributed to the wrong contact or classified with the wrong message direction. - Callback queries from unsupported group chats could bypass the private-chat guard. ### What changed - Acknowledge callbacks immediately in the webhook JSON response using Telegram's `answerCallbackQuery` method, before background processing starts. - Treat `callback_query.from` as the user who pressed the button and compare it with `message.chat.id` to distinguish customer clicks from Business-account-owner clicks. - Preserve the customer contact and conversation routing for outgoing Business callbacks. - Validate callback payload shape and ignore unsupported inline-mode, game, malformed, group, and supergroup callbacks during message ingestion while still acknowledging them at the webhook boundary. ### Tests - Standard callback ingestion and response-only acknowledgement. - Incoming customer and outgoing Business-account-owner callback classification. - Customer contact attributes and conversation routing. - Group, supergroup, inline-mode, game, mismatched-participant, and malformed payload filtering. - Webhook parameter forwarding and immediate `answerCallbackQuery` response. Fixes #13677 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature - [ ] Breaking change - [ ] Documentation update required --------- Co-authored-by: Artur Saakyan <artur@Arturs-MacBook-Air-2.local> Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com> Co-authored-by: Sony Mathew <ynos1234@gmail.com> Co-authored-by: Muhsin <12408980+muhsin-k@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Sojan Jose <sojan@pepalo.com>
This commit is contained in:
@@ -1,6 +1,11 @@
|
||||
class Webhooks::TelegramController < ActionController::API
|
||||
def process_payload
|
||||
Webhooks::TelegramEventsJob.perform_later(params.to_unsafe_hash)
|
||||
head :ok
|
||||
|
||||
callback_query = params[:callback_query]
|
||||
callback_query_id = callback_query[:id] if callback_query.respond_to?(:key?)
|
||||
return head :ok unless callback_query_id.is_a?(String) && callback_query_id.present?
|
||||
|
||||
render json: { method: 'answerCallbackQuery', callback_query_id: callback_query_id }
|
||||
end
|
||||
end
|
||||
|
||||
76
app/services/telegram/callback_query_param_helpers.rb
Normal file
76
app/services/telegram/callback_query_param_helpers.rb
Normal file
@@ -0,0 +1,76 @@
|
||||
module Telegram::CallbackQueryParamHelpers
|
||||
def callback_query_params?
|
||||
callback_query.present?
|
||||
end
|
||||
|
||||
def processable_private_callback_query?
|
||||
valid_callback_payload? && valid_callback_participants? && private_callback_chat? && callback_participants_match?
|
||||
end
|
||||
|
||||
def callback_query
|
||||
value = params[:callback_query]
|
||||
value if value.respond_to?(:key?)
|
||||
end
|
||||
|
||||
def callback_query_id
|
||||
callback_query&.[](:id)
|
||||
end
|
||||
|
||||
def callback_query_data
|
||||
callback_query&.[](:data)
|
||||
end
|
||||
|
||||
def callback_message_chat
|
||||
callback_chat = callback_message&.[](:chat)
|
||||
callback_chat if callback_chat.respond_to?(:key?)
|
||||
end
|
||||
|
||||
def callback_sender
|
||||
sender = callback_query&.[](:from)
|
||||
sender if sender.respond_to?(:key?)
|
||||
end
|
||||
|
||||
def callback_chat_id
|
||||
callback_message_chat&.[](:id)
|
||||
end
|
||||
|
||||
def callback_sender_id
|
||||
callback_sender&.[](:id)
|
||||
end
|
||||
|
||||
def callback_business_connection_id
|
||||
callback_message&.[](:business_connection_id)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def valid_callback_payload?
|
||||
callback_query_id.is_a?(String) && callback_query_id.present? &&
|
||||
callback_query_data.is_a?(String) && callback_query_data.present?
|
||||
end
|
||||
|
||||
def valid_callback_participants?
|
||||
valid_telegram_id?(callback_sender_id) && valid_telegram_id?(callback_chat_id)
|
||||
end
|
||||
|
||||
def valid_telegram_id?(value)
|
||||
value.is_a?(Integer) && value.positive?
|
||||
end
|
||||
|
||||
def private_callback_chat?
|
||||
callback_message_chat[:type] == 'private'
|
||||
end
|
||||
|
||||
def callback_participants_match?
|
||||
business_callback_query? || callback_sender_id == callback_chat_id
|
||||
end
|
||||
|
||||
def business_callback_query?
|
||||
callback_business_connection_id.is_a?(String) && callback_business_connection_id.present?
|
||||
end
|
||||
|
||||
def callback_message
|
||||
message = callback_query&.[](:message)
|
||||
message if message.respond_to?(:key?)
|
||||
end
|
||||
end
|
||||
@@ -1,7 +1,9 @@
|
||||
module Telegram::ParamHelpers
|
||||
include Telegram::CallbackQueryParamHelpers
|
||||
|
||||
# ensures that message is from a private chat and not a group chat
|
||||
def private_message?
|
||||
return true if callback_query_params?
|
||||
return processable_private_callback_query? if callback_query_params?
|
||||
|
||||
params.dig(:message, :chat, :type) == 'private'
|
||||
end
|
||||
@@ -21,6 +23,8 @@ module Telegram::ParamHelpers
|
||||
# This is our messages posted via telegram client.
|
||||
# Such messages should be outgoing (from us to client)
|
||||
def business_message_outgoing?
|
||||
return business_message? && callback_chat_id != callback_sender_id if callback_query_params?
|
||||
|
||||
business_message? && telegram_params_base_object[:chat][:id] != telegram_params_base_object[:from][:id]
|
||||
end
|
||||
|
||||
@@ -28,19 +32,17 @@ module Telegram::ParamHelpers
|
||||
params[:message].present?
|
||||
end
|
||||
|
||||
def callback_query_params?
|
||||
params[:callback_query].present?
|
||||
end
|
||||
|
||||
def telegram_params_base_object
|
||||
if callback_query_params?
|
||||
params[:callback_query]
|
||||
callback_query
|
||||
else
|
||||
params[:message]
|
||||
end
|
||||
end
|
||||
|
||||
def contact_params
|
||||
return callback_query_contact_params if callback_query_params?
|
||||
|
||||
if business_message_outgoing?
|
||||
telegram_params_base_object[:chat]
|
||||
else
|
||||
@@ -48,7 +50,19 @@ module Telegram::ParamHelpers
|
||||
end
|
||||
end
|
||||
|
||||
def callback_query_contact_params
|
||||
return callback_message_chat if business_message_outgoing?
|
||||
|
||||
callback_sender.presence || callback_message_chat
|
||||
end
|
||||
|
||||
def telegram_params_from_id
|
||||
if callback_query_params?
|
||||
return callback_chat_id if business_message? && callback_chat_id.present?
|
||||
|
||||
return callback_sender_id
|
||||
end
|
||||
|
||||
return telegram_params_base_object[:chat][:id] if business_message?
|
||||
|
||||
telegram_params_base_object[:from][:id]
|
||||
@@ -72,7 +86,7 @@ module Telegram::ParamHelpers
|
||||
|
||||
def telegram_params_chat_id
|
||||
if callback_query_params?
|
||||
params[:callback_query][:message][:chat][:id]
|
||||
callback_chat_id
|
||||
else
|
||||
telegram_params_base_object[:chat][:id]
|
||||
end
|
||||
@@ -80,7 +94,7 @@ module Telegram::ParamHelpers
|
||||
|
||||
def telegram_params_business_connection_id
|
||||
if callback_query_params?
|
||||
params[:callback_query][:message][:business_connection_id]
|
||||
callback_business_connection_id
|
||||
else
|
||||
telegram_params_base_object[:business_connection_id]
|
||||
end
|
||||
@@ -88,7 +102,7 @@ module Telegram::ParamHelpers
|
||||
|
||||
def telegram_params_message_content
|
||||
if callback_query_params?
|
||||
params[:callback_query][:data]
|
||||
callback_query_data
|
||||
else
|
||||
params[:message][:text].presence || params[:message][:caption]
|
||||
end
|
||||
@@ -96,7 +110,7 @@ module Telegram::ParamHelpers
|
||||
|
||||
def telegram_params_message_id
|
||||
if callback_query_params?
|
||||
params[:callback_query][:id]
|
||||
callback_query_id
|
||||
else
|
||||
params[:message][:message_id]
|
||||
end
|
||||
|
||||
@@ -2,11 +2,48 @@ require 'rails_helper'
|
||||
|
||||
RSpec.describe 'Webhooks::TelegramController', type: :request do
|
||||
describe 'POST /webhooks/telegram/{:bot_token}' do
|
||||
it 'call the telegram events job with the params' do
|
||||
before do
|
||||
allow(Webhooks::TelegramEventsJob).to receive(:perform_later)
|
||||
end
|
||||
|
||||
it 'calls the telegram events job with the params' do
|
||||
expect(Webhooks::TelegramEventsJob).to receive(:perform_later)
|
||||
post '/webhooks/telegram/random_bot_token', params: { content: 'hello' }
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
|
||||
it 'acknowledges callback queries in the webhook response' do
|
||||
post '/webhooks/telegram/random_bot_token', params: {
|
||||
update_id: 123,
|
||||
callback_query: { id: 'callback-query-id', data: 'Option 1' }
|
||||
}, as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
expect(response.parsed_body).to eq(
|
||||
'method' => 'answerCallbackQuery',
|
||||
'callback_query_id' => 'callback-query-id'
|
||||
)
|
||||
expect(Webhooks::TelegramEventsJob).to have_received(:perform_later).with(
|
||||
hash_including(
|
||||
'bot_token' => 'random_bot_token',
|
||||
'telegram' => hash_including('callback_query' => hash_including('id' => 'callback-query-id'))
|
||||
)
|
||||
).once
|
||||
end
|
||||
|
||||
it 'returns an empty response for non-callback updates' do
|
||||
post '/webhooks/telegram/random_bot_token', params: { update_id: 123, message: { text: 'hello' } }, as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
expect(response.body).to be_empty
|
||||
end
|
||||
|
||||
it 'returns an empty response for a malformed callback query' do
|
||||
post '/webhooks/telegram/random_bot_token', params: { update_id: 123, callback_query: 'invalid' }, as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
expect(response.body).to be_empty
|
||||
expect(Webhooks::TelegramEventsJob).to have_received(:perform_later).once
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -407,7 +407,7 @@ describe Telegram::IncomingMessageService do
|
||||
'language_code' => 'en',
|
||||
'is_premium' => true
|
||||
},
|
||||
'message' => message_params,
|
||||
'message' => message_params.deep_merge('chat' => { 'id' => 5_171_248 }),
|
||||
'chat_instance' => '-89923842384923492',
|
||||
'data' => 'Option 1'
|
||||
}
|
||||
@@ -418,6 +418,206 @@ describe Telegram::IncomingMessageService do
|
||||
expect(contact_for(5_171_248).name).to eq('Sojan Jose')
|
||||
expect(contact_for(5_171_248).additional_attributes['social_telegram_user_id']).to eq(5_171_248)
|
||||
expect(telegram_channel.inbox.messages.first.content).to eq('Option 1')
|
||||
expect(a_request(:post, %r{/answerCallbackQuery})).not_to have_been_made
|
||||
end
|
||||
end
|
||||
|
||||
context 'when callback_query params include business_connection_id' do
|
||||
it 'creates an incoming message for the customer contact' do
|
||||
params = {
|
||||
'update_id' => 2_342_342_343_242,
|
||||
'callback_query' => {
|
||||
'id' => '2342342309929423',
|
||||
'from' => {
|
||||
'id' => 23,
|
||||
'is_bot' => false,
|
||||
'first_name' => 'Sojan',
|
||||
'last_name' => 'Jose',
|
||||
'username' => 'sojan',
|
||||
'language_code' => 'en'
|
||||
},
|
||||
'message' => message_params.deep_merge(
|
||||
'business_connection_id' => 'eooW3KF5WB5HxTD7T826',
|
||||
'from' => {
|
||||
'id' => 42,
|
||||
'is_bot' => false,
|
||||
'first_name' => 'John',
|
||||
'last_name' => 'Doe',
|
||||
'username' => 'johndoe'
|
||||
}
|
||||
),
|
||||
'chat_instance' => '-89923842384923492',
|
||||
'data' => 'Option 1'
|
||||
}
|
||||
}.with_indifferent_access
|
||||
|
||||
described_class.new(inbox: telegram_channel.inbox, params: params).perform
|
||||
|
||||
message = telegram_channel.inbox.messages.first
|
||||
contact = contact_for(23)
|
||||
expect(message.content).to eq('Option 1')
|
||||
expect(message.message_type).to eq('incoming')
|
||||
aggregate_failures do
|
||||
expect(message.sender).to eq(contact)
|
||||
expect(message.conversation.contact).to eq(contact)
|
||||
expect(contact.name).to eq('Sojan Jose')
|
||||
expect(contact.additional_attributes).to include(
|
||||
'social_telegram_user_id' => 23,
|
||||
'social_telegram_user_name' => 'sojan',
|
||||
'language_code' => 'en'
|
||||
)
|
||||
expect(message.conversation.additional_attributes).to include(
|
||||
'chat_id' => 23,
|
||||
'business_connection_id' => 'eooW3KF5WB5HxTD7T826'
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the business account owner triggers a callback query' do
|
||||
it 'creates an outgoing message in the customer conversation' do
|
||||
params = {
|
||||
'update_id' => 2_342_342_343_242,
|
||||
'callback_query' => {
|
||||
'id' => '2342342309929423',
|
||||
'from' => {
|
||||
'id' => 42,
|
||||
'is_bot' => false,
|
||||
'first_name' => 'John',
|
||||
'last_name' => 'Doe',
|
||||
'username' => 'johndoe'
|
||||
},
|
||||
'message' => message_params.deep_merge(
|
||||
'business_connection_id' => 'eooW3KF5WB5HxTD7T826',
|
||||
'from' => {
|
||||
'id' => 42,
|
||||
'is_bot' => false,
|
||||
'first_name' => 'John',
|
||||
'last_name' => 'Doe',
|
||||
'username' => 'johndoe'
|
||||
}
|
||||
),
|
||||
'chat_instance' => '-89923842384923492',
|
||||
'data' => 'Option 1'
|
||||
}
|
||||
}.with_indifferent_access
|
||||
|
||||
described_class.new(inbox: telegram_channel.inbox, params: params).perform
|
||||
|
||||
message = telegram_channel.inbox.messages.first
|
||||
expect(message.content).to eq('Option 1')
|
||||
expect(message.message_type).to eq('outgoing')
|
||||
expect(message.sender).to be_nil
|
||||
contact = contact_for(23)
|
||||
expect(message.conversation.contact).to eq(contact)
|
||||
expect(contact.name).to eq('Sojan Jose')
|
||||
expect(contact.additional_attributes['social_telegram_user_name']).to eq('sojan')
|
||||
end
|
||||
end
|
||||
|
||||
%w[group supergroup].each do |chat_type|
|
||||
context "when callback_query comes from a #{chat_type}" do
|
||||
it 'does not create a conversation' do
|
||||
params = {
|
||||
'update_id' => 2_342_342_343_242,
|
||||
'callback_query' => {
|
||||
'id' => '2342342309929423',
|
||||
'from' => {
|
||||
'id' => 5_171_248,
|
||||
'is_bot' => false,
|
||||
'first_name' => 'Sojan',
|
||||
'last_name' => 'Jose',
|
||||
'username' => 'sojan',
|
||||
'language_code' => 'en'
|
||||
},
|
||||
'message' => message_params.deep_merge('chat' => { 'type' => chat_type }),
|
||||
'chat_instance' => '-89923842384923492',
|
||||
'data' => 'Option 1'
|
||||
}
|
||||
}.with_indifferent_access
|
||||
|
||||
expect { described_class.new(inbox: telegram_channel.inbox, params: params).perform }
|
||||
.not_to(change { [Conversation.count, Message.count, Contact.count, ContactInbox.count] })
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when callback_query comes from inline mode without a message' do
|
||||
it 'does not create a conversation' do
|
||||
params = {
|
||||
'update_id' => 2_342_342_343_242,
|
||||
'callback_query' => {
|
||||
'id' => '2342342309929423',
|
||||
'from' => {
|
||||
'id' => 5_171_248,
|
||||
'is_bot' => false,
|
||||
'first_name' => 'Sojan',
|
||||
'last_name' => 'Jose',
|
||||
'username' => 'sojan',
|
||||
'language_code' => 'en'
|
||||
},
|
||||
'inline_message_id' => 'inline-message-id',
|
||||
'chat_instance' => '-89923842384923492',
|
||||
'data' => 'Option 1'
|
||||
}
|
||||
}.with_indifferent_access
|
||||
|
||||
expect { described_class.new(inbox: telegram_channel.inbox, params: params).perform }
|
||||
.not_to(change { [Conversation.count, Message.count, Contact.count, ContactInbox.count] })
|
||||
end
|
||||
end
|
||||
|
||||
context 'when a non-business callback sender does not match the private chat' do
|
||||
it 'does not create a conversation' do
|
||||
params = {
|
||||
'update_id' => 2_342_342_343_242,
|
||||
'callback_query' => {
|
||||
'id' => '2342342309929423',
|
||||
'from' => {
|
||||
'id' => 5_171_248,
|
||||
'is_bot' => false,
|
||||
'first_name' => 'Sojan',
|
||||
'last_name' => 'Jose',
|
||||
'username' => 'sojan'
|
||||
},
|
||||
'message' => message_params,
|
||||
'chat_instance' => '-89923842384923492',
|
||||
'data' => 'Option 1'
|
||||
}
|
||||
}.with_indifferent_access
|
||||
|
||||
expect { described_class.new(inbox: telegram_channel.inbox, params: params).perform }
|
||||
.not_to(change { [Conversation.count, Message.count, Contact.count, ContactInbox.count] })
|
||||
end
|
||||
end
|
||||
|
||||
context 'when callback_query has a game payload instead of data' do
|
||||
it 'does not create a conversation' do
|
||||
params = {
|
||||
'update_id' => 2_342_342_343_242,
|
||||
'callback_query' => {
|
||||
'id' => '2342342309929423',
|
||||
'from' => message_params['from'],
|
||||
'message' => message_params,
|
||||
'chat_instance' => '-89923842384923492',
|
||||
'game_short_name' => 'test-game'
|
||||
}
|
||||
}.with_indifferent_access
|
||||
|
||||
expect { described_class.new(inbox: telegram_channel.inbox, params: params).perform }
|
||||
.not_to(change { [Conversation.count, Message.count, Contact.count, ContactInbox.count] })
|
||||
end
|
||||
end
|
||||
|
||||
context 'when callback_query is malformed' do
|
||||
it 'does not create a conversation' do
|
||||
params = {
|
||||
'update_id' => 2_342_342_343_242,
|
||||
'callback_query' => 'invalid'
|
||||
}.with_indifferent_access
|
||||
|
||||
expect { described_class.new(inbox: telegram_channel.inbox, params: params).perform }
|
||||
.not_to(change { [Conversation.count, Message.count, Contact.count, ContactInbox.count] })
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
Reference in New Issue
Block a user