[FIX] Use form model to create/edit resources (fixes #4, fixes #5, refs #3)v0.1.10

Since we must associate other models (e.g., classifications) to a Resource,
we use a composite model to save all changes inside a database transaction.
This approach makes it simpler to handle resources and their associations.

Work remains to fix the geolocation and reverse geolocation to ensure
these are in sync.

8 files changed, 193 insertions, 39 deletions

diff --git a/app/controllers/resources_controller.rb b/app/controllers/resources_controller.rb
index 2fa392f..4a99f2a 100644
--- a/app/controllers/resources_controller.rb
+++ b/app/controllers/resources_controller.rb
@@ -3,7 +3,8 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
 
 class ResourcesController < ApplicationController
-  before_action :set_resource, only: [:new, :show, :edit, :update, :delete, :destroy]
+  before_action :set_resource, only: [:show, :delete, :destroy]
+#  before_action :set_resource_form, only: [:new, :create, :edit, :update]
 
   # GET /resources
   def index
@@ -12,6 +13,7 @@ class ResourcesController < ApplicationController
 
   # GET /resources/new
   def new
+    @resource = ResourceForm.new(current_agent)
   end
 
   # POST /resources
@@ -19,18 +21,12 @@ class ResourcesController < ApplicationController
     # TODO Background job to list similar items
     # TODO If there's a match, return to user with new record or list of mergeable ones
 
-    classification = resource_params.delete(:classification) || { section_ids: [] }
-
-    Rails.logger.info resource_params
-
-    @resource = current_agent.resources.build(resource_params)
+    @resource = ResourceForm.new(current_agent, resource_form_params)
 
     respond_to do |format|
-      Rails.logger.info "format: #{format} - Res: #{@resource.inspect}"
       if @resource.save
-        classification[:section_ids].each { |id| @resource.classifications.find_or_create_by(section_id: id)  }
-        format.html { redirect_to @resource, notice: 'Merci de votre contribution !' }
-        format.json { render :show, status: :created, location: @resource }
+        format.html { redirect_to @resource.resource, notice: 'Merci de votre contribution !' }
+        format.json { render :show, status: :created, location: @resource.resource }
       else
         format.html { render :new }
         format.json { render json: @resource.errors, status: :unprocessable_entity }
@@ -62,8 +58,8 @@ class ResourcesController < ApplicationController
 
     respond_to do |format|
       if @resource.update(resource_params)
-        format.html { redirect_to @resource, notice: 'Merci de votre contribution !' }
-        format.json { render :show, status: :ok, location: @resource }
+        format.html { redirect_to @resource.resource, notice: 'Merci de votre contribution !' }
+        format.json { render :show, status: :ok, location: @resource.resource }
       else
         format.html { render :edit }
         format.json { render json: @resource.errors, status: :unprocessable_entity }
@@ -97,9 +93,9 @@ class ResourcesController < ApplicationController
 
   private
 
-  def resource_params
+  def resource_form_params
     params
-      .require(:resource)
+      .require(:resource_form)
       .permit(:agent_id,
               :uuid,
               :name,
@@ -111,11 +107,12 @@ class ResourcesController < ApplicationController
               :address,
               :postal_code,
               :city,
-              :entry_number,
-              :categories,
+              :entry_number, # Unused on new resources
+              :categories,   #
               :latitude,
               :longitude,
-              classification: [ :section_ids ])
+              :source,
+              section_ids: [])
   end
 
   def set_resource
diff --git a/app/helpers/resources_helper.rb b/app/helpers/resources_helper.rb
index a949311..792a78a 100644
--- a/app/helpers/resources_helper.rb
+++ b/app/helpers/resources_helper.rb
@@ -4,13 +4,14 @@
 
 module ResourcesHelper
   # Return a SELECT tag to choose a section in a given taxonomy
-  def section_select(taxonomy, selected = nil)
+  def section_select(taxonomy, selected = nil, options = {})
     grouped_options = []
