Cldc 1377 scheme status (#775)

* reset managing org for sheme

* only display schemes with managing org id

* only display relevant answers in show scheme page

* tests

* check arrangement type from saved scheme if incoming params for it is nil

* don't override managing org id if it's coming through as nil

* convert to use values that won't change for checks

* Revert "convert to use values that won't change for checks" because the value coming through from the params is the value and not a key

This reverts commit 50053ec0cf233e3a6c36db9af2528c354d3d60e7.

* Test

* check constant values

* Add test case for controller, check if the scheme is confirmed before displaying

* validate all the required scheme questions are answered

* fix ui

* fix test

* Add test, change error message wording

* Display error message next to the relevant field

* update context and query

* lint

Co-authored-by: Dushan Despotovic <dushan@madetech.com>

app/controllers/schemes_controller.rb

@@ -103,6 +103,10 @@ private
         @scheme.errors.add(key.to_sym)
       end
     end
+
+    if @scheme.arrangement_type_same? && arrangement_type_value(scheme_params[:arrangement_type]) != "D"
+      @scheme.errors.delete(:managing_organisation_id)
+    end
   end
 
   def confirm_secondary_page?(page)
@@ -124,6 +128,8 @@ private
       "schemes/details"
     elsif page.include?("edit")
       "schemes/edit_name"
+    elsif page.include?("check-answers")
+      "schemes/check_answers"
     end
   end
 
@@ -170,14 +176,32 @@ private
                                                      :intended_stay,
                                                      :confirmed)
 
-    full_params = required_params[:arrangement_type] == "D" && required_params[:owning_organisation_id].present? ? required_params.merge(managing_organisation_id: required_params[:owning_organisation_id]) : required_params
+    if arrangement_type_changed_to_different_org?(required_params)
+      required_params[:managing_organisation_id] = nil
+    end
+
+    if arrangement_type_set_to_same_org?(required_params)
+      required_params[:managing_organisation_id] = required_params[:owning_organisation_id] || @scheme.owning_organisation_id
+    end
 
-    full_params[:sensitive] = full_params[:sensitive].to_i if full_params[:sensitive]
+    required_params[:sensitive] = required_params[:sensitive].to_i if required_params[:sensitive]
 
     if current_user.data_coordinator?
-      full_params[:owning_organisation_id] = current_user.organisation_id
+      required_params[:owning_organisation_id] = current_user.organisation_id
+    end
+    required_params
   end
-    full_params
+
+  def arrangement_type_set_to_same_org?(required_params)
+    arrangement_type_value(required_params[:arrangement_type]) == "D" || (required_params[:arrangement_type].blank? && @scheme.present? && @scheme.arrangement_type_same?)
+  end
+
+  def arrangement_type_changed_to_different_org?(required_params)
+    @scheme.present? && @scheme.arrangement_type_same? && arrangement_type_value(required_params[:arrangement_type]) != "D" && required_params[:managing_organisation_id].blank?
+  end
+
+  def arrangement_type_value(key)
+    key.present? ? Scheme::ARRANGEMENT_TYPE[key.to_sym] : nil
   end
 
   def search_term

app/frontend/styles/_errors.scss

