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

Unify APIs for the 'autocomplete' and 'select with search' components #9958

Merged
merged 17 commits into from
Feb 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
17 changes: 1 addition & 16 deletions app/assets/javascripts/components/autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,7 @@ window.GOVUK.Modules = window.GOVUK.Modules || {}
selectElement: $select,
minLength: 3,
showAllValues: $select.multiple,
showNoOptionsFound: true,
onConfirm: function (query) {
let matchingOption
if (query) {
matchingOption = [].filter.call($select.options, function (option) {
return (option.textContent || option.innerText) === query
})[0]
} else {
matchingOption = [].filter.call($select.options, function (option) {
return option.value === ''
})[0]
}
if (matchingOption) {
matchingOption.selected = true
}
}
showNoOptionsFound: true
}

const assignedOptions = JSON.parse(
Expand Down
223 changes: 119 additions & 104 deletions app/helpers/admin/taggable_content_helper.rb
Original file line number Diff line number Diff line change
@@ -1,40 +1,34 @@
# A bunch of helpers for efficiently generating select options for taggable
# content, e.g. topics, organisations, etc.
module Admin::TaggableContentHelper
# Returns an Array that represents the current set of taggable topical
# events. Each element of the array consists of two values: the name and ID
# of the topical event.
def taggable_topical_events_container
Rails.cache.fetch(taggable_topical_events_cache_digest, expires_in: 1.day) do
TopicalEvent.order(:name).map { |te| [te.name, te.id] }
def taggable_organisations_container(selected_ids = [])
cached_taggable_organisations.map do |o|
{
text: o.select_name,
value: o.id,
selected: selected_ids.include?(o.id),
}
end
end

# Returns an Array that represents the current set of taggable organisations.
# Each element of the array consists of two values: the select_name and the
# ID of the organisation
def taggable_organisations_container
Rails.cache.fetch(taggable_organisations_cache_digest, expires_in: 1.day) do
Organisation.with_translations.order("organisation_translations.name").map { |o| [o.select_name, o.id] }
def taggable_ministerial_role_appointments_container(selected_ids = [])
cached_taggable_ministerial_role_appointments.map do |appointment|
{
text: role_appointment_label(appointment),
value: appointment.id,
selected: selected_ids.include?(appointment.id),
}
end
end

# Returns an Array that represents the current set of taggable ministerial
# role appointments (both past and present). Each element of the array
# consists of two values: a selectable label (consisting of the person, the
# role, the date the role was held if it's in the past, and the organisations
# the person belongs to) and the ID of the role appointment.
def taggable_ministerial_role_appointments_container
Rails.cache.fetch(taggable_ministerial_role_appointments_cache_digest, expires_in: 1.day) do
role_appointments_container_for(RoleAppointment.for_ministerial_roles)
end
end

def taggable_needs_container
Rails.cache.fetch("need.linkables", expires_in: 1.minute) do
Services.publishing_api.get_linkables(document_type: "need").to_a.map do |need|
need.values_at("title", "content_id")
end
def taggable_needs_container(selected_ids)
Services.publishing_api.get_linkables(document_type: "need").to_a.map do |need|
title, content_id = need.values_at("title", "content_id")
{
text: title,
value: content_id,
selected: selected_ids.include?(content_id),
}
end
rescue GdsApi::TimedOutException, GdsApi::HTTPServerError
stale_data = Rails.cache.fetch("need.linkables")
Expand All @@ -43,92 +37,79 @@ def taggable_needs_container
raise
end

# Returns an Array that represents the current set of taggable roles (both
# past and present). Each element of the array consists of two values: a
# selectable label (consisting of the person, the role, the date the role was
# held if it's in the past, and the organisations the person belongs to) and
# the ID of the role appointment.
def taggable_role_appointments_container
Rails.cache.fetch(taggable_role_appointments_cache_digest, expires_in: 1.day) do
role_appointments_container_for(RoleAppointment)
def taggable_role_appointments_container(selected_ids = [])
cached_taggable_role_appointments.map do |appointment|
{
text: role_appointment_label(appointment),
value: appointment.id,
selected: selected_ids.include?(appointment.id),
}
end
end

# Returns an Array that represents the taggable ministerial roles. Each
# element of the array consists of two values: the name of the ministerial
# role with the organisation and current holder and its ID.
def taggable_ministerial_roles_container
Rails.cache.fetch(taggable_ministerial_roles_cache_digest, expires_in: 1.day) do
MinisterialRole.with_translations.with_translations_for(:organisations).alphabetical_by_person.map do |role|
["#{role.name}, #{role.organisations.map(&:name).to_sentence} (#{role.current_person_name})", role.id]
end
def taggable_detailed_guides_container(selected_ids = [])
cached_taggable_detailed_guides.map do |d|
{
text: d.title,
value: d.id,
selected: selected_ids.include?(d.id),
}
end
end

# Returns an Array that represents the current set of taggable detauled
# guides. Each element of the array consists of two values: the guide title
# and its ID.
def taggable_detailed_guides_container
Rails.cache.fetch(taggable_detailed_guides_cache_digest, expires_in: 1.day) do
DetailedGuide.alphabetical.latest_edition.active.map { |d| [d.title, d.id] }
def taggable_statistical_data_sets_container(selected_ids = [])
cached_taggable_statistical_data_sets.map do |data_set|
{
text: data_set.title,
value: data_set.document_id,
selected: selected_ids.include?(data_set.document_id),
}
end
end

# Returns an Array that represents the current set of taggable statistical
# data sets. Each elements of the array consists of two values: the data
# set title and its ID.
def taggable_statistical_data_sets_container
Rails.cache.fetch(taggable_statistical_data_sets_cache_digest, expires_in: 1.day) do
StatisticalDataSet.with_translations.latest_edition.map do |data_set|
[data_set.title, data_set.document_id]
end
def taggable_world_locations_container(selected_ids = [])
cached_taggable_world_locations.map do |w|
{
text: w.name,
value: w.id,
selected: selected_ids.include?(w.id),
}
end
end

# Returns an Array that represents the taggable world locations. Each element
# of the array consists of two values: the location name and its ID
def taggable_world_locations_container
Rails.cache.fetch(taggable_world_locations_cache_digest, expires_in: 1.day) do
WorldLocation.ordered_by_name.where(active: true).map { |w| [w.name, w.id] }
def taggable_roles_container(selected_ids = [])
cached_taggable_roles.map do |w|
{
text: w.name,
value: w.id,
selected: selected_ids.include?(w.id),
}
end
end

# Returns an Array that represents the taggable roles. Each element of the
# array consists of two values: the role name and its ID
def taggable_roles_container
Rails.cache.fetch(taggable_roles_cache_digest, expires_in: 1.day) do
Role.order(:name).map { |w| [w.name, w.id] }
def taggable_alternative_format_providers_container(selected_ids = [])
cached_taggable_alternative_format_providers.map do |o|
{
text: "#{o.name} (#{o.alternative_format_contact_email.presence || '-'})",
value: o.id,
selected: selected_ids.include?(o.id),
}
end
end

# Returns an Array that represents the taggable alternative format providers.
# Each element of the array consists of two values: the label (organisation
# and the email address if avaiable) and the ID of the organisation.
def taggable_alternative_format_providers_container
Rails.cache.fetch(taggable_alternative_format_providers_cache_digest, expires_in: 1.day) do
Organisation.alphabetical.map do |o|
["#{o.name} (#{o.alternative_format_contact_email.presence || '-'})", o.id]
end
def taggable_worldwide_organisations_container(selected_ids = [])
cached_taggable_worldwide_organisations.map do |wo|
{
text: wo.title,
value: wo.document.id,
selected: selected_ids.include?(wo.document.id),
}
end
end

# Returns an Array representing the taggable document collections and their
# groups. Each element of the array consists of two values: the
# collection/group name and the ID of the group.
def taggable_document_collection_groups_container
Rails.cache.fetch(taggable_document_collection_groups_cache_digest, expires_in: 1.day) do
DocumentCollection.latest_edition.alphabetical.includes(:groups).flat_map do |collection|
collection.groups.map { |group| ["#{collection.title} (#{group.heading})", group.id] }
end
end
end

# Returns an Array that represents the taggable worldwide organisations.
# Each element of the array consists of two values: the name of the worldwide
# organisation and its ID.
def taggable_worldwide_organisations_container
Rails.cache.fetch(taggable_worldwide_organisations_cache_digest, expires_in: 1.day) do
WorldwideOrganisation.with_translations.latest_edition.map { |wo| [wo.title, wo.document.id] }
def cached_taggable_organisations
Rails.cache.fetch(taggable_organisations_cache_digest, expires_in: 1.day) do
Organisation.with_translations.order("organisation_translations.name")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these queries (this one being a good example) shouldn't really be very slow - if it is it might be a missing index on the translation name. I can very much imagine a MoG change scenario where we put an org live and then have to shell into prod to clear the cache because we can't tag any content to it.

I suppose my question is could we apply the caching a bit more selectively/check we've got the indexes we need? Probably a bit outside the scope of this PR, to be fair

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I suspected as much too. Digging into the commit history (read 0a2f1b7), it looks like some of the cached queries have a significant impact on page load times, so I didn't want to open that can of worms. Would be ripe for a revisit in a future PR though.

end
end

Expand All @@ -146,6 +127,12 @@ def taggable_organisations_cache_digest
@taggable_organisations_cache_digest ||= calculate_digest(Organisation.order(:id), "organisations")
end

def cached_taggable_ministerial_role_appointments
Rails.cache.fetch(taggable_ministerial_role_appointments_cache_digest, expires_in: 1.day) do
role_appointments_container_for(RoleAppointment.for_ministerial_roles)
end
end

# Returns an MD5 digest representing the current set of taggable ministerial
# role appointments. This will change if any role appointments are added or
# changed, and also if an occupied MinisterialRole is updated.
Expand All @@ -159,18 +146,23 @@ def taggable_ministerial_role_appointments_cache_digest
)
end

