Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve SitesNeighbourhoods selector #887

Merged
merged 17 commits into from
Feb 1, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions app/controllers/admin/neighbourhoods_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions app/datatables/neighbourhood_datatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
9 changes: 9 additions & 0 deletions app/helpers/sites_helper.rb
Original file line number Diff line number Diff line change
@@ -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
5 changes: 2 additions & 3 deletions app/javascript/packs/admin.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require("cocoon")
require("select2")
require("datatables.net-bs4")
require("@nathanvda/cocoon")
lexiwitch marked this conversation as resolved.
Show resolved Hide resolved
require("select2")

import 'bootstrap'
import 'vue'
Expand All @@ -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()
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/behaviors/all_behaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
20 changes: 20 additions & 0 deletions app/javascript/src/behaviors/behaviors.site.js
Original file line number Diff line number Diff line change
@@ -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 });
});
}
}
}
});
30 changes: 19 additions & 11 deletions app/javascript/src/datatable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
});
21 changes: 19 additions & 2 deletions app/models/neighbourhood.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
4 changes: 4 additions & 0 deletions app/models/sites_neighbourhood.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
27 changes: 11 additions & 16 deletions app/views/admin/sites/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -53,30 +53,25 @@
<%= f.fields_for :sites_neighbourhood do |sn| %>
<% if @primary_neighbourhood_id %>
<p><%= @all_neighbourhoods.find(@primary_neighbourhood_id).name %></p>
<%= 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.select :neighbourhood_id, options_from_collection_for_select(@all_neighbourhoods, 'id', 'name', @primary_neighbourhood_id), class: 'form-control select2', include_blank: true %>
<% end %>
<% end %>

<h2>Other neighbourhoods to include</h2>
<p>These neighbourhoods are added to the site event directory</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 Secondary Neighbourhood', f, :sites_neighbourhoods, class: "btn btn-primary" %>
</div>
<br></br>
</div>

<%= f.button :submit, class: "btn btn-primary " %>
Expand Down
16 changes: 16 additions & 0 deletions app/views/admin/sites/_sites_neighbourhood_fields.html.erb
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class AddParentNameFieldToNeighbourhoods < ActiveRecord::Migration[6.1]
def change
add_column :neighbourhoods, :parent_name, :string

Neighbourhood.find_each(&:save)
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
21 changes: 14 additions & 7 deletions test/integration/admin/sites_integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
9 changes: 8 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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==
Expand Down