Validate that an event's course belongs to the event group's organization

app/models/event.rb

@@ -35,6 +35,7 @@ class Event < ApplicationRecord
  validates :scheduled_start_time, :laps_required, presence: true
  validates :short_name, uniqueness: { case_sensitive: false, scope: :event_group_id }
  validate :course_is_consistent
+ validate :course_matches_event_group_organization
 
  before_validation :add_default_results_template
  before_validation :conform_changed_course
@@ -95,6 +96,13 @@ def course_is_consistent
  errors.add(:course_id, "does not reconcile with one or more splits")
  end
 
+ def course_matches_event_group_organization
+ return unless course && event_group&.organization_id
+ return if course.organization_id == event_group.organization_id
+
+ errors.add(:course_id, "must belong to the same organization as the event group")
+ end
+
  def to_s
  slug
  end
@@ -164,6 +172,8 @@ def add_default_results_template
 
  def conform_changed_course
  return unless persisted? && course_id_changed?
+ # Let the belongs_to presence validation reject a blank course_id
+ return if course.nil?
 
  response = Interactors::ChangeEventCourse.perform!(event: self, new_course: course)
  response.errors.each { |error| errors.add(:base, error[:title]) }

spec/controllers/api/v1/aid_stations_controller_spec.rb

@@ -3,7 +3,7 @@
 RSpec.describe Api::V1::AidStationsController do
  let(:course) { create(:course) }
  let(:split) { create(:split, course: course) }
- let(:event) { create(:event, course: course) }
+ let(:event) { create(:event, course: course, event_group: create(:event_group, organization: course.organization)) }
  let(:aid_station) { create(:aid_station, split: split, event: event) }
  let(:type) { "aid_stations" }
 
@@ -12,7 +12,7 @@
 
  via_login_and_jwt do
  context "when an existing aid_station.id is provided" do
- let(:params) { {id: aid_station} }
+ let(:params) { {id: aid_station} }
 
  it "returns a successful 200 response" do
  make_request
@@ -21,18 +21,18 @@
 
  it "returns data of a single aid_station" do
  make_request
- parsed_response = JSON.parse(response.body)
+ parsed_response = response.parsed_body
  expect(parsed_response["data"]["id"].to_i).to eq(aid_station.id)
  expect(response.body).to be_jsonapi_response_for(type)
  end
  end
 
- context "if the aid_station does not exist" do
- let(:params) { {id: 0} }
+ context "when the aid_station does not exist" do
+ let(:params) { {id: 0} }
 
  it "returns an error" do
  make_request
- parsed_response = JSON.parse(response.body)
+ parsed_response = response.parsed_body
  expect(parsed_response["errors"]).to include(/not found/)
  expect(response.status).to eq(404)
  end
@@ -45,25 +45,25 @@
 
  via_login_and_jwt do
  context "when provided data is valid" do
- let(:params) { {data: {type: type, attributes: {split_id: split.id, event_id: event.id}}} }
+ let(:params) { {data: {type: type, attributes: {split_id: split.id, event_id: event.id } } } }
 
  it "returns a successful json response" do
  make_request
  expect(response.body).to be_jsonapi_response_for(type)
- parsed_response = JSON.parse(response.body)
+ parsed_response = response.parsed_body
  expect(parsed_response["data"]["id"]).not_to be_nil
  expect(response.status).to eq(201)
  end
 
  it "creates a aid_station record" do
- expect { make_request }.to change { AidStation.count }.by(1)
+ expect { make_request }.to change(AidStation, :count).by(1)
  end
  end
  end
  end
 
  describe "#destroy" do
- subject(:make_request) { delete :destroy, params: {id: aid_station_id} }
+ subject(:make_request) { delete :destroy, params: {id: aid_station_id} }
 
  via_login_and_jwt do
  context "when the record exists" do
@@ -76,7 +76,7 @@
  end
 
  it "destroys the aid_station record" do
