From ed299aba5730fc4fe29a16d77e0542231e4f7d5e Mon Sep 17 00:00:00 2001 From: Malaber Date: Mon, 4 May 2026 15:25:26 +0200 Subject: [PATCH] feat(accounts): support multiple team players Teams now use a join table so organizers can attach more than one player account while players still get public profile data. --- app/controllers/teams_controller.rb | 87 +++++++++++++------ app/controllers/tournaments_controller.rb | 14 +-- app/models/team.rb | 3 +- app/models/team_player.rb | 6 ++ app/models/user.rb | 3 +- app/serializers/team_serializer.rb | 6 +- app/serializers/tournament_serializer.rb | 2 +- app/services/tournament_sync_schema.rb | 2 +- .../20260504120000_add_player_to_teams.rb | 7 -- .../20260504120000_create_team_players.rb | 18 ++++ db/schema.rb | 15 +++- spec/controllers/accounts_controller_spec.rb | 4 +- spec/controllers/teams_controller_spec.rb | 40 ++++++--- .../tournaments_controller_spec.rb | 12 ++- spec/e2e/http/api_surface_spec.rb | 25 ++++-- spec/models/team_spec.rb | 3 +- spec/models/user_spec.rb | 1 + 17 files changed, 174 insertions(+), 74 deletions(-) create mode 100644 app/models/team_player.rb delete mode 100644 db/migrate/20260504120000_add_player_to_teams.rb create mode 100644 db/migrate/20260504120000_create_team_players.rb diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb index 0a05825..0450240 100644 --- a/app/controllers/teams_controller.rb +++ b/app/controllers/teams_controller.rb @@ -13,16 +13,19 @@ class TeamsController < ApplicationController # PATCH/PUT /teams/1 def update - @team.assign_attributes(team_params) - assign_player_from_params if player_assignment_requested? + Team.transaction do + @team.assign_attributes(team_params) + assign_players_from_params if player_assignment_requested? + raise ActiveRecord::Rollback if performed? + + @team.save! + end return if performed? - if @team.save - push_sync_if_needed!(@team.tournament) - render json: @team - else - render json: @team.errors, status: :unprocessable_content - end + push_sync_if_needed!(@team.tournament) + render json: @team + rescue ActiveRecord::RecordInvalid => e + render json: e.record.errors, status: :unprocessable_content end private @@ -36,40 +39,74 @@ class TeamsController < ApplicationController end def player_assignment_requested? - params.key?(:player_email) || params.key?(:player_id) + params.key?(:player_email) || params.key?(:player_id) || + params.key?(:player_emails) || params.key?(:player_ids) end - def assign_player_from_params + def assign_players_from_params unless tournament_owner? render_forbidden_error return end - @team.player = if params.key?(:player_email) - player_from_email - else - player_from_id - end + if params.key?(:player_emails) + @team.players = players_from_emails(params[:player_emails]) + elsif params.key?(:player_ids) + @team.players = players_from_ids(params[:player_ids]) + elsif params.key?(:player_email) + add_or_clear_player_from_email + else + add_or_clear_player_from_id + end rescue ActiveRecord::RecordInvalid => e render json: e.record.errors, status: :unprocessable_content end - def player_from_email - email = params[:player_email].to_s.strip - return nil if email.blank? - - User.find_or_create_player_by_email!(email) + def players_from_emails(emails) + email_values(emails).map { |email| User.find_or_create_player_by_email!(email) } end - def player_from_id - player_id = params[:player_id].to_s.strip - return nil if player_id.blank? + def players_from_ids(player_ids) + id_values = Array.wrap(player_ids).map(&:to_s).map(&:strip).reject(&:blank?) + return [] if id_values.empty? - User.find(player_id) + User.where(id: id_values).tap do |players| + unless players.size == id_values.uniq.size + raise ActiveRecord::RecordNotFound.new('Could not find all players', 'User') + end + end + end + + def add_or_clear_player_from_email + email = params[:player_email].to_s.strip + if email.blank? + @team.players = [] + return + end + + add_player(User.find_or_create_player_by_email!(email)) + end + + def add_or_clear_player_from_id + player_id = params[:player_id].to_s.strip + if player_id.blank? + @team.players = [] + return + end + + add_player(User.find(player_id)) + end + + def add_player(player) + @team.players << player unless @team.player_ids.include?(player.id) + end + + def email_values(emails) + Array.wrap(emails).map(&:to_s).map(&:strip).reject(&:blank?) end def require_team_update_actor! - return if tournament_owner? || @team.player == current_user + return if tournament_owner? || @team.players.include?(current_user) render_forbidden_error end diff --git a/app/controllers/tournaments_controller.rb b/app/controllers/tournaments_controller.rb index ca202f5..b539bb0 100644 --- a/app/controllers/tournaments_controller.rb +++ b/app/controllers/tournaments_controller.rb @@ -236,15 +236,15 @@ class TournamentsController < ApplicationController if team[:id] Team.find team[:id] elsif team[:name] - Team.create name: team[:name], player: player_from_team_params(team) + Team.create(name: team[:name], players: players_from_team_params(team)) end end - def player_from_team_params(team) - player_email = team[:player_email].to_s.strip - return nil if player_email.blank? - - User.find_or_create_player_by_email!(player_email) + def players_from_team_params(team) + player_emails = team[:player_emails].presence || team[:player_email] + Array.wrap(player_emails).map(&:to_s).map(&:strip).reject(&:blank?).map do |player_email| + User.find_or_create_player_by_email!(player_email) + end end def set_tournament @@ -256,7 +256,7 @@ class TournamentsController < ApplicationController @tournament = profiling.measure('load_tournament') do Tournament.includes( :user, - { teams: :player }, + { teams: :players }, stages: [ { matches: { match_scores: :team } }, { groups: [ diff --git a/app/models/team.rb b/app/models/team.rb index 6f39fd5..57c2a44 100644 --- a/app/models/team.rb +++ b/app/models/team.rb @@ -2,7 +2,8 @@ class Team < ApplicationRecord belongs_to :tournament, optional: true - belongs_to :player, class_name: 'User', optional: true, inverse_of: :player_teams + has_many :team_players, dependent: :destroy + has_many :players, through: :team_players has_many :group_scores, dependent: :destroy has_many :match_scores, dependent: :destroy has_many :bets, dependent: :destroy diff --git a/app/models/team_player.rb b/app/models/team_player.rb new file mode 100644 index 0000000..ada0c3a --- /dev/null +++ b/app/models/team_player.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class TeamPlayer < ApplicationRecord + belongs_to :team + belongs_to :player, class_name: 'User' +end diff --git a/app/models/user.rb b/app/models/user.rb index a887116..7e156c0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -11,7 +11,8 @@ class User < ApplicationRecord validates :username, presence: true, uniqueness: { case_sensitive: false } has_many :tournaments, dependent: :destroy - has_many :player_teams, class_name: 'Team', foreign_key: :player_id, inverse_of: :player, dependent: :nullify + has_many :team_players, foreign_key: :player_id, inverse_of: :player, dependent: :destroy + has_many :player_teams, through: :team_players, source: :team has_many :played_tournaments, -> { distinct }, through: :player_teams, source: :tournament has_many :bets, dependent: :destroy diff --git a/app/serializers/team_serializer.rb b/app/serializers/team_serializer.rb index eaa4fb5..5799663 100644 --- a/app/serializers/team_serializer.rb +++ b/app/serializers/team_serializer.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class TeamSerializer < ApplicationSerializer - attributes :name, :player + attributes :name, :players - def player - AccountSummarySerializer.new(object.player).as_json if object.player + def players + object.players.map { |player| AccountSummarySerializer.new(player).as_json } end end diff --git a/app/serializers/tournament_serializer.rb b/app/serializers/tournament_serializer.rb index cb60514..f23cf83 100644 --- a/app/serializers/tournament_serializer.rb +++ b/app/serializers/tournament_serializer.rb @@ -34,7 +34,7 @@ class TournamentSerializer < SimpleTournamentSerializer { id: team.id, name: team.name, - player: team.player && AccountSummarySerializer.new(team.player).as_json, + players: team.players.map { |player| AccountSummarySerializer.new(player).as_json }, advancing_from_group_stage: adv_teams.include?(team) } end diff --git a/app/services/tournament_sync_schema.rb b/app/services/tournament_sync_schema.rb index a103b5c..ee0a9ac 100644 --- a/app/services/tournament_sync_schema.rb +++ b/app/services/tournament_sync_schema.rb @@ -33,7 +33,7 @@ class TournamentSyncSchema }.freeze, Team => { synced: %w[id name].freeze, - ignored: %w[tournament_id player_id created_at updated_at sync_source_id].freeze + ignored: %w[tournament_id created_at updated_at sync_source_id].freeze }.freeze, Beamer => { synced: %w[id name display_state is_default config].freeze, diff --git a/db/migrate/20260504120000_add_player_to_teams.rb b/db/migrate/20260504120000_add_player_to_teams.rb deleted file mode 100644 index b300983..0000000 --- a/db/migrate/20260504120000_add_player_to_teams.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -class AddPlayerToTeams < ActiveRecord::Migration[7.0] - def change - add_reference :teams, :player, type: :integer, foreign_key: { to_table: :users }, index: true - end -end diff --git a/db/migrate/20260504120000_create_team_players.rb b/db/migrate/20260504120000_create_team_players.rb new file mode 100644 index 0000000..da3af52 --- /dev/null +++ b/db/migrate/20260504120000_create_team_players.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CreateTeamPlayers < ActiveRecord::Migration[7.0] + def change + create_table :team_players do |t| + t.references :team, null: false, type: :integer, foreign_key: { on_delete: :cascade }, index: true + t.references :player, + null: false, + type: :integer, + foreign_key: { to_table: :users, on_delete: :cascade }, + index: true + + t.timestamps + end + + add_index :team_players, %i[team_id player_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 67ad9a4..1777a61 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -121,14 +121,22 @@ ActiveRecord::Schema[8.1].define(version: 2026_05_04_120000) do t.index ["tournament_id"], name: "index_team_action_lists_on_tournament_id" end + create_table "team_players", force: :cascade do |t| + t.datetime "created_at", null: false + t.integer "player_id", null: false + t.integer "team_id", null: false + t.datetime "updated_at", null: false + t.index ["player_id"], name: "index_team_players_on_player_id" + t.index ["team_id", "player_id"], name: "index_team_players_on_team_id_and_player_id", unique: true + t.index ["team_id"], name: "index_team_players_on_team_id" + end + create_table "teams", force: :cascade do |t| t.datetime "created_at", precision: nil, null: false t.string "name" - t.integer "player_id" t.integer "sync_source_id" t.integer "tournament_id" t.datetime "updated_at", precision: nil, null: false - t.index ["player_id"], name: "index_teams_on_player_id" t.index ["tournament_id"], name: "index_teams_on_tournament_id" end @@ -235,8 +243,9 @@ ActiveRecord::Schema[8.1].define(version: 2026_05_04_120000) do add_foreign_key "team_action_items", "team_action_lists", on_delete: :cascade add_foreign_key "team_action_items", "teams", on_delete: :cascade add_foreign_key "team_action_lists", "tournaments", on_delete: :cascade + add_foreign_key "team_players", "teams", on_delete: :cascade + add_foreign_key "team_players", "users", column: "player_id", on_delete: :cascade add_foreign_key "teams", "tournaments", on_delete: :cascade - add_foreign_key "teams", "users", column: "player_id" add_foreign_key "tournament_sync_queue_entries", "tournaments", on_delete: :cascade add_foreign_key "tournament_transaction_log_entries", "tournaments", on_delete: :cascade add_foreign_key "tournament_transaction_log_entries", "users", on_delete: :nullify diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index e73d58a..95ac9b4 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/controllers/accounts_controller_spec.rb @@ -5,8 +5,8 @@ require 'rails_helper' RSpec.describe AccountsController, type: :controller do describe 'GET #show' do let(:account) { create(:user, username: 'player-one') } - let(:public_team) { create(:team, name: 'Public Team', player: account) } - let(:private_team) { create(:team, name: 'Private Team', player: account) } + let(:public_team) { create(:team, name: 'Public Team', players: [account]) } + let(:private_team) { create(:team, name: 'Private Team', players: [account]) } let(:public_tournament) { create(:tournament, public: true, teams: [public_team]) } let(:private_tournament) { create(:tournament, public: false, teams: [private_team]) } let(:created_public_tournament) { create(:tournament, user: account, public: true) } diff --git a/spec/controllers/teams_controller_spec.rb b/spec/controllers/teams_controller_spec.rb index 778e551..bd6cc19 100644 --- a/spec/controllers/teams_controller_spec.rb +++ b/spec/controllers/teams_controller_spec.rb @@ -20,15 +20,15 @@ RSpec.describe TeamsController, type: :controller do expect(body[:name]).to eq(@team.name) end - it 'returns player account summary when linked' do + it 'returns player account summaries when linked' do player = create(:user, username: 'linked-player') - @team.update!(player: player) + @team.players << player get :show, params: { id: @team.to_param } body = deserialize_response response - expect(body.dig(:player, :id)).to eq(player.id) - expect(body.dig(:player, :name)).to eq('linked-player') + expect(body[:players].first[:id]).to eq(player.id) + expect(body[:players].first[:name]).to eq('linked-player') end end @@ -64,8 +64,8 @@ RSpec.describe TeamsController, type: :controller do put :update, params: { id: @team.to_param, player_email: player.email } expect(response).to be_successful - expect(@team.reload.player).to eq(player) - expect(deserialize_response(response).dig(:player, :id)).to eq(player.id) + expect(@team.reload.players).to contain_exactly(player) + expect(deserialize_response(response)[:players].first[:id]).to eq(player.id) end it 'creates a player account by email when none exists' do @@ -74,17 +74,31 @@ RSpec.describe TeamsController, type: :controller do end.to change(User, :count).by(1) expect(response).to be_successful - expect(@team.reload.player.email).to eq('new-player@example.com') - expect(@team.player.username).to eq('new-player') + expect(@team.reload.players.first.email).to eq('new-player@example.com') + expect(@team.players.first.username).to eq('new-player') end - it 'clears player association with blank player_id' do - @team.update!(player: create(:user)) + it 'associates multiple player accounts by email' do + first_player = create(:user, email: 'first-player@example.com') + + put :update, params: { + id: @team.to_param, + player_emails: [first_player.email, 'second-player@example.com'] + } + + expect(response).to be_successful + expect(@team.reload.players.map(&:email)).to match_array( + ['first-player@example.com', 'second-player@example.com'] + ) + end + + it 'clears player associations with blank player_id' do + @team.players << create(:user) put :update, params: { id: @team.to_param, player_id: '' } expect(response).to be_successful - expect(@team.reload.player).to be_nil + expect(@team.reload.players).to be_empty end end @@ -102,7 +116,7 @@ RSpec.describe TeamsController, type: :controller do context 'with valid params as linked player' do before(:each) do @player = create(:user) - @team.update!(player: @player) + @team.players << @player apply_authentication_headers_for @player end @@ -119,7 +133,7 @@ RSpec.describe TeamsController, type: :controller do put :update, params: { id: @team.to_param, player_email: new_player.email } expect(response).to have_http_status(:forbidden) - expect(@team.reload.player).to eq(@player) + expect(@team.reload.players).to contain_exactly(@player) end end end diff --git a/spec/controllers/tournaments_controller_spec.rb b/spec/controllers/tournaments_controller_spec.rb index 5362bba..467499a 100644 --- a/spec/controllers/tournaments_controller_spec.rb +++ b/spec/controllers/tournaments_controller_spec.rb @@ -546,7 +546,10 @@ RSpec.describe TournamentsController, type: :controller do data = create_playoff_tournament_data data.delete :teams data[:teams] = [ - { name: 'Alpha', player_email: 'alpha-player@example.com' }, + { + name: 'Alpha', + player_emails: ['alpha-player@example.com', 'alpha-partner@example.com'] + }, { name: 'Beta' }, { name: 'Gamma' }, { name: 'Delta' } @@ -554,11 +557,12 @@ RSpec.describe TournamentsController, type: :controller do expect do post :create, params: data - end.to change(User, :count).by(1) + end.to change(User, :count).by(2) tournament = Tournament.find(deserialize_response(response)[:id]) - expect(tournament.teams.find_by(name: 'Alpha').player.email).to eq('alpha-player@example.com') - expect(tournament.teams.find_by(name: 'Beta').player).to be_nil + expect(tournament.teams.find_by(name: 'Alpha').players.map(&:email)) + .to match_array(['alpha-player@example.com', 'alpha-partner@example.com']) + expect(tournament.teams.find_by(name: 'Beta').players).to be_empty end end diff --git a/spec/e2e/http/api_surface_spec.rb b/spec/e2e/http/api_surface_spec.rb index 02eb505..43ce856 100644 --- a/spec/e2e/http/api_surface_spec.rb +++ b/spec/e2e/http/api_surface_spec.rb @@ -139,25 +139,34 @@ RSpec.describe 'Backend API surface HTTP E2E' do player_reassign = player_client.patch("/teams/#{linked_team.fetch(:id)}", body: { player_email: "stolen-#{unique}@example.com" }) + co_player_email = "http-e2e-co-player-#{unique}@example.com" + link_co_player = owner_client.patch("/teams/#{linked_team.fetch(:id)}", body: { + player_email: co_player_email + }) placeholder_email = "http-e2e-placeholder-#{unique}@example.com" link_placeholder = owner_client.patch("/teams/#{unlinked_team.fetch(:id)}", body: { player_email: placeholder_email }) - profile = anonymous_client.get("/accounts/#{link_existing.dig(:json, :player, :id)}") - placeholder_profile = anonymous_client.get("/accounts/#{link_placeholder.dig(:json, :player, :id)}") + profile = anonymous_client.get("/accounts/#{player_id_by_name(link_existing, player_username)}") + placeholder_profile = anonymous_client.get("/accounts/#{player_id_by_name(link_placeholder, placeholder_email)}") expect(player_client.authenticated?).to eq(true) - expect(linked_team[:player]).to be_nil + expect(linked_team[:players]).to eq([]) expect(link_existing[:status]).to eq(200) - expect(link_existing.dig(:json, :player, :name)).to eq(player_username) + expect(link_existing.dig(:json, :players).map { |player| player.fetch(:name) }).to eq([player_username]) expect(player_update[:status]).to eq(200) expect(player_update.dig(:json, :name)).to end_with('Player Rename') expect(player_reassign[:status]).to eq(403) + expect(link_co_player[:status]).to eq(200) + co_player_names = link_co_player.dig(:json, :players).map { |player| player.fetch(:name) } + expect(co_player_names).to include(player_username) + expect(co_player_names.any? { |name| name.start_with?('http-e2e-co-player') }).to eq(true) + expect(link_placeholder[:status]).to eq(200) - expect(link_placeholder.dig(:json, :player, :name)).to start_with('http-e2e-placeholder') + expect(link_placeholder.dig(:json, :players).first[:name]).to start_with('http-e2e-placeholder') expect(placeholder_profile[:status]).to eq(200) expect(placeholder_profile.dig(:json, :teams).map { |team| team.fetch(:id) }).to include(unlinked_team.fetch(:id)) @@ -617,6 +626,12 @@ RSpec.describe 'Backend API surface HTTP E2E' do expect(response[:json].map { |match| match.fetch(:state) }.uniq).to eq(['in_progress']) end + def player_id_by_name(response, name) + response.dig(:json, :players).find do |player| + player.fetch(:name) == name || player.fetch(:name).start_with?(name.split('@').first) + end.fetch(:id) + end + def create_playoff_tournament(client:, public:, name_prefix:, teams: nil) payload = { name: "#{name_prefix} #{unique_suffix}", diff --git a/spec/models/team_spec.rb b/spec/models/team_spec.rb index 6764c69..33bcc69 100644 --- a/spec/models/team_spec.rb +++ b/spec/models/team_spec.rb @@ -9,7 +9,8 @@ RSpec.describe Team, type: :model do describe 'association' do it { should belong_to(:tournament).optional } - it { should belong_to(:player).optional } + it { should have_many :team_players } + it { should have_many :players } it { should have_many :group_scores } it { should have_many :match_scores } it { should have_many :bets } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ead323b..0571095 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5,6 +5,7 @@ require 'rails_helper' RSpec.describe User, type: :model do describe 'association' do it { should have_many :tournaments } + it { should have_many :team_players } it { should have_many :player_teams } it { should have_many :played_tournaments } it { should have_many :bets }