Address review notes on the training video notifications

- can_send_training_video? moves to ApplicationController: the checkbox is
  rendered by games/_media, which GamesController serves, so the helper
  declared on GameMediaController blew up games#show (CI red).
- delivery fans out into one job per recipient with retries, so a transient
  email/telegram failure retries and ends up in failed jobs instead of being
  swallowed by a log line.
- the notification names the actual uploader (an admin or an accepted coach
  can post the video too), falling back to neutral wording without a name.
- a photo uploaded with the box ticked now says plainly that only videos are
  sent, instead of silently ignoring the choice.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: denis1011101

app/controllers/application_controller.rb

@@ -8,7 +8,8 @@ class ApplicationController < ActionController::Base
   protect_from_forgery with: :exception
   before_action :authenticate_user!
 
-  helper_method :current_user, :user_signed_in?, :sign_in, :sign_out, :geocoding_exceeded?, :can_manage?, :can_remove_participant?
+  helper_method :current_user, :user_signed_in?, :sign_in, :sign_out, :geocoding_exceeded?, :can_manage?, :can_remove_participant?,
+                :can_send_training_video?
 
   private
 
@@ -54,6 +55,15 @@ def can_remove_participant?(game, participation_user)
     AccessControl.can_remove_participant?(current_user, game, participation_user)
   end
 
+  # Рассылку ролика инициирует тот, кто ведёт игру: организатор, админ или
+  # принятый тренер. Живёт здесь, а не в GameMediaController, потому что
+  # галочку рисует games/_media, а его отдаёт GamesController.
+  def can_send_training_video?(game)
+    return false unless current_user
+
+    can_manage?(game) || (game.coach_id == current_user.id && game.coach_accepted?)
+  end
+
   def current_user
     @current_user ||= User.find_by(id: session[:user_id]) if session[:user_id].present?
   end

app/controllers/game_media_controller.rb

@@ -1,6 +1,5 @@
 class GameMediaController < ApplicationController
   before_action :set_game
-  helper_method :can_send_training_video?
 
   # Прикладывать может организатор, админ и любой записавшийся участник:
   # снимает обычно не тот, кто игру создал.
@@ -12,12 +11,7 @@ def create
     end
 
     if medium.save
-      if notify_participants?(medium)
-        SendTrainingVideoJob.perform_later(medium.id)
-        redirect_to @game, notice: t("game_media.uploaded_and_queued")
-      else
-        redirect_to @game, notice: t("game_media.uploaded")
-      end
+      redirect_to @game, notice: upload_notice(medium)
     else
       redirect_to @game, alert: medium.errors.full_messages.to_sentence.presence || t("game_media.failed"),
                   status: :see_other
@@ -52,11 +46,25 @@ def contributor?
     @game.participations.approved.exists?(user_id: current_user.id)
   end
 
+  # Один и тот же file input принимает и фото, и видео, а рассылать мы умеем
+  # только ролики. Поэтому галочка на фотографии не проглатывается молча:
+  # файл сохраняем, но честно говорим, что рассылки не будет.
+  def upload_notice(medium)
+    if notify_participants?(medium)
+      SendTrainingVideoJob.perform_later(medium.id)
+      t("game_media.uploaded_and_queued")
+    elsif notify_requested? && can_send_training_video?(@game)
+      t("game_media.uploaded_videos_only")
+    else
+      t("game_media.uploaded")
+    end
+  end
+
   def notify_participants?(medium)
-    medium.video? && can_send_training_video?(@game) && ActiveModel::Type::Boolean.new.cast(params[:notify_participants])
+    medium.video? && can_send_training_video?(@game) && notify_requested?
   end
 
-  def can_send_training_video?(game)
-    can_manage?(game) || (game.coach == current_user && game.coach_accepted?)
+  def notify_requested?
+    ActiveModel::Type::Boolean.new.cast(params[:notify_participants])
   end
 end

app/jobs/send_training_video_job.rb

@@ -1,29 +1,50 @@
 class SendTrainingVideoJob < ApplicationJob
   queue_as :default
 
