Cldc 1394 schemes cya (#797)

* Mark scheme location as confirmed

* Route to check your answers after any changes are made to the scheme. Display change for all editable fields

* Add some functionality

* Display banners and redirect to check answers after editing location name

* Remove a test

* update locations confirmation

* reuse can_change_scheme_answer?

* Use path helpers

* Redirect to view scheme when trying to access create form pages for the scheme

(cherry picked from commit 761bf97dc2)

* add before action for confirmed schemes

* update location path

* lint

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

app/controllers/locations_controller.rb

@@ -56,7 +56,7 @@ class LocationsController < ApplicationController
       when "edit"
         location_params[:add_another_location] == "Yes" ? redirect_to(new_location_path(@location.scheme)) : redirect_to(scheme_check_answers_path(@scheme, anchor: "locations"))
       when "edit-name"
-        redirect_to(locations_path(@scheme))
+        redirect_to(scheme_check_answers_path(@scheme, anchor: "locations"))
       end
     else
       render :edit, status: :unprocessable_entity

app/controllers/schemes_controller.rb

@@ -5,9 +5,9 @@ class SchemesController < ApplicationController
   before_action :authenticate_user!
   before_action :find_resource, except: %i[index]
   before_action :authenticate_scope!
+  before_action :redirect_if_scheme_confirmed, only: %i[primary_client_group confirm_secondary_client_group secondary_client_group support details]
 
   def index
-    flash[:notice] = "#{Scheme.find(params[:scheme_id].to_i).service_name} has been created." if params[:scheme_id]
     redirect_to schemes_organisation_path(current_user.organisation) unless current_user.support?
     all_schemes = Scheme.all.order("service_name ASC")
 
@@ -52,10 +52,19 @@ class SchemesController < ApplicationController
 
     check_answers = params[:scheme][:check_answers]
     page = params[:scheme][:page]
+    scheme_previously_confirmed = @scheme.confirmed?
 
     validation_errors scheme_params
     if @scheme.errors.empty? && @scheme.update(scheme_params)
-      if check_answers
+      if scheme_params[:confirmed] == "true"
+        @scheme.locations.update!(confirmed: true)
+        flash[:notice] = if scheme_previously_confirmed
+                           "#{@scheme.service_name} has been updated."
+                         else
+                           "#{@scheme.service_name} has been created."
+                         end
+        redirect_to scheme_path(@scheme)
+      elsif check_answers
         if confirm_secondary_page? page
           redirect_to scheme_secondary_client_group_path(@scheme, check_answers: "true")
         else
@@ -177,7 +186,7 @@ private
         scheme_details_path(@scheme)
       end
     when "edit-name"
-      scheme_path(@scheme)
+      scheme_check_answers_path(@scheme)
     when "check-answers"
       schemes_path(scheme_id: @scheme.id)
     end
@@ -246,4 +255,8 @@ private
       render_not_found and return
     end
   end
+
+  def redirect_if_scheme_confirmed
+    redirect_to @scheme if @scheme.confirmed?
+  end
 end

app/helpers/check_answers_helper.rb

@@ -11,6 +11,19 @@ module CheckAnswersHelper
     end
   end
 
+  def can_change_scheme_answer?(attribute_name, scheme)
+    editable_attributes = current_user.support? ? ["Name", "Confidential information", "Housing stock owned by"] : ["Name", "Confidential information"]
+    !scheme.confirmed? || editable_attributes.include?(attribute_name)
+  end
+
+  def get_location_change_link_href(scheme, location)
+    if location.confirmed?
+      location_edit_name_path(id: scheme.id, location_id: location.id)
+    else
+      location_edit_path(id: scheme.id, location_id: location.id)
+    end
+  end
+
 private
 
   def answered_questions_count(subsection, case_log, current_user)

app/views/locations/edit.html.erb

@@ -7,7 +7,9 @@
   ) %>
 <% end %>
 
-<%= form_for(@location, method: :patch, url: location_path) do |f| %>
+<%= render partial: "organisations/headings", locals: { main: "Add a location to this scheme", sub: @scheme.service_name } %>
+
+<%= form_for(@location, method: :patch, url: location_path(@location)) do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
       <%= f.govuk_error_summary %>

app/views/schemes/_scheme_summary_list_row.html.erb

@@ -1,4 +1,4 @@
-<div class="<%= "govuk-summary-list__row #{scheme.confirmed? && attribute[:name] != 'Name' ? 'govuk-summary-list__row--no-actions' : ''}" %>">
+<div class="<%= "govuk-summary-list__row #{scheme.confirmed? && !can_change_scheme_answer?(attribute[:name], @scheme) ? 'govuk-summary-list__row--no-actions' : ''}" %>">
     <dt class="govuk-summary-list__key">
         <%= attribute[:name].to_s %>
     </dt>