- expect { make_request }.to change { AidStation.count }.by(-1)
+ expect { make_request }.to change(AidStation, :count).by(-1)
  end
  end
 
@@ -85,7 +85,7 @@
 
  it "returns an error if the aid_station does not exist" do
  make_request
- parsed_response = JSON.parse(response.body)
+ parsed_response = response.parsed_body
  expect(parsed_response["errors"]).to include(/not found/)
  expect(response.status).to eq(404)
  end

spec/controllers/api/v1/events_controller_spec.rb

@@ -3,7 +3,7 @@
 RSpec.describe Api::V1::EventsController do
  let(:type) { "events" }
  let(:event) { create(:event, course: course, event_group: event_group) }
- let(:course) { create(:course) }
+ let(:course) { create(:course, organization: event_group.organization) }
  let(:event_group) { create(:event_group) }
 
  let(:parsed_response) { response.parsed_body }

spec/controllers/api/v1/splits_controller_spec.rb

@@ -10,7 +10,7 @@
 
  via_login_and_jwt do
  context "when an existing split.id is provided" do
- let(:params) { {id: split} }
+ let(:params) { {id: split} }
 
  it "returns a successful 200 response" do
  make_request
@@ -19,18 +19,18 @@
 
  it "returns data of a single split" do
  make_request
- parsed_response = JSON.parse(response.body)
+ parsed_response = response.parsed_body
  expect(parsed_response["data"]["id"].to_i).to eq(split.id)
  expect(response.body).to be_jsonapi_response_for(type)
  end
  end
 
- context "if the split does not exist" do
- let(:params) { {id: 0} }
+ context "when the split does not exist" do
+ let(:params) { {id: 0} }
 
  it "returns an error" do
  make_request
- parsed_response = JSON.parse(response.body)
+ parsed_response = response.parsed_body
  expect(parsed_response["errors"]).to include(/not found/)
  expect(response.status).to eq(404)
  end
@@ -40,34 +40,36 @@
 
  describe "#create" do
  subject(:make_request) { post :create, params: params }
- let(:params) { {data: {type: "splits", attributes: attributes}} }
+
+ let(:params) { { data: { type: "splits", attributes: attributes } } }
 
  via_login_and_jwt do
  context "when provided data is valid" do
  let(:attributes) do
- {base_name: "Test Split", course_id: course.id, distance_from_start: 100,
- kind: "intermediate", sub_split_bitkey: 1}
+ {base_name: "Test Split", course_id: course.id, distance_from_start: 100,
+kind: "intermediate", sub_split_bitkey: 1}
  end
 
  it "returns a successful json response" do
  make_request
  expect(response.body).to be_jsonapi_response_for(type)
- parsed_response = JSON.parse(response.body)
+ parsed_response = response.parsed_body
  expect(parsed_response["data"]["id"]).not_to be_nil
  expect(response.status).to eq(201)
  end
 
  it "creates a split record" do
- expect { make_request }.to change { Split.count }.by(1)
+ expect { make_request }.to change(Split, :count).by(1)
  end
  end
  end
  end
 
  describe "#update" do
  subject(:make_request) { put :update, params: params }
- let(:params) { {id: split_id, data: {type: type, attributes: attributes}} }
- let(:attributes) { {base_name: "Updated Split Name", latitude: 40, longitude: -105, elevation: 2000} }
+
+ let(:params) { { id: split_id, data: { type: type, attributes: attributes } } }
+ let(:attributes) { { base_name: "Updated Split Name", latitude: 40, longitude: -105, elevation: 2000 } }
 
  via_login_and_jwt do
  context "when the split exists" do
@@ -93,7 +95,7 @@
 
  it "returns an error if the split does not exist" do
  make_request
- parsed_response = JSON.parse(response.body)
+ parsed_response = response.parsed_body
  expect(parsed_response["errors"]).to include(/not found/)
  expect(response.status).to eq(404)
  end
