Refactor: Event forms use form objects

I'd like to decouple the form more from the model in order to provide a
better UX with form fields which don't exist on the model.

A first step is getting the existing functionality working with form
objects.

Note that on Edit we print the dates rather than returning the date
array from the model.

Get rid of usage of date_array - this hasn't actually been what we use
for making the listings for a while now, but it's still been used in
parallel - I guess because I wasn't sure about the SwingDates approach
(which I still think is kind of batshit crazy and a bad idea, but it
does mostly work)

Building the URL separately based on @event is a tradeoff: without this
we need to somehow get the event ID into the form so that `form_with`
can do its thing and infer which URL to use, but I couldn't find an
approach for that which works well:

  - adding an extra initialisation parameter breaks the validation specs
    which are expecting to be able to `build` an activemodel instance
    with FactoryBot
  - we could add ID as an attribute and set it in the validation error
    case on update, but that feels awkward.

Note that we need to monkeypatch Shoulda Matchers to run it without
ActiveRecord. This is NOT a "responsible" way of monkeypatching, but I
couldn't get it to work and all the examples I can find online are about
monkeypatching instance methods not class methods. Raised an issue on
the Shoulda Matchers repo here:

  thoughtbot/shoulda-matchers#1553

Gemfile.lock

@@ -345,7 +345,7 @@ GEM
       rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2)
     sexp_processor (4.16.1)
-    shoulda-matchers (5.1.0)
+    shoulda-matchers (5.3.0)
       activesupport (>= 5.2.0)
     simplecov (0.21.2)
       docile (~> 1.1)

app/controllers/events_controller.rb

@@ -13,20 +13,21 @@ def show
   end
 
   def new
-    venue = Venue.find_by(id: params[:venue_id])
-    @event = Event.new(venue:)
+    @form = CreateEventForm.new(venue_id: params[:venue_id])
   end
 
   def edit
     @event = Event.find(params[:id])
+    @form = EditEventForm.from_event(@event)
   end
 
   def create
-    @event = Event.new(event_params)
+    @form = CreateEventForm.new(event_params)
 
-    if @event.save
+    if @form.valid?
+      event = Event.create!(@form.to_h)
       flash[:notice] = t("flash.success", model: "Event", action: "created")
-      redirect_to(@event)
+      redirect_to(event)
     else
       render action: "new"
     end
@@ -35,10 +36,11 @@ def create
   def update
     @event = Event.find(params[:id])
 
-    audit_comment = EventParamsCommenter.new.comment(@event, event_params)
-    update_params = event_params.merge!(audit_comment)
-
-    if @event.update(update_params)
+    @form = EditEventForm.new(event_params)
+    if @form.valid?
+      audit_comment = EventParamsCommenter.new.comment(@event, event_params)
+      update_params = @form.to_h.merge!(audit_comment)
+      @event.update!(update_params)
       flash[:notice] = t("flash.success", model: "Event", action: "updated")
       redirect_to(@event)
     else
@@ -78,8 +80,8 @@ def event_params
         course_length
         day
         frequency
-        date_array
-        cancellation_array
+        dates
+        cancellations
         first_date
         last_date
         url

app/controllers/external_events_controller.rb

@@ -5,15 +5,17 @@ class ExternalEventsController < CmsBaseController
 
   def edit
     @event = Event.find_by!(organiser_token: params[:id])
+    @form = OrganiserEditEventForm.from_event(@event)
   end
 
   def update
     @event = Event.find_by!(organiser_token: params[:id])
 
-    audit_comment = EventParamsCommenter.new.comment(@event, event_params)
-    update_params = event_params.merge!(audit_comment)
-
-    if @event.update(update_params)
+    @form = OrganiserEditEventForm.new(event_params)
+    if @form.valid?
+      audit_comment = EventParamsCommenter.new.comment(@event, event_params)
+      update_params = @form.to_h.merge!(audit_comment)
+      @event.update!(update_params)
       flash[:success] = t("flash.success", model: "Event", action: "updated")
       redirect_to edit_external_event_path(@event.organiser_token)
     else
@@ -36,8 +38,8 @@ def event_params
     params.require(:event).permit(
       %i[
         venue_id
-        date_array
-        cancellation_array
+        dates
+        cancellations
         last_date
       ]
     )

app/forms/create_event_form.rb

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+class CreateEventForm
+  include ActiveModel::Model
+  include ActiveModel::Attributes
+
+  attribute :title, :string
+  attribute :url, :string
+  attribute :venue_id, :integer
+  attribute :event_type, :string
+
+  attribute :has_social, :boolean
+  attribute :social_organiser_id, :integer
+
+  attribute :has_class, :boolean
+  attribute :has_taster, :boolean
+  attribute :class_style, :string
+  attribute :course_length, :string
+  attribute :class_organiser_id, :integer
+
+  attribute :frequency, :integer
+  attribute :day, :string
+  attribute :dates, :string
+  attribute :cancellations, :string
+  attribute :first_date, :string
+  attribute :last_date, :string
+
+  def self.model_name
+    Event.model_name
+  end
+
+  validates :url, presence: true, uri: true
+  validates :venue_id, presence: true
+  validates :event_type, presence: true
+  validates :frequency, presence: true
+  validates :course_length, numericality: { only_integer: true, greater_than: 0, allow_blank: true }
+
+  validates_with ValidSocialOrClass
+  validates_with ValidWeeklyEvent
+
+  def action
+    "Create"
+  end
+
+  def weekly?
+    frequency == 1
+  end
+
+  def infrequent?
+    return false if frequency.nil?
+
+    frequency.zero? || frequency >= 4
+  end
+
+  def has_social? # rubocop:disable Naming/PredicateName
+    !!has_social
+  end
+
+  def has_class? # rubocop:disable Naming/PredicateName
+    !!has_class
+  end
+
+  def to_h
+    attributes.merge(
+      "dates" => DatesStringParser.new.parse(dates),
+      "cancellations" => DatesStringParser.new.parse(cancellations)
+    )
+  end
+end