@@ -14,7 +14,7 @@
             <%= details_html(attribute) %>
         </dd>
     <% end %>
-    <% if !scheme.confirmed? || attribute[:name] == "Name" %>
+    <% if can_change_scheme_answer?(attribute[:name], scheme) %>
         <dd class="govuk-summary-list__actions">
             <a class="govuk-link" href="<%= change_link %>">Change</a>
         </dd>

app/views/schemes/check_answers.html.erb

@@ -10,7 +10,7 @@
       <dl class="govuk-summary-list">
         <% @scheme.check_details_attributes.each do |attr| %>
           <% next if current_user.data_coordinator? && attr[:name] == ("owned by") %>
-          <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_details_path(@scheme, check_answers: true) } %>
+          <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: @scheme.confirmed? ? scheme_edit_name_path(@scheme) : scheme_details_path(@scheme, check_answers: true) } %>
         <% end %>
         <% if !@scheme.arrangement_type_same? %>
         <% @scheme.check_support_services_provider_attributes.each do |attr| %>
@@ -68,7 +68,7 @@
           <%= table.body do |body| %>
             <%= body.row do |row| %>
               <% row.cell(text: location.id) %>
-              <% row.cell(text: simple_format(location_cell(location, "/schemes/#{@scheme.id}/locations/#{location.id}/edit"), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %>
+              <% row.cell(text: simple_format(location_cell(location, get_location_change_link_href(@scheme, location)), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %>
               <% row.cell(text: location.units) %>
               <% row.cell(text: simple_format("<span>#{location.type_of_unit}</span>")) %>
               <% row.cell(text: location.mobility_type) %>

config/routes.rb

@@ -48,6 +48,7 @@ Rails.application.routes.draw do
     member do
       resources :locations do
         get "edit-name", to: "locations#edit_name"
+        get "edit", to: "locations#edit"
       end
     end
   end

db/migrate/20220729110846_add_confirmed_location.rb

@@ -0,0 +1,7 @@
+class AddConfirmedLocation < ActiveRecord::Migration[7.0]
+  def change
+    change_table :locations, bulk: true do |t|
+      t.column :confirmed, :boolean
+    end
+  end
+end

db/schema.rb

@@ -250,6 +250,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_08_02_125711) do
     t.string "mobility_type"
     t.datetime "startdate", precision: nil
     t.string "location_admin_district"
+    t.boolean "confirmed"
     t.index ["old_id"], name: "index_locations_on_old_id", unique: true
     t.index ["scheme_id"], name: "index_locations_on_scheme_id"
   end

spec/features/schemes_spec.rb

@@ -284,13 +284,34 @@ RSpec.describe "Schemes scheme Features" do
                   expect(page).not_to have_button("Create scheme")
                 end
 
+                it "allows you to edit the newly added location" do
+                  click_link "Locations"
+                  expect(page).to have_link(nil, href: /edit/)
+                end
+
+                context "when you click save" do
+                  it "displays a updated banner" do
+                    click_button "Save"
+                    expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
+                    expect(page).to have_content("has been updated")
+                  end
+
+                  it "does not let you edit the saved location" do
+                    click_link "Locations"
+                    expect(page).to have_link(nil, href: /edit(?!-name)/)
+                    click_button "Save"
+                    click_link "Locations"
+                    expect(page).not_to have_link(nil, href: /edit(?!-name)/)
+                  end
+                end
+
                 context "when you click to view the scheme details" do
                   before do
                     click_link("Scheme")
                   end
 
-                  it "does not let you change details other than the name" do
+                  it "does not let you change details other than the name, confidential information and housing stock owner" do
-                    assert_selector "a", text: "Change", count: 1
+                    assert_selector "a", text: "Change", count: 3
                   end
                 end
               end
@@ -451,6 +472,7 @@ RSpec.describe "Schemes scheme Features" do
 
           it "lets me check my answers after adding a location" do
             fill_in_and_save_location
+            expect(page).to have_current_path("/schemes/#{scheme.id}/check-answers")
             expect(page).to have_content "Check your changes before creating this scheme"
           end
 
@@ -561,11 +583,12 @@ RSpec.describe "Schemes scheme Features" do
           end
 
           it "adds scheme to the list of schemes" do
+            expect(page).to have_content "#{scheme.service_name} has been created."
+            click_link "Schemes"
             expect(page).to have_content "Supported housing schemes"
             expect(page).to have_content scheme.id_to_display
             expect(page).to have_content scheme.service_name
             expect(page).to have_content scheme.owning_organisation.name
-            expect(page).to have_content "#{scheme.service_name} has been created."
           end
         end
 
@@ -704,9 +727,17 @@ RSpec.describe "Schemes scheme Features" do
                 click_button "Save changes"
               end
 
-              it "lets me see amended details on the show page" do
+              it "lets me see amended details on the check answers page" do
                 expect(page).to have_content "FooBar"
+                expect(page).to have_current_path("/schemes/#{scheme.id}/check-answers")
+                assert_selector "a", text: "Change", count: 3
+              end
+
+              it "lets me save the scheme" do
+                click_button "Save"
                 expect(page).to have_current_path("/schemes/#{scheme.id}")
+                expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
+                expect(page).to have_content("has been updated")
               end
             end
           end
@@ -748,10 +779,10 @@ RSpec.describe "Schemes scheme Features" do
                   click_button "Save and continue"
                 end
 
-                it "returns to locations page and shows the new name" do
+                it "returns to locations check your answers page and shows the new name" do
                   expect(page).to have_content location.id
                   expect(page).to have_content "NewName"
-                  expect(page).to have_current_path("/schemes/#{scheme.id}/locations")
+                  expect(page.current_url.split("/").last).to eq("check-answers#locations")
                 end
               end
             end
@@ -834,8 +865,8 @@ RSpec.describe "Schemes scheme Features" do
                     click_link("Scheme")
                   end
 
-                  it "does not let you change details other than the name" do
+                  it "does not let you change details other than the name, Confidential information	and Housing stock owned by" do
-                    assert_selector "a", text: "Change", count: 1
+                    assert_selector "a", text: "Change", count: 3
                   end
                 end
               end
@@ -846,6 +877,55 @@ RSpec.describe "Schemes scheme Features" do
     end
   end
 
+  context "when I am signed in as a data coordinator" do
+    let(:user) { FactoryBot.create(:user, :data_coordinator, last_sign_in_at: Time.zone.now) }
+    let!(:schemes) { FactoryBot.create_list(:scheme, 5, owning_organisation_id: user.organisation_id) }
+
+    before do
+      visit("/logs")
+      fill_in("user[email]", with: user.email)
+      fill_in("user[password]", with: user.password)
+      click_button("Sign in")
+    end
+
+    context "when editing a scheme" do
+      context "when I visit schemes page" do
+        before do
+          visit("schemes")
+        end
+
+        context "when I click to see individual scheme" do
+          let(:scheme) { schemes.first }
+
+          before do
+            FactoryBot.create(:location, scheme:)
+            click_link(scheme.service_name)
+          end
+
+          context "when I click to change scheme name" do
+            before do
+              click_link("Change", href: "/schemes/#{scheme.id}/edit-name", match: :first)
+            end
+
+            context "when I edit details" do
+              before do
+                fill_in "Scheme name", with: "FooBar"
+                check "This scheme contains confidential information"
+                click_button "Save changes"
+              end
+
+              it "lets me see amended details on the check answers page" do
+                expect(page).to have_content "FooBar"
+                expect(page).to have_current_path("/schemes/#{scheme.id}/check-answers")
+                expect(page).to have_link("Change", href:  /schemes\/#{scheme.id}\/edit-name/, count: 2)
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+
   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, managing_organisation: user.organisation, arrangement_type: "The same organisation that owns the housing stock") }

spec/requests/schemes_controller_spec.rb

@@ -37,14 +37,6 @@ RSpec.describe SchemesController, type: :request do
         get "/schemes"
       end
 
-      context "when params scheme_id is present" do
-        it "shows a success banner" do
-          get "/schemes", params: { scheme_id: schemes.first.id }
-          follow_redirect!
-          expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
-        end
-      end
-
       it "redirects to the organisation schemes path" do
         follow_redirect!
         expect(path).to match("/organisations/#{user.organisation.id}/schemes")
@@ -93,13 +85,6 @@ RSpec.describe SchemesController, type: :request do
         expect(CGI.unescape_html(response.body)).to match("<strong>#{schemes.count}</strong> total schemes")
       end
 
-      context "when params scheme_id is present" do
-        it "shows a success banner" do
-          get "/schemes", params: { scheme_id: schemes.first.id }
-          expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
-        end
-      end
-
       context "when paginating over 20 results" do
         let(:total_schemes_count) { Scheme.count }
 
@@ -884,9 +869,11 @@ RSpec.describe SchemesController, type: :request do
 
     context "when signed in as a support" do
       let(:user) { FactoryBot.create(:user, :support) }
-      let(:scheme_to_update) { FactoryBot.create(:scheme, owning_organisation: user.organisation) }
+      let(:scheme_to_update) { FactoryBot.create(:scheme, owning_organisation: user.organisation, confirmed: nil) }
+      # let!(:location) { FactoryBot.create(:location, scheme: scheme_to_update) }
 
       before do
+        FactoryBot.create(:location, scheme: scheme_to_update)
         allow(user).to receive(:need_two_factor_authentication?).and_return(false)
         sign_in user
         patch "/schemes/#{scheme_to_update.id}", params:
@@ -939,6 +926,21 @@ RSpec.describe SchemesController, type: :request do
             expect(scheme_to_update.reload.primary_client_group).to eq("Homeless families with support needs")
           end
         end
+
+        context "when saving a scheme" do
+          let(:params) { { scheme: { page: "check-answers", confirmed: "true" } } }
+
+          it "marks the scheme as confirmed" do
+            expect(scheme_to_update.reload.confirmed?).to eq(true)
+          end
+
+          it "marks all the scheme locations as confirmed" do
+            expect(scheme_to_update.locations.count > 0).to eq(true)
+            scheme_to_update.locations.each do |location|
+              expect(location.confirmed?).to eq(true)
+            end
+          end
+        end
       end
 
       context "when updating primary client group" do
@@ -1175,7 +1177,7 @@ RSpec.describe SchemesController, type: :request do
 
     context "when signed in as a data coordinator" do
       let(:user) { FactoryBot.create(:user, :data_coordinator) }
-      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) }
+      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation, confirmed: nil) }
       let!(:another_scheme) { FactoryBot.create(:scheme) }
 
       before do