-  def perform(game_medium_id)
+  # Отправка идёт по одной задаче на получателя: временный отказ почты или
+  # телеграма роняет только свою доставку, она уходит в ретрай, а исчерпав
+  # попытки — в failed jobs. Общий rescue вместо этого молча терял игрока.
+  retry_on StandardError, wait: :polynomially_longer, attempts: 5
+
+  def perform(game_medium_id, user_id = nil)
     medium = GameMedium.includes(file_attachment: :blob).find_by(id: game_medium_id)
     return unless medium&.video? && !medium.hidden?
 
-    notification = build_notification(medium)
-    medium.game.participations.approved.where.not(user_id: nil).includes(:user).find_each do |participation|
-      NotificationDelivery.deliver(user: participation.user, notification: notification)
-    rescue StandardError => e
-      Rails.logger.warn("SendTrainingVideoJob failed for user_id=#{participation.user_id}: #{e.message}")
+    return deliver(medium, user_id) if user_id
+
+    medium.game.participations.approved.where.not(user_id: nil).find_each do |participation|
+      self.class.perform_later(game_medium_id, participation.user_id)
     end
   end
 
   private
 
+  def deliver(medium, user_id)
+    user = User.find_by(id: user_id)
+    return unless user
+
+    NotificationDelivery.deliver(user: user, notification: build_notification(medium))
+  end
+
   def build_notification(medium)
     url = Rails.application.routes.url_helpers.rails_blob_url(
       medium.file,
       **Rails.application.config.action_mailer.default_url_options.to_h.symbolize_keys
     )
