Merge pull request #177 from denis1011101/fix/game-chat-reply-delivery

Fix/game chat reply delivery

Author: denis1011101

app/jobs/telegram/deliver_chat_message_job.rb

@@ -19,19 +19,38 @@ def perform(game_id, recipient_id, text)
       # доставкой человека могли вывести из состава.
       return unless game.chat_open? && game.team_member_ids.include?(recipient.id)
 
+      # Отвечать в том же окне, где пришло сообщение, — первое, что делает
+      # человек. Без указателя ответ пропадал: Relay не знает, в какую игру его
+      # отдать. Поэтому доставка сама включает получателю чат этой игры — но
+      # только после того, как сообщение действительно ушло.
+      arming = Telegram::Chat::Session.automatic_start_needed?(
+        recipient.telegram_chat_id, recipient, game
+      )
+
       # Без parse_mode: это чужой текст, а не наш шаблон. С Markdown одиночный
       # `_` или `[` либо исказит сообщение, либо уронит отправку четырёхсоткой.
+      params = {
+        "chat_id" => recipient.telegram_chat_id.to_s,
+        "link_preview_options" => Telegram::Api::LINK_PREVIEW_DISABLED,
+        "text" => text.to_s
+      }
+      params["reply_markup"] = { inline_keyboard: Telegram::Chat::Flow.controls(recipient) }.to_json if arming
+
       response = begin
-        Telegram::Api.post("sendMessage", {
-          "chat_id" => recipient.telegram_chat_id.to_s,
-          "link_preview_options" => Telegram::Api::LINK_PREVIEW_DISABLED,
-          "text" => text.to_s
-        })
+        Telegram::Api.post("sendMessage", params)
       rescue StandardError => error
         raise TransientDeliveryError, error.message
       end
 
-      handle_response(response, game_id, recipient_id, text)
+      delivered = handle_response(response, game_id, recipient_id, text)
+      # Ретрай и постоянная ошибка не должны оставлять человека в чате, о котором
+      # он не узнал: сообщение с кнопками до него не дошло. Следующая попытка
+      # увидит, что указателя нет, и пришлёт кнопки снова. Запись — только если
+      # выбора всё ещё нет: пока шла отправка, человек мог открыть другую игру.
+      if arming && delivered
+        Telegram::Chat::Session.start_automatically(recipient.telegram_chat_id, recipient, game)
+      end
+      delivered
     end
 
     private

app/services/telegram/chat/flow.rb

@@ -43,7 +43,8 @@ def enter(user, game)
           return false if chat_id.blank? || game.nil?
           return false unless game.chat_open? && game.team_member_ids.include?(user.id)
 
-          Session.start(chat_id, game)
+          return false unless Session.start_automatically(chat_id, user, game)
+
           show_status(chat_id.to_s, user, game)
           true
         end
@@ -96,11 +97,17 @@ def stop(chat_id, user)
         def show_status(chat_id, user, game)
           recipients = game.chat_members.where.not(id: user.id).count
           text = t(user, :chat_started, game: Message.game_label(game), count: recipients)
