Skip to content

Commit

Permalink
Add confirmation page for finder facets
Browse files Browse the repository at this point in the history
Introduces page to confirm changes to the facets of a finder:

- Hides diff / generated schema in a details component.
- Have a shortened summary, don't bother attempting to summarise
  for humans, they can always click on view diff if needed.
- Minimises the diff by sorting the properties as per the most
  common ordering in the existing JSON files (we don't want any
  diffs if someone submits without making any changes!)
- Has summary list facets in the order they're defined (with the
  exception of deleted facets, which are a bit fiddly to slot into
  the correct spot. Jotting them onto the end is sufficient).
  • Loading branch information
dnkrj authored and ChrisBAshton committed Jan 3, 2025
1 parent 1292218 commit 13060ce
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 0 deletions.
69 changes: 69 additions & 0 deletions app/controllers/admin_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,75 @@ def edit_facets; end

def edit_metadata; end

def confirm_facets
# TODO: should 'require' :facets
allowed_facets = %i[
key
name
short_name
type
preposition
display_as_result_metadata
filterable
allowed_values
_destroy
]
@params = params.permit(
facets: allowed_facets,
)
@params["facets"] = @params["facets"].values
@params["facets"].each do |facet|
if facet["_destroy"] == "1"
@params["facets"].delete(facet)
end

if facet["type"] == "enum_text_multiple"
facet["type"] = "text"
facet["specialist_publisher_properties"] = {
"select": "multiple",
}
elsif facet["type"] == "enum_text_single"
facet["type"] = "text"
facet["specialist_publisher_properties"] = {
"select": "one",
}
else
facet.delete("allowed_values")
end

if facet["display_as_result_metadata"]
facet["display_as_result_metadata"] = facet["display_as_result_metadata"] == "true"
end
if facet["filterable"]
facet["filterable"] = facet["filterable"] == "true"
end
if facet["preposition"] == ""
facet.delete("preposition")
end
if facet["short_name"] == ""
facet.delete("short_name")
end

if facet["allowed_values"]
facet["allowed_values"] = facet["allowed_values"].split("\n").map do |str|
label = str.match(/^(.+){/)
label = label.nil? ? str.strip : label[1].strip
value = str.match(/{(.+)}/)
value = value.nil? ? str.strip.downcase.gsub(/[^\w\d\s]/, "").gsub(/\s/u, "-") : value[1]
{ label:, value: }
end
end

# reorder facet keys according to `allowed_facets`
facet.slice(*allowed_facets.map(&:to_s))
end

@proposed_schema = FinderSchema.new(@current_format.finder_schema.attributes)
@proposed_schema.update(@params)

render :confirm_facets
end

def confirm_metadata
@params = params.permit(
:name,
Expand Down
1 change: 1 addition & 0 deletions app/policies/document_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def can_request_edits_to_finder?
end
alias_method :summary?, :can_request_edits_to_finder?
alias_method :edit_facets?, :can_request_edits_to_finder?
alias_method :confirm_facets?, :can_request_edits_to_finder?
alias_method :edit_metadata?, :can_request_edits_to_finder?
alias_method :confirm_metadata?, :can_request_edits_to_finder?
alias_method :zendesk?, :can_request_edits_to_finder?
Expand Down
32 changes: 32 additions & 0 deletions app/views/admin/_facets_summary_card.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<%
summary_card_actions ||= []

# Value for the summary page, where we only want to summarise the current facets
summary_rows = schema.facets.map do |facet|
{
key: facet["name"],
Expand All @@ -9,6 +10,37 @@
: facet["type"].humanize,
}
end

if (defined? previous_schema)
# We're on the edit page, so want to summarise the changes. So overriding `summary_rows`.
removed_facets = previous_schema.facets.map { |facet| facet["name"] } - schema.facets.map { |facet| facet["name"] }
previous_schema_facet_names = previous_schema.facets.map { |facet| facet["name"] }
proposed_schema_facet_names = schema.facets.map { |facet| facet["name"] }

summary_rows = proposed_schema_facet_names.map do |facet_name|
if (previous_schema_facet_names.include?(facet_name))
old_facet_config = previous_schema.facets.find { |f| f["name"] == facet_name }
new_facet_config = schema.facets.find { |f| f["name"] == facet_name }
updated = !facet_name.in?(removed_facets) && !old_facet_config.nil? && old_facet_config != new_facet_config

{
key: facet_name,
value: raw("Updated (click on 'View diff' for details)"),
} if updated
else
{
key: facet_name,
value: raw("Added (click on 'View diff' for details)"),
}
end
end
summary_rows.compact!.concat(removed_facets.map do |facet_name|
{
key: facet_name,
value: raw("Deleted"),
}
end)
end
%>
<%= render "govuk_publishing_components/components/summary_card", {
title: "Filters and options",
Expand Down
67 changes: 67 additions & 0 deletions app/views/admin/confirm_facets.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<% content_for :breadcrumbs do %>
<%= render "govuk_publishing_components/components/breadcrumbs", {
collapse_on_mobile: true,
breadcrumbs: [
{
title: "All finders",
url: root_path
},
{
title: "#{current_format.title} finder",
url: "/admin/#{current_format.admin_slug}"
},
{
title: "Request change",
url: "/admin/facets/#{current_format.admin_slug}"
},
{
title: "Check changes",
url: request.original_url
}
]
} %>
<% end %>
<% content_for :page_title, "Edit #{current_format.title} finder" %>
<% content_for :title, "Check your changes before submitting" %>
<% content_for :context, "#{current_format.title} finder" %>


<div class="govuk-grid-row">
<div class="govuk-grid-column-full govuk-body govuk-!-margin-top-8">
<%= render "facets_summary_card", {
schema: @proposed_schema,
previous_schema: @current_format.finder_schema,
} %>

<details>
<summary>View diff</summary>
<%=
Diffy::Diff.new(
JSON.pretty_generate(@current_format.finder_schema.as_json).to_s,
JSON.pretty_generate(@proposed_schema.as_json).to_s,

Check notice

Code scanning / Brakeman

Unescaped model attribute. Note

Unescaped model attribute.
allow_empty_diff: false,
).to_s(:html).html_safe
%>
</details>

<details>
<summary>View generated schema</summary>
<pre><code><%= JSON.pretty_generate(@proposed_schema.as_json) %></code></pre>
</details>

<p class="govuk-body govuk-body govuk-!-margin-top-7">
By submitting you are confirming that these changes are required to the specialist finder.
</p>

<div class="govuk-button-group govuk-!-margin-top-7">
<%= form_tag "/admin/zendesk/#{current_format.admin_slug}", method: 'post' do %>
<%= hidden_field_tag :proposed_schema, JSON.pretty_generate(@proposed_schema) %>
<%= render "govuk_publishing_components/components/button", {
text: "Submit changes"
} %>
<% end %>

<%= link_to("Cancel", "/admin/#{current_format.admin_slug}", class: "govuk-link govuk-link--no-visited-state") %>
</div>
</div>
</div>
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

get "/admin/:document_type_slug", to: "admin#summary"
get "/admin/facets/:document_type_slug", to: "admin#edit_facets"
post "/admin/facets/:document_type_slug", to: "admin#confirm_facets"
get "/admin/metadata/:document_type_slug", to: "admin#edit_metadata"
post "/admin/metadata/:document_type_slug", to: "admin#confirm_metadata"
post "/admin/zendesk/:document_type_slug", to: "admin#zendesk"
Expand Down

0 comments on commit 13060ce

Please sign in to comment.