+    # Ролик может выложить не только организатор, но и админ или принятый
+    # тренер, поэтому имя берём из самой загрузки. Без имени — нейтральный
+    # текст: приписывать видео организатору наугад нельзя.
+    author = medium.user&.name.to_s.strip.presence
 
     NotificationDelivery::Notification.new(
       subject: ->(locale) { I18n.t("game_media.training_video.subject", locale: locale) },
-      body: ->(locale) { I18n.t("game_media.training_video.body", locale: locale) },
+      body: lambda { |locale|
+        if author
+          I18n.t("game_media.training_video.body_by", author: author, locale: locale)
+        else
+          I18n.t("game_media.training_video.body", locale: locale)
+        end
+      },
       actions: lambda { |locale|
         [ { label: I18n.t("game_media.training_video.action", locale: locale), url: url } ]
       }

config/locales/web.en.yml

@@ -73,13 +73,15 @@ en:
   game_media:
     uploaded: Attachment uploaded.
     uploaded_and_queued: Video uploaded and queued for delivery to the players.
+    uploaded_videos_only: Photo uploaded. Only videos can be sent to the players, so nothing was delivered.
     removed: Attachment removed.
     hidden: Attachment hidden from the feed.
     failed: Could not upload the attachment.
     not_allowed: Only the organizer and the players of this game can attach media.
     training_video:
       subject: Training video for your game
-      body: The organizer shared a training video with the players of your game.
+      body: A training video has been shared with the players of your game.
+      body_by: "%{author} shared a training video with the players of your game."
       action: Watch video
   games:
     show:

config/locales/web.es.yml

@@ -73,13 +73,15 @@ es:
   game_media:
     uploaded: Archivo subido.
     uploaded_and_queued: El vídeo se ha subido y está en cola para enviarlo a los jugadores.
+    uploaded_videos_only: Foto subida. A los jugadores solo se les pueden enviar vídeos, así que no se ha enviado nada.
     removed: Archivo eliminado.
     hidden: Archivo oculto del feed.
     failed: No se ha podido subir el archivo.
     not_allowed: Solo el organizador y los jugadores del partido pueden adjuntar archivos.
     training_video:
       subject: Vídeo de entrenamiento para tu partido
-      body: El organizador ha compartido un vídeo de entrenamiento con los jugadores de tu partido.
+      body: Se ha compartido un vídeo de entrenamiento con los jugadores de tu partido.
+      body_by: "%{author} ha compartido un vídeo de entrenamiento con los jugadores de tu partido."
       action: Ver vídeo
   games:
     show:

config/locales/web.ru.yml

@@ -73,13 +73,15 @@ ru:
   game_media:
     uploaded: Вложение загружено.
     uploaded_and_queued: Видео загружено и поставлено в очередь на отправку игрокам.
+    uploaded_videos_only: Фото загружено. Игрокам можно отправлять только видео, поэтому рассылки не было.
     removed: Вложение удалено.
     hidden: Вложение скрыто из ленты.
     failed: Не удалось загрузить вложение.
     not_allowed: Прикладывать фото и видео могут организатор и участники игры.
     training_video:
       subject: Обучающее видео для вашей игры
-      body: Организатор поделился обучающим видео с участниками вашей игры.
+      body: Обучающим видео поделились с участниками вашей игры.
+      body_by: "%{author} поделился обучающим видео с участниками вашей игры."
       action: Смотреть видео
   games:
     show:

test/controllers/game_media_controller_test.rb

@@ -42,6 +42,18 @@ class GameMediaControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to game_path(@game)
   end
 
+  test "a photo with the box ticked is uploaded but says nothing was sent" do
+    sign_in_as @owner
+
+    assert_no_enqueued_jobs only: SendTrainingVideoJob do
+      assert_difference -> { @game.game_media.count }, 1 do
+        post game_media_path(@game), params: { file: upload, notify_participants: "1" }
+      end
+    end
+
+    assert_equal I18n.t("game_media.uploaded_videos_only"), flash[:notice]
+  end
+
   test "a player cannot queue an uploaded video for everyone" do
     sign_in_as @participant
 

test/jobs/send_training_video_job_test.rb

@@ -20,11 +20,7 @@ class SendTrainingVideoJobTest < ActiveJob::TestCase
   end
 
   test "delivers a GetCourt video link only to approved registered players" do
-    deliveries = []
-
-    with_stubbed_singleton_method(NotificationDelivery, :deliver, ->(**args) { deliveries << args }) do
-      SendTrainingVideoJob.perform_now(@medium.id)
-    end
+    deliveries = deliver_all
 
     assert_equal [ @approved.id ], deliveries.map { |delivery| delivery[:user].id }
     notification = deliveries.first[:notification]
@@ -33,19 +29,60 @@ class SendTrainingVideoJobTest < ActiveJob::TestCase
     assert_match %r{http://example.com/rails/active_storage/blobs/redirect/}, notification.actions(:en).first[:url]
   end
 
+  test "names the person who uploaded the video instead of the organizer" do
+    coach = User.create!(name: "Coach Marina", email: "training-coach@example.com")
+    @medium.update!(user: coach)
+
+    assert_equal "Coach Marina shared a training video with the players of your game.",
+                 deliver_all.first[:notification].body(:en)
+  end
+
+  test "falls back to sender-neutral wording when the uploader has no name" do
+    @medium.user.update_column(:name, nil)
+
+    assert_equal "A training video has been shared with the players of your game.",
+                 deliver_all.first[:notification].body(:en)
+  end
+
   test "does not deliver a hidden video" do
-    deliveries = []
     @medium.hide!
 
-    with_stubbed_singleton_method(NotificationDelivery, :deliver, ->(**args) { deliveries << args }) do
+    assert_empty deliver_all
+  end
+
+  test "each approved player gets their own delivery job" do
+    other = User.create!(email: "training-approved-2@example.com", locale: "en", notification_channel: "email")
+    @game.participations.create!(user: other)
+
+    # Раскладка по получателям: одна задача на игрока, чтобы падение доставки
+    # не уносило с собой остальных.
+    assert_enqueued_jobs 2, only: SendTrainingVideoJob do
       SendTrainingVideoJob.perform_now(@medium.id)
     end
+  end
 
-    assert_empty deliveries
+  test "a failed delivery is retried instead of being swallowed" do
+    failing = ->(**_args) { raise "telegram is down" }
+
+    assert_enqueued_with(job: SendTrainingVideoJob, args: [ @medium.id, @approved.id ]) do
+      with_stubbed_singleton_method(NotificationDelivery, :deliver, failing) do
+        SendTrainingVideoJob.perform_now(@medium.id, @approved.id)
+      end
+    end
   end
 
   private
 
+  def deliver_all
+    deliveries = []
+
+    with_stubbed_singleton_method(NotificationDelivery, :deliver, ->(**args) { deliveries << args }) do
+      perform_enqueued_jobs(only: SendTrainingVideoJob) { SendTrainingVideoJob.perform_now(@medium.id) }
+    end
+
+    deliveries
+  end
+
   def with_stubbed_singleton_method(target, method_name, replacement)
     singleton = target.singleton_class
     original = singleton.instance_method(method_name)