diff --git a/app/models/competition.rb b/app/models/competition.rb index ec5b021a14d..63e561368d0 100644 --- a/app/models/competition.rb +++ b/app/models/competition.rb @@ -643,9 +643,9 @@ def user_can_pre_register?(user) delegates.include?(user) || trainee_delegates.include?(user) || organizers.include?(user) end - attr_accessor :being_cloned_from_id + attr_accessor :being_cloned_from_id, :being_cloned_from_cache def being_cloned_from - Competition.find_by(id: being_cloned_from_id) + @being_cloned_from_cache ||= Competition.find_by(id: being_cloned_from_id) end def build_clone @@ -709,7 +709,7 @@ def build_clone # Clone competition tabs. if clone_tabs being_cloned_from&.tabs&.each do |tab| - tabs.create(tab.attributes.slice(*CompetitionTab::CLONEABLE_ATTRIBUTES)) + tabs.create!(tab.attributes.slice(*CompetitionTab::CLONEABLE_ATTRIBUTES)) end end end @@ -2449,7 +2449,13 @@ def to_form_data # It is quite uncool that we have to duplicate the internal form_data formatting like this # but as long as we let our backend handle the complete error validation we literally have no other choice def form_errors - return {} if self.valid? + self_valid = self.valid? + # If we're cloning, we also need to check the parent's associations. + # Otherwise, the user may be surprised by a silent fail if some tabs/venues/schedules + # of the parent are invalid. (This can happen if we introduce new validations on old data) + self_valid &= being_cloned_from&.tabs&.all?(&:valid?) if being_cloned_from_id.present? + + return {} if self_valid { # for historic reasons, we keep 'name' errors listed under ID. Don't ask. @@ -2542,7 +2548,7 @@ def form_errors }, "cloning" => { "fromId" => errors[:being_cloned_from_id], - "cloneTabs" => errors[:clone_tabs], + "cloneTabs" => being_cloned_from_id.present? ? being_cloned_from&.association_errors(:tabs) : errors[:clone_tabs], }, "other" => { "competitionEvents" => errors[:competition_events], @@ -2550,6 +2556,10 @@ def form_errors } end + def association_errors(association_name) + self.public_send(association_name).map(&:errors).flat_map(&:to_a) + end + def set_form_data(form_data, current_user) JSON::Validator.validate!(Competition.form_data_json_schema, form_data) diff --git a/app/models/competition_tab.rb b/app/models/competition_tab.rb index ea9a8d77647..4d97a728d2f 100644 --- a/app/models/competition_tab.rb +++ b/app/models/competition_tab.rb @@ -34,6 +34,16 @@ def slug competition.tabs.where("display_order > ?", display_order).update_all("display_order = display_order - 1") end + validate :verify_if_full_urls + private def verify_if_full_urls + content.scan(/\[(.*?)\]\((.*?)\)/).any? do |match| + url = match[1] + unless url.starts_with?('http://', 'https://') + errors.add(:content, I18n.t('competitions.errors.not_full_url', url: url)) + end + end + end + def reorder(direction) current_display_order = display_order other_display_order = display_order + (direction.to_s == "up" ? -1 : 1) diff --git a/config/locales/en.yml b/config/locales/en.yml index 05c850e70fa..e811690d3a7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1963,6 +1963,7 @@ en: series_distance_km: "The competition is located too far away from %{competition}." series_distance_days: "The competition is scheduled too many days away from %{competition}." must_ask_about_guests_if_specifying_limit: "Must ask about guests if a guest limit is specified." + not_full_url: "You have a relative URL (%{url}) in the content. Please enter full URLs with prefix http:// or https://." new: create_competition: "Create competition" note_clone: "To clone an existing competition visit the information page for that competition and click the \"Clone\" link in the menu." diff --git a/spec/models/competition_tab_spec.rb b/spec/models/competition_tab_spec.rb index 2f0b72c14ec..0b346eba7f9 100644 --- a/spec/models/competition_tab_spec.rb +++ b/spec/models/competition_tab_spec.rb @@ -74,4 +74,18 @@ expect(competition.tabs.to_a).to eq [tab1, tab2, tab3] end end + + context "#verify_if_full_urls" do + let(:competition_tab) { FactoryBot.build(:competition_tab) } + + it "doesn't allow relative URLs" do + competition_tab.update(content: "[Link](/relative)") + expect(competition_tab).to be_invalid + end + + it "allows full URLs" do + competition_tab.update(content: "[Link](http://full)") + expect(competition_tab).to be_valid + end + end end