chore: remove empty code changes in locatable_location and one modules

Author: rsmithlal

app/models/better_together/geography/locatable_location.rb

@@ -9,8 +9,13 @@ class LocatableLocation < ApplicationRecord
       belongs_to :locatable, polymorphic: true
       belongs_to :location, polymorphic: true, optional: true
 
-      validates :name, presence: true, if: :simple_location?
-      validate :at_least_one_location_source
+      # If a persisted nested location is submitted with all empty fields, mark it
+      # for destruction so accepts_nested_attributes_for with allow_destroy will remove it
+      before_validation :mark_for_destruction_if_empty
+
+      # Validate name only for simple locations and not when marked for destruction
+      validates :name, presence: true, if: -> { simple_location? && !marked_for_destruction? }
+      validate :at_least_one_location_source, unless: :marked_for_destruction?
 
       def self.permitted_attributes(id: false, destroy: false)
         super + %i[
@@ -129,7 +134,19 @@ def self.available_buildings_for(context = nil) # rubocop:todo Metrics/MethodLen
 
       private
 
+      def mark_for_destruction_if_empty
+        name_blank = name.blank?
+        location_blank = location.blank?
+
+        # If both the simple name and structured location are blank, mark for destruction
+        # for both new and persisted records so validations won't block form submission.
+        mark_for_destruction if name_blank && location_blank
+      end
+
       def at_least_one_location_source
+        # If this record is scheduled for destruction or otherwise empty, don't add errors here
+        return if marked_for_destruction?
+
         sources = [name.present?, location.present?]
         return if sources.any?
 

app/models/concerns/better_together/geography/locatable/one.rb

@@ -12,8 +12,18 @@ module One
                   class_name: 'BetterTogether::Geography::LocatableLocation',
                   as: :locatable
 
+          # Reject nested location attributes when all relevant fields are blank so
+          # we don't build an invalid empty LocatableLocation during create/update.
+          # Only reject when this is a new nested record (no id) and all meaningful
+          # fields are blank. Persisted records with blank fields should be allowed
+          # through so they can be marked for destruction by the model callback.
           accepts_nested_attributes_for :location,
-                                        allow_destroy: true, reject_if: :blank?
+                                        allow_destroy: true,
+                                        reject_if: ->(attrs) {
+                                          attrs.blank? || (
+                                            attrs['id'].blank? && attrs['name'].blank? && attrs['location_id'].blank? && attrs['location_type'].blank?
+                                          )
+                                        }
         end
 
         class_methods do

spec/requests/better_together/events_controller_spec.rb

@@ -128,4 +128,113 @@
       end
     end
   end
+
+  describe 'creating events with different location types' do
+    let(:locale) { I18n.default_locale }
+
+    context 'as platform manager', :as_platform_manager do
+      it 'creates an event with a simple (name) location' do
+        params = {
+          event: {
+            name: 'Simple Location Event',
+            starts_at: 1.day.from_now.iso8601,
+            identifier: SecureRandom.uuid,
+            privacy: 'public',
+            location_attributes: {
+              name: 'Community Hall'
+            }
+          },
+          locale: locale
+        }
+
+        post better_together.events_path(locale: locale), params: params
+
+        expect(response).to have_http_status(:found)
+        event = BetterTogether::Event.order(:created_at).last
+        expect(event).to be_present
+        expect(event.location).to be_present
+        expect(event.location.name).to eq('Community Hall')
+        expect(event.location.location).to be_nil
+      end
+
+      it 'creates an event with an Address location' do
+        address = create(:better_together_address, privacy: 'public')
+
+        params = {
+          event: {
+            name: 'Address Location Event',
+            starts_at: 1.day.from_now.iso8601,
+            identifier: SecureRandom.uuid,
+            privacy: 'public',
+            location_attributes: {
+              location_id: address.id,
+              location_type: 'BetterTogether::Address'
+            }
+          },
+          locale: locale
+        }
+
+        post better_together.events_path(locale: locale), params: params
+
+        expect(response).to have_http_status(:found)
+        event = BetterTogether::Event.order(:created_at).last
+        expect(event).to be_present
+        expect(event.location).to be_present
+        expect(event.location.location_type).to eq('BetterTogether::Address')
+        expect(event.location.address).to be_present
+        expect(event.location.address.id).to eq(address.id)
+      end
+
+      it 'creates an event with a Building location' do
+        manager_user = BetterTogether::User.find_by(email: '[email protected]') ||
+                       create(:better_together_user, :confirmed, :platform_manager, email: '[email protected]')
+        building = create(:better_together_infrastructure_building, creator: manager_user.person, privacy: 'private')
+
+        params = {
+          event: {
+            name: 'Building Location Event',
+            starts_at: 1.day.from_now.iso8601,
+            identifier: SecureRandom.uuid,
+            privacy: 'public',
+            location_attributes: {
+              location_id: building.id,
+              location_type: 'BetterTogether::Infrastructure::Building'
+            }
+          },
+          locale: locale
+        }
+
+        post better_together.events_path(locale: locale), params: params
+
+        expect(response).to have_http_status(:found)
+        event = BetterTogether::Event.order(:created_at).last
+        expect(event).to be_present
+        expect(event.location).to be_present
+        expect(event.location.location_type).to eq('BetterTogether::Infrastructure::Building')
+        expect(event.location.building).to be_present
+        expect(event.location.building.id).to eq(building.id)
+      end
+
+      it 'creates a draft event with no location assigned' do
+        params = {
+          event: {
+            name: 'Draft Event Without Location',
+            identifier: SecureRandom.uuid,
+            privacy: 'public'
+            # intentionally omit starts_at to keep it a draft and omit location_attributes
+          },
+          locale: locale
+        }
+
+        post better_together.events_path(locale: locale), params: params
+
+        expect(response).to have_http_status(:found)
+        event = BetterTogether::Event.order(:created_at).last
+        expect(event).to be_present
+        expect(event.starts_at).to be_nil
+        expect(event).to be_draft
+        expect(event.location).to be_nil
+      end
+    end
+  end
 end