From 9310fa613b3d5eeec63a4e7f542f93d7ec277be9 Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Mon, 28 May 2018 12:30:47 +0300 Subject: [PATCH 1/5] Use bang-methods as actions for commands --- CHANGELOG.md | 2 + README.md | 23 ++-- lib/telegram/bot/updates_controller.rb | 74 +++++------ .../callback_query_context.rb | 8 +- .../bot/updates_controller/commands.rb | 44 +++++++ .../bot/updates_controller/message_context.rb | 7 +- spec/integration_helper.rb | 4 +- spec/support/examples/integration.rb | 2 +- .../bot/rspec/message_helpers_spec.rb | 2 +- .../bot/updates_controller/commands_spec.rb | 111 ++++++++++++++++ .../instrumentation_spec.rb | 4 +- .../message_context_spec.rb | 4 +- .../bot/updates_controller/rescue_spec.rb | 4 +- .../bot/updates_controller/session_spec.rb | 4 +- spec/telegram/bot/updates_controller_spec.rb | 122 +----------------- 15 files changed, 225 insertions(+), 190 deletions(-) create mode 100644 lib/telegram/bot/updates_controller/commands.rb create mode 100644 spec/telegram/bot/updates_controller/commands_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index ce712c5..e7ca600 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - Requiring `telegram/bot/rspec/integration` is deprecated in favor of `telegram/bot/rspec/integration/rails`. - `:telegram_bot` rspec tag is replaced with `telegram_bot: :rails`. +- __Breaking change__. Use bang-methods as actions for commands. + This prevents calling context contextual actions and payload specific actions with commands. # 0.13.1 diff --git a/README.md b/README.md index 9c6c9de..d319ed9 100644 --- a/README.md +++ b/README.md @@ -154,14 +154,11 @@ class Telegram::WebhookController < Telegram::Bot::UpdatesController # chosen_inline_result(result_id, query) # callback_query(data) - # Define public methods to respond to commands. + # Define public methods ending with `!` to handle commands. # Command arguments will be parsed and passed to the method. # Be sure to use splat args and default values to not get errors when # someone passed more or less arguments in the message. - # - # For some commands like /message or /123 method names should start with - # `on_` to avoid conflicts. - def start(data = nil, *) + def start!(data = nil, *) # do_smth_with(data) # There are `chat` & `from` shortcut methods. @@ -257,11 +254,11 @@ class Telegram::WebhookController < Telegram::Bot::UpdatesController # You can override global config for this controller. self.session_store = :file_store - def write(text = nil, *) + def write!(text = nil, *) session[:text] = text end - def read(*) + def read!(*) respond_with :message, text: session[:text] end @@ -284,7 +281,7 @@ it asks you for additional argument. There is `MessageContext` for this: class Telegram::WebhookController < Telegram::Bot::UpdatesController include Telegram::Bot::UpdatesController::MessageContext - def rename(*) + def rename!(*) # set context for the next message save_context :rename respond_with :message, text: 'What name do you like?' @@ -297,18 +294,18 @@ class Telegram::WebhookController < Telegram::Bot::UpdatesController end # You can do it in other way: - def rename(name = nil, *) + def rename!(name = nil, *) if name update_name name respond_with :message, text: 'Renamed!' else - save_context :rename + save_context :rename! respond_with :message, text: 'What name do you like?' end end - # This will call #rename like if it is called with message '/rename %text%' - context_handler :rename + # This will call #rename! like if it is called with message '/rename %text%' + context_handler :rename! # If you have a lot of such methods you can call this method # to use context value as action name for all contexts which miss handlers: @@ -460,7 +457,7 @@ RSpec.describe TelegramWebhooksController, telegram_bot: :rails do expect { dispatch_message('Hi') }.to send_telegram_message(bot, /msg regexp/, some: :option) end - describe '#start' do + describe '#start!' do subject { -> { dispatch_command :start } } # Using built in matcher for `respond_to`: it { should respond_with_message 'Hi there!' } diff --git a/lib/telegram/bot/updates_controller.rb b/lib/telegram/bot/updates_controller.rb index d2c749a..0aa8c3c 100644 --- a/lib/telegram/bot/updates_controller.rb +++ b/lib/telegram/bot/updates_controller.rb @@ -54,6 +54,7 @@ module Telegram abstract! %w[ + commands instrumentation log_subscriber reply_helpers @@ -79,6 +80,7 @@ module Telegram end include AbstractController::Translation + include Commands include Rescue include ReplyHelpers include Instrumentation @@ -96,8 +98,6 @@ module Telegram shipping_query pre_checkout_query ].freeze - CMD_REGEX = %r{\A/([a-z\d_]{,31})(@(\S+))?(\s|$)}i - CONFLICT_CMD_REGEX = Regexp.new("^(#{PAYLOAD_TYPES.join('|')}|\\d)") class << self # Initialize controller and process update. @@ -105,27 +105,6 @@ module Telegram new(*args).dispatch end - # Overrid it to filter or transform commands. - # Default implementation is to convert to downcase and add `on_` prefix - # for conflicting commands. - def action_for_command(cmd) - cmd.downcase! - cmd.match(CONFLICT_CMD_REGEX) ? "on_#{cmd}" : cmd - end - - # Fetches command from text message. All subsequent words are returned - # as arguments. - # If command has mention (eg. `/test@SomeBot`), it returns commands only - # for specified username. Set `username` to `true` to accept - # any commands. - def command_from_text(text, username = nil) - return unless text - match = text.match(CMD_REGEX) - return unless match - mention = match[3] - [match[1], text.split.drop(1)] if username == true || !mention || mention == username - end - def payload_from_update(update) update && PAYLOAD_TYPES.find do |type| item = update[type] @@ -134,8 +113,7 @@ module Telegram end end - attr_internal_reader :update, :bot, :payload, :payload_type, :is_command - alias_method :command?, :is_command + attr_internal_reader :update, :bot, :payload, :payload_type delegate :username, to: :bot, prefix: true, allow_nil: true # Second argument can be either update object with hash access & string @@ -175,49 +153,57 @@ module Telegram # Processes current update. def dispatch - @_is_command, action, args = action_for_payload + action, args = action_for_payload process(action, *args) end + attr_internal_reader :action_options + + # It provides support for passing array as action, where first vaule + # is action name and second is action metadata. + # This metadata is stored inside action_options + def process(action, *args) + action, options = action if action.is_a?(Array) + @_action_options = options || {} + super + end + + # There are multiple ways how action name is calculated for update + # (see Commands, MessageContext, etc.). This method represents the + # way how action was calculated for current udpate. + # + # Some of possible values are `:payload, :command, :message_context`. + def action_type + action_options[:type] || :payload + end + # Calculates action name and args for payload. # Uses `action_for_#{payload_type}` methods. # If this method doesn't return anything # it uses fallback with action same as payload type. - # Returns array `[is_command?, action, args]`. + # Returns array `[action, args]`. def action_for_payload if payload_type send("action_for_#{payload_type}") || action_for_default_payload else - [false, :unsupported_payload_type, []] + [:unsupported_payload_type, []] end end def action_for_default_payload - [false, payload_type, [payload]] + [payload_type, [payload]] end - # If payload is a message with command, then returned action is an - # action for this command. - # Separate method, so it can be easily overriden (ex. MessageContext). - # - # This is not used for edited messages/posts. It process them as basic updates. - def action_for_message - cmd, args = self.class.command_from_text(payload['text'], bot_username) - cmd &&= self.class.action_for_command(cmd) - [true, cmd, args] if cmd - end - alias_method :action_for_channel_post, :action_for_message - def action_for_inline_query - [false, payload_type, [payload['query'], payload['offset']]] + [payload_type, [payload['query'], payload['offset']]] end def action_for_chosen_inline_result - [false, payload_type, [payload['result_id'], payload['query']]] + [payload_type, [payload['result_id'], payload['query']]] end def action_for_callback_query - [false, payload_type, [payload['data']]] + [payload_type, [payload['data']]] end # Silently ignore unsupported messages. diff --git a/lib/telegram/bot/updates_controller/callback_query_context.rb b/lib/telegram/bot/updates_controller/callback_query_context.rb index 21a1eeb..1dddff1 100644 --- a/lib/telegram/bot/updates_controller/callback_query_context.rb +++ b/lib/telegram/bot/updates_controller/callback_query_context.rb @@ -17,8 +17,12 @@ module Telegram context, new_data = context_from_callback_query if context action_name = "#{context}_callback_query" - [false, action_name, [new_data]] if action_method?(action_name) - end || super + if action_method?(action_name) + action_options = {type: :callback_query_context, context: context} + return [[action_name, action_options], [new_data]] + end + end + super end def context_from_callback_query diff --git a/lib/telegram/bot/updates_controller/commands.rb b/lib/telegram/bot/updates_controller/commands.rb new file mode 100644 index 0000000..7e26abf --- /dev/null +++ b/lib/telegram/bot/updates_controller/commands.rb @@ -0,0 +1,44 @@ +module Telegram + module Bot + class UpdatesController + # Support for parsing commands + module Commands + CMD_REGEX = %r{\A/([a-z\d_]{,31})(@(\S+))?(\s|$)}i + + class << self + # Fetches command from text message. All subsequent words are returned + # as arguments. + # If command has mention (eg. `/test@SomeBot`), it returns commands only + # for specified username. Set `username` to `true` to accept + # any commands. + def command_from_text(text, username = nil) + return unless text + match = text.match(CMD_REGEX) + return unless match + mention = match[3] + [match[1], text.split.drop(1)] if username == true || !mention || mention == username + end + end + + # Override it to filter or transform commands. + # Default implementation is to downcase and add `!` suffix. + def action_for_command(cmd) + "#{cmd.downcase}!" + end + + # If payload is a message with command, then returned action is an + # action for this command. + # Separate method, so it can be easily overriden (ex. MessageContext). + # + # This is not used for edited messages/posts. It process them as basic updates. + def action_for_message + cmd, args = Commands.command_from_text(payload['text'], bot_username) + return unless cmd + [[action_for_command(cmd), type: :command, command: cmd], args] + end + + alias_method :action_for_channel_post, :action_for_message + end + end + end +end diff --git a/lib/telegram/bot/updates_controller/message_context.rb b/lib/telegram/bot/updates_controller/message_context.rb index 3186300..01146ed 100644 --- a/lib/telegram/bot/updates_controller/message_context.rb +++ b/lib/telegram/bot/updates_controller/message_context.rb @@ -48,7 +48,7 @@ module Telegram end # Action to clear context. - def cancel + def cancel! # Context is already cleared in action_for_message end @@ -71,7 +71,10 @@ module Telegram @context = val && val.to_sym super || context && begin handler = handler_for_context - [true, handler, payload['text'].try!(:split) || []] if handler + if handler + action_options = {type: :message_context, context: context} + [[handler, action_options], payload['text'].try!(:split) || []] + end end end diff --git a/spec/integration_helper.rb b/spec/integration_helper.rb index e6eef97..dcd91f5 100644 --- a/spec/integration_helper.rb +++ b/spec/integration_helper.rb @@ -35,7 +35,7 @@ Rails.application.initialize! # # Controllers %w[default other named].each do |bot_name| controller = Class.new(Telegram::Bot::UpdatesController) do - define_method :start do |*| + define_method :start! do |*| respond_with :message, text: "from #{bot_name}" end end @@ -46,7 +46,7 @@ end klass.class_eval do use_session! - define_method :load_session do |*| + define_method :load_session! do |*| session[:test] end end diff --git a/spec/support/examples/integration.rb b/spec/support/examples/integration.rb index 00ee1ac..cb79400 100644 --- a/spec/support/examples/integration.rb +++ b/spec/support/examples/integration.rb @@ -2,7 +2,7 @@ RSpec.shared_examples 'shared integration examples' do let(:bot) { Telegram::Bot::ClientStub.new('token') } let(:controller_class) do Class.new(Telegram::Bot::UpdatesController) do - def start(data = nil, *) + def start!(data = nil, *) respond_with :message, text: "Hi #{data}" end diff --git a/spec/telegram/bot/rspec/message_helpers_spec.rb b/spec/telegram/bot/rspec/message_helpers_spec.rb index 372783d..63fb315 100644 --- a/spec/telegram/bot/rspec/message_helpers_spec.rb +++ b/spec/telegram/bot/rspec/message_helpers_spec.rb @@ -69,7 +69,7 @@ RSpec.describe 'Integration: message helpers', telegram_bot: :poller do let(:bot) { Telegram::Bot::ClientStub.new('token') } let(:controller_class) do Class.new(Telegram::Bot::UpdatesController) do - def start(*args) + def start!(*args) respond_with :message, text: "Start: #{args.inspect}, option: #{payload['option']}" end end diff --git a/spec/telegram/bot/updates_controller/commands_spec.rb b/spec/telegram/bot/updates_controller/commands_spec.rb new file mode 100644 index 0000000..5dfb196 --- /dev/null +++ b/spec/telegram/bot/updates_controller/commands_spec.rb @@ -0,0 +1,111 @@ +RSpec.describe Telegram::Bot::UpdatesController::Commands do + describe '#action_for_command' do + subject { ->(*args) { object.action_for_command(*args) } } + let(:object) { Object.new.tap { |x| x.extend described_class } } + + def assert_subject(input, expected) + expect(subject.call input).to eq expected + end + + it 'bypasses and downcases not conflictint commands' do + assert_subject 'test', 'test!' + assert_subject 'TeSt', 'test!' + assert_subject '_Te1St', '_te1st!' + end + end + + describe '.command_from_text' do + subject { ->(*args) { described_class.command_from_text(*args) } } + + def assert_subject(input, cmd, *args) + expected = cmd ? [cmd, args] : cmd + expect(subject.call(*input)).to eq expected + end + + let(:max_cmd_size) { 32 } + let(:long_cmd) { 'a' * (max_cmd_size - 1) } + let(:too_long_cmd) { 'a' * max_cmd_size } + + it 'works for simple commands' do + assert_subject '/test', 'test' + assert_subject '/tE_2_St', 'tE_2_St' + assert_subject '/123', '123' + assert_subject "/#{long_cmd}", long_cmd + end + + it 'works for simple messages' do + assert_subject 'text', nil + assert_subject ' ', nil + assert_subject ' text', nil + assert_subject ' 1', nil + assert_subject ' /text', nil + assert_subject '/te-xt', nil + assert_subject 'text /cmd ', nil + assert_subject "/#{too_long_cmd}", nil + end + + it 'works for mentioned commands' do + assert_subject ['/test@bot', 'bot'], 'test' + assert_subject ['/test@otherbot', 'bot'], nil + assert_subject ['/test@Bot', 'bot'], nil + assert_subject '/test@bot', nil + assert_subject ['/test@bot', true], 'test' + assert_subject ['/test@otherbot', true], 'test' + end + + it 'works for commands with args' do + assert_subject '/test arg', 'test', 'arg' + assert_subject '/test arg 1 2', 'test', 'arg', '1', '2' + assert_subject ['/test@bot arg', 'bot'], 'test', 'arg' + assert_subject ['/test@otherbot arg', 'bot'], nil + assert_subject '/test@bot arg', nil + end + + it 'works for commands with multiline args' do + assert_subject "/test arg\nother", 'test', 'arg', 'other' + assert_subject "/test one\ntwo\n\nthree", 'test', 'one', 'two', 'three' + end + end + + describe '#action_for_payload' do + include_context 'telegram/bot/updates_controller' + let(:controller_class) { Telegram::Bot::UpdatesController } + subject { controller.action_for_payload } + + %w[message channel_post].each do |type| + context "when payload is edited_#{type}" do + let(:payload_type) { "edited_#{type}" } + it { should eq [payload_type, [payload]] } + end + + context 'when payload is message' do + let(:payload_type) { type } + let(:payload) { {'text' => text} } + let(:text) { 'test' } + + it { should eq [payload_type, [payload]] } + + context 'with command' do + let(:text) { "/test#{"@#{mention}" if mention} arg 1 2" } + let(:mention) {} + it { should eq [['test!', type: :command, command: 'test'], %w[arg 1 2]] } + + context 'with mention' do + let(:mention) { bot.username } + it { should eq [['test!', type: :command, command: 'test'], %w[arg 1 2]] } + end + + context 'with mention for other bot' do + let(:mention) { 'other_bot_name' } + it { should eq [payload_type, [payload]] } + end + end + + context 'without text' do + let(:payload) { {'audio' => {'file_id' => 123}} } + it { should eq [payload_type, [payload]] } + end + end + end + end +end diff --git a/spec/telegram/bot/updates_controller/instrumentation_spec.rb b/spec/telegram/bot/updates_controller/instrumentation_spec.rb index c8a7258..cb0c190 100644 --- a/spec/telegram/bot/updates_controller/instrumentation_spec.rb +++ b/spec/telegram/bot/updates_controller/instrumentation_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Telegram::Bot::UpdatesController::Instrumentation do let(:controller_class) do Class.new(Telegram::Bot::UpdatesController) do - def start(*) + def start!(*) end end end @@ -72,7 +72,7 @@ RSpec.describe Telegram::Bot::UpdatesController::Instrumentation do describe '#respond_with' do before do - def controller.start(*) + def controller.start!(*) respond_with :message, text: 'sample response' end end diff --git a/spec/telegram/bot/updates_controller/message_context_spec.rb b/spec/telegram/bot/updates_controller/message_context_spec.rb index ff0248a..0aaef81 100644 --- a/spec/telegram/bot/updates_controller/message_context_spec.rb +++ b/spec/telegram/bot/updates_controller/message_context_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Telegram::Bot::UpdatesController::MessageContext do [:method_result, *args] end - def action(*args) + def action!(*args) [:action_result, *args] end @@ -109,7 +109,7 @@ RSpec.describe Telegram::Bot::UpdatesController::MessageContext do end context 'when context is action`s name but not mapped' do - before { session[:context] = :action } + before { session[:context] = :action! } its(:call) { should eq [:action_result, *text.split] } it { should_not change(controller, :filter_done) } it { should change { session[:context] }.to nil } diff --git a/spec/telegram/bot/updates_controller/rescue_spec.rb b/spec/telegram/bot/updates_controller/rescue_spec.rb index ae03f76..d7dd275 100644 --- a/spec/telegram/bot/updates_controller/rescue_spec.rb +++ b/spec/telegram/bot/updates_controller/rescue_spec.rb @@ -8,11 +8,11 @@ RSpec.describe Telegram::Bot::UpdatesController::Rescue do Class.new(Telegram::Bot::UpdatesController) do rescue_from ArgumentError, with: -> { respond_with :message, text: 'Rescued' } - def rescuable(*) + def rescuable!(*) raise ArgumentError, 'rescuable' end - def not_rescuable(*) + def not_rescuable!(*) raise 'not_rescuable' end end diff --git a/spec/telegram/bot/updates_controller/session_spec.rb b/spec/telegram/bot/updates_controller/session_spec.rb index 97406fb..8bb092a 100644 --- a/spec/telegram/bot/updates_controller/session_spec.rb +++ b/spec/telegram/bot/updates_controller/session_spec.rb @@ -19,11 +19,11 @@ RSpec.describe Telegram::Bot::UpdatesController::Session do controller_class.class_eval do self.session_store = :memory_store - def write(text) + def write!(text) session[:text] = text end - def read + def read! session[:text] end diff --git a/spec/telegram/bot/updates_controller_spec.rb b/spec/telegram/bot/updates_controller_spec.rb index 93550ad..29efd6c 100644 --- a/spec/telegram/bot/updates_controller_spec.rb +++ b/spec/telegram/bot/updates_controller_spec.rb @@ -1,81 +1,5 @@ RSpec.describe Telegram::Bot::UpdatesController do include_context 'telegram/bot/updates_controller' - let(:other_bot_name) { 'other_bot' } - - describe '.action_for_command' do - subject { ->(*args) { described_class.action_for_command(*args) } } - - def assert_subject(input, expected) - expect(subject.call input).to eq expected - end - - it 'bypasses and downcases not conflictint commands' do - assert_subject 'test', 'test' - assert_subject 'TeSt', 'test' - assert_subject '_Te1St', '_te1st' - end - - it 'adds _on to conflicting commands' do - described_class::PAYLOAD_TYPES.each do |x| - assert_subject x, "on_#{x}" - assert_subject x.upcase, "on_#{x}" - end - assert_subject '1TeSt', 'on_1test' - end - end - - describe '.command_from_text' do - subject { ->(*args) { described_class.command_from_text(*args) } } - - def assert_subject(input, cmd, *args) - expected = cmd ? [cmd, args] : cmd - expect(subject.call(*input)).to eq expected - end - - let(:max_cmd_size) { 32 } - let(:long_cmd) { 'a' * (max_cmd_size - 1) } - let(:too_long_cmd) { 'a' * max_cmd_size } - - it 'works for simple commands' do - assert_subject '/test', 'test' - assert_subject '/tE_2_St', 'tE_2_St' - assert_subject '/123', '123' - assert_subject "/#{long_cmd}", long_cmd - end - - it 'works for simple messages' do - assert_subject 'text', nil - assert_subject ' ', nil - assert_subject ' text', nil - assert_subject ' 1', nil - assert_subject ' /text', nil - assert_subject '/te-xt', nil - assert_subject 'text /cmd ', nil - assert_subject "/#{too_long_cmd}", nil - end - - it 'works for mentioned commands' do - assert_subject ['/test@bot', 'bot'], 'test' - assert_subject ['/test@otherbot', 'bot'], nil - assert_subject ['/test@Bot', 'bot'], nil - assert_subject '/test@bot', nil - assert_subject ['/test@bot', true], 'test' - assert_subject ['/test@otherbot', true], 'test' - end - - it 'works for commands with args' do - assert_subject '/test arg', 'test', 'arg' - assert_subject '/test arg 1 2', 'test', 'arg', '1', '2' - assert_subject ['/test@bot arg', 'bot'], 'test', 'arg' - assert_subject ['/test@otherbot arg', 'bot'], nil - assert_subject '/test@bot arg', nil - end - - it 'works for commands with multiline args' do - assert_subject "/test arg\nother", 'test', 'arg', 'other' - assert_subject "/test one\ntwo\n\nthree", 'test', 'one', 'two', 'three' - end - end describe '#action_for_payload' do subject { controller.action_for_payload } @@ -87,60 +11,24 @@ RSpec.describe Telegram::Bot::UpdatesController do context 'when payload is inline_query' do let(:payload_type) { 'inline_query' } let(:payload) { stub_payload(:id, :from, :location, :query, :offset) } - it { should eq [false, payload_type, payload.values_at(:query, :offset)] } + it { should eq [payload_type, payload.values_at(:query, :offset)] } end context 'when payload is chosen_inline_result' do let(:payload_type) { 'chosen_inline_result' } let(:payload) { stub_payload(:result_id, :from, :location, :inline_message_id, :query) } - it { should eq [false, payload_type, payload.values_at(:result_id, :query)] } + it { should eq [payload_type, payload.values_at(:result_id, :query)] } end context 'when payload is callback_query' do let(:payload_type) { 'callback_query' } let(:payload) { stub_payload(:id, :from, :message, :inline_message_id, :data) } - it { should eq [false, payload_type, payload.values_at(:data)] } + it { should eq [payload_type, payload.values_at(:data)] } end context 'when payload is not supported' do let(:payload_type) { '_unsupported_' } - it { should eq [false, :unsupported_payload_type, []] } - end - - %w[message channel_post].each do |type| - context "when payload is edited_#{type}" do - let(:payload_type) { "edited_#{type}" } - it { should eq [false, payload_type, [payload]] } - end - - context 'when payload is message' do - let(:payload_type) { type } - let(:payload) { {'text' => text} } - let(:text) { 'test' } - - it { should eq [false, payload_type, [payload]] } - - context 'with command' do - let(:text) { "/test#{"@#{mention}" if mention} arg 1 2" } - let(:mention) {} - it { should eq [true, 'test', %w[arg 1 2]] } - - context 'with mention' do - let(:mention) { bot.username } - it { should eq [true, 'test', %w[arg 1 2]] } - end - - context 'with mention for other bot' do - let(:mention) { other_bot_name } - it { should eq [false, payload_type, [payload]] } - end - end - - context 'without text' do - let(:payload) { {'audio' => {'file_id' => 123}} } - it { should eq [false, payload_type, [payload]] } - end - end + it { should eq [:unsupported_payload_type, []] } end custom_payload_types = %w[ @@ -155,7 +43,7 @@ RSpec.describe Telegram::Bot::UpdatesController do (described_class::PAYLOAD_TYPES - custom_payload_types).each do |type| context "when payload is #{type}" do let(:payload_type) { type } - it { should eq [false, payload_type, [payload]] } + it { should eq [payload_type, [payload]] } end end end From 6dd705fe89d1daef0f54558caf3c607f15cbf82c Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Mon, 28 May 2018 18:41:40 +0300 Subject: [PATCH 2/5] Drop `.context_handler`, `.context_to_action!` methods --- CHANGELOG.md | 3 + README.md | 11 +- lib/telegram/bot/updates_controller.rb | 8 +- .../bot/updates_controller/message_context.rb | 100 +++++++----------- .../message_context_spec.rb | 87 +++++---------- 5 files changed, 77 insertions(+), 132 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7ca600..8f250dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ - `:telegram_bot` rspec tag is replaced with `telegram_bot: :rails`. - __Breaking change__. Use bang-methods as actions for commands. This prevents calling context contextual actions and payload specific actions with commands. +- __Breaking change__. Drop `.context_handler`, `.context_to_action!` methods. + Use pass action name directly to `#save_context`. + It's the same as `.context_to_action!` is enabled by default. # 0.13.1 diff --git a/README.md b/README.md index d319ed9..3483295 100644 --- a/README.md +++ b/README.md @@ -288,12 +288,12 @@ class Telegram::WebhookController < Telegram::Bot::UpdatesController end # register context handlers to handle this context - context_handler :rename do |*words| + def rename(*words) update_name words[0] respond_with :message, text: 'Renamed!' end - # You can do it in other way: + # You can use same action name as context name: def rename!(name = nil, *) if name update_name name @@ -303,13 +303,6 @@ class Telegram::WebhookController < Telegram::Bot::UpdatesController respond_with :message, text: 'What name do you like?' end end - - # This will call #rename! like if it is called with message '/rename %text%' - context_handler :rename! - - # If you have a lot of such methods you can call this method - # to use context value as action name for all contexts which miss handlers: - context_to_action! end ``` diff --git a/lib/telegram/bot/updates_controller.rb b/lib/telegram/bot/updates_controller.rb index 0aa8c3c..742378f 100644 --- a/lib/telegram/bot/updates_controller.rb +++ b/lib/telegram/bot/updates_controller.rb @@ -206,9 +206,11 @@ module Telegram [payload_type, [payload['data']]] end - # Silently ignore unsupported messages. - # Params are `action, *args`. - def action_missing(*) + # Silently ignore unsupported messages to not fail when user crafts + # an update with usupported command, callback query context, etc. + def action_missing(action, *_args) + logger.debug { "The action '#{action}' is not defined in #{self.class.name}" } if logger + nil end PAYLOAD_TYPES.each do |type| diff --git a/lib/telegram/bot/updates_controller/message_context.rb b/lib/telegram/bot/updates_controller/message_context.rb index 01146ed..b1b3496 100644 --- a/lib/telegram/bot/updates_controller/message_context.rb +++ b/lib/telegram/bot/updates_controller/message_context.rb @@ -2,51 +2,35 @@ module Telegram module Bot class UpdatesController # Allows to store context in session and treat next message according to this context. + # + # It provides `save_context` method to store method name + # to be used as action for next update: + # + # def set_location!(*) + # save_context(:set_location_from_message) + # respond_with :message, text: 'Where are you?' + # end + # + # def set_location_from_messge(city = nil, *) + # # update + # end + # + # # OR + # # This will support both `/set_location city_name`, and `/set_location` + # # with subsequent refinement. + # def set_location!(city = nil, *) + # if city + # # update + # else + # save_context(:set_location!) + # respond_with :message, text: 'Where are you?' + # end + # end module MessageContext extend ActiveSupport::Concern include Session - module ClassMethods - def context_handlers - @_context_handlers ||= {} - end - - # Registers handler for context. - # - # context_handler :rename do |*| - # resource.update!(name: payload['text']) - # end - # - # # To run other action with all the callbacks: - # context_handler :rename do |*words| - # process(:rename, *words) - # end - # - # # Or just - # context_handler :rename, :your_action_to_call - # context_handler :rename # to call :rename - # - def context_handler(context = nil, action = nil, &block) - context &&= context.to_sym - if block - action = "_context_handler_#{context}" - define_method(action, &block) - end - context_handlers[context] = action || context - end - - attr_reader :context_to_action - - # Use it to use context value as action name for all contexts - # which miss handlers. - # For security reasons it supports only action methods and will - # raise AbstractController::ActionNotFound if context is invalid. - def context_to_action! - @context_to_action = true - end - end - # Action to clear context. def cancel! # Context is already cleared in action_for_message @@ -54,10 +38,6 @@ module Telegram private - # Context is read from the session to treat messages - # according to previous request. - attr_reader :context - # Controller may have multiple sessions, let it be possible # to select session for message context. def message_context_session @@ -68,13 +48,11 @@ module Telegram # it has higher priority than contextual action. def action_for_message val = message_context_session.delete(:context) - @context = val && val.to_sym + context = val && val.to_s super || context && begin - handler = handler_for_context - if handler - action_options = {type: :message_context, context: context} - [[handler, action_options], payload['text'].try!(:split) || []] - end + args = payload['text'].try!(:split) || [] + action = action_for_message_context(context) + [[action, type: :message_context, context: context], args] end end @@ -83,18 +61,16 @@ module Telegram message_context_session[:context] = context end - def handler_for_context - self.class.context_handlers[context] || self.class.context_to_action && begin - action_name = context.to_s - unless action_method?(action_name) - raise AbstractController::ActionNotFound, - "The action '#{action_name}' could not be set from context " \ - "for #{self.class.name}. " \ - 'context_to_action! supports only action methods for security reasons. ' \ - 'If you need to call this action use context_handler for it.' - end - action_name - end + # Returns action name for message context. By default it's the same as context name. + # Raises AbstractController::ActionNotFound if action is not available. + # This differs from other cases where invalid actions are silently ignored, + # because message context is controlled by developer, and users are not able + # to construct update to run any specific context. + def action_for_message_context(context) + action = context.to_s + return action if action_method?(action) + raise AbstractController::ActionNotFound, + "The context action '#{action}' is not found in #{self.class.name}" end end end diff --git a/spec/telegram/bot/updates_controller/message_context_spec.rb b/spec/telegram/bot/updates_controller/message_context_spec.rb index 0aaef81..9ab4952 100644 --- a/spec/telegram/bot/updates_controller/message_context_spec.rb +++ b/spec/telegram/bot/updates_controller/message_context_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Telegram::Bot::UpdatesController::MessageContext do include described_class attr_accessor :filter_done - before_action only: :redirect do + before_action only: :context_with_filter do self.filter_done = true end @@ -17,19 +17,16 @@ RSpec.describe Telegram::Bot::UpdatesController::MessageContext do [:no_context, *args] end - context_handler :block do |*args| - [:block_result, *args] + def handler_method(*args) + [:method_result_1, *args] end - context_handler :redirect - context_handler :other_redirect, :redirect - - def redirect(*args) - [:method_result, *args] + def context_with_filter(*args) + [:method_result_2, *args] end def action!(*args) - [:action_result, *args] + [:command_result, *args] end private @@ -52,85 +49,59 @@ RSpec.describe Telegram::Bot::UpdatesController::MessageContext do it { should_not change { session[:context] } } end - context 'when context is handled by block' do - before { session[:context] = :block } - its(:call) { should eq [:block_result, *text.split] } + context 'when context is handled by handler_method' do + before { session[:context] = :handler_method } + its(:call) { should eq [:method_result_1, *text.split] } it { should_not change(controller, :filter_done) } it { should change { session[:context] }.to nil } context 'when message has no text' do let(:payload) { {'audio' => {'file_id' => 123}} } - its(:call) { should eq [:block_result] } + its(:call) { should eq [:method_result_1] } end context 'when message has new command' do let(:text) { '/action a s d' } - its(:call) { should eq [:action_result, 'a', 's', 'd'] } + its(:call) { should eq [:command_result, 'a', 's', 'd'] } it { should change { session[:context] }.to nil } end end - context 'when context is handled by short redirect' do - before { session[:context] = :redirect } - its(:call) { should eq [:method_result, *text.split] } + context 'when context is handled by short context_with_filter' do + before { session[:context] = :context_with_filter } + its(:call) { should eq [:method_result_2, *text.split] } it { should change(controller, :filter_done).to true } it { should change { session[:context] }.to nil } it { should change(controller, :callbacks_runs).to 1 } context 'when message has no text' do let(:payload) { {'audio' => {'file_id' => 123}} } - its(:call) { should eq [:method_result] } + its(:call) { should eq [:method_result_2] } it { should change(controller, :filter_done).to true } it { should change { session[:context] }.to nil } end end - context 'when context is handled by custom redirect' do - before { session[:context] = :other_redirect } - its(:call) { should eq [:method_result, *text.split] } - it { should change(controller, :filter_done).to true } - it { should change { session[:context] }.to nil } - end - - context 'when context is action`s name but not mapped' do - before { session[:context] = :action } - its(:call) { should eq [:no_context, payload] } + context 'when context is command-action`s name' do + before { session[:context] = :action! } + its(:call) { should eq [:command_result, *text.split] } it { should_not change(controller, :filter_done) } it { should change { session[:context] }.to nil } end - context 'when context_to_action is true' do - before { controller_class.context_to_action! } - - context 'when context is not set' do - its(:call) { should eq [:no_context, payload] } - it { should_not change(controller, :filter_done) } - it { should_not change { session[:context] } } + context 'when context is not an action`s name' do + before { session[:context] = :not_action } + it do + should raise_error(AbstractController::ActionNotFound). + and change { session[:context] }.to nil end + end - context 'when context is action`s name but not mapped' do - before { session[:context] = :action! } - its(:call) { should eq [:action_result, *text.split] } - it { should_not change(controller, :filter_done) } - it { should change { session[:context] }.to nil } - end - - context 'when context is invalid' do - before { session[:context] = :invalid } - it 'raises error and clears context' do - expect do - should raise_error AbstractController::ActionNotFound - end.to change { session[:context] }.to nil - end - end - - context 'when context is private method`s name' do - before { session[:context] = :not_action } - it 'raises error and clears context' do - expect do - should raise_error AbstractController::ActionNotFound - end.to change { session[:context] }.to nil - end + context 'when context is invalid name' do + before { session[:context] = :invalid } + it do + should raise_error(AbstractController::ActionNotFound). + and change { session[:context] }.to nil end end end From 7ff89d012c4fe133c974090007429eed9990c3c9 Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Thu, 7 Jun 2018 14:10:22 +0600 Subject: [PATCH 3/5] Translation helper strips `!` from action name for lazy translations --- CHANGELOG.md | 1 + lib/telegram/bot/updates_controller.rb | 20 ++++++----- .../bot/updates_controller/translation.rb | 36 +++++++++++++++++++ .../updates_controller/translation_spec.rb | 17 +++++++++ 4 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 lib/telegram/bot/updates_controller/translation.rb create mode 100644 spec/telegram/bot/updates_controller/translation_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f250dc..cd40380 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - `:telegram_bot` rspec tag is replaced with `telegram_bot: :rails`. - __Breaking change__. Use bang-methods as actions for commands. This prevents calling context contextual actions and payload specific actions with commands. + Translation helper strips `!` from action name for lazy translations. - __Breaking change__. Drop `.context_handler`, `.context_to_action!` methods. Use pass action name directly to `#save_context`. It's the same as `.context_to_action!` is enabled by default. diff --git a/lib/telegram/bot/updates_controller.rb b/lib/telegram/bot/updates_controller.rb index 742378f..0d4a182 100644 --- a/lib/telegram/bot/updates_controller.rb +++ b/lib/telegram/bot/updates_controller.rb @@ -1,4 +1,5 @@ require 'abstract_controller' +require 'active_support/core_ext/string/inflections' require 'active_support/callbacks' require 'active_support/version' @@ -54,13 +55,14 @@ module Telegram abstract! %w[ - commands - instrumentation - log_subscriber - reply_helpers - rescue - session - ].each { |file| require "telegram/bot/updates_controller/#{file}" } + Commands + Instrumentation + LogSubscriber + ReplyHelpers + Rescue + Session + Translation + ].each { |name| require "telegram/bot/updates_controller/#{name.underscore}" } %w[ CallbackQueryContext @@ -79,10 +81,12 @@ module Telegram skip_after_callbacks_if_terminated: true end - include AbstractController::Translation include Commands include Rescue include ReplyHelpers + include Translation + # Add instrumentations hooks at the bottom, to ensure they instrument + # all the methods properly. include Instrumentation extend Session::ConfigMethods diff --git a/lib/telegram/bot/updates_controller/translation.rb b/lib/telegram/bot/updates_controller/translation.rb new file mode 100644 index 0000000..9af9dcf --- /dev/null +++ b/lib/telegram/bot/updates_controller/translation.rb @@ -0,0 +1,36 @@ +module Telegram + module Bot + class UpdatesController + # Provides helpers similar to AbstractController::Translation + # but by default uses `action_name_i18n_key` in lazy translation keys + # which strips `!` from action names by default. This makes translating + # strings for commands more convenient. + # + # To disable this behaviour use `alias_method :action_name_i18n_key, :action_name`. + module Translation + # See toplevel description. + def translate(key, options = {}) + if key.to_s.start_with?('.') + path = controller_path.tr('/', '.') + defaults = [:"#{path}#{key}"] + defaults << options[:default] if options[:default] + options[:default] = defaults.flatten + key = "#{path}.#{action_name_i18n_key}#{key}" + end + I18n.translate(key, options) + end + alias :t :translate + + # Strips trailing `!` from action_name. + def action_name_i18n_key + action_name.chomp('!') + end + + def localize(*args) + I18n.localize(*args) + end + alias :l :localize + end + end + end +end diff --git a/spec/telegram/bot/updates_controller/translation_spec.rb b/spec/telegram/bot/updates_controller/translation_spec.rb new file mode 100644 index 0000000..d9cdfb5 --- /dev/null +++ b/spec/telegram/bot/updates_controller/translation_spec.rb @@ -0,0 +1,17 @@ +RSpec.describe Telegram::Bot::UpdatesController::Translation do + describe '#translate' do + let(:controller) do + double( + controller_path: 'telegram/webhooks', + action_name: 'start!', + ).tap { |x| x.extend(described_class) } + end + + it 'uses action_name without ! for lazy translations' do + expect(I18n).to receive(:translate).with('telegram.webhooks.start.hello', + default: [:'telegram.webhooks.hello'], + ) + controller.t('.hello') + end + end +end From b5f4c97f443567078dadc5f194521c9c0961405f Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Thu, 7 Jun 2018 14:26:13 +0600 Subject: [PATCH 4/5] Class-level helper for lazy translations --- CHANGELOG.md | 1 + lib/telegram/bot/updates_controller/translation.rb | 11 +++++++++++ .../bot/updates_controller/translation_spec.rb | 14 ++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd40380..68516e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - __Breaking change__. Drop `.context_handler`, `.context_to_action!` methods. Use pass action name directly to `#save_context`. It's the same as `.context_to_action!` is enabled by default. +- Class-level helper for lazy translations. # 0.13.1 diff --git a/lib/telegram/bot/updates_controller/translation.rb b/lib/telegram/bot/updates_controller/translation.rb index 9af9dcf..ee2ba12 100644 --- a/lib/telegram/bot/updates_controller/translation.rb +++ b/lib/telegram/bot/updates_controller/translation.rb @@ -8,6 +8,17 @@ module Telegram # # To disable this behaviour use `alias_method :action_name_i18n_key, :action_name`. module Translation + extend ActiveSupport::Concern + + module ClassMethods + # Class-level helper for lazy translations. + def translate(key, options = {}) + key = "#{controller_path.tr('/', '.')}#{key}" if key.to_s.start_with?('.') + I18n.translate(key, options) + end + alias :t :translate + end + # See toplevel description. def translate(key, options = {}) if key.to_s.start_with?('.') diff --git a/spec/telegram/bot/updates_controller/translation_spec.rb b/spec/telegram/bot/updates_controller/translation_spec.rb index d9cdfb5..534a20c 100644 --- a/spec/telegram/bot/updates_controller/translation_spec.rb +++ b/spec/telegram/bot/updates_controller/translation_spec.rb @@ -14,4 +14,18 @@ RSpec.describe Telegram::Bot::UpdatesController::Translation do controller.t('.hello') end end + + describe described_class::ClassMethods do + describe '#translate' do + let(:controller_class) do + double(controller_path: 'telegram/webhooks'). + tap { |x| x.extend(described_class) } + end + + it 'uses controller_path for lazy translations' do + expect(I18n).to receive(:translate).with('telegram.webhooks.hello', {}) + controller_class.t('.hello') + end + end + end end From 409d6e4aa8ca5f3d85ac2aaa65b7c1c709220271 Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Thu, 7 Jun 2018 14:38:51 +0600 Subject: [PATCH 5/5] Use not confusing naming in readme --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3483295..aa1b7ce 100644 --- a/README.md +++ b/README.md @@ -283,12 +283,12 @@ class Telegram::WebhookController < Telegram::Bot::UpdatesController def rename!(*) # set context for the next message - save_context :rename + save_context :rename_from_message respond_with :message, text: 'What name do you like?' end # register context handlers to handle this context - def rename(*words) + def rename_from_message(*words) update_name words[0] respond_with :message, text: 'Renamed!' end