@@ -102,7 +104,7 @@
  end
 
  describe "#destroy" do
- subject(:make_request) { delete :destroy, params: {id: split_id} }
+ subject(:make_request) { delete :destroy, params: {id: split_id} }
 
  via_login_and_jwt do
  context "when the record exists" do
@@ -115,19 +117,19 @@
  end
 
  it "destroys the split record" do
- expect { make_request }.to change { Split.count }.by(-1)
+ expect { make_request }.to change(Split, :count).by(-1)
  end
  end
 
  context "when any split_times are associated with the split" do
  let(:split_id) { split.id }
 
  it "returns an error message" do
- event = create(:event, course: course)
+ event = create(:event, course: course, event_group: create(:event_group, organization: course.organization))
  effort = create(:effort, event: event)
  create(:split_time, split: split, effort: effort)
  make_request
- parsed_response = JSON.parse(response.body)
+ parsed_response = response.parsed_body
  expect(parsed_response["errors"].first["detail"]["messages"]).to include(/Split has 1 associated split times/)
  expect(response.status).to eq(422)
  end
@@ -138,7 +140,7 @@
 
  it "returns an error if the split does not exist" do
  make_request
- parsed_response = JSON.parse(response.body)
+ parsed_response = response.parsed_body
  expect(parsed_response["errors"]).to include(/not found/)
  expect(response.status).to eq(404)
  end

spec/factories/event.rb

@@ -2,8 +2,8 @@
  factory :event do
  scheduled_start_time { FFaker::Time.datetime }
  laps_required { 1 }
- course
  event_group
+ course { association :course, organization: event_group&.organization }
 
  trait :with_short_name do
  short_name { "#{rand(25..2000)}#{%w[-mile -kilo k M].sample}" }

spec/lib/etl/importer_spec.rb

@@ -23,7 +23,7 @@
  let(:source_data) { file_fixture("test_splits.csv") }
  let(:data_format) { :csv_splits }
  let(:options) { { parent: event, current_user_id: 1 } }
- let(:event) { create(:event, course: course) }
+ let(:event) { create(:event, course: course, event_group: create(:event_group, organization: course.organization)) }
  let(:course) { create(:course) }
 
  it "creates all new splits within the given course" do
@@ -40,7 +40,7 @@
  let(:source_data) { file_fixture("test_splits.csv") }
  let(:data_format) { :csv_splits }
  let(:options) { { parent: event, current_user_id: 1 } }
- let(:event) { create(:event, course: course) }
+ let(:event) { create(:event, course: course, event_group: create(:event_group, organization: course.organization)) }
  let(:course) { create(:course) }
  before do
  create(:split, :start, course: course)
@@ -61,7 +61,7 @@
  let(:source_data) { file_fixture("test_splits_minimal.csv") }
  let(:data_format) { :csv_splits }
  let(:options) { { parent: event, current_user_id: 1 } }
- let(:event) { create(:event, course: course) }
+ let(:event) { create(:event, course: course, event_group: create(:event_group, organization: course.organization)) }
  let(:course) { create(:course) }
  before do
  create(:split, :start, base_name: "Start", course: course)

spec/lib/etl/loaders/upsert_strategy_spec.rb

@@ -4,7 +4,7 @@
  subject { described_class.new(proto_records, options) }
 
  let(:course) { create(:course, id: 10) }