@@ -0,0 +1,4 @@
+.app-summary-list__value--error {
+  border-left: $govuk-border-width solid $govuk-error-colour;
+  padding-left: govuk-spacing(3);
+}

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 {

app/helpers/details_table_helper.rb

@@ -4,7 +4,7 @@ module DetailsTableHelper
       list = attribute[:value].map { |value| "<li>#{value}</li>" }.join
       simple_format(list, { class: "govuk-list govuk-list--bullet" }, wrapper_tag: "ul")
     else
-      value = attribute[:value].is_a?(Array) ? attribute[:value].first : attribute[:value] || "None"
+      value = attribute[:value].is_a?(Array) ? attribute[:value].first : attribute[:value] || "<span class=\"app-!-colour-muted\">You didn’t answer this question</span>".html_safe
 
       simple_format(value.to_s, { class: "govuk-body" }, wrapper_tag: "p")
     end

app/models/form/setup/questions/scheme_id.rb

@@ -21,7 +21,7 @@ class Form::Setup::Questions::SchemeId < ::Form::Question
 
   def displayed_answer_options(case_log)
     organisation = case_log.owning_organisation || case_log.created_by&.organisation
-    schemes = organisation ? Scheme.select(:id).where(owning_organisation_id: organisation.id) : Scheme.select(:id)
+    schemes = organisation ? Scheme.select(:id).where(owning_organisation_id: organisation.id, confirmed: true) : Scheme.select(:id).where(confirmed: true)
     filtered_scheme_ids = schemes.joins(:locations).merge(Location.where("startdate <= ? or startdate IS NULL", Time.zone.today)).map(&:id)
     answer_options.select do |k, _v|
       filtered_scheme_ids.include?(k.to_i) || k.blank?

app/models/scheme.rb

@@ -9,6 +9,8 @@ class Scheme < ApplicationRecord
   scope :search_by_postcode, ->(postcode) { joins(:locations).where("locations.postcode ILIKE ?", "%#{postcode.delete(' ')}%") }
   scope :search_by, ->(param) { search_by_postcode(param).or(search_by_service_name(param)).or(filter_by_id(param)).distinct }
 
+  validate :validate_confirmed
+
   SENSITIVE = {
     No: 0,
     Yes: 1,
@@ -100,49 +102,49 @@ class Scheme < ApplicationRecord
 
   def check_details_attributes
     [
-      { name: "Service code", value: id_to_display },
+      { name: "Service code", value: id_to_display, id: "id" },
-      { name: "Name", value: service_name },
+      { name: "Name", value: service_name, id: "service_name" },
-      { name: "Confidential information", value: sensitive },
+      { name: "Confidential information", value: sensitive, id: "sensitive" },
-      { name: "Type of scheme", value: scheme_type },
+      { name: "Type of scheme", value: scheme_type, id: "scheme_type" },
-      { name: "Registered under Care Standards Act 2000", value: registered_under_care_act },
+      { 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 },
+      { name: "Housing stock owned by", value: owning_organisation.name, id: "owning_organisation_id" },
-      { name: "Support services provided by", value: arrangement_type },
+      { 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: "Level of support given", value: support_type, id: "support_type" },
-      { name: "Intended length of stay", value: intended_stay },
+      { name: "Intended length of stay", value: intended_stay, id: "intended_stay" },
     ]
   end
 
   def display_attributes
-    [
+    base_attributes = [
       { name: "Scheme code", value: id_to_display },
       { name: "Name", value: service_name, edit: true },
       { name: "Confidential information", value: sensitive, edit: true },
@@ -157,6 +159,11 @@ class Scheme < ApplicationRecord
       { name: "Level of support given", value: support_type },
       { name: "Intended length of stay", value: intended_stay },
     ]
+
+    if arrangement_type_same?
+      base_attributes.delete({ name: "Organisation providing support", value: managing_organisation&.name })
+    end
+    base_attributes
   end
 
   def synonyms
@@ -196,4 +203,22 @@ class Scheme < ApplicationRecord
     }
     Scheme.intended_stays.keys.excluding("Missing").map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize, description: hints[key.to_sym]) }
   end
+
+  def arrangement_type_same?
+    arrangement_type.present? && ARRANGEMENT_TYPE[arrangement_type.to_sym] == "D"
+  end
+
+  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? do |attribute|
+        if self[attribute].blank?
+          errors.add :base
+          errors.add attribute.to_sym
+          self.confirmed = false
+        end
+      end
+    end
+  end
 end

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>

app/views/schemes/check_answers.html.erb

@@ -1,87 +1,37 @@
 <% content_for :title, "Check your answers before creating this scheme" %>
 <%= render partial: "organisations/headings", locals: { main: "Check your changes before creating this scheme", sub: @scheme.service_name } %>
 
-<div class="govuk-grid-row">
+<%= form_for(@scheme, as: :scheme, method: :patch) do |f| %>
+  <div class="govuk-grid-row">
     <div class="govuk-grid-column-three-quarters-from-desktop">
+    <%= 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| %>
+              <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_details_path(@scheme, check_answers: true) } %>
-              <% 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 %>
             <% end %>
+            <% if !@scheme.arrangement_type_same? %>
             <% @scheme.check_support_services_provider_attributes.each do |attr| %>
-            <%= summary_list.row do |row| %>
+              <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_support_services_provider_path(@scheme, check_answers: true) } %>
-              <% 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 %>
             <% end %>
             <% @scheme.check_primary_client_attributes.each do |attr| %>
-            <%= summary_list.row do |row| %>
+              <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_primary_client_group_path(@scheme, check_answers: true) } %>
-              <% 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 %>
             <% end %>
             <% @scheme.check_secondary_client_confirmation_attributes.each do |attr| %>
-            <%= summary_list.row do |row| %>
+              <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_confirm_secondary_client_group_path(@scheme, check_answers: true) } %>
-              <% 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 %>
             <% end %>
             <% if @scheme.has_other_client_group == "Yes" %>
               <% @scheme.check_secondary_client_attributes.each do |attr| %>
-              <%= summary_list.row do |row| %>
+                <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_secondary_client_group_path(@scheme, check_answers: true) } %>
-                <% 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 %>
               <% end %>
             <% end %>
             <% @scheme.check_support_attributes.each do |attr| %>