+    tag_name = options[:name] || 'classification[section_ids]'
 
     taxonomy.categories.each do |c|
       grouped_options << [c.name, c.sections.map { |s| [s.name, s.id] }]
     end
 
-    select_tag('classification[section_ids]', grouped_options_for_select(grouped_options), selected: selected, multiple: true)
+    select_tag(tag_name, grouped_options_for_select(grouped_options, selected), multiple: true)
   end
 end
diff --git a/app/models/classification.rb b/app/models/classification.rb
index 0754054..e2b180f 100644
--- a/app/models/classification.rb
+++ b/app/models/classification.rb
@@ -5,4 +5,6 @@
 class Classification < ApplicationRecord
   belongs_to :resource
   belongs_to :section
+
+  self.primary_key = :resource_id
 end
diff --git a/app/models/resource.rb b/app/models/resource.rb
index c603978..b803afb 100644
--- a/app/models/resource.rb
+++ b/app/models/resource.rb
@@ -21,7 +21,7 @@ class Resource < ApplicationRecord
             length: { in: 3..64 }
 
   validates :email,
-            with: { format: URI::MailTo::EMAIL_REGEXP },
+            format: { with: URI::MailTo::EMAIL_REGEXP },
             allow_blank: true
 
   validates :source,
@@ -31,7 +31,8 @@ class Resource < ApplicationRecord
 
   phony_normalize :phone_number, default_country_code: 'BE', normalize_when_valid: true
   validates :phone_number,
-            phony_plausible: { ignore_record_country_code: true, ignore_record_country_number: true }
+            phony_plausible: { ignore_record_country_code: true, ignore_record_country_number: true },
+            allow_blank: true
 
   # Depends on validate_url Gem
   validates :website,
@@ -47,19 +48,19 @@ class Resource < ApplicationRecord
     format('%<lon>3.7f', lon: feature['geometry']['coordinates'][0]).to_f
   end
   def longitude=(value)
-    feature['geometry']['coordinates'][0] = format('%<lon>3.7f', lon: value).to_f
+    feature['geometry']['coordinates'][0] = format('%<lon>3.7f', lon: value.to_f).to_f
   end
   # You can use, e.g.: res.latitude = 0.123
   def latitude
     format('%<lat>2.7f', lat: feature['geometry']['coordinates'][1]).to_f
   end
   def latitude=(value)
-    feature['geometry']['coordinates'][1] = format('%<lat>2.7f', lat: value).to_f
+    feature['geometry']['coordinates'][1] = format('%<lat>2.7f', lat: value.to_f).to_f
   end
 
   # Properties
 
-  [:name, :summary, :description, :email, :source, :address, :postal_code, :city, :phone_number, :website].each do |prop|
+  [:name, :summary, :description, :email, :source, :address, :postal_code, :city, :phone_number, :website, :entry_number].each do |prop|
     # Define a reader
     define_method prop do
       properties[prop.to_s]
