diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb index de8a2d835..b88b895fb 100644 --- a/app/controllers/locations_controller.rb +++ b/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 diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb index c10784c2d..d4168e355 100644 --- a/app/controllers/schemes_controller.rb +++ b/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 diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 982cab7cc..760c08ab0 100644 --- a/app/helpers/check_answers_helper.rb +++ b/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) diff --git a/app/views/locations/edit.html.erb b/app/views/locations/edit.html.erb index 6fe2496b2..361e050ef 100644 --- a/app/views/locations/edit.html.erb +++ b/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| %>
<%= f.govuk_error_summary %> diff --git a/app/views/schemes/_scheme_summary_list_row.html.erb b/app/views/schemes/_scheme_summary_list_row.html.erb index a02e5d25f..df8939df1 100644 --- a/app/views/schemes/_scheme_summary_list_row.html.erb +++ b/app/views/schemes/_scheme_summary_list_row.html.erb @@ -1,4 +1,4 @@ -
"> +
">
<%= attribute[:name].to_s %>
@@ -14,7 +14,7 @@ <%= details_html(attribute) %> <% end %> - <% if !scheme.confirmed? || attribute[:name] == "Name" %> + <% if can_change_scheme_answer?(attribute[:name], scheme) %>
Change
diff --git a/app/views/schemes/check_answers.html.erb b/app/views/schemes/check_answers.html.erb index 7319259f4..f2f13c7e1 100644 --- a/app/views/schemes/check_answers.html.erb +++ b/app/views/schemes/check_answers.html.erb @@ -10,7 +10,7 @@
<% @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("#{location.type_of_unit}")) %> <% row.cell(text: location.mobility_type) %> diff --git a/config/routes.rb b/config/routes.rb index a6b0b9654..65130cd35 100644 --- a/config/routes.rb +++ b/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 diff --git a/db/migrate/20220729110846_add_confirmed_location.rb b/db/migrate/20220729110846_add_confirmed_location.rb new file mode 100644 index 000000000..3656a999e --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 8ead19234..417f03725 100644 --- a/db/schema.rb +++ b/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 diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 9792dfc42..2df56bb3d 100644 --- a/spec/features/schemes_spec.rb +++ b/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 - assert_selector "a", text: "Change", count: 1 + it "does not let you change details other than the name, confidential information and housing stock owner" do + 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 - assert_selector "a", text: "Change", count: 1 + 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: 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") } diff --git a/spec/requests/schemes_controller_spec.rb b/spec/requests/schemes_controller_spec.rb index 3ccc9e461..fe133253f 100644 --- a/spec/requests/schemes_controller_spec.rb +++ b/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("#{schemes.count} 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)