app/forms/edit_event_form.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+class EditEventForm < CreateEventForm
+  include ActiveModel::Model
+  include ActiveModel::Attributes
+
+  class << self
+    def from_event(event)
+      new(
+        {
+          title: event.title,
+          url: event.url,
+          venue_id: event.venue_id,
+          event_type: event.event_type,
+
+          has_social: event.has_social,
+          social_organiser_id: event.social_organiser_id,
+
+          has_class: event.has_class,
+          has_taster: event.has_taster,
+          class_style: event.class_style,
+          course_length: event.course_length,
+          class_organiser_id: event.class_organiser_id,
+
+          frequency: event.frequency,
+          day: event.day,
+          dates: event.print_dates,
+          cancellations: event.print_cancellations,
+          first_date: event.first_date,
+          last_date: event.last_date
+        }
+      )
+    end
+  end
+
+  def action
+    "Update"
+  end
+
+  def persisted?
+    true
+  end
+end

app/forms/organiser_edit_event_form.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+class OrganiserEditEventForm
+  include ActiveModel::Model
+  include ActiveModel::Attributes
+
+  class << self
+    def from_event(event)
+      new(
+        {
+          venue_id: event.venue_id,
+          dates: event.print_dates,
+          cancellations: event.print_cancellations,
+          last_date: event.last_date
+        }
+      )
+    end
+  end
+
+  attribute :venue_id, :integer
+  attribute :dates, :string
+  attribute :cancellations, :string
+  attribute :last_date, :string
+
+  def self.model_name
+    Event.model_name
+  end
+
+  validates :venue_id, presence: true
+
+  def to_h
+    attributes.merge(
+      "dates" => DatesStringParser.new.parse(dates),
+      "cancellations" => DatesStringParser.new.parse(cancellations)
+    )
+  end
+
+  def persisted?
+    true
+  end
+end

app/helpers/listings_helper.rb

@@ -122,7 +122,7 @@ def postcode_link(postcode_string, map_url = nil)
 
   # Return a span containing a message about cancelled dates:
   def swingclass_cancelledmsg(swingclass)
-    return "" if swingclass.cancellation_array(future: true).empty?
+    return "" if swingclass.future_cancellations.empty?
 
     date_printer = DatePrinter.new(separator: ", ", format: :short_date)
     cancellations = date_printer.print(swingclass.future_cancellations)

app/models/event.rb

@@ -144,10 +144,6 @@ def dates=(array_of_new_dates)
     array_of_new_dates.each { |nd| add_date(nd) }
   end
 
-  def date_array=(date_string)
-    self.dates = DatesStringParser.new.parse(date_string)
-  end
-
   def cancellations
     swing_cancellations.collect(&:date)
   end
@@ -161,24 +157,10 @@ def add_cancellation(new_cancellation)
     swing_cancellations << SwingDate.find_or_initialize_by(date: new_cancellation)
   end
 
-  def cancellation_array=(date_string)
-    self.cancellations = DatesStringParser.new.parse(date_string)
-  end
-
   def future_cancellations
     filter_future(cancellations)
   end
 
-  # READ METHODS #
-
-  def date_array(future: false)
-    return_array_of_dates(dates, future:)
-  end
-
-  def cancellation_array(future: false)
-    return_array_of_dates(cancellations, future:)
-  end
-
   private
 
   # Given an array of dates, return an appropriately filtered array

app/services/event_params_commenter.rb

@@ -17,19 +17,23 @@ def comment(event, update_params)
   private
 
   def updated_dates_comment(event, update_params)
-    comment_text("Updated dates", event.print_dates, update_params[:date_array])
+    comment_text("Updated dates", event.print_dates, update_params[:dates])
   end
 
   def updated_cancellations_comment(event, update_params)
-    comment_text("Updated cancellations", event.print_cancellations, update_params[:cancellation_array])
+    comment_text("Updated cancellations", event.print_cancellations, update_params[:cancellations])
   end
 
   def changed_dates(event, update_params)
-    (event.print_dates != update_params[:date_array])
+    return false if update_params[:dates].nil?
+
+    (event.print_dates != update_params[:dates])
   end
 
   def changed_cancellations(event, update_params)
-    (event.print_cancellations != update_params[:cancellation_array])
+    return false if update_params[:cancellations].nil?
+
+    (event.print_cancellations != update_params[:cancellations])
   end
 
   def comment_text(message, old, new)

app/validators/valid_weekly_event.rb

@@ -15,8 +15,8 @@ def weekly_events_must_have_day(event)
   end
 
   def cannot_be_weekly_and_have_dates(event)
-    return unless event.weekly? && !event.dates.empty?
+    return unless event.weekly? && event.dates.present?
 
-    event.errors.add(:date_array, "must be empty for weekly events")
+    event.errors.add(:dates, "must be empty for weekly events")
   end
 end

app/views/events/_event_listing_table_row.html.erb

@@ -30,7 +30,7 @@
     <td class="actions last">
       <%= link_to 'Show', event %>
       <%= link_to 'Delete', event, data: { confirm: 'Are you sure you want to delete this event?' }, method: :delete %>
-      <%= link_to 'Archive', archive_event_path(event), :method => :put %>
+      <%= link_to 'Archive', archive_event_path(event), data: { "turbo-method" => :put } %>
     </td>
   </tr>
 <% end %>