@@ -1202,7 +1204,7 @@ RSpec.describe SchemesController, type: :request do
 
     context "when signed in as a support user" do
       let(:user) { FactoryBot.create(:user, :support) }
-      let!(:scheme) { FactoryBot.create(:scheme) }
+      let!(:scheme) { FactoryBot.create(:scheme, confirmed: nil) }
 
       before do
         allow(user).to receive(:need_two_factor_authentication?).and_return(false)
@@ -1214,6 +1216,21 @@ RSpec.describe SchemesController, type: :request do
         expect(response).to have_http_status(:ok)
         expect(page).to have_content("What client group is this scheme intended for?")
       end
+
+      context "and the scheme is confirmed" do
+        before do
+          scheme.update!(confirmed: true)
+          get "/schemes/#{scheme.id}/primary-client-group"
+        end
+
+        it "redirects to a view scheme page" do
+          follow_redirect!
+          expect(response).to have_http_status(:ok)
+          expect(path).to match("/schemes/#{scheme.id}")
+          expect(page).to have_content(scheme.service_name)
+          assert_select "a", text: /Change/, count: 3
+        end
+      end
     end
   end
 
@@ -1241,7 +1258,7 @@ RSpec.describe SchemesController, type: :request do
 
     context "when signed in as a data coordinator" do
       let(:user) { FactoryBot.create(:user, :data_coordinator) }