diff --git a/app/models/resource_form.rb b/app/models/resource_form.rb
new file mode 100644
index 0000000..14275a4
--- /dev/null
+++ b/app/models/resource_form.rb
@@ -0,0 +1,138 @@
+class ResourceForm
+  include ActiveModel::Model
+
+  attr_accessor :agent, :resource
+  attr_accessor :agent_id, :uuid, :name, :summary, :description, :email,
+                :website, :phone_number, :address, :postal_code, :city,
+                :entry_number, :latitude, :longitude, :section_ids, :source
+
+  with_options presence: true do
+    validates :name
+    validates :email, allow_blank: true
+#    validates :source
+    validates :phone_number, allow_blank: true
+    validates :website, allow_blank: true
+  end
+
+  validate :validate_models
+
+  def initialize(agent, attributes = nil)
+    @agent = agent
+    @attributes = attributes || default_attributes
+
+    raise ArgumentError "Attributes must be permitted" unless @attributes.permitted?
+
+    # We use an empty hidden field to ensure this is always set, even if no
+    # section is selected, so let's remove the empty value
+    @section_ids = @attributes.extract!(:section_ids)[:section_ids].filter_map { |x| x.presence } rescue []
+    # We may have an existing and correct source, but anyway we must have one
+    @resource = @agent.resources.build({ source: @agent.name }.merge(@attributes))
+  end
+
+  def new_record?
+    resource&.new_record? || true
+  end
+
+  # New forms can provide an existing classification,
+  # and updates can prefer passed arguments to ensure correct changes
+  def section_ids
+    resource.present? && @section_ids.empty? ? resource.section_ids : @section_ids
+  end
+
+  # Ensure field values stick around
+  # OMG this is ugly
+  # TODO: use delegator or metaprog
+  def name
+    @name ||= resource&.name
+  end
+  def summary
+    @summary ||= resource&.summary
+  end
+  def description
+    @description ||= resource&.description
+  end
+  def email
+    @email ||= resource&.email
+  end
+  def website
+    @website ||= resource&.website
+  end
+  def phone_number
+    @phone_number ||= resource&.phone_number
+  end
+  def address
+    @address ||= resource&.address
+  end
+  def postal_code
+    @postal_code ||= resource&.postal_code
+  end
+  def city
+    @city ||= resource&.city
+  end
+  def longitude
+    @longitude ||= resource&.longitude
+  end
+  def latitude
+    @latitude ||= resource&.latitude
+  end
+
+
+  def save
+    Rails.logger.info "--- Calling SAVE ---"
+    return false if invalid?
+
+    ActiveRecord::Base.transaction do
+      resource.save! && \
+      @section_ids.each do |sec_id|
+        Rails.logger.info("section " + sec_id)
+        resource.categories << resource.classifications.find_or_create_by!(section_id: sec_id)
+      end
+
+      true
+    rescue ActiveRecord::StatementInvalid => e
+      errors.add(:base, e.message)
+      false
+    end
+
+  rescue Exception => e
+    Rails.logger.info "Unhandled Exception #{e.class}: #{e.message}"
+    false
+  end
+
+  private
+
+  def default_attributes
+    ActionController::Parameters.new(
+      {
+        agent_id: @agent.id,
+        uuid: '',
+        name: '',
+        summary: '',
+        description: '',
+        email: '',
+        website: '',
+        phone_number: '',
+        address: '',
+        postal_code: '',
+        city: '',
+        entry_number: nil,
+        latitude: 0.0,
+        longitude: 0.0,
+        source: @agent.name,
+        section_ids: []
+      }).permit!
+  end
+
+  # Validate underlying models
+  def validate_models
+    Rails.logger.info('--- validate_models ---')
+    resource.valid? || promote_errors(resource.errors)
+  end
+
+  # Promote an error from the Model to the Form
+  def promote_errors(model_errors)
+    model_errors.each do |attribute, error|
+      errors.add(attribute, error.full_message)
+    end
+  end
+end
diff --git a/app/views/resources/_edit.html.erb b/app/views/resources/_edit.html.erb
index 55c59f6..f22aee3 100644
--- a/app/views/resources/_edit.html.erb
+++ b/app/views/resources/_edit.html.erb
@@ -1,6 +1,6 @@
-<h3>Edit <%= @resource.presence.feature['properties']['name'] || 'new resource' %></h3>
+<h3>Edit <%= @resource.presence.name || 'new resource' %></h3>
 <h2>Context: <%= current_agent %></h2>
 
-<%= form_with model: [current_agent,@resource], url: controller.action_name == 'new' ? agent_resources_path(agent: current_agent) : resource_path(@resource) do |f| %>
+<%= form_with model: @resource, url: @resource.presence.new_record? ? agent_resources_path(current_agent) : resource_path(@resource.resource) do |f| %>
     <%= render partial: 'form', locals: { resource: resource, f: f } %>
 <% end %>
