Add competition validation: If confirmed, then at least one organizer (…

…#9771)

* Add competition validation: If confirmed, then at least one organizer

* Fix Delegate/Organizer ID unpacking

* Include organizer in :confirmed and :visible test traits/mocks

* Fix failing tests (1): RSpec Factories

* Fix failing tests (2): Using new factory throughout tests

* Fix failing tests (3): Adjusting competition validations

* Fix failing tests (4): More model validators for organizers

* Fix organizer assumptions in competitions_controller_spec

* Add organizer to dummy competition

app/models/competition.rb

@@ -413,6 +413,13 @@ def must_have_at_least_one_delegate
     end
   end
 
+  validate :must_have_at_least_one_organizer, if: :confirmed_or_visible?
+  def must_have_at_least_one_organizer
+    if organizer_ids.empty?
+      errors.add(:organizer_ids, I18n.t('competitions.errors.must_contain_organizer'))
+    end
+  end
+
   def confirmed_or_visible?
     self.confirmed? || self.showAtAll
   end
@@ -705,15 +712,15 @@ def create_id_and_cell_name(force_override: false)
 
   attr_writer :staff_delegate_ids, :organizer_ids, :trainee_delegate_ids
   def staff_delegate_ids
-    @staff_delegate_ids || staff_delegates.pluck(:id).join(",")
+    @staff_delegate_ids || staff_delegates.map(&:id).join(",")
   end
 
   def organizer_ids
-    @organizer_ids || organizers.pluck(:id).join(",")
+    @organizer_ids || organizers.map(&:id).join(",")
   end
 
   def trainee_delegate_ids
-    @trainee_delegate_ids || trainee_delegates.pluck(:id).join(",")
+    @trainee_delegate_ids || trainee_delegates.map(&:id).join(",")
   end
 
   def enable_v2_registrations!

config/locales/en.yml

@@ -1893,6 +1893,7 @@ en:
       must_contain_event: "must contain at least one event for this competition"
       schedule_must_match_rounds: "must have at least one round, and all rounds must be included in the schedule."
       must_contain_delegate: "must contain at least one WCA Delegate"
+      must_contain_organizer: "must contain at least one organizer"
       not_all_delegates: "are not all Delegates"
       registration_close_after_open: "registration close must be after registration open"
       registration_already_closed: "registration must not already be closed."

spec/controllers/api_competitions_controller_spec.rb

@@ -16,12 +16,11 @@ def get_wcif_and_compare_persons_to(id, expected)
     let(:competition) {
       FactoryBot.create(
         :competition,
-        :with_delegate,
+        :visible,
         id: "TestComp2014",
         start_date: "2014-02-03",
         end_date: "2014-02-05",
         external_website: "http://example.com",
-        showAtAll: true,
       )
     }
 
@@ -57,11 +56,11 @@ def get_wcif_and_compare_persons_to(id, expected)
         :competition,
         :with_delegate,
         :with_valid_schedule,
+        :visible,
         id: "TestComp2014",
         start_date: "2014-02-03",
         end_date: "2014-02-05",
         external_website: "http://example.com",
-        showAtAll: true,
       )
     }
 