-          buttons = [ [
+          Telegram::Api.send_with_buttons(chat_id, text, controls(user), parse_mode: nil)
+        end
+
+        # Кнопки управления режимом. Их же вешает на доставленное сообщение
+        # DeliverChatMessageJob, когда чат включается получателю: состояние видно
+        # там же, где оно появилось, без отдельной карточки вдогонку.
+        def controls(user)
+          [ [
             { text: t(user, :chat_switch_btn), callback_data: "chat:pick" },
             { text: t(user, :chat_exit_btn), callback_data: "chat:exit" }
           ] ]
-          Telegram::Api.send_with_buttons(chat_id, text, buttons, parse_mode: nil)
         end
 
         private

app/services/telegram/chat/session.rb

@@ -10,36 +10,62 @@ module Chat
     # игры уже после того, как включил режим.
     module Session
       KEY_PREFIX = "tg:chat:active".freeze
+      AUTOMATIC_KEY_PREFIX = "tg:chat:auto".freeze
 
       class << self
+        # Явный выбор пользователя всегда имеет приоритет над автоматическим.
         def start(chat_id, game)
           closes_at = game&.chat_open_until
           return false unless closes_at
 
           ttl = [ closes_at - Time.current, 1.minute ].max
           Rails.cache.write(key(chat_id), game.id, expires_in: ttl)
+          Rails.cache.delete(automatic_key(chat_id))
           true
         end
 
+        # Автоматические сессии сменяют друг друга, но лежат отдельно от явного
+        # выбора и поэтому не могут его затереть даже при параллельной записи.
+        def start_automatically(chat_id, user, game)
+          closes_at = game&.chat_open_until
+          return false unless closes_at
+
+          ttl = [ closes_at - Time.current, 1.minute ].max
+          Rails.cache.write(automatic_key(chat_id), game.id, expires_in: ttl)
+
+          # Явный выбор мог появиться, пока шла доставка. Он имеет приоритет;
+          # автоматический указатель заодно убираем, чтобы тот не ожил после TTL.
+          if active_game_for(explicit_game_id(chat_id), user)
+            Rails.cache.delete(automatic_key(chat_id))
+            return false
+          end
+
+          true
+        end
+
+        def automatic_start_needed?(chat_id, user, game)
+          return false if active_game_for(explicit_game_id(chat_id), user)
+
+          active_game_for(automatic_game_id(chat_id), user)&.id != game.id
+        end
+
         def stop(chat_id)
           Rails.cache.delete(key(chat_id))
+          Rails.cache.delete(automatic_key(chat_id))
         end
 
         def game_id(chat_id)
-          Rails.cache.read(key(chat_id))
+          explicit_game_id(chat_id) || automatic_game_id(chat_id)
         end
 
-        # Игра, в которую человек пишет прямо сейчас, — или nil, и тогда режим
-        # гасится: игра прошла, человека вывели из состава, игру удалили.
+        # Игра, в которую человек пишет прямо сейчас. Протухшие указатели не
+        # удаляем здесь: параллельный явный выбор мог уже записать в тот же ключ
+        # новое значение. Они безвредны и исчезнут сами по TTL.
         def active_game(chat_id, user)
-          id = game_id(chat_id)
-          return nil unless id && user
-
-          game = Game.find_by(id: id)
-          return game if game&.chat_open? && game.team_member_ids.include?(user.id)
+          return nil unless user
 
-          stop(chat_id)
-          nil
+          active_game_for(explicit_game_id(chat_id), user) ||
+            active_game_for(automatic_game_id(chat_id), user)
         end
 
         # Человек вышел из игры (или его вывели) — гасим режим, но только если
@@ -56,6 +82,27 @@ def stop_for(user, game)
         def key(chat_id)
           "#{KEY_PREFIX}:#{chat_id}"
         end
+
+        def automatic_key(chat_id)
+          "#{AUTOMATIC_KEY_PREFIX}:#{chat_id}"
+        end
+
+        private
+
+        def explicit_game_id(chat_id)
+          Rails.cache.read(key(chat_id))
+        end
+
+        def automatic_game_id(chat_id)
+          Rails.cache.read(automatic_key(chat_id))
+        end
+
+        def active_game_for(id, user)
+          return nil unless id && user
+
+          game = Game.find_by(id: id)
+          game if game&.chat_open? && game.team_member_ids.include?(user.id)
+        end
       end
     end
   end

test/jobs/telegram/chat_relay_test.rb

@@ -1,7 +1,9 @@
 require "test_helper"
+require "support/cache_helper"
 
 class Telegram::ChatRelayTest < ActiveSupport::TestCase
   include ActiveJob::TestHelper
+  include CacheHelper
 
   setup do
     @court = Court.create!(name: "Relay Court", city_name: "Yekaterinburg")
@@ -53,6 +55,174 @@ class Telegram::ChatRelayTest < ActiveSupport::TestCase
     assert_not params.key?("parse_mode")
   end
 
+  # Ответ в том же окне — первое, что делает получатель. Раньше он пропадал:
+  # у человека не было указателя, и Relay не знал, в какую игру его отдать.
+  test "delivery turns the chat mode on for a recipient who has none" do
+    params = nil
+
+    with_memory_cache do
+      stub_singleton(Telegram::Api, :post, ->(_path, sent) { params = sent; { "ok" => true } }) do
+        Telegram::DeliverChatMessageJob.perform_now(@game.id, @owner.id, "во сколько?")
+      end
+
+      assert_equal @game.id, Telegram::Chat::Session.active_game(@owner.telegram_chat_id, @owner).id
+    end
+
+    buttons = JSON.parse(params["reply_markup"])["inline_keyboard"].first
+    assert_equal [ "chat:pick", "chat:exit" ], buttons.map { |button| button["callback_data"] }
+  end
+
+  test "delivery leaves an existing chat choice alone" do
+    other = Game.create!(court: @court, user: @owner, date: Date.current, kind: "game")
+    params = nil
+
+    with_memory_cache do
+      Telegram::Chat::Session.start(@owner.telegram_chat_id, other)
+
+      stub_singleton(Telegram::Api, :post, ->(_path, sent) { params = sent; { "ok" => true } }) do
+        Telegram::DeliverChatMessageJob.perform_now(@game.id, @owner.id, "во сколько?")
+      end
+
+      assert_equal other.id, Telegram::Chat::Session.game_id(@owner.telegram_chat_id)
+    end
+
+    assert_not params.key?("reply_markup")
+  ensure
+    other&.destroy
+  end
+
+  test "a later automatic delivery becomes the reply target" do
+    other = Game.create!(court: @court, user: @owner, date: Date.current, kind: "game")
+    second_delivery = nil
+
+    with_memory_cache do
+      stub_singleton(Telegram::Api, :post, ->(*) { { "ok" => true } }) do
+        Telegram::DeliverChatMessageJob.perform_now(@game.id, @owner.id, "из первой игры")
+      end
+
+      stub_singleton(Telegram::Api, :post, ->(_path, sent) { second_delivery = sent; { "ok" => true } }) do
+        Telegram::DeliverChatMessageJob.perform_now(other.id, @owner.id, "из второй игры")
+      end
+
+      assert_equal other.id, Telegram::Chat::Session.active_game(@owner.telegram_chat_id, @owner).id
+    end
+
+    assert second_delivery.key?("reply_markup")
+  ensure
+    other&.destroy
+  end
+
+  test "a failed delivery leaves no chat mode behind" do
+    # 403 — постоянная ошибка: сообщение с кнопками до человека не дошло, и
+    # оказаться в чате втихаря он не должен.
+    with_memory_cache do
+      stub_singleton(Telegram::Api, :post, ->(*) { { "ok" => false, "error_code" => 403, "description" => "Forbidden" } }) do
+        Telegram::DeliverChatMessageJob.perform_now(@game.id, @owner.id, "во сколько?")
+      end
+
+      assert_nil Telegram::Chat::Session.game_id(@owner.telegram_chat_id)
+    end
+  end
+
+  test "a throttled delivery arms nothing and sends the buttons again on the retry" do
+    throttled = { "ok" => false, "error_code" => 429, "parameters" => { "retry_after" => 7 } }
+    later = Class.new { def perform_later(*) = true }.new
+    second_attempt = nil
+
+    with_memory_cache do
+      stub_singleton(Telegram::Api, :post, ->(*) { throttled }) do
+        stub_singleton(Telegram::DeliverChatMessageJob, :set, ->(wait:) { later }) do
+          Telegram::DeliverChatMessageJob.perform_now(@game.id, @owner.id, "во сколько?")
+        end
+      end
+
+      assert_nil Telegram::Chat::Session.game_id(@owner.telegram_chat_id)
+
+      stub_singleton(Telegram::Api, :post, ->(_path, sent) { second_attempt = sent; { "ok" => true } }) do
+        Telegram::DeliverChatMessageJob.perform_now(@game.id, @owner.id, "во сколько?")
+      end
+
+      assert_equal @game.id, Telegram::Chat::Session.game_id(@owner.telegram_chat_id)
+    end
+
+    assert second_attempt.key?("reply_markup")
+  end
+
+  test "a retried server error leaves no chat mode behind" do
+    with_memory_cache do
+      assert_enqueued_jobs 1, only: Telegram::DeliverChatMessageJob do
+        stub_singleton(Telegram::Api, :post, ->(*) { { "ok" => false, "error_code" => 503 } }) do
+          Telegram::DeliverChatMessageJob.perform_now(@game.id, @owner.id, "во сколько?")
+        end
+      end
+
+      assert_nil Telegram::Chat::Session.game_id(@owner.telegram_chat_id)
+    end
+  end
+
+  # Указатель на игру, которой больше нет, — мусор: он не должен навсегда
+  # запирать включение чата для живой игры.
+  test "a stale pointer does not block arming the chat mode" do
+    other = Game.create!(court: @court, user: @owner, date: Date.current, kind: "game")
+    params = nil
+
+    with_memory_cache do
+      Telegram::Chat::Session.start(@owner.telegram_chat_id, other)
+      other.destroy
+
+      stub_singleton(Telegram::Api, :post, ->(_path, sent) { params = sent; { "ok" => true } }) do
+        Telegram::DeliverChatMessageJob.perform_now(@game.id, @owner.id, "во сколько?")
+      end
+
+      assert_equal @game.id, Telegram::Chat::Session.active_game(@owner.telegram_chat_id, @owner).id
+    end
+
+    assert params.key?("reply_markup")
+  end
+
+  # Отправка не мгновенна: пока она идёт, человек мог сам открыть другую игру.
+  test "a choice made while the message was in flight is not overwritten" do
+    other = Game.create!(court: @court, user: @owner, date: Date.current, kind: "game")
+
+    with_memory_cache do
+      posting = lambda do |*|
+        Telegram::Chat::Session.start(@owner.telegram_chat_id, other)
+        { "ok" => true }
+      end
+
+      stub_singleton(Telegram::Api, :post, posting) do
+        Telegram::DeliverChatMessageJob.perform_now(@game.id, @owner.id, "во сколько?")
+      end
+
+      assert_equal other.id, Telegram::Chat::Session.game_id(@owner.telegram_chat_id)
+    end
+  ensure
+    other&.destroy
+  end
+
+  test "stale validation does not delete a concurrent explicit choice" do
+    stale = Game.create!(court: @court, user: @owner, date: Date.current, kind: "game")
+    fresh = Game.create!(court: @court, user: @owner, date: Date.current, kind: "game")
+
+    with_memory_cache do
+      Telegram::Chat::Session.start(@owner.telegram_chat_id, stale)
+      stale.destroy
+
+      lookup = lambda do |**|
+        Telegram::Chat::Session.start(@owner.telegram_chat_id, fresh)
+        nil
+      end
+      stub_singleton(Game, :find_by, lookup) do
+        assert_nil Telegram::Chat::Session.active_game(@owner.telegram_chat_id, @owner)
+      end
+
+      assert_equal fresh.id, Telegram::Chat::Session.game_id(@owner.telegram_chat_id)
+    end
+  ensure
+    fresh&.destroy
+    stale&.destroy
+  end
+
   test "delivery re-checks membership right before sending" do
     @participation.destroy