Improve SitesNeighbourhoods selector (geeksforsocialchange#887)

* Add select2 to primary neighbourhoods form
* Add select2 + cocoon to secondary neighbourhoods form
* Memoize parent_name field in Neighbourhoods
* Give neighbourhoods more context
* Add try/catch block to datatable.js to stop errors appearing on pages that do not contain datatables
* Remove filtering of primary id from options_for_sites_neighbourhoods
* Correct Sites behaviours file to remove primary neighbourhood from form field input
* Fix CSS on Cocoon
* Buttons: Red. Descriptions: Added

Co-authored-by: Dr Kim Foale <[email protected]>

app/controllers/admin/neighbourhoods_controller.rb

@@ -10,9 +10,10 @@ def index
       respond_to do |format|
         format.html
         format.json { render json: NeighbourhoodDatatable.new(
-                                    params, view_context: view_context, 
+                                    params,
+                                    view_context: view_context,
                                     neighbourhoods: @neighbourhoods
-                                  ) 
+                                  )
                     }
       end
     end

app/datatables/neighbourhood_datatable.rb

@@ -8,6 +8,7 @@ def view_columns
       unit_name:       { source: 'Neighbourhood.unit_name' },
       unit_code_key:   { source: 'Neighbourhood.unit_code_key' },
       unit_code_value: { source: 'Neighbourhood.unit_code_value' },
+      parent_name:     { source: 'Neighbourhood.parent_name' },
     }
   end
 
@@ -19,10 +20,7 @@ def data
         unit_name:       record.unit_name,
         unit_code_key:   record.unit_code_key,
         unit_code_value: record.unit_code_value,
-        # county:   record.county,
-        # district: record.district,
-        # region:   record.region,
-        # country:  record.country
+        parent_name:     record.parent_name,
       }
     end
   end

app/helpers/sites_helper.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module SitesHelper
+  def options_for_sites_neighbourhoods
+    # Remove the primary neighbourhood from the list
+    @all_neighbourhoods.filter { |e| e.name != '' }
+                       .collect { |e| { name: e.contextual_name, id: e.id } }
+  end
+end

app/javascript/packs/admin.js

@@ -1,6 +1,6 @@
-require("cocoon")
-require("select2")
 require("datatables.net-bs4")
+require("@nathanvda/cocoon")
+require("select2")
 
 import 'bootstrap'
 import 'vue'
@@ -12,7 +12,6 @@ import '../src/datatable.js'
 import '../src/opening-times.js'
 import '../src/ward-picker.js'
 