-            <%= summary_list.row do |row| %>
+              <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_support_path(@scheme, check_answers: true) } %>
-              <% 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 %>
-          <% end %>
             <% end %>
+          </dl>
         <% end %>
         <% component.tab(label: "Locations") do %>
           <%= govuk_table do |table| %>
@@ -127,9 +77,7 @@
         <% end %>
       <% end %>
     </div>
-</div>
+  </div>
-
-<%= form_for(@scheme, as: :scheme, method: :patch) do |f| %>
   <%= f.hidden_field :page, value: "check-answers" %>
   <%= f.hidden_field :confirmed, value: "true" %>
   <% button_label = @scheme.confirmed? ? "Save" : "Create scheme" %>

config/locales/en.yml

@@ -65,6 +65,8 @@ en:
               invalid: "Select if this scheme provides for another client group"
             arrangement_type:
               invalid: "Select who provides the support services used by this scheme"
+            base:
+              invalid: "You must answer all the required questions for this scheme"
         location:
           attributes:
             startdate:

spec/factories/scheme.rb

@@ -10,6 +10,7 @@ FactoryBot.define do
     primary_client_group { %w[O H M L A G F B D E I S N R Q P X].sample }
     secondary_client_group { %w[O H M L A G F B D E I S N R Q P X].sample }
     owning_organisation { FactoryBot.create(:organisation) }
+    managing_organisation { FactoryBot.create(:organisation) }
     confirmed { true }
     created_at { Time.zone.now }
     trait :export do

spec/features/form/accessible_autocomplete_spec.rb

@@ -60,7 +60,7 @@ RSpec.describe "Accessible Automcomplete" do
   end
 
   context "when searching schemes" do
-    let(:scheme) { FactoryBot.create(:scheme, owning_organisation_id: case_log.created_by.organisation_id, primary_client_group: "Q", secondary_client_group: "P") }
+    let(:scheme) { FactoryBot.create(:scheme, owning_organisation_id: case_log.created_by.organisation_id, managing_organisation_id: case_log.created_by.organisation_id, primary_client_group: "Q", secondary_client_group: "P") }
 
     before do
       FactoryBot.create(:location, scheme:, postcode: "W6 0ST")
@@ -76,7 +76,7 @@ RSpec.describe "Accessible Automcomplete" do
 
     it "displays appended text next to the options", js: true do
       find("#case-log-scheme-id-field").click.native.send_keys("w", "6", :down, :enter)
-      expect(find(".autocomplete__option__append", visible: :hidden, text: scheme.service_name)).to be_present
+      expect(find(".autocomplete__option", visible: :hidden, text: scheme.service_name)).to be_present
       expect(find("span", visible: :hidden, text: "2 locations")).to be_present
     end
 

spec/features/schemes_spec.rb

@@ -519,7 +519,7 @@ RSpec.describe "Schemes scheme Features" do
           end
 
           it "displays change links" do
-            assert_selector "a", text: "Change", count: 13
+            assert_selector "a", text: "Change", count: 12
           end
 
           it "allows changing details questions" do
@@ -539,6 +539,13 @@ RSpec.describe "Schemes scheme Features" do
             expect(page).to have_current_path("/schemes/#{scheme.id}/check-answers")
             expect(page).to have_content "Check your changes before creating this scheme"
           end
+
+          it "indicates if the scheme is not complete" do
+            click_link("Change", href: "/schemes/#{scheme.id}/details?check_answers=true", match: :first)
+            choose "Another registered housing provider"
+            click_button "Save and continue"
+            expect(page).to have_content("You didn’t answer this question")
+          end
         end
 
         context "when selecting 'create a scheme'" do
@@ -623,6 +630,20 @@ RSpec.describe "Schemes scheme Features" do
               expect(page).to have_current_path("/schemes/#{scheme.id}/check-answers")
               expect(page).to have_content "Check your changes before creating this scheme"
             end
+
+            it "keeps the provider answer when swithing between other provider options" do
+              click_link("Change", href: "/schemes/#{scheme.id}/details?check_answers=true", match: :first)
+              choose "Another organisation"
+              click_button "Save and continue"
+              expect(page).to have_content(another_organisation.name)
+            end
+
+            it "does not display the answer if it's changed to the same support provider" do
+              click_link("Change", href: "/schemes/#{scheme.id}/details?check_answers=true", match: :first)
+              choose "The same organisation that owns the housing stock"
+              click_button "Save and continue"
+              expect(page).not_to have_content("Organisation providing support")
+            end
           end
         end
       end
@@ -822,7 +843,7 @@ RSpec.describe "Schemes scheme Features" do
 
   context "when selecting a scheme" do
     let!(:user) { FactoryBot.create(:user, :data_coordinator, last_sign_in_at: Time.zone.now) }