-      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) }
+      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation, confirmed: nil) }
       let!(:another_scheme) { FactoryBot.create(:scheme) }
 
       before do
@@ -1268,7 +1285,7 @@ RSpec.describe SchemesController, type: :request do
 
     context "when signed in as a support user" do
       let(:user) { FactoryBot.create(:user, :support) }
-      let!(:scheme) { FactoryBot.create(:scheme) }
+      let!(:scheme) { FactoryBot.create(:scheme, confirmed: nil) }
 
       before do
         allow(user).to receive(:need_two_factor_authentication?).and_return(false)
@@ -1280,6 +1297,21 @@ RSpec.describe SchemesController, type: :request do
         expect(response).to have_http_status(:ok)
         expect(page).to have_content("Does this scheme provide for another client group?")
       end
+
+      context "and the scheme is confirmed" do
+        before do
+          scheme.update!(confirmed: true)
+          get "/schemes/#{scheme.id}/confirm-secondary-client-group"
+        end
+
+        it "redirects to a view scheme page" do
+          follow_redirect!
+          expect(response).to have_http_status(:ok)
+          expect(path).to match("/schemes/#{scheme.id}")
+          expect(page).to have_content(scheme.service_name)
+          assert_select "a", text: /Change/, count: 3
+        end
+      end
     end
   end
 