-
 $(document).on('turbolinks:load', function () {
 
   $('body').init_behaviors()

app/javascript/src/behaviors/all_behaviors.js

@@ -4,4 +4,5 @@ import './behaviors.collection.js'
 import './behaviors.neighbourhood.js'
 import './behaviors.partner.js'
 import './behaviors.place.js'
+import './behaviors.site.js'
 import './behaviors.user.js'

app/javascript/src/behaviors/behaviors.site.js

@@ -0,0 +1,20 @@
+jQuery.extend(Behaviors, {
+  site: {
+    form: {
+      init: function() {
+        // If the sites neighbourhood's relation_type is primary, it generates a label
+        // with the class 'cocoon_delete-this'. This allows us to remove it from the form
+        // so it does not submit a primary relation as being a secondary one or remove it
+        $('.cocoon_delete-this').parents('.nested-fields').remove();
+
+        // Attach select2 to the current select2 nodes
+        $('.select2').each(function () { $(this).select2({ multiple: false }); });
+
+        // Attach select2 to all future select2 nodes
+        $('.sites_neighbourhoods').bind('cocoon:after-insert', function (_, element) {
+          $('.select2', element).select2({ multiple: false });
+        });
+      }
+    }
+  }
+});

app/javascript/src/datatable.js

@@ -10,15 +10,23 @@ document.addEventListener("turbolinks:before-cache", function () {
 });
 
 document.addEventListener('turbolinks:load', function () {
-  dataTable = $('#datatable').DataTable({
-    "processing": true,
-    "serverSide": true,
-    "pageLength": 15,
-    "ajax": {
-      "url": $('#datatable').data('source')
-    },
-    "pagingType": "full_numbers",
-    // Column spec is loaded from a script tag in the view
-    "columns": columns
-  })
+  try {
+    dataTable = $('#datatable').DataTable({
+      "processing": true,
+      "serverSide": true,
+      "pageLength": 15,
+      "ajax": {
+        "url": $('#datatable').data('source')
+      },
+      "pagingType": "full_numbers",
+      // Column spec is loaded from a script tag in the view
+      "columns": columns
+    })
+  } catch (e) {
+    // On pages where DataTables shouldn't be used, columns is not defined.
+    // This catches that and stops it throwing an error to the console.
+    if (!(e instanceof ReferenceError)) {
+      console.error(e);
+    }
+  }
 });

app/models/neighbourhood.rb

@@ -19,6 +19,8 @@ class Neighbourhood < ApplicationRecord
             length: { is: 9 },
             allow_blank: true
 
+  before_update :inject_parent_name_field
+
   def shortname
     if name_abbr.present?
       name_abbr
@@ -29,13 +31,21 @@ def shortname
     end
   end
 
+  def contextual_name
+    # "Wardname, Countryname (Region)"
+    return "#{shortname}, #{parent_name} (#{unit.titleize})" if parent_name
+
+    # "Wardname (Region)"
+    "#{shortname} (#{unit.titleize})"
+  end
+
   def fullname
     if name.present?
-      name 
+      name
     elsif name_abbr.present?
       name_abbr
     else
-      "[not set]"
+      '[not set]'
     end
   end
 
@@ -67,4 +77,11 @@ def find_from_postcodesio_response(res)
                              unit_name: res['admin_ward'])
     end
   end
+
+  private
+
+  def inject_parent_name_field
+    self.parent_name = parent.name if parent
+    true
+  end
 end

app/models/sites_neighbourhood.rb

@@ -9,4 +9,8 @@ class SitesNeighbourhood < ApplicationRecord
               scope: :site_id,
               message: 'Neighbourhood cannot be assigned more than once to a site'
             }
+
+  def name
+    neighbourhood.to_s
+  end
 end

app/views/admin/sites/_form.html.erb

@@ -49,38 +49,38 @@
   <p><em>Who took this photo?</em></p>
 
   <h2>Main neighbourhood</h2>
+  <p class="font-weight-light">This neighbourhood will be listed in the main PlaceCal directory. It cannot be changed after the site is created.</p>
 
-  <%= f.fields_for :sites_neighbourhood do |sn| %>
+  <%= f.simple_fields_for :sites_neighbourhood do |sn| %>
     <% if @primary_neighbourhood_id %>
-      <p><%= @all_neighbourhoods.find(@primary_neighbourhood_id).name %></p>
+      <h3><span class="badge badge-secondary"><%= @all_neighbourhoods.find(@primary_neighbourhood_id).contextual_name %></span></h3>
+      <%= sn.hidden_field :relation_type, value: "Primary" %>
+      <%= sn.hidden_field :neighbourhood_id, value: @primary_neighbourhood_id %>
     <% else %>
       <%= sn.hidden_field :relation_type, value: "Primary" %>
-      <%= sn.select :neighbourhood_id, options_from_collection_for_select(@all_neighbourhoods, 'id', 'name', @primary_neighbourhood_id), class: 'form-control', include_blank: true %>
+      <%= sn.input :neighbourhood_id, collection: options_for_sites_neighbourhoods, include_blank: false,
+          value_method: ->(obj) { obj[:id] }, label_method: ->(obj) { obj[:name] },
+          input_html: { class: 'form-control select2 col-6' },
+          label: '', label_html: { hidden: true } %>
     <% end %>
   <% end %>
