Allow meeting addresses to be unset/TBD (decidim#14389)

antopalidi · alecslupu · microstudi · commit 4fb895913da8 · 2025-07-31T10:21:31.000+02:00
* update validations, add pending_location_text

* add tests

* update notifications

* add tests

* refactor: move pending_location? logic to model

* Apply suggestions from code review

Co-authored-by: Alexandru Emil Lupu &lt;contact@alecslupu.ro&gt;

* fix test

* fix validations

* change validations

* fix validations

* change tests

---------

Co-authored-by: Alexandru Emil Lupu &lt;contact@alecslupu.ro&gt;
diff --git a/decidim-core/app/cells/decidim/address/show.erb b/decidim-core/app/cells/decidim/address/show.erb
@@ -24,6 +24,7 @@
             <%= start_and_end_time %>
           </div>
         </div>
+      </div>
     </li>
   <% end %>
 </ul>
diff --git a/decidim-core/app/cells/decidim/address_cell.rb b/decidim-core/app/cells/decidim/address_cell.rb
@@ -24,11 +24,17 @@ def location_hints
     end
 
     def location
+      return pending_location_text if model.respond_to?(:pending_location?) && model.pending_location?
+
       decidim_sanitize_translated(model.location)
     end
 
     def address
-      decidim_sanitize_translated(model.address)
+      decidim_sanitize_translated(model.address) if model.respond_to?(:address) && model.address.present?
+    end
+
+    def pending_location_text
+      t("show.pending_address", scope: "decidim.meetings.meetings")
     end
 
     def display_start_and_end_time?
diff --git a/decidim-core/spec/cells/decidim/address_cell_spec.rb b/decidim-core/spec/cells/decidim/address_cell_spec.rb
@@ -37,6 +37,19 @@
     end
   end
 
+  context "when address is pending" do
+    let(:location) { { "ca" => "", "en" => "", "es" => "", "machine_translations" => { "es" => "Location" } } }
+
+    before do
+      allow(model).to receive(:location).and_return location
+      allow(model).to receive(:pending_location?).and_return(true)
+    end
+
+    it "renders pending address text" do
+      expect(subject.find(".address__location")).to have_content(I18n.t("show.pending_address", scope: "decidim.meetings.meetings"))
+    end
+  end
+
   context "with an online meeting url" do
     let(:my_cell) { cell("decidim/address", model, online: true) }
     let(:model) { create(:dummy_resource) }
diff --git a/decidim-meetings/app/cells/decidim/meetings/dates_and_map_cell.rb b/decidim-meetings/app/cells/decidim/meetings/dates_and_map_cell.rb
@@ -32,7 +32,7 @@ def same_day?
       end
 
       def display_map?
-        maps_enabled? && !online?
+        maps_enabled? && !online? && model.address.present?
       end
     end
   end
diff --git a/decidim-meetings/app/commands/decidim/meetings/admin/update_meeting.rb b/decidim-meetings/app/commands/decidim/meetings/admin/update_meeting.rb
@@ -45,7 +45,8 @@ def send_notification
             event: "decidim.events.meetings.meeting_updated",
             event_class: Decidim::Meetings::UpdateMeetingEvent,
             resource:,
-            followers: resource.followers
+            followers: resource.followers,
+            extra: { changed_fields: resource.previous_changes.keys & important_attributes }
           )
         end
 
@@ -54,13 +55,17 @@ def should_notify_followers?
         end
 
         def important_attributes
-          %w(start_time end_time address)
+          %w(start_time end_time address location)
         end
 
         def start_time_changed?
           resource.previous_changes["start_time"].present?
         end
 
+        def address_changed?
+          resource.previous_changes["address"].present? || resource.previous_changes["location"].present?
+        end
+
         def schedule_upcoming_meeting_notification
           return if resource.start_time < Time.zone.now
           return unless resource.reminder_enabled
diff --git a/decidim-meetings/app/events/decidim/meetings/update_meeting_event.rb b/decidim-meetings/app/events/decidim/meetings/update_meeting_event.rb
@@ -4,6 +4,31 @@ module Decidim
   module Meetings
     class UpdateMeetingEvent < Decidim::Events::SimpleEvent
       include Decidim::Meetings::MeetingEvent