diff --git a/app/views/resources/_form.html.erb b/app/views/resources/_form.html.erb
index 89745bd..648834d 100644
--- a/app/views/resources/_form.html.erb
+++ b/app/views/resources/_form.html.erb
@@ -1,12 +1,24 @@
-<fieldset>
+<% if resource.errors.any? %>
+    <div id="error_explanation">
+        <h2><%= pluralize(resource.errors.count, "error") %> prohibited this resource from being saved:</h2>
+
+        <ul>
+            <% resource.errors.each do |error| %>
+                <li><%= error.full_message %></li>
+            <% end %>
+        </ul>
+    </div>
+<% end %>
+
+    <fieldset>
     <%= tag.legend "Propriétés de la ressource" %>
 
-    <dl>
+    <dl class="field">
         <dt><%= f.label :name %></dt>
-        <dd><%= f.text_field :name, maxlength: 64, placeholder: 'La ferme des animaux', value: resource.name %></dd>
+        <dd><%= f.text_field :name, maxlength: 64, placeholder: 'La ferme des animaux' %></dd>
 
         <dt><%= f.label :summary %></dt>
-        <dd><%= f.text_field :summary, maxlength: 136, placeholder: 'Une ferme locale', value: resource.name %></dd>
+        <dd><%= f.text_field :summary, maxlength: 136, placeholder: 'Une ferme locale' %></dd>
 
         <dt><%= f.label :description %></dt>
         <dd><%= f.text_area :description, cols: 72, rows: 10, placeholder: '## Un choix pertinent
@@ -29,33 +41,34 @@ La description _peut_ comporter du [Markdown].
 <fieldset>
     <%= tag.legend "Classification" %>
     <%# TODO Add a taxonomy selector %>
-    <%= section_select(current_agent.taxonomies&.first || Taxonomy.first) %>
+    <%= section_select(current_agent.taxonomies&.first || Taxonomy.first, resource.section_ids, name: 'resource_form[section_ids]') %>
 </fieldset>
 
 <fieldset>
     <%= tag.legend "Coordonées géographiques" %>
     <%# TODO Add a graphical geo selector %>
 
-    <dl>
+    <dl class="field">
         <dt><%= f.label :address %></dt>
-        <dd><%= f.text_field :address, value: resource.address, placeholder: 'Rue aux herbes, 123' %></dd>
+        <dd><%= f.text_field :address, placeholder: 'Rue aux herbes, 123' %></dd>
 
         <dt><%= f.label :postal_code %></dt>
-        <dd><%= f.text_field :postal_code, value: resource.postal_code, placeholder: '1000' %></dd>
+        <dd><%= f.text_field :postal_code, placeholder: '1000' %></dd>
 
         <dt><%= f.label :city %></dt>
-        <dd><%= f.text_field :city, value: resource.city, placeholder: 'Bruxelles' %></dd>
+        <dd><%= f.text_field :city, placeholder: 'Bruxelles' %></dd>
     </dl>
 
-    <dl>
+    <dl class="field">
         <dt><%= f.label :longitude %></dt>
-        <dd><%= f.text_field :longitude, value: resource.longitude, placeholder: '0.56789012' %></dd>
+        <dd><%= f.text_field :longitude, placeholder: '0.56789012', pattern: '[\d]+.[\d]+' %></dd>
 
         <dt><%= f.label :latitude %></dt>
-        <dd><%= f.text_field :latitude, value: resource.latitude, placeholder: '50.12345678' %></dd>
+        <dd><%= f.text_field :latitude, placeholder: '50.12345678', pattern: '[\d]+.[\d]+' %></dd>
     </dl>
 </fieldset>
 
 <%= f.hidden_field :agent_id, value: current_agent.id %>
+<%= f.hidden_field :source, value: current_agent.name %>
 
 <p><%= f.submit 'Save' %></p>
diff --git a/app/views/resources/new.html.erb b/app/views/resources/new.html.erb
index 80d8206..e8347b4 100644
--- a/app/views/resources/new.html.erb
+++ b/app/views/resources/new.html.erb
@@ -1,2 +1,4 @@
-<%= render partial: 'edit', locals: { resource: @resource } %>
+<%= render 'edit', resource: @resource %>
+
+<#%= render partial: 'edit', locals: { resource: @resource } %>