refactor: update notification marking methods to use record IDs for improved clarity and consistency

Author: rsmithlal

app/controllers/better_together/joatu/agreements_controller.rb

@@ -56,7 +56,7 @@ def respond_to_create_failure(format) # rubocop:todo Metrics/MethodLength
 
       # GET /joatu/agreements/:id
       def show
-        mark_notifications_read_for_record(@joatu_agreement)
+        mark_notifications_read_for_record_id(@joatu_agreement.id)
         super
       end
 

app/controllers/better_together/notifications_controller.rb

@@ -45,7 +45,7 @@ def mark_notification_as_read(id)
     end
 
     def mark_record_notification_as_read(id)
-      mark_notifications_read_for_record(Struct.new(id: id), recipient: helpers.current_person)
+      mark_notifications_read_for_record_id(id)
     end
   end
 end

app/controllers/concerns/better_together/notification_readable.rb

@@ -5,12 +5,12 @@ module BetterTogether
   module NotificationReadable
     extend ActiveSupport::Concern
 
-    # Marks notifications (for the current person) as read for events bound to a record
+    # Marks notifications (for the current person) as read for events bound to a record ID
     # via Noticed::Event#record_id (generic helper used across features).
-    def mark_notifications_read_for_record(record, recipient: helpers.current_person)
-      return unless recipient && record.respond_to?(:id)
+    def mark_notifications_read_for_record_id(record_id, recipient: helpers.current_person)
+      return unless recipient && record_id.present?
 
-      event_ids = Noticed::Event.where(record_id: record.id).select(:id)
+      event_ids = Noticed::Event.where(record_id: record_id).select(:id)
 
       Noticed::Notification
         .where(recipient:)

app/models/better_together/geography/locatable_location.rb

@@ -197,11 +197,15 @@ def self.available_buildings_for(context = nil) # rubocop:todo Metrics/MethodLen
       private
 
       def mark_for_destruction_if_empty
+        # Only mark for destruction if this is a persisted nested record that becomes empty
+        # Don't auto-mark new records for destruction as they should validate normally
+        return unless persisted?
+
         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.
+        # for persisted records so accepts_nested_attributes_for with allow_destroy will remove them
         mark_for_destruction if name_blank && location_blank
       end