From 13060ceb4bb91e7d4be377e3ae95416ea710cd0a Mon Sep 17 00:00:00 2001 From: Daniel Karaj Date: Mon, 4 Nov 2024 15:40:59 +0000 Subject: [PATCH] Add confirmation page for finder facets 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). --- app/controllers/admin_controller.rb | 69 +++++++++++++++++++ app/policies/document_policy.rb | 1 + app/views/admin/_facets_summary_card.html.erb | 32 +++++++++ app/views/admin/confirm_facets.html.erb | 67 ++++++++++++++++++ config/routes.rb | 1 + 5 files changed, 170 insertions(+) create mode 100644 app/views/admin/confirm_facets.html.erb diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index b3f467824..5aef88386 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -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, diff --git a/app/policies/document_policy.rb b/app/policies/document_policy.rb index d00ffec19..eaedcf15f 100644 --- a/app/policies/document_policy.rb +++ b/app/policies/document_policy.rb @@ -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? diff --git a/app/views/admin/_facets_summary_card.html.erb b/app/views/admin/_facets_summary_card.html.erb index 23160c6e5..26155c2c4 100644 --- a/app/views/admin/_facets_summary_card.html.erb +++ b/app/views/admin/_facets_summary_card.html.erb @@ -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"], @@ -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", diff --git a/app/views/admin/confirm_facets.html.erb b/app/views/admin/confirm_facets.html.erb new file mode 100644 index 000000000..72da6c0c6 --- /dev/null +++ b/app/views/admin/confirm_facets.html.erb @@ -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" %> + + +
+
+ <%= render "facets_summary_card", { + schema: @proposed_schema, + previous_schema: @current_format.finder_schema, + } %> + +
+ View diff + <%= + Diffy::Diff.new( + JSON.pretty_generate(@current_format.finder_schema.as_json).to_s, + JSON.pretty_generate(@proposed_schema.as_json).to_s, + allow_empty_diff: false, + ).to_s(:html).html_safe + %> +
+ +
+ View generated schema + 
<%= JSON.pretty_generate(@proposed_schema.as_json) %>
+
+ +

+ By submitting you are confirming that these changes are required to the specialist finder. +

+ +
+ <%= 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") %> +
+
+
diff --git a/config/routes.rb b/config/routes.rb index 23d16d51b..523ad179e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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"