def cached_taggable_role_appointments
Rails.cache.fetch(taggable_role_appointments_cache_digest, expires_in: 1.day) do
role_appointments_container_for(RoleAppointment)
end
end

# Returns an MD5 digest representing the current set of taggable ministerial
# role appointments. This will change if any role appointments are added or
# changed, and also if an occupied Role is updated.
def taggable_role_appointments_cache_digest
@taggable_role_appointments_cache_digest ||= calculate_digest(RoleAppointment.order(:id), "role-appointments")
end

# Returns an MD5 digest representing the current set of taggable ministerial
# rile appointments. THis will change if any ministerial role is added or
# updated.
def taggable_ministerial_roles_cache_digest
@taggable_ministerial_roles_cache_digest ||= calculate_digest(MinisterialRole.order(:id), "ministerial-roles")
def cached_taggable_detailed_guides
Rails.cache.fetch(taggable_detailed_guides_cache_digest, expires_in: 1.day) do
DetailedGuide.alphabetical.latest_edition.active
end
end

# Returns an MD5 digest representing all the detailed guides. This wil change
Expand All @@ -179,36 +171,59 @@ def taggable_detailed_guides_cache_digest
@taggable_detailed_guides_cache_digest ||= calculate_digest(Document.where(document_type: "DetailedGuide").order(:id), "detailed-guides")
end