@@ -1307,7 +1339,7 @@ RSpec.describe SchemesController, type: :request do
 
     context "when signed in as a data coordinator" do
       let(:user) { FactoryBot.create(:user, :data_coordinator) }
-      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) }
+      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation, confirmed: nil) }
       let!(:another_scheme) { FactoryBot.create(:scheme) }
 
       before do
@@ -1334,7 +1366,7 @@ RSpec.describe SchemesController, type: :request do
 
     context "when signed in as a support user" do
       let(:user) { FactoryBot.create(:user, :support) }
-      let!(:scheme) { FactoryBot.create(:scheme, primary_client_group: Scheme::PRIMARY_CLIENT_GROUP[:"Homeless families with support needs"]) }
+      let!(:scheme) { FactoryBot.create(:scheme, confirmed: nil, primary_client_group: Scheme::PRIMARY_CLIENT_GROUP[:"Homeless families with support needs"]) }
 
       before do
         allow(user).to receive(:need_two_factor_authentication?).and_return(false)
@@ -1347,6 +1379,21 @@ RSpec.describe SchemesController, type: :request do
         expect(page).to have_content("What is the other client group?")
       end
 
+      context "and the scheme is confirmed" do
+        before do
+          scheme.update!(confirmed: true)
+          get "/schemes/#{scheme.id}/secondary-client-group"
+        end
+
+        it "redirects to a view scheme page" do
+          follow_redirect!
+          expect(response).to have_http_status(:ok)
+          expect(path).to match("/schemes/#{scheme.id}")
+          expect(page).to have_content(scheme.service_name)
+          assert_select "a", text: /Change/, count: 3
+        end
+      end
+
       it "does not show the primary client group as an option" do
         expect(scheme.primary_client_group).not_to be_nil
         expect(page).not_to have_content("Homeless families with support needs")
@@ -1378,7 +1425,7 @@ RSpec.describe SchemesController, type: :request do
 
     context "when signed in as a data coordinator" do
       let(:user) { FactoryBot.create(:user, :data_coordinator) }
-      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) }
+      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation, confirmed: nil) }
       let!(:another_scheme) { FactoryBot.create(:scheme) }
 
       before do
@@ -1401,11 +1448,26 @@ RSpec.describe SchemesController, type: :request do
           expect(response).to have_http_status(:not_found)
         end
       end
+
+      context "and the scheme is confirmed" do
+        before do
+          scheme.update!(confirmed: true)
+          get "/schemes/#{scheme.id}/support"
+        end
+
+        it "redirects to a view scheme page" do
+          follow_redirect!
+          expect(response).to have_http_status(:ok)
+          expect(path).to match("/schemes/#{scheme.id}")
+          expect(page).to have_content(scheme.service_name)
+          assert_select "a", text: /Change/, count: 2
+        end
+      end
     end
 
     context "when signed in as a support user" do
       let(:user) { FactoryBot.create(:user, :support) }
-      let!(:scheme) { FactoryBot.create(:scheme) }
+      let!(:scheme) { FactoryBot.create(:scheme, confirmed: nil) }
 
       before do
         allow(user).to receive(:need_two_factor_authentication?).and_return(false)
@@ -1510,7 +1572,7 @@ RSpec.describe SchemesController, type: :request do
 
     context "when signed in as a data coordinator" do
       let(:user) { FactoryBot.create(:user, :data_coordinator) }
-      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) }
+      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation, confirmed: nil) }
       let!(:another_scheme) { FactoryBot.create(:scheme) }
 
       before do
@@ -1533,11 +1595,26 @@ RSpec.describe SchemesController, type: :request do
           expect(response).to have_http_status(:not_found)
         end
       end
+
+      context "and the scheme is confirmed" do
+        before do
+          scheme.update!(confirmed: true)
+          get "/schemes/#{scheme.id}/details"
+        end
+
+        it "redirects to a view scheme page" do
+          follow_redirect!
+          expect(response).to have_http_status(:ok)
+          expect(path).to match("/schemes/#{scheme.id}")
+          expect(page).to have_content(scheme.service_name)
+          assert_select "a", text: /Change/, count: 2
+        end
+      end
     end
 
     context "when signed in as a support user" do
       let(:user) { FactoryBot.create(:user, :support) }
-      let!(:scheme) { FactoryBot.create(:scheme) }
+      let!(:scheme) { FactoryBot.create(:scheme, confirmed: nil) }
 
       before do
         allow(user).to receive(:need_two_factor_authentication?).and_return(false)