- let(:event) { create(:event, id: 1, course: course) }
+ let(:event) { create(:event, id: 1, course: course, event_group: create(:event_group, organization: course.organization)) }
  let(:options) { { parent: course, event: event, unique_key: [:course_id, :distance_from_start], current_user_id: 111 } }
  let(:valid_proto_records) do
  [ProtoRecord.new({ record_type: :split, kind: 0, sub_split_bitmap: 1, base_name: "Start", distance_from_start: 0, course_id: 10 }),

spec/models/aid_station_spec.rb

@@ -51,10 +51,10 @@
  let(:event_1) { create(:event, :with_short_name, course: course_1, event_group: event_group) }
  let(:event_2) { create(:event, :with_short_name, course: course_2, event_group: event_group) }
  let(:event_group) { create(:event_group, home_time_zone: "Arizona") }
- let(:course_1) { create(:course) }
+ let(:course_1) { create(:course, organization: event_group.organization) }
  let(:course_1_split_1) { create(:split, :start, course: course_1, base_name: "Start", latitude: 40, longitude: -105) }
  let(:course_1_split_2) { create(:split, :finish, course: course_1, base_name: "Finish", latitude: 42, longitude: -107) }
- let(:course_2) { create(:course) }
+ let(:course_2) { create(:course, organization: event_group.organization) }
  let(:course_2_split_1) { create(:split, :start, course: course_2, base_name: "Start", latitude: 40, longitude: -105) }
  before do
  event_1.splits << course_1_split_1

spec/models/event_spec.rb

@@ -126,6 +126,48 @@
  end
  end
 
+ describe "course organization consistency" do
+ it "is invalid when the course belongs to a different organization than the event group" do
+ event = build(:event, :with_short_name, course: courses(:d30_12m_course), event_group: event_groups(:hardrock_2016))
+ expect(event).not_to be_valid
+ expect(event.errors[:course_id]).to include("must belong to the same organization as the event group")
+ end
+
+ it "is valid when the course belongs to the event group's organization" do
+ event = build(:event, :with_short_name, course: courses(:hardrock_ccw), event_group: event_groups(:hardrock_2016))
+ expect(event).to be_valid
+ end
+
+ it "gives factory-built events a course in the event group's organization" do
+ event = build(:event)
+ expect(event.course.organization).to eq(event.event_group.organization)
+ end
+
+ context "when changing a persisted event's course to another organization's course" do
+ it "is invalid and does not persist the change" do
+ event = events(:hardrock_2016)
+ original_course = event.course
+
+ response = event.update(course_id: courses(:d30_12m_course).id)
+
+ expect(response).to eq(false)
+ expect(event.errors[:course_id]).to include("must belong to the same organization as the event group")
+ expect(event.reload.course).to eq(original_course)
+ end
+ end
+ end
+
+ describe "clearing the course on a persisted event" do
+ it "is invalid rather than raising from the course-change conforming" do
+ event = events(:hardrock_2016)
+ event.course_id = nil
+
+ expect { event.valid? }.not_to raise_error
+ expect(event).not_to be_valid
+ expect(event.errors[:course]).to include("must exist")
+ end
+ end
+
  describe "methods that produce lap_splits and time_points" do
  let(:event) { build_stubbed(:event, laps_required: laps_required) }
  let(:laps_required) { 2 }

spec/models/split_spec.rb

@@ -189,10 +189,10 @@
  let(:event_1) { create(:event, :with_short_name, course: course_1, event_group: event_group) }
  let(:event_2) { create(:event, :with_short_name, course: course_2, event_group: event_group) }
  let(:event_group) { create(:event_group) }
- let(:course_1) { create(:course) }
+ let(:course_1) { create(:course, organization: event_group.organization) }
  let(:course_1_split_1) { create(:split, :start, course: course_1, base_name: "Start", latitude: 40, longitude: -105) }
  let(:course_1_split_2) { create(:split, :finish, course: course_1, base_name: "Finish", latitude: 42, longitude: -107) }
- let(:course_2) { create(:course) }
+ let(:course_2) { create(:course, organization: event_group.organization) }
  let(:course_2_split_1) { create(:split, :start, course: course_2, base_name: "Start", latitude: 40, longitude: -105) }
  let(:course_2_split_2) { create(:split, :finish, course: course_2, base_name: "Finish", latitude: 42, longitude: -107) }
  before do