+  <br />
 
   <h2>Other neighbourhoods to include</h2>
+  <p class="font-weight-light">Information from these neighbourhoods will also be displayed on this site</p>
 
-  <div class="site__neighbourhoods">
-    <% @all_neighbourhoods.each do |neighbourhood| %>
-      <%= fields_for "site[sites_neighbourhoods_attributes][#{neighbourhood.id}]" do |sna|%>
-        <label>
-          <%- if (@secondary_neighbourhood_ids.include?(neighbourhood.id)) %>
-            <%= sna.hidden_field :id, value: @sites_neighbourhoods_ids[neighbourhood.id] %>
-            <%# if box is unchecked, this will result in the removal of the association %>
-            <%= sna.check_box :_destroy, {checked: true}, false, true %>
-          <% else %>
-             <%= sna.check_box :neighbourhood_id, {include_hidden: false, checked: false}, neighbourhood.id %>
-             <%= sna.hidden_field :relation_type, value: "Secondary" %>
-          <% end %>
-          <%= neighbourhood.name %>
-        </label><br>
-      <% end %>
+  <div class="sites_neighbourhoods">
+    <%= f.simple_fields_for :sites_neighbourhoods do |neighbourhood| %>
+      <%= render 'sites_neighbourhood_fields', :f => neighbourhood %>
     <% end %>
+    <div class="links">
+      <%= link_to_add_association 'Add neighbourhood', f, :sites_neighbourhoods, class: "btn btn-primary btn-sm" %>
+    </div>
+    <br></br>
   </div>
 
-  <%= f.button :submit, class: "btn btn-primary " %>
+  <%= f.button :submit, class: "btn btn-primary btn-lg" %>
   <% unless @site.new_record? %>
-    <%= link_to "Destroy Site", admin_site_path(@site), method: :delete, class: "btn btn-danger" %>
+    <%= link_to "Destroy Site", admin_site_path(@site), method: :delete, class: "btn btn-danger btn-lg" %>
   <% end %>
 <% end %>

app/views/admin/sites/_sites_neighbourhood_fields.html.erb

@@ -0,0 +1,16 @@
+<fieldset class="input-group nested-fields p-0">
+    <%# The style width setting here is applied to dynamically created elements, we do not know why. %>
+    <%# TODO: Fix the dynamic styling so that width does not get applied, so we can remove this smell %>
+    <%= f.input :neighbourhood_id, collection: options_for_sites_neighbourhoods, include_blank: false,
+        value_method: ->(obj) { obj[:id] },
+        label_method: ->(obj) { obj[:name] },
+        input_html: { class: 'form-control select2', style: "width: 599.8px;" },
+        label: '', label_html: { hidden: true } %>
+
+    <%# This label causes behaviours.site.js to remove the association from the form %>
+    <%= f.label :ignore, class: 'cocoon_delete-this' if f.object.relation_type == 'Primary' %>
+    <div class="input-group-append p-0">
+        <%= f.hidden_field :relation_type, value: 'Secondary' %>
+        <%= link_to_remove_association 'Remove', f, class: "pl-2 pt-1 text-danger" %>
+    </div>
+</fieldset>

db/migrate/20220125175817_add_parent_name_field_to_neighbourhoods.rb

@@ -0,0 +1,7 @@
+class AddParentNameFieldToNeighbourhoods < ActiveRecord::Migration[6.1]
+  def change
+    add_column :neighbourhoods, :parent_name, :string
+
+    Neighbourhood.find_each(&:save)
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2022_01_24_170103) do
+ActiveRecord::Schema.define(version: 2022_01_25_175817) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -125,6 +125,7 @@
     t.string "unit_code_key", default: "WD19CD"
     t.string "unit_code_value"
     t.string "unit_name"
+    t.string "parent_name"
     t.index ["ancestry"], name: "index_neighbourhoods_on_ancestry"
   end
 

