From 995a2cc2503f7a08814c89a1151e191fbc75e949 Mon Sep 17 00:00:00 2001 From: Malaber Date: Mon, 4 May 2026 14:52:59 +0200 Subject: [PATCH 1/2] feat(accounts): add player team profiles Teams can link to player accounts by email while keeping profile tournament data public unless viewed by the account owner. --- app/controllers/accounts_controller.rb | 15 +++++ app/controllers/application_controller.rb | 7 +- app/controllers/teams_controller.rb | 51 ++++++++++++++- app/controllers/tournaments_controller.rb | 11 +++- app/models/team.rb | 3 +- app/models/user.rb | 32 ++++++++++ app/serializers/account_profile_serializer.rb | 60 +++++++++++++++++ app/serializers/account_summary_serializer.rb | 9 +++ app/serializers/team_serializer.rb | 6 +- app/serializers/tournament_serializer.rb | 1 + app/services/tournament_sync_schema.rb | 2 +- config/routes.rb | 1 + .../20260504120000_add_player_to_teams.rb | 7 ++ db/schema.rb | 5 +- spec/controllers/accounts_controller_spec.rb | 54 ++++++++++++++++ spec/controllers/teams_controller_spec.rb | 64 +++++++++++++++++++ .../tournaments_controller_spec.rb | 19 ++++++ spec/e2e/http/api_surface_spec.rb | 59 ++++++++++++++++- spec/models/team_spec.rb | 1 + spec/models/user_spec.rb | 2 + spec/routing/accounts_routing_spec.rb | 11 ++++ 21 files changed, 409 insertions(+), 11 deletions(-) create mode 100644 app/controllers/accounts_controller.rb create mode 100644 app/serializers/account_profile_serializer.rb create mode 100644 app/serializers/account_summary_serializer.rb create mode 100644 db/migrate/20260504120000_add_player_to_teams.rb create mode 100644 spec/controllers/accounts_controller_spec.rb create mode 100644 spec/routing/accounts_routing_spec.rb diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb new file mode 100644 index 0000000..67f43be --- /dev/null +++ b/app/controllers/accounts_controller.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AccountsController < ApplicationController + before_action :set_account, only: %i[show] + + def show + render json: @account, serializer: AccountProfileSerializer, scope: current_user + end + + private + + def set_account + @account = User.includes(:tournaments, player_teams: :tournament).find(params[:id]) + end +end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 97043de..a3451c5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -9,6 +9,7 @@ class ApplicationController < ActionController::API rescue_from ActionController::ParameterMissing do |e| render json: { error: e.message }, status: :bad_request end + rescue_from ActiveRecord::RecordInvalid, with: :render_record_invalid_error rescue_from ActiveRecord::RecordNotFound, with: :render_not_found_error protected @@ -56,7 +57,7 @@ class ApplicationController < ActionController::API end def require_writable_tournament!(tournament) - render_read_only_error if tournament.read_only_mode? + render_read_only_error if tournament&.read_only_mode? end def render_not_found_error(exception) @@ -67,4 +68,8 @@ class ApplicationController < ActionController::API end render json: { error: error }, status: :not_found end + + def render_record_invalid_error(exception) + render json: exception.record.errors, status: :unprocessable_content + end end diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb index d6e07c4..0a05825 100644 --- a/app/controllers/teams_controller.rb +++ b/app/controllers/teams_controller.rb @@ -3,7 +3,7 @@ class TeamsController < ApplicationController before_action :set_team, only: %i[show update] before_action :authenticate_user!, only: %i[update] - before_action -> { require_owner! @team.owner }, only: %i[update] + before_action :require_team_update_actor!, only: %i[update] before_action -> { require_writable_tournament!(@team.tournament) }, only: %i[update] # GET /teams/1 @@ -13,7 +13,11 @@ class TeamsController < ApplicationController # PATCH/PUT /teams/1 def update - if @team.update(team_params) + @team.assign_attributes(team_params) + assign_player_from_params if player_assignment_requested? + return if performed? + + if @team.save push_sync_if_needed!(@team.tournament) render json: @team else @@ -31,6 +35,49 @@ class TeamsController < ApplicationController params.slice(:name).permit! end + def player_assignment_requested? + params.key?(:player_email) || params.key?(:player_id) + end + + def assign_player_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 + 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) + end + + def player_from_id + player_id = params[:player_id].to_s.strip + return nil if player_id.blank? + + User.find(player_id) + end + + def require_team_update_actor! + return if tournament_owner? || @team.player == current_user + + render_forbidden_error + end + + def tournament_owner? + @team.owner == current_user + end + def push_sync_if_needed!(tournament) TournamentSyncEnqueue.call(tournament) end diff --git a/app/controllers/tournaments_controller.rb b/app/controllers/tournaments_controller.rb index f878c77..ca202f5 100644 --- a/app/controllers/tournaments_controller.rb +++ b/app/controllers/tournaments_controller.rb @@ -236,10 +236,17 @@ class TournamentsController < ApplicationController if team[:id] Team.find team[:id] elsif team[:name] - Team.create name: team[:name] + Team.create name: team[:name], player: player_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) + end + def set_tournament @tournament = Tournament.find(params[:id]) end @@ -249,7 +256,7 @@ class TournamentsController < ApplicationController @tournament = profiling.measure('load_tournament') do Tournament.includes( :user, - :teams, + { teams: :player }, stages: [ { matches: { match_scores: :team } }, { groups: [ diff --git a/app/models/team.rb b/app/models/team.rb index 4ad8508..6f39fd5 100644 --- a/app/models/team.rb +++ b/app/models/team.rb @@ -2,6 +2,7 @@ class Team < ApplicationRecord belongs_to :tournament, optional: true + belongs_to :player, class_name: 'User', optional: true, inverse_of: :player_teams has_many :group_scores, dependent: :destroy has_many :match_scores, dependent: :destroy has_many :bets, dependent: :destroy @@ -9,5 +10,5 @@ class Team < ApplicationRecord validates :name, presence: true - delegate :owner, to: :tournament + delegate :owner, to: :tournament, allow_nil: true end diff --git a/app/models/user.rb b/app/models/user.rb index 6d2a109..a887116 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -11,5 +11,37 @@ 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 :played_tournaments, -> { distinct }, through: :player_teams, source: :tournament has_many :bets, dependent: :destroy + + class << self + def find_or_create_player_by_email!(email) + normalized_email = email.to_s.strip.downcase + where('LOWER(email) = ?', normalized_email).first || create!(email: normalized_email) do |user| + generated_password = Devise.friendly_token.first(24) + user.username = unique_player_username(normalized_email) + user.password = generated_password + user.password_confirmation = generated_password + user.confirmed_at = Time.current + user.uid = normalized_email + user.provider = 'email' + end + end + + private + + def unique_player_username(email) + base = email.split('@').first.to_s.parameterize.presence || 'player' + candidate = base + suffix = 2 + + while where('LOWER(username) = ?', candidate.downcase).exists? + candidate = "#{base}-#{suffix}" + suffix += 1 + end + + candidate + end + end end diff --git a/app/serializers/account_profile_serializer.rb b/app/serializers/account_profile_serializer.rb new file mode 100644 index 0000000..de9a2ed --- /dev/null +++ b/app/serializers/account_profile_serializer.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +class AccountProfileSerializer < AccountSummarySerializer + attributes :tournaments, :created_tournaments, :teams + + def tournaments + visible_played_tournaments.map { |tournament| tournament_summary(tournament) } + end + + def created_tournaments + visible_created_tournaments.map { |tournament| tournament_summary(tournament) } + end + + def teams + visible_player_teams.map do |team| + { + id: team.id, + name: team.name, + tournament: tournament_summary(team.tournament) + } + end + end + + private + + def visible_played_tournaments + visible_tournaments(object.played_tournaments.order(:created_at)) + end + + def visible_created_tournaments + visible_tournaments(object.tournaments.order(:created_at)) + end + + def visible_player_teams + object.player_teams.includes(:tournament).order(:created_at).select do |team| + profile_owner? || team.tournament&.public? + end + end + + def visible_tournaments(scope) + return scope if profile_owner? + + scope.where(public: true) + end + + def profile_owner? + scope == object + end + + def tournament_summary(tournament) + return nil if tournament.nil? + + { + id: tournament.id, + name: tournament.name, + code: tournament.code, + public: tournament.public + } + end +end diff --git a/app/serializers/account_summary_serializer.rb b/app/serializers/account_summary_serializer.rb new file mode 100644 index 0000000..6d2ae03 --- /dev/null +++ b/app/serializers/account_summary_serializer.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AccountSummarySerializer < ApplicationSerializer + attributes :name, :username + + def name + object.username + end +end diff --git a/app/serializers/team_serializer.rb b/app/serializers/team_serializer.rb index cf33a86..eaa4fb5 100644 --- a/app/serializers/team_serializer.rb +++ b/app/serializers/team_serializer.rb @@ -1,5 +1,9 @@ # frozen_string_literal: true class TeamSerializer < ApplicationSerializer - attributes :name + attributes :name, :player + + def player + AccountSummarySerializer.new(object.player).as_json if object.player + end end diff --git a/app/serializers/tournament_serializer.rb b/app/serializers/tournament_serializer.rb index 8777d91..cb60514 100644 --- a/app/serializers/tournament_serializer.rb +++ b/app/serializers/tournament_serializer.rb @@ -34,6 +34,7 @@ class TournamentSerializer < SimpleTournamentSerializer { id: team.id, name: team.name, + player: team.player && AccountSummarySerializer.new(team.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 ee0a9ac..a103b5c 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 created_at updated_at sync_source_id].freeze + ignored: %w[tournament_id player_id created_at updated_at sync_source_id].freeze }.freeze, Beamer => { synced: %w[id name display_state is_default config].freeze, diff --git a/config/routes.rb b/config/routes.rb index 3176d07..be17e08 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -19,6 +19,7 @@ Rails.application.routes.draw do end resources :stages, only: %i[show update] resources :teams, only: %i[show update] + resources :accounts, only: %i[show] resources :team_action_items, only: %i[update] resources :tournaments do resources :team_action_lists, only: %i[create] diff --git a/db/migrate/20260504120000_add_player_to_teams.rb b/db/migrate/20260504120000_add_player_to_teams.rb new file mode 100644 index 0000000..b300983 --- /dev/null +++ b/db/migrate/20260504120000_add_player_to_teams.rb @@ -0,0 +1,7 @@ +# 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/schema.rb b/db/schema.rb index 773e654..67ad9a4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_04_30_121000) do +ActiveRecord::Schema[8.1].define(version: 2026_05_04_120000) do create_table "beamers", force: :cascade do |t| t.json "config", default: {}, null: false t.datetime "created_at", null: false @@ -124,9 +124,11 @@ ActiveRecord::Schema[8.1].define(version: 2026_04_30_121000) do 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 @@ -234,6 +236,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_04_30_121000) do add_foreign_key "team_action_items", "teams", on_delete: :cascade add_foreign_key "team_action_lists", "tournaments", 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 new file mode 100644 index 0000000..e73d58a --- /dev/null +++ b/spec/controllers/accounts_controller_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +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_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) } + let(:created_private_tournament) { create(:tournament, user: account, public: false) } + + before do + public_tournament + private_tournament + created_public_tournament + created_private_tournament + end + + it 'returns public account profile information' do + get :show, params: { id: account.to_param } + + body = deserialize_response(response) + expect(response).to be_successful + expect(body[:id]).to eq(account.id) + expect(body[:name]).to eq('player-one') + expect(body[:username]).to eq('player-one') + expect(body[:teams].map { |team| team[:id] }).to eq([public_team.id]) + expect(body[:tournaments].map { |tournament| tournament[:id] }).to eq([public_tournament.id]) + expect(body[:created_tournaments].map { |tournament| tournament[:id] }).to eq([created_public_tournament.id]) + end + + it 'returns private profile information to the account owner' do + apply_authentication_headers_for account + + get :show, params: { id: account.to_param } + + body = deserialize_response(response) + expect(body[:teams].map { |team| team[:id] }).to match_array([public_team.id, private_team.id]) + expect(body[:tournaments].map { |tournament| tournament[:id] }) + .to match_array([public_tournament.id, private_tournament.id]) + expect(body[:created_tournaments].map { |tournament| tournament[:id] }) + .to match_array([created_public_tournament.id, created_private_tournament.id]) + end + + it 'returns not found for unknown accounts' do + get :show, params: { id: User.maximum(:id).to_i + 1 } + + expect(response).to have_http_status(:not_found) + end + end +end diff --git a/spec/controllers/teams_controller_spec.rb b/spec/controllers/teams_controller_spec.rb index 8dc9eb1..778e551 100644 --- a/spec/controllers/teams_controller_spec.rb +++ b/spec/controllers/teams_controller_spec.rb @@ -19,6 +19,17 @@ RSpec.describe TeamsController, type: :controller do body = deserialize_response response expect(body[:name]).to eq(@team.name) end + + it 'returns player account summary when linked' do + player = create(:user, username: 'linked-player') + @team.update!(player: 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') + end end describe 'PUT #update' do @@ -46,6 +57,35 @@ RSpec.describe TeamsController, type: :controller do body = deserialize_response response expect(body[:name]).to eq(valid_update[:name]) end + + it 'associates an existing player account by email' do + player = create(:user, username: 'player-account', email: 'player@example.com') + + 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) + end + + it 'creates a player account by email when none exists' do + expect do + put :update, params: { id: @team.to_param, player_email: 'new-player@example.com' } + 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') + end + + it 'clears player association with blank player_id' do + @team.update!(player: create(:user)) + + put :update, params: { id: @team.to_param, player_id: '' } + + expect(response).to be_successful + expect(@team.reload.player).to be_nil + end end context 'with valid params as another user' do @@ -58,5 +98,29 @@ RSpec.describe TeamsController, type: :controller do expect(response).to have_http_status(:forbidden) end end + + context 'with valid params as linked player' do + before(:each) do + @player = create(:user) + @team.update!(player: @player) + apply_authentication_headers_for @player + end + + it 'updates the requested team name' do + put :update, params: { id: @team.to_param }.merge(valid_update) + + expect(response).to be_successful + expect(@team.reload.name).to eq(valid_update[:name]) + end + + it 'cannot reassign the linked player' do + new_player = create(:user, email: 'other-player@example.com') + + 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) + end + end end end diff --git a/spec/controllers/tournaments_controller_spec.rb b/spec/controllers/tournaments_controller_spec.rb index 4f396b3..5362bba 100644 --- a/spec/controllers/tournaments_controller_spec.rb +++ b/spec/controllers/tournaments_controller_spec.rb @@ -541,6 +541,25 @@ RSpec.describe TournamentsController, type: :controller do post :create, params: data end.to change(Team, :count).by(data[:teams].count) end + + it 'creates player accounts from team email values' do + data = create_playoff_tournament_data + data.delete :teams + data[:teams] = [ + { name: 'Alpha', player_email: 'alpha-player@example.com' }, + { name: 'Beta' }, + { name: 'Gamma' }, + { name: 'Delta' } + ] + + expect do + post :create, params: data + end.to change(User, :count).by(1) + + 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 + end end context 'with invalid parameters' do diff --git a/spec/e2e/http/api_surface_spec.rb b/spec/e2e/http/api_surface_spec.rb index f8600d7..02eb505 100644 --- a/spec/e2e/http/api_surface_spec.rb +++ b/spec/e2e/http/api_surface_spec.rb @@ -114,6 +114,61 @@ RSpec.describe 'Backend API surface HTTP E2E' do expect(unauthenticated[:status]).to eq(401) end + it 'links player accounts to teams and exposes public account profiles' do + unless ENV['TURNIERE_E2E_ALT_EMAIL'] && ENV['TURNIERE_E2E_ALT_USERNAME'] + skip('player profile flow requires TURNIERE_E2E_ALT_EMAIL and TURNIERE_E2E_ALT_USERNAME') + end + + unique = unique_suffix + player_email = ENV.fetch('TURNIERE_E2E_ALT_EMAIL') + player_username = ENV.fetch('TURNIERE_E2E_ALT_USERNAME') + player_client = other_client + player_created_tournament = create_playoff_tournament( + client: player_client, + public: true, + name_prefix: 'Player Created' + ) + tournament = create_playoff_tournament(client: owner_client, public: true, name_prefix: 'Player Profile') + linked_team = tournament.fetch(:teams).first + unlinked_team = tournament.fetch(:teams).fetch(1) + + link_existing = owner_client.patch("/teams/#{linked_team.fetch(:id)}", body: { player_email: player_email }) + player_update = player_client.patch("/teams/#{linked_team.fetch(:id)}", body: { + name: "#{linked_team.fetch(:name)} Player Rename" + }) + player_reassign = player_client.patch("/teams/#{linked_team.fetch(:id)}", body: { + player_email: "stolen-#{unique}@example.com" + }) + 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)}") + + expect(player_client.authenticated?).to eq(true) + expect(linked_team[:player]).to be_nil + + expect(link_existing[:status]).to eq(200) + expect(link_existing.dig(:json, :player, :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_placeholder[:status]).to eq(200) + expect(link_placeholder.dig(:json, :player, :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)) + + expect(profile[:status]).to eq(200) + expect(profile.dig(:json, :name)).to eq(player_username) + expect(profile.dig(:json, :teams).map { |team| team.fetch(:id) }).to include(linked_team.fetch(:id)) + expect(profile.dig(:json, :tournaments).map { |item| item.fetch(:id) }).to include(tournament.fetch(:id)) + expect(profile.dig(:json, :created_tournaments).map { |item| item.fetch(:id) }) + .to include(player_created_tournament.fetch(:id)) + end + it 'updates and destroys tournaments, enforces owner checks, and validates timer updates' do tournament = create_group_stage_tournament(client: owner_client, public: false, name_prefix: 'Mutable Tournament') @@ -562,12 +617,12 @@ RSpec.describe 'Backend API surface HTTP E2E' do expect(response[:json].map { |match| match.fetch(:state) }.uniq).to eq(['in_progress']) end - def create_playoff_tournament(client:, public:, name_prefix:) + def create_playoff_tournament(client:, public:, name_prefix:, teams: nil) payload = { name: "#{name_prefix} #{unique_suffix}", description: 'HTTP API surface E2E playoff tournament', public: public, - teams: 4.times.map do |index| + teams: teams || 4.times.map do |index| { name: "#{name_prefix.tr(' ', '')}-S#{index + 1}" } end } diff --git a/spec/models/team_spec.rb b/spec/models/team_spec.rb index d6d097e..6764c69 100644 --- a/spec/models/team_spec.rb +++ b/spec/models/team_spec.rb @@ -9,6 +9,7 @@ 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 :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 83ab130..ead323b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5,6 +5,8 @@ require 'rails_helper' RSpec.describe User, type: :model do describe 'association' do it { should have_many :tournaments } + it { should have_many :player_teams } + it { should have_many :played_tournaments } it { should have_many :bets } end diff --git a/spec/routing/accounts_routing_spec.rb b/spec/routing/accounts_routing_spec.rb new file mode 100644 index 0000000..a6f008d --- /dev/null +++ b/spec/routing/accounts_routing_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe AccountsController, type: :routing do + describe 'routing' do + it 'routes to #show' do + expect(get: '/accounts/1').to route_to('accounts#show', id: '1') + end + end +end From ed299aba5730fc4fe29a16d77e0542231e4f7d5e Mon Sep 17 00:00:00 2001 From: Malaber Date: Mon, 4 May 2026 15:25:26 +0200 Subject: [PATCH 2/2] 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 }