@@ -264,12 +263,12 @@ def get_wcif_and_compare_persons_to(id, expected)
       FactoryBot.create(
         :competition,
         :with_delegate,
+        :visible,
         id: "TestComp2014",
         name: "Test Comp 2014",
         start_date: "2014-02-03",
         end_date: "2014-02-05",
         external_website: "http://example.com",
-        showAtAll: true,
         event_ids: %w(333 444),
         latitude: 43_641_740,
         longitude: -79_376_902,
@@ -357,15 +356,15 @@ def get_wcif_and_compare_persons_to(id, expected)
           comp_id += 1
           last_registration = FactoryBot.create(:registration, :accepted, competition: competition, user: user)
         end
-        get_wcif_and_compare_persons_to(competition.id, user_competitor_ids + [[competition.delegates.first.id, nil]])
+        get_wcif_and_compare_persons_to(competition.id, user_competitor_ids + [[competition.organizers.first.id, nil], [competition.delegates.first.id, nil]])
 
         # Move last registration to deleted
         last_registration.touch :deleted_at
         # Create and register one new user
         user = FactoryBot.create(:user)
         last_registration = FactoryBot.create(:registration, :accepted, competition: competition, user: user)
         user_competitor_ids << [user.id, comp_id]
-        get_wcif_and_compare_persons_to(competition.id, user_competitor_ids + [[competition.delegates.first.id, nil]])
+        get_wcif_and_compare_persons_to(competition.id, user_competitor_ids + [[competition.organizers.first.id, nil], [competition.delegates.first.id, nil]])
       end
 
       it 'gets announced and unannounced series competitions ids' do

spec/controllers/competitions_controller_spec.rb

@@ -3,7 +3,7 @@
 require 'rails_helper'
 
 RSpec.describe CompetitionsController do
-  let(:competition) { FactoryBot.create(:competition, :with_delegate, :registration_open, :with_valid_schedule, :with_guest_limit, :with_event_limit, name: "my long competition name above 32 chars 2023") }
+  let(:competition) { FactoryBot.create(:competition, :with_delegate, :with_organizer, :registration_open, :with_valid_schedule, :with_guest_limit, :with_event_limit, name: "my long competition name above 32 chars 2023") }
   let(:future_competition) { FactoryBot.create(:competition, :with_delegate, :ongoing) }
 
   describe 'GET #index' do
@@ -218,6 +218,7 @@
       it 'cannot see unconfirmed nearby competitions' do
         get :nearby_competitions_json, params: my_competition.serializable_hash
         expect(JSON.parse(response.body)).to eq []
+        other_competition.organizers = [organizer]
         other_competition.confirmed = true
         other_competition.save!
         get :nearby_competitions_json, params: my_competition.serializable_hash
@@ -363,7 +364,7 @@
         expect(new_comp.id).to eq ""
 
         # Cloning a competition should clone its organizers.
-        expect(new_comp.organizers.sort_by(&:id)).to eq []
+        expect(new_comp.organizers.sort_by(&:id)).to eq competition.organizers.sort_by(&:id)
         # When a delegate clones a competition, it should clone its organizers, and add
         # the delegate doing the cloning.
         expect(new_comp.delegates.sort_by(&:id)).to eq [delegate]
@@ -481,16 +482,17 @@
       end
 
       it "organizer cannot demote oneself" do
+        original_organizer = competition.organizers.first
         # Attempt to remove ourself as an organizer. This should not be allowed, because
         # we would not be allowed to access the page anymore.
-        update_params = build_competition_update(competition, staff: { organizerIds: [] })
+        update_params = build_competition_update(competition, staff: { organizerIds: [original_organizer.id] })
         patch :update, params: update_params, as: :json
         expect(response).to have_http_status(:bad_request)
         errors = JSON.parse(response.body)
         expect(errors['staff']['staffDelegateIds']).to eq ["You cannot demote yourself"]
         expect(errors['staff']['traineeDelegateIds']).to eq ["You cannot demote yourself"]
         expect(errors['staff']['organizerIds']).to eq ["You cannot demote yourself"]
-        expect(competition.reload.organizers).to eq [organizer]
+        expect(competition.reload.organizers).to eq [original_organizer, organizer]
       end
 
       it "can update the registration fees when there is no payment" do
@@ -563,7 +565,6 @@
       end
 
       it "adds another organizer and expects him to receive a notification email" do
-        competition.organizers << organizer1
         new_organizer = FactoryBot.create :user
         expect(CompetitionsMailer).to receive(:notify_organizer_of_addition_to_competition).with(competition.delegates.last, competition, new_organizer).and_call_original
         organizers = [competition.organizers.first, new_organizer]
@@ -575,7 +576,7 @@
 
       it "notifies organizers correctly when id changes" do
         new_organizer = FactoryBot.create :user
-        update_params = build_competition_update(competition, competitionId: "NewId2018", staff: { organizerIds: [new_organizer.id] })
+        update_params = build_competition_update(competition, competitionId: "NewId2018", staff: { organizerIds: [competition.organizers.last.id, new_organizer.id] })
         competition.id = "NewId2018"
         expect(CompetitionsMailer).to receive(:notify_organizer_of_addition_to_competition).with(competition.delegates.last, competition, new_organizer).and_call_original
         expect do
@@ -586,16 +587,15 @@
       it "removes an organizer and expects him to receive a notification email" do
         competition.organizers << [organizer1, organizer2]
         expect(CompetitionsMailer).to receive(:notify_organizer_of_removal_from_competition).with(competition.delegates.last, competition, organizer2).and_call_original
-        update_params = build_competition_update(competition, staff: { organizerIds: [organizer1.id] })
+        update_params = build_competition_update(competition, staff: { organizerIds: [competition.organizers.first.id, organizer1.id] })
         expect do
           patch :update, params: update_params, as: :json
         end.to change { enqueued_jobs.size }.by(1)
       end
 
       it "can confirm a competition and expects wcat and organizers to receive a notification email" do
-        competition.organizers << organizer1
         competition.update(start_date: 5.week.from_now, end_date: 5.week.from_now)
-        expect(CompetitionsMailer).to receive(:notify_organizer_of_confirmed_competition).with(competition.delegates.last, competition, organizer1).and_call_original
+        expect(CompetitionsMailer).to receive(:notify_organizer_of_confirmed_competition).with(competition.delegates.last, competition, competition.organizers.last).and_call_original
         expect(CompetitionsMailer).to receive(:notify_wcat_of_confirmed_competition).with(competition.delegates.last, competition).and_call_original
         expect do
           put :confirm, params: { competition_id: competition }
@@ -734,7 +734,6 @@
       end
 
       it "adds another organizer and expects him to receive a notification email" do
-        competition.organizers << organizer1
         new_organizer = FactoryBot.create :user
         expect(CompetitionsMailer).to receive(:notify_organizer_of_addition_to_competition).with(competition.trainee_delegates.last, competition, new_organizer).and_call_original
         organizers = [competition.organizers.first, new_organizer]
@@ -746,7 +745,7 @@
 
       it "notifies organizers correctly when id changes" do
         new_organizer = FactoryBot.create :user
-        update_params = build_competition_update(competition, competitionId: "NewId2018", staff: { organizerIds: [new_organizer.id] })
+        update_params = build_competition_update(competition, competitionId: "NewId2018", staff: { organizerIds: [competition.organizers.last.id, new_organizer.id] })
         competition.id = "NewId2018"
         expect(CompetitionsMailer).to receive(:notify_organizer_of_addition_to_competition).with(competition.trainee_delegates.last, competition, new_organizer).and_call_original
         expect do
@@ -756,8 +755,9 @@
 
       it "removes an organizer and expects him to receive a notification email" do
         competition.organizers << [organizer1, organizer2]
+        puts competition.organizers.count
         expect(CompetitionsMailer).to receive(:notify_organizer_of_removal_from_competition).with(competition.trainee_delegates.last, competition, organizer2).and_call_original
-        update_params = build_competition_update(competition, staff: { organizerIds: [organizer1.id] })
+        update_params = build_competition_update(competition, staff: { organizerIds: [competition.organizers.first.id, organizer1.id] })
         expect do
           patch :update, params: update_params, as: :json
         end.to change { enqueued_jobs.size }.by(1)
@@ -857,11 +857,9 @@
       it 'announces and expects organizers to receive a notification email' do
         sign_in wcat_member
         competition.update(start_date: "2011-12-04", end_date: "2011-12-05")
-        organizer = FactoryBot.create :user
-        competition.organizers << organizer
         expect(competition.announced_at).to be nil
         expect(competition.announced_by).to be nil
-        expect(CompetitionsMailer).to receive(:notify_organizer_of_announced_competition).with(competition, organizer).and_call_original
+        expect(CompetitionsMailer).to receive(:notify_organizer_of_announced_competition).with(competition, competition.organizers.last).and_call_original
         expect do
           put :announce, params: { competition_id: competition }
         end.to change { enqueued_jobs.size }.by(1)

spec/controllers/registrations_controller_spec.rb

@@ -261,8 +261,8 @@
     let!(:delegate) { FactoryBot.create(:delegate) }
     let!(:other_delegate) { FactoryBot.create(:delegate) }
 
-    let!(:competition) { FactoryBot.create(:competition, :registration_open, delegates: [delegate], showAtAll: true) }
-    let!(:other_competition) { FactoryBot.create(:competition, :registration_open, delegates: [other_delegate], showAtAll: true) }
+    let!(:competition) { FactoryBot.create(:competition, :registration_open, :visible, delegates: [delegate]) }
+    let!(:other_competition) { FactoryBot.create(:competition, :registration_open, :visible, delegates: [other_delegate]) }
 
     before :each do
       sign_in delegate
@@ -364,7 +364,7 @@
   context "signed in as competitor" do
     let!(:user) { FactoryBot.create(:user, :wca_id) }
     let!(:delegate) { FactoryBot.create(:delegate) }
-    let!(:competition) { FactoryBot.create(:competition, :registration_open, delegates: [delegate], showAtAll: true) }
+    let!(:competition) { FactoryBot.create(:competition, :visible, :registration_open, delegates: [delegate]) }
     let(:threes_comp_event) { competition.competition_events.find_by(event_id: "333") }
 
     before :each do

spec/factories/competitions.rb

@@ -144,6 +144,7 @@
 
     trait :confirmed do
       with_delegate
+      with_organizer
       with_valid_schedule
       confirmed_at { Time.now }
     end
@@ -154,6 +155,7 @@
 
     trait :visible do
       with_delegate
+      with_organizer
       showAtAll { true }
     end
 

spec/factories/contacts.rb

@@ -15,14 +15,15 @@
     transient do
       competition_contact { nil }
       competition_delegates { [] }
+      competition_organizers { [] }
     end
 
     trait :with_invalid_competition_id do
       competition_id { "FooBar1900" }
     end
 
     trait :with_competition do
-      competition_id { FactoryBot.create(:competition, :announced, contact: competition_contact, delegates: competition_delegates).id }
+      competition_id { FactoryBot.create(:competition, :announced, contact: competition_contact, delegates: competition_delegates, organizers: competition_organizers).id }
     end
   end
 

spec/features/competition_management_spec.rb

@@ -99,7 +99,7 @@ def wca_registration_checkbox
     end
 
     scenario "User confirms a competition" do
-      competition = FactoryBot.create(:competition, :future, :with_delegate, :with_valid_schedule)
+      competition = FactoryBot.create(:competition, :future, :with_delegate, :with_organizer, :with_valid_schedule)
       visit edit_competition_path(competition)
       click_button "Confirm"
 
@@ -226,7 +226,7 @@ def wca_registration_checkbox
   context "when signed in as delegate" do
     let!(:delegate) { FactoryBot.create(:delegate) }
     let(:cloned_delegate) { FactoryBot.create(:delegate) }
-    let(:competition_to_clone) { FactoryBot.create :competition, cityName: 'Melbourne, Victoria', countryId: "Australia", delegates: [cloned_delegate], showAtAll: true }
+    let(:competition_to_clone) { FactoryBot.create :competition, :visible, cityName: 'Melbourne, Victoria', countryId: "Australia", delegates: [cloned_delegate] }
 
     let(:threes) { Event.find("333") }
     let(:fours) { Event.find("444") }

spec/features/register_for_competition_spec.rb

@@ -5,7 +5,7 @@
 RSpec.feature "Registering for a competition" do
   let!(:user) { FactoryBot.create :user }
   let!(:delegate) { FactoryBot.create :delegate }
-  let(:competition) { FactoryBot.create :competition, :registration_open, delegates: [delegate], showAtAll: true }
+  let(:competition) { FactoryBot.create :competition, :registration_open, :visible, delegates: [delegate] }
 
   context "signed in as user" do
     before :each do

spec/models/competition_spec.rb

@@ -783,7 +783,7 @@
   end
 
   describe "when confirming or making visible" do
-    let(:competition_with_delegate) { FactoryBot.build :competition, :with_delegate, generate_website: false }
+    let(:competition_with_delegate) { FactoryBot.build :competition, :with_delegate, :with_organizer, generate_website: false }
     let(:competition_without_delegate) { FactoryBot.build :competition }
 
     [:confirmed, :showAtAll].each do |action|
@@ -816,7 +816,7 @@
     end
 
     it "sets confirmed_at when setting confirmed true" do
-      competition = FactoryBot.create :competition, :future, :with_delegate, :with_valid_schedule
+      competition = FactoryBot.create :competition, :future, :with_delegate, :with_organizer, :with_valid_schedule
       expect(competition.confirmed_at).to be_nil
 
       now = Time.at(Time.now.to_i)
@@ -1085,16 +1085,17 @@
     let(:delegate2) { FactoryBot.create(:delegate) }
     let(:organizer1) { FactoryBot.create(:user) }
     let(:organizer2) { FactoryBot.create(:user) }
+    let(:organizer3) { FactoryBot.create(:user) }
     let!(:competition) {
       FactoryBot.create(:competition, :confirmed, delegates: [delegate1, delegate2], organizers: [organizer1, organizer2])
     }
-    let!(:competition_without_organizers) {
-      FactoryBot.create(:competition, :confirmed, delegates: [delegate1, delegate2], organizers: [])
+    let!(:competition_with_different_organizers) {
+      FactoryBot.create(:competition, :confirmed, delegates: [delegate1, delegate2], organizers: [organizer3])
     }
     let!(:other_comp) { FactoryBot.create(:competition) }
 
     it "finds comps by delegate" do
-      expect(Competition.managed_by(delegate1.id)).to match_array [competition, competition_without_organizers]
+      expect(Competition.managed_by(delegate1.id)).to match_array [competition, competition_with_different_organizers]
     end
 
     it "finds comps by organizer" do

spec/models/competition_wcif_spec.rb

@@ -6,23 +6,30 @@
   let!(:competition) {
     FactoryBot.create(
       :competition,
-      :with_delegate,
+      :visible,
       :with_competitor_limit,
       id: "TestComp2014",
       name: "Test Comp 2014",
       cellName: "Test 2014",
       start_date: "2014-02-03",
       end_date: "2014-02-05",
       external_website: "http://example.com",
-      showAtAll: true,
       event_ids: %w(333 444 333fm 333mbf),
       with_schedule: true,
       competitor_limit: 50,
       registration_open: "2013-12-01",
       registration_close: "2013-12-31",
     )
   }
-  let(:partner_competition) { FactoryBot.create(:competition, :with_delegate, id: "PartnerComp2014", series_base: competition, series_distance_days: 3, showAtAll: true) }
+  let(:partner_competition) {
+    FactoryBot.create(
+      :competition,
+      :visible,
+      id: "PartnerComp2014",
+      series_base: competition,
+      series_distance_days: 3,
+    )
+  }
   let!(:competition_series) {
     FactoryBot.create(
       :competition_series,
@@ -33,6 +40,7 @@
     )
   }
   let(:delegate) { competition.delegates.first }
+  let(:organizer) { competition.organizers.first }
   let(:sixty_second_2_attempt_cutoff) { Cutoff.new(number_of_attempts: 2, attempt_result: 1.minute.in_centiseconds) }
   let(:top_16_advance) { AdvancementConditions::RankingCondition.new(16) }
   let!(:round333_1) { FactoryBot.create(:round, competition: competition, event_id: "333", number: 1, cutoff: sixty_second_2_attempt_cutoff, advancement_condition: top_16_advance, scramble_set_count: 16, total_number_of_rounds: 2) }
@@ -59,7 +67,7 @@
           "shortName" => "Spectacular 2014",
           "competitionIds" => %w[TestComp2014 PartnerComp2014],
         },
-        "persons" => [delegate.to_wcif(competition)],
+        "persons" => [organizer.to_wcif(competition), delegate.to_wcif(competition)],
         "events" => [
           {
             "id" => "333",