def cached_taggable_statistical_data_sets
Rails.cache.fetch(taggable_statistical_data_sets_cache_digest, expires_in: 1.day) do
StatisticalDataSet.with_translations.latest_edition
end
end

# Returns an MD5 digest representing the taggable statistical data sets. This
# will change if any statistical data set is added or updated.
def taggable_statistical_data_sets_cache_digest
@taggable_statistical_data_sets_cache_digest ||= calculate_digest(Document.where(document_type: "StatisticalDataSet").order(:id), "statistical-data-sets")
end

def cached_taggable_world_locations
Rails.cache.fetch(taggable_world_locations_cache_digest, expires_in: 1.day) do
WorldLocation.ordered_by_name.where(active: true)
end
end

# Returns an MD5 digest representing the taggable world locations. This will
# change if any world locations are added or updated.
def taggable_world_locations_cache_digest
@taggable_world_locations_cache_digest ||= calculate_digest(WorldLocation.order(:id), "world-locations")
end

def cached_taggable_roles
Rails.cache.fetch(taggable_roles_cache_digest, expires_in: 1.day) do
Role.order(:name)
end
end

# Returns an MD5 digest representing the taggable roles. This will
# change if any world locations are added or updated.
def taggable_roles_cache_digest
@taggable_roles_cache_digest ||= calculate_digest(Role.order(:id), "roles")
end

