From 13698ef6fb95aba107b7e85d2ec3c6e4180e6dd8 Mon Sep 17 00:00:00 2001
From: Kat <katrina@madetech.com>
Date: Mon, 25 Jul 2022 14:57:46 +0100
Subject: [PATCH] Display error message next to the relevant field

---
 app/frontend/styles/_errors.scss              |  5 ++
 app/frontend/styles/application.scss          |  1 +
 app/models/scheme.rb                          | 36 ++++++----
 .../schemes/_scheme_summary_list_row.html.erb | 22 ++++++
 app/views/schemes/check_answers.html.erb      | 70 +++----------------
 5 files changed, 57 insertions(+), 77 deletions(-)
 create mode 100644 app/frontend/styles/_errors.scss
 create mode 100644 app/views/schemes/_scheme_summary_list_row.html.erb

diff --git a/app/frontend/styles/_errors.scss b/app/frontend/styles/_errors.scss
new file mode 100644
index 000000000..3e1d2ff7d
--- /dev/null
+++ b/app/frontend/styles/_errors.scss
@@ -0,0 +1,5 @@
+.app-summary-list__value--error {
+    border-left: $govuk-border-width solid $govuk-error-colour;
+    padding-left: govuk-spacing(3);
+  }
+  
\ No newline at end of file
diff --git a/app/frontend/styles/application.scss b/app/frontend/styles/application.scss
index dd4dc3b97..4b26044d1 100644
--- a/app/frontend/styles/application.scss
+++ b/app/frontend/styles/application.scss
@@ -40,6 +40,7 @@ $govuk-breakpoints: (
 @import "primary-navigation";
 @import "search";
 @import "sub-navigation";
+@import "errors";
 
 // App utilities
 .app-\!-colour-muted {
diff --git a/app/models/scheme.rb b/app/models/scheme.rb
index 405dfe8d6..accc4f17c 100644
--- a/app/models/scheme.rb
+++ b/app/models/scheme.rb
@@ -102,44 +102,44 @@ class Scheme < ApplicationRecord
 
   def check_details_attributes
     [
-      { name: "Service code", value: id_to_display },
-      { name: "Name", value: service_name },
-      { name: "Confidential information", value: sensitive },
-      { name: "Type of scheme", value: scheme_type },
-      { name: "Registered under Care Standards Act 2000", value: registered_under_care_act },
-      { name: "Housing stock owned by", value: owning_organisation.name },
-      { name: "Support services provided by", value: arrangement_type },
+      { name: "Service code", value: id_to_display, id: "id" },
+      { name: "Name", value: service_name, id: "service_name" },
+      { name: "Confidential information", value: sensitive, id: "sensitive" },
+      { name: "Type of scheme", value: scheme_type, id: "scheme_type" },
+      { name: "Registered under Care Standards Act 2000", value: registered_under_care_act, id: "registered_under_care_act" },
+      { name: "Housing stock owned by", value: owning_organisation.name, id: "owning_organisation_id" },
+      { name: "Support services provided by", value: arrangement_type, id: "arrangement_type" },
     ]
   end
 
   def check_support_services_provider_attributes
     [
-      { name: "Organisation providing support", value: managing_organisation&.name },
+      { name: "Organisation providing support", value: managing_organisation&.name, id: "managing_organisation_id" },
     ]
   end
 
   def check_primary_client_attributes
     [
-      { name: "Primary client group", value: primary_client_group },
+      { name: "Primary client group", value: primary_client_group, id: "primary_client_group" },
     ]
   end
 
   def check_secondary_client_confirmation_attributes
     [
-      { name: "Has another client group", value: has_other_client_group },
+      { name: "Has another client group", value: has_other_client_group, id: "has_other_client_group" },
     ]
   end
 
   def check_secondary_client_attributes
     [
-      { name: "Secondary client group", value: secondary_client_group },
+      { name: "Secondary client group", value: secondary_client_group, id: "secondary_client_group" },
     ]
   end
 
   def check_support_attributes
     [
-      { name: "Level of support given", value: support_type },
-      { name: "Intended length of stay", value: intended_stay },
+      { name: "Level of support given", value: support_type, id: "support_type" },
+      { name: "Intended length of stay", value: intended_stay, id: "intended_stay" },
     ]
   end
 
@@ -211,8 +211,14 @@ class Scheme < ApplicationRecord
   def validate_confirmed
     required_attributes = attribute_names - %w[id created_at updated_at old_id old_visible_id confirmed end_date sensitive secondary_client_group total_units has_other_client_group]
 
-    if confirmed == true && required_attributes.any? { |attribute| self[attribute].blank? }
-      errors.add :base
+    if confirmed == true
+      required_attributes.any? do |attribute|
+        if self[attribute].blank?
+          errors.add :base
+          errors.add attribute.to_sym
+          self.confirmed = false
+        end
+      end
     end
   end
 end
diff --git a/app/views/schemes/_scheme_summary_list_row.html.erb b/app/views/schemes/_scheme_summary_list_row.html.erb
new file mode 100644
index 000000000..a02e5d25f
--- /dev/null
+++ b/app/views/schemes/_scheme_summary_list_row.html.erb
@@ -0,0 +1,22 @@
+<div class="<%= "govuk-summary-list__row #{scheme.confirmed? && attribute[:name] != 'Name' ? 'govuk-summary-list__row--no-actions' : ''}" %>">
+    <dt class="govuk-summary-list__key">
+        <%= attribute[:name].to_s %>
+    </dt>
+    <% if scheme.errors[attribute[:id].to_sym].count > 0 %>
+        <dd class="govuk-summary-list__value app-summary-list__value--error">
+            <p class="govuk-error-message">
+                <span class="govuk-error-message govuk-visually-hidden">Error:</span> <%= scheme.errors[attribute[:id].to_sym][0] %>
+            </p>
+            <%= details_html(attribute) %>
+        </dd>
+    <% else %>
+        <dd class="govuk-summary-list__value">
+            <%= details_html(attribute) %>
+        </dd>
+    <% end %>
+    <% if !scheme.confirmed? || attribute[:name] == "Name" %>
+        <dd class="govuk-summary-list__actions">
+            <a class="govuk-link" href="<%= change_link %>">Change</a>
+        </dd>
+    <% end %>
+</div>
diff --git a/app/views/schemes/check_answers.html.erb b/app/views/schemes/check_answers.html.erb
index 701238d54..4d068e9ca 100644
--- a/app/views/schemes/check_answers.html.erb
+++ b/app/views/schemes/check_answers.html.erb
@@ -7,85 +7,31 @@
     <%= f.govuk_error_summary %>
       <%= govuk_tabs(title: "Check your answers before creating this scheme") do |component| %>
         <% component.tab(label: "Scheme") do %>
-          <%= govuk_summary_list do |summary_list| %>
+          <dl class="govuk-summary-list">
             <% @scheme.check_details_attributes.each do |attr| %>
               <% next if current_user.data_coordinator? && attr[:name] == ("owned by") %>
-              <%= summary_list.row do |row| %>
-                <% row.key { attr[:name].to_s } %>
-                <% row.value { details_html(attr) } %>
-                <% unless @scheme.confirmed? && attr[:name] != "Name" %>
-                  <% row.action(
-                      text: "Change",
-                      href: scheme_details_path(@scheme, check_answers: true),
-                    ) %>
-                  <% end %>
-              <% end %>
+              <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_details_path(@scheme, check_answers: true) } %>
             <% end %>
             <% if !@scheme.arrangement_type_same? %>
             <% @scheme.check_support_services_provider_attributes.each do |attr| %>
-                <%= summary_list.row do |row| %>
-                  <% row.key { attr[:name].to_s } %>
-                  <% row.value { details_html(attr) } %>
-                  <% unless @scheme.confirmed? %>
-                    <% row.action(
-                        text: "Change",
-                        href: scheme_support_services_provider_path(@scheme, check_answers: true),
-                      ) %>
-                    <% end %>
-                <% end %>
+              <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_support_services_provider_path(@scheme, check_answers: true) } %>
               <% end %>
             <% end %>
             <% @scheme.check_primary_client_attributes.each do |attr| %>