-    let!(:schemes) { FactoryBot.create_list(:scheme, 5, owning_organisation: user.organisation) }
+    let!(:schemes) { FactoryBot.create_list(:scheme, 5, owning_organisation: user.organisation, managing_organisation: user.organisation, arrangement_type: "The same organisation that owns the housing stock") }
     let(:location) { FactoryBot.create(:location, scheme: schemes[2]) }
     let!(:case_log) { FactoryBot.create(:case_log, created_by: user, needstype: 2) }
 
@@ -865,5 +886,11 @@ RSpec.describe "Schemes scheme Features" do
       visit("/logs/#{case_log.id}/scheme")
       expect(find("#case-log-scheme-id-field").all("option").count).to eq(4)
     end
+
+    it "does display the schemes that are not completed" do
+      schemes[2].update!(confirmed: false)
+      visit("/logs/#{case_log.id}/scheme")
+      expect(find("#case-log-scheme-id-field").all("option").count).to eq(3)
+    end
   end
 end

spec/models/form/setup/questions/scheme_id_spec.rb

@@ -43,7 +43,7 @@ RSpec.describe Form::Setup::Questions::SchemeId, type: :model do
     let(:organisation) { FactoryBot.create(:organisation) }
     let(:organisation_2) { FactoryBot.create(:organisation) }
     let(:user) { FactoryBot.create(:user, organisation:) }
-    let(:scheme) { FactoryBot.create(:scheme, owning_organisation: organisation) }
+    let(:scheme) { FactoryBot.create(:scheme, owning_organisation: organisation, managing_organisation: organisation) }
     let(:case_log) { FactoryBot.create(:case_log, created_by: user, needstype: 2) }
 
     before do

spec/requests/form_controller_spec.rb

@@ -110,7 +110,8 @@ RSpec.describe FormController, type: :request do
               end
 
               before do
-                FactoryBot.create_list(:location, 5)
+                locations = FactoryBot.create_list(:location, 5)
+                locations.each { |location| location.scheme.update!(arrangement_type: "The same organisation that owns the housing stock", managing_organisation_id: location.scheme.owning_organisation_id) }
               end
 
               it "returns an unfiltered list of schemes" do

spec/requests/schemes_controller_spec.rb

@@ -557,13 +557,22 @@ RSpec.describe SchemesController, type: :request do
 
     context "when signed in as a data coordinator" do
       let(:user) { FactoryBot.create(:user, :data_coordinator) }
-      let(:scheme_to_update) { FactoryBot.create(:scheme, owning_organisation: user.organisation) }
+      let(:scheme_to_update) { FactoryBot.create(:scheme, owning_organisation: user.organisation, confirmed: nil) }
 
       before do
         sign_in user
         patch "/schemes/#{scheme_to_update.id}", params:
       end
 
+      context "when confirming unfinished scheme" do
+        let(:params) { { scheme: { owning_organisation_id: user.organisation.id, arrangement_type: "V", confirmed: true, page: "check-answers" } } }
+
+        it "does not allow the scheme to be confirmed" do
+          expect(response).to have_http_status(:unprocessable_entity)
+          expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.base.invalid"))
+        end
+      end
+
       context "when params are missing" do
         let(:params) do
           { scheme: {
@@ -612,7 +621,7 @@ RSpec.describe SchemesController, type: :request do
       end
 
       context "when updating support services provider" do
-        let(:params) { { scheme: { managing_organisation_id: organisation.id, page: "support-services-provider" } } }
+        let(:params) { { scheme: { arrangement_type: "Another organisation", managing_organisation_id: organisation.id, page: "support-services-provider" } } }
 
         it "renders primary client group after successful update" do
           follow_redirect!
@@ -786,6 +795,7 @@ RSpec.describe SchemesController, type: :request do
                       registered_under_care_act: "No",
                       page: "details",
                       owning_organisation_id: organisation.id,
+                      managing_organisation_id: organisation.id,
                       arrangement_type: "D" } }
         end
 
@@ -1043,7 +1053,7 @@ RSpec.describe SchemesController, type: :request do
                       scheme_type: "Foyer",
                       registered_under_care_act: "No",
                       page: "details",
-                      arrangement_type: "D",
+                      arrangement_type: "The same organisation that owns the housing stock",
                       owning_organisation_id: another_organisation.id } }
         end
 
@@ -1078,6 +1088,7 @@ RSpec.describe SchemesController, type: :request do
             expect(scheme_to_update.reload.scheme_type).to eq("Foyer")
             expect(scheme_to_update.reload.sensitive).to eq("Yes")
             expect(scheme_to_update.reload.registered_under_care_act).to eq("No")
+            expect(scheme_to_update.reload.managing_organisation_id).to eq(scheme_to_update.owning_organisation_id)
           end
         end
       end