def cached_taggable_alternative_format_providers
Rails.cache.fetch(taggable_alternative_format_providers_cache_digest, expires_in: 1.day) do
Organisation.alphabetical
end
end

# Returns an MD5 digest representing the taggable alternative format
# providers. This will change if any alternative format providers are
# changed.
def taggable_alternative_format_providers_cache_digest
@taggable_alternative_format_providers_cache_digest ||= calculate_digest(Organisation.order(:id), "alternative-format-providers")
end

# Returns an MD5 digest representing the taggable document collection
# groups. This will change if any document collection or group within
# the collection is changed or any new ones are added.
def taggable_document_collection_groups_cache_digest
@taggable_document_collection_groups_cache_digest ||= calculate_digest(Document.where(document_type: "DocumentCollection").order(:id), "document-collection-groups")
def cached_taggable_worldwide_organisations
Rails.cache.fetch(taggable_worldwide_organisations_cache_digest, expires_in: 1.day) do
WorldwideOrganisation.with_translations.latest_edition
end
end

# Returns an MD5 digest representing the taggable worldwide organisations. This
Expand All @@ -229,7 +244,7 @@ def role_appointments_container_for(scope)
.includes(:person)
.with_translations_for(:organisations)
.with_translations_for(:role)
.ascending_start_date.map { |appointment| [role_appointment_label(appointment), appointment.id] }
.ascending_start_date
end

def role_appointment_label(appointment)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
name: "edition[corporate_information_page_type_id]",
label: "Type",
heading_size: "l",
error_message: errors_for_input(edition.errors, :corporate_information_page_type_id),
error_items: errors_for(edition.errors, :corporate_information_page_type_id),
include_blank: true,
options: corporate_information_page_types(@organisation).map do |type, value|
{
Expand Down
10 changes: 4 additions & 6 deletions app/views/admin/detailed_guides/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,12 @@
id: "edition_related_detailed_guide_ids",
name: "edition[related_detailed_guide_ids][]",
error_items: errors_for(edition.errors, :related_detailed_guide_ids),
label: {
text: "Related guides",
heading_size: "m",
},
include_blank: true,
label: "Related guides",
heading_size: "m",
options: taggable_detailed_guides_container(edition.related_detailed_guide_ids),
select: {
options: [""] + taggable_detailed_guides_container,
multiple: true,
selected: edition.related_detailed_guide_ids,
},
} %>
<% end %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
name: "social_media_account[social_media_service_id]",
heading_size: "l",
include_blank: true,
error_message: errors_for_input(social_media_account.errors, :social_media_service_id),
error_items: errors_for(social_media_account.errors, :social_media_service_id),
options: SocialMediaService.all.map do |service|
{
text: service.name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@

<select name="edition[alternative_format_provider_id]" id="edition_alternative_format_provider_id" class="govuk-select" aria-describedby="alternative-format-provider-hint">
<option></option>
<% taggable_alternative_format_providers_container.each do |name, id| %>
<%
taggable_alternative_format_providers_container.each do |hash|
name = hash[:text]
id = hash[:value]
%>
<option
value="<%= id %>"
<%= "disabled='disabled'" if name.end_with?("(-)") %>
Expand Down
Loading
Loading