package.json

@@ -7,6 +7,7 @@
     "yarn": ">= 1.21.1"
   },
   "dependencies": {
+    "@nathanvda/cocoon": "^1.2.14",
     "@rails/ujs": "^6.0.2",
     "@rails/webpacker": "5.4.3",
     "axios": "^0.24.0",

test/integration/admin/sites_integration_test.rb

@@ -35,10 +35,17 @@ class AdminSitesIntegrationTest < ActionDispatch::IntegrationTest
     assert_select 'label', 'Hero image'
     assert_select 'label', 'Hero image credit'
 
-    # See all neighbourhoods
-    assert_select '.site__neighbourhoods' do
-      assert_select 'label', @number_of_neighbourhoods
-    end
+    # See just neighbourhoods they admin
+    # In short:
+    # - Find the cocoon template for the Secondary Neighbourhoods <select> item
+    # - Grep for all occurences of "option value=" which grabs only the first <option> tag (not the closing tag)
+    # - That gives us all the neighbourhoods it is displaying
+    #
+    # Please replace this with Capybara in the future lol
+
+    cocoon_select_template = assert_select('.add_fields').first['data-association-insertion-template']
+    neighbourhoods_shown = cocoon_select_template.scan(/(option value=)/).size
+    assert neighbourhoods_shown == @number_of_neighbourhoods
   end
 
   test 'site admin users see appropriate fields' do
@@ -63,8 +70,8 @@ class AdminSitesIntegrationTest < ActionDispatch::IntegrationTest
     assert_select 'label', 'Hero image credit'
 
     # See just neighbourhoods they admin
-    assert_select '.site__neighbourhoods' do
-      assert_select 'label', 2
-    end
+    cocoon_select_template = assert_select('.add_fields').first['data-association-insertion-template']
+    neighbourhoods_shown = cocoon_select_template.scan(/(option value=)/).size
+    assert neighbourhoods_shown == 2
   end
 end

yarn.lock

@@ -910,6 +910,13 @@
   resolved "https://registry.yarnpkg.com/@gar/promisify/-/promisify-1.1.2.tgz#30aa825f11d438671d585bd44e7fd564535fc210"
   integrity sha512-82cpyJyKRoQoRi+14ibCeGPu0CwypgtBAdBhq1WfvagpCZNKqwXbKwXllYSMG91DhmG4jt9gN8eP6lGOtozuaw==
 
+"@nathanvda/cocoon@^1.2.14":
+  version "1.2.14"
+  resolved "https://registry.yarnpkg.com/@nathanvda/cocoon/-/cocoon-1.2.14.tgz#aaea910e4b9c0d28d5bdcb7f3743617db46b09af"
+  integrity sha512-WcEt2vVp50de2i7rkD4O+96O1iMtMIcTBNGPocrHfcmHDujKOngoLHFF8Ektgoh8PjwFAJMxx8WyGv0BtKTjxQ==
+  dependencies:
+    jquery "^3.3.1"
+
 "@nodelib/[email protected]":
   version "2.1.5"
   resolved "https://registry.yarnpkg.com/@nodelib/fs.scandir/-/fs.scandir-2.1.5.tgz#7619c2eb21b25483f6d167548b4cfd5a7488c3d5"
@@ -4033,7 +4040,7 @@ jest-worker@^26.5.0:
     merge-stream "^2.0.0"
     supports-color "^7.0.0"
 
-jquery@>=1.7, jquery@^3.4.1:
+jquery@>=1.7, jquery@^3.3.1, jquery@^3.4.1:
   version "3.6.0"
   resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.6.0.tgz#c72a09f15c1bdce142f49dbf1170bdf8adac2470"
   integrity sha512-JVzAR/AjBvVt2BmYhxRCSYysDsPcssdmTFnzyLEts9qNwmjmu4JTAMYubEfwVOSwpQ1I1sKKFcxhZCI2buerfw==