+
+      i18n_attributes :changed_fields
+
+      def notification_title
+        I18n.t(
+          "notification_title",
+          scope: i18n_scope,
+          changed_fields: changed_fields,
+          resource_title: translated_attribute(resource.title),
+          resource_path: resource_path
+        ).html_safe
+      end
+
+      private
+
+      def changed_field_keys
+        extra[:changed_fields] || []
+      end
+
+      def changed_fields
+        keys = changed_field_keys
+        return "" if keys.empty?
+
+        keys.map { |key| I18n.t("field_names.#{key}", scope: i18n_scope) }.to_sentence
+      end
     end
   end
 end
diff --git a/decidim-meetings/app/forms/decidim/meetings/admin/meeting_form.rb b/decidim-meetings/app/forms/decidim/meetings/admin/meeting_form.rb
@@ -36,7 +36,9 @@ class MeetingForm < ::Decidim::Meetings::BaseMeetingForm
         validates :registration_type, presence: true
         validates :registration_url, presence: true, url: true, if: ->(form) { form.on_different_platform? }
         validates :type_of_meeting, presence: true
-        validates :location, translatable_presence: true, if: ->(form) { form.in_person_meeting? || form.hybrid_meeting? }
+        validates :address, presence: true, if: ->(form) { form.needs_address? && form.location.values.any?(&:present?) && form.address.blank? }
+        validates :location, translatable_presence: true, if: ->(form) { form.needs_address? && form.address.present? }
+        validates :address, geocoding: true, if: ->(form) { form.has_address? && !form.geocoded? && form.needs_address? }
         validates :online_meeting_url, url: true, if: ->(form) { form.online_meeting? || form.hybrid_meeting? }
         validates :comments_start_time, date: { before: :comments_end_time, allow_blank: true, if: proc { |obj| obj.comments_end_time.present? } }
         validates :comments_end_time, date: { after: :comments_start_time, allow_blank: true, if: proc { |obj| obj.comments_start_time.present? } }
diff --git a/decidim-meetings/app/forms/decidim/meetings/base_meeting_form.rb b/decidim-meetings/app/forms/decidim/meetings/base_meeting_form.rb
@@ -13,8 +13,6 @@ class BaseMeetingForm < Decidim::Form
 
       validates :current_component, presence: true
 
-      validates :address, presence: true, if: ->(form) { form.needs_address? }
-      validates :address, geocoding: true, if: ->(form) { form.has_address? && !form.geocoded? && form.needs_address? }
       validates :start_time, presence: true, date: { before: :end_time }
       validates :end_time, presence: true, date: { after: :start_time }
 
diff --git a/decidim-meetings/app/forms/decidim/meetings/meeting_form.rb b/decidim-meetings/app/forms/decidim/meetings/meeting_form.rb
@@ -20,6 +20,8 @@ class MeetingForm < ::Decidim::Meetings::BaseMeetingForm
       attribute :iframe_embed_type, String, default: "none"
       attribute :iframe_access_level, String
 
+      validates :address, presence: true, if: ->(form) { form.needs_address? }
+      validates :address, geocoding: true, if: ->(form) { form.has_address? && !form.geocoded? && form.needs_address? }
       validates :iframe_embed_type, inclusion: { in: Decidim::Meetings::Meeting.participants_iframe_embed_types }
       validates :title, presence: true
       validates :description, presence: true
diff --git a/decidim-meetings/app/models/decidim/meetings/meeting.rb b/decidim-meetings/app/models/decidim/meetings/meeting.rb
@@ -213,6 +213,10 @@ def has_registration_for?(user)
         registrations.where(user:).any?
       end
 
+      def pending_location?
+        !online? && location.except("machine_translations").values.all?(&:blank?)
+      end
+
       def maps_enabled?
         component.settings.maps_enabled?
       end
diff --git a/decidim-meetings/config/locales/en.yml b/decidim-meetings/config/locales/en.yml
@@ -582,6 +582,8 @@ en:
           micro_camera_permissions_warning: When you click on the button below, you will be asked for microphone and/or camera permissions, and you will join the videoconference
           no_slots_available: No slots available
           organizations: Attending organizations
