Bug: handle_event("next_round") always executes unconditional DB fetch and broadcast — intermediate returns are dead code

[immortal71]

Describe the bug

In player_live/show.ex, handle_event("next_round", ...) contains two early {:noreply, ...} returns inside the if round_open? branches. Because these are not the last expression in the function, Elixir discards them. The function always falls through to the unconditional Game.find + Endpoint.broadcast at the end.

Affected file

copi.owasp.org/lib/copi_web/live/player_live/show.ex

Code snippet

def handle_event("next_round", _, socket) do
  game = socket.assigns.game

  if round_open?(game) do
    if Copi.Cornucopia.Game.can_continue_round?(game) do
      Copi.Cornucopia.update_game(game, %{round_open: false})
      Process.send_after(self(), :proceed_to_next_round, 100)
      {:noreply, assign(socket, :game, game)}  # ← ignored; not the last expression
    else
      {:noreply, socket}                        # ← ignored; not the last expression
    end
  else
    Copi.Cornucopia.update_game(game, %{rounds_played: game.rounds_played + 1, round_open: true})
    ...
  end

  # These always execute, regardless of which branch was taken above
  {:ok, updated_game} = Game.find(game.id)
  CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
  {:noreply, assign(socket, :game, updated_game)}
end

Impact

• When the round can't close yet (round_open? && !can_continue?), the handler is supposed to be a no-op, but it still executes a Game.find DB query and broadcasts a game:updated PubSub message to all connected players — causing unnecessary load and spurious UI reloads.
• The intended early returns are dead code and will never be the actual function return value.

Expected behavior

When no state change occurs (round is open and cannot yet continue), the handler should return {:noreply, socket} immediately with no DB query and no broadcast.

Proposed fix

Restructure the handler so each branch is a complete, independent case/cond arm that returns its own {:noreply, ...} including the appropriate fetch and broadcast only when a state change actually occurred.

[sydseter]

@immortal71 I have assigned you.

[sydseter]

The code still contains the problematic handle_event("next_round", ...) structure in copi.owasp.org/lib/copi_web/live/player_live/show.ex: the round_open? branches return correctly, and the DB fetch/broadcast occur only in the else branch.

PR #2836 proposes this fix but remains open, so the issue itself remains unresolved.