-              <%= summary_list.row do |row| %>
-                <% row.key { attr[:name].to_s } %>
-                <% row.value { details_html(attr) } %>
-                <% unless @scheme.confirmed? %>
-                  <% row.action(
-                      text: "Change",
-                      href: scheme_primary_client_group_path(@scheme, check_answers: true),
-                    ) %>
-                  <% end %>
-              <% end %>
+              <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_primary_client_group_path(@scheme, check_answers: true) } %>
             <% end %>
             <% @scheme.check_secondary_client_confirmation_attributes.each do |attr| %>
-              <%= summary_list.row do |row| %>
-                <% row.key { attr[:name].to_s } %>
-                <% row.value { details_html(attr) } %>
-                <% unless @scheme.confirmed? %>
-                  <% row.action(
-                      text: "Change",
-                      href: scheme_confirm_secondary_client_group_path(@scheme, check_answers: true),
-                    ) %>
-                <% end %>
-              <% end %>
+              <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_confirm_secondary_client_group_path(@scheme, check_answers: true) } %>
             <% end %>
             <% if @scheme.has_other_client_group == "Yes" %>
               <% @scheme.check_secondary_client_attributes.each do |attr| %>
-                <%= summary_list.row do |row| %>
-                  <% row.key { attr[:name].to_s } %>
-                  <% row.value { details_html(attr) } %>
-                  <% unless @scheme.confirmed? %>
-                    <% row.action(
-                        text: "Change",
-                        href: scheme_secondary_client_group_path(@scheme, check_answers: true),
-                      ) %>
-                    <% end %>
-                <% end %>
+                <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_secondary_client_group_path(@scheme, check_answers: true) } %>
               <% end %>
             <% end %>
             <% @scheme.check_support_attributes.each do |attr| %>
-              <%= summary_list.row do |row| %>
-                <% row.key { attr[:name].to_s } %>
-                <% row.value { details_html(attr) } %>
-                <% unless @scheme.confirmed? %>
-                  <% row.action(
-                      text: "Change",
-                      href: scheme_support_path(@scheme, check_answers: true),
-                    ) %>
-                <% end %>
-              <% end %>
+              <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_support_path(@scheme, check_answers: true) } %>
             <% end %>
-          <% end %>
+          </dl>
         <% end %>
         <% component.tab(label: "Locations") do %>
           <%= govuk_table do |table| %>