+          pending_address: The location will be announced soon.
+          redirect_notice: This meeting is part of another space, so you have been moved to %{current_space_name}. <br>If you prefer, you can go back to <a href="%{previous_space_url}">%{previous_space_name}</a>.
           registration_code_help_text: Your registration code
           registration_state:
             validated: VALIDATED
diff --git a/decidim-meetings/spec/commands/admin/update_meeting_spec.rb b/decidim-meetings/spec/commands/admin/update_meeting_spec.rb
@@ -224,7 +224,8 @@ module Decidim::Meetings
                 event: "decidim.events.meetings.meeting_updated",
                 event_class: UpdateMeetingEvent,
                 resource: meeting,
-                followers: [user]
+                followers: [user],
+                extra: { changed_fields: %w(start_time) }
               )
 
             subject.call
@@ -246,7 +247,8 @@ module Decidim::Meetings
                 event: "decidim.events.meetings.meeting_updated",
                 event_class: UpdateMeetingEvent,
                 resource: meeting,
-                followers: [user]
+                followers: [user],
+                extra: { changed_fields: %w(end_time) }
               )
 
             subject.call
@@ -263,7 +265,8 @@ module Decidim::Meetings
                 event: "decidim.events.meetings.meeting_updated",
                 event_class: UpdateMeetingEvent,
                 resource: meeting,
-                followers: [user]
+                followers: [user],
+                extra: { changed_fields: %w(address) }
               )
 
             subject.call
diff --git a/decidim-meetings/spec/events/decidim/meetings/update_meeting_event_spec.rb b/decidim-meetings/spec/events/decidim/meetings/update_meeting_event_spec.rb
@@ -17,6 +17,38 @@
     end
   end
 
+  describe "notification_title" do
+    context "with one changed field" do
+      let(:extra) { { changed_fields: ["address"] } }
+
+      it "includes the changed field" do
+        expect(subject.notification_title).to include("the address")
+        expect(subject.notification_title).to include(resource_title)
+        expect(subject.notification_title).to include("has been updated with changes to")
+      end
+    end
+
+    context "with multiple changed fields" do
+      let(:extra) { { changed_fields: %w(start_time address location) } }
+
+      it "lists all changed fields with proper grammar" do
+        expect(subject.notification_title).to include("the start time, the address, and the location")
+      end
+    end
+
+    context "with no changed fields" do
+      let(:extra) { { changed_fields: [] } }
+
+      it "returns a safe fallback title when no fields are changed" do
+        expect(subject.notification_title).to be_a(String)
+        expect(subject.notification_title).not_to include("translation missing")
+        expect(subject.notification_title).not_to include("address")
+        expect(subject.notification_title).not_to include("location")
+        expect(subject.notification_title).not_to include("time")
+      end
+    end
+  end
+
   describe "resource_text" do
     it "returns the meeting description" do
       expect(subject.resource_text).to eq translated(resource.description)
diff --git a/decidim-meetings/spec/forms/admin/meeting_form_spec.rb b/decidim-meetings/spec/forms/admin/meeting_form_spec.rb
@@ -107,17 +107,31 @@ module Decidim::Meetings
       it { is_expected.not_to be_valid }
     end
 
-    describe "when location is missing and type of meeting is in_person" do
+    describe "address and location" do
       let(:type_of_meeting) { "in_person" }
-      let(:location) { { en: nil } }
 
-      it { is_expected.not_to be_valid }
-    end
+      context "when both location and address are blank" do
+        let(:address) { nil }
+        let(:location) { { "en" => "" } }
+
+        it { is_expected.to be_valid }
+      end
 
-    describe "when address is missing" do
-      let(:address) { nil }
+      context "when both location and address are present" do
+        it { is_expected.to be_valid }
+      end
 
-      it { is_expected.not_to be_valid }
+      context "when location is present but address is blank" do
+        let(:address) { nil }
+
+        it { is_expected.not_to be_valid }
+      end
+
+      context "when address is present but location is blank" do
+        let(:location) { { "en" => "" } }
+
+        it { is_expected.not_to be_valid }
+      end
     end
 
     describe "when start_time is missing" do
diff --git a/decidim-meetings/spec/system/admin/admin_manages_meetings_spec.rb b/decidim-meetings/spec/system/admin/admin_manages_meetings_spec.rb
@@ -5,17 +5,18 @@
 
 describe "Admin manages meetings", serves_geocoding_autocomplete: true, serves_map: true do
   let(:manifest_name) { "meetings" }
-  let!(:meeting) { create(:meeting, :published, scope:, services: [], component: current_component) }
+  let!(:meeting) { create(:meeting, :published, services: [], component: current_component, start_time: base_date + 1.day, end_time: base_date + 26.hours) }
   let(:address) { "Some address" }
   let(:latitude) { 40.1234 }
   let(:longitude) { 2.1234 }
   let(:service_titles) { ["This is the first service", "This is the second service"] }
-  let(:base_date) { Time.new.utc }
+  let(:base_date) { Time.zone.now.change(usec: 0) }
   let(:meeting_start_date) { base_date.strftime("%d/%m/%Y") }
   let(:meeting_start_time) { base_date.utc.strftime("%H:%M") }
   let(:meeting_end_date) { ((base_date + 2.days) + 1.month).strftime("%d/%m/%Y") }
   let(:meeting_end_time) { (base_date + 4.hours).strftime("%H:%M") }
-  let(:attributes) { attributes_for(:meeting, component: current_component) }
+  let(:attributes) { attributes_for(:meeting, component: current_component, skip_injection: true) }
+  let!(:follow) { create(:follow, followable: meeting, user:) }
 
   include_context "when managing a component as an admin" do
     let(:participatory_process) { create(:participatory_process, :published, :with_steps, organization:) }
@@ -176,7 +177,9 @@
 
       fill_in_geocoding :meeting_address, with: address
 
-      find("*[type=submit]").click
+      perform_enqueued_jobs do
+        find("*[type=submit]").click
+      end
     end
 
     expect(page).to have_admin_callout("successfully")
@@ -185,8 +188,15 @@
       expect(page).to have_content(translated(attributes[:title]))
     end
 
+    email = last_email
+    sleep 1
+    expect(email.subject).to include("updated")
+    expect(email.body.encoded).to include(%(The "#{decidim_sanitize_translated(attributes[:title])}" meeting has been updated with changes to the address and the location))
+    page.visit decidim.notifications_path
+    expect(page).to have_content("The #{decidim_sanitize_translated(attributes[:title])} meeting has been updated with changes to the address and the location")
+
     visit decidim_admin.root_path
-    expect(page).to have_content("updated the #{translated(attributes[:title])} meeting on the")
+    expect(page).to have_content("updated the #{decidim_sanitize_translated(attributes[:title])} meeting on the")
   end
 
   it "sets registration enabled to true when registration type is on this platform" do
@@ -330,6 +340,32 @@
     visit decidim_admin.root_path
     expect(page).to have_content("created the #{translated(attributes[:title])} meeting on the")
   end
+  
+  context "when the venue has not been decided yet" do
+    it "creates a new meeting without an address" do
+      click_on "New meeting"
+
+      fill_in_i18n(:meeting_title, "#meeting-title-tabs", **attributes[:title].except("machine_translations"))
+      fill_in_i18n_editor(:meeting_description, "#meeting-description-tabs", **attributes[:description].except("machine_translations"))
+      select "In person", from: :meeting_type_of_meeting
+      select "Registration disabled", from: :meeting_registration_type
+      fill_in_datepicker :meeting_start_time_date, with: meeting_start_date
+      fill_in_timepicker :meeting_start_time_time, with: meeting_start_time
+      fill_in_datepicker :meeting_end_time_date, with: meeting_end_date
+      fill_in_timepicker :meeting_end_time_time, with: meeting_end_time
+
+      within ".new_meeting" do
+        find("*[type=submit]").click
+      end
+
+      expect(page).to have_admin_callout("successfully")
+
+      new_meeting = Decidim::Meetings::Meeting.last
+      puts "Meeting location: #{new_meeting.location}"
+      expect(new_meeting.location.values).to all(be_blank)
+      expect(new_meeting.address).to be_empty
+    end
+  end
 
   context "when using the front-end geocoder", :serves_geocoding_autocomplete do
     it_behaves_like(
diff --git a/decidim-meetings/spec/system/meeting_spec.rb b/decidim-meetings/spec/system/meeting_spec.rb
