diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb index 1468bc013..be5d3f7ef 100644 --- a/app/controllers/schemes_controller.rb +++ b/app/controllers/schemes_controller.rb @@ -145,8 +145,8 @@ class SchemesController < ApplicationController if @scheme.errors.empty? && @scheme.update(scheme_params) @scheme.update!(secondary_client_group: nil) if @scheme.has_other_client_group == "No" if scheme_params[:confirmed] == "true" || @scheme.confirmed? - if check_answers && confirm_secondary_page?(page) - redirect_to scheme_secondary_client_group_path(@scheme, check_answers: "true") + if check_answers && should_direct_via_secondary_client_group_page?(page) + redirect_to scheme_secondary_client_group_path(@scheme, referrer: "check-answers") else @scheme.locations.update!(confirmed: true) flash[:notice] = if scheme_previously_confirmed @@ -157,8 +157,8 @@ class SchemesController < ApplicationController redirect_to scheme_path(@scheme) end elsif check_answers - if confirm_secondary_page?(page) - redirect_to scheme_secondary_client_group_path(@scheme, check_answers: "true") + if should_direct_via_secondary_client_group_page?(page) + redirect_to scheme_secondary_client_group_path(@scheme, referrer: "check-answers") else redirect_to scheme_check_answers_path(@scheme) end @@ -249,8 +249,8 @@ private end end - def confirm_secondary_page?(page) - page == "confirm-secondary" && @scheme.has_other_client_group == "Yes" + def should_direct_via_secondary_client_group_page?(page) + page == "confirm-secondary" && @scheme.has_other_client_group == "Yes" && @scheme.secondary_client_group.nil? end def current_template(page) diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb index 7efb9fffd..bcd40b082 100644 --- a/app/helpers/schemes_helper.rb +++ b/app/helpers/schemes_helper.rb @@ -64,15 +64,15 @@ module SchemesHelper def change_answer_link(scheme, question_id, user) case question_id when "service_name", "sensitive", "scheme_type", "registered_under_care_act", "owning_organisation_id", "arrangement_type" - user.support? || !scheme.confirmed? ? scheme_details_path(scheme, check_answers: true) : scheme_edit_name_path(scheme) + user.support? || !scheme.confirmed? ? scheme_details_path(scheme, referrer: "check-answers") : scheme_edit_name_path(scheme) when "primary_client_group" - scheme_primary_client_group_path(scheme, check_answers: true) + scheme_primary_client_group_path(scheme, referrer: "check-answers") when "has_other_client_group" - scheme_confirm_secondary_client_group_path(scheme, check_answers: true) + scheme_confirm_secondary_client_group_path(scheme, referrer: "check-answers") when "secondary_client_group" - scheme_secondary_client_group_path(scheme, check_answers: true) + scheme_secondary_client_group_path(scheme, referrer: "check-answers") when "support_type", "intended_stay" - scheme_support_path(scheme, check_answers: true) + scheme_support_path(scheme, referrer: "check-answers") end end diff --git a/app/views/schemes/confirm_secondary.html.erb b/app/views/schemes/confirm_secondary.html.erb index 7e7cfeebf..9b715c264 100644 --- a/app/views/schemes/confirm_secondary.html.erb +++ b/app/views/schemes/confirm_secondary.html.erb @@ -1,9 +1,7 @@ <% content_for :title, "Does this scheme provide for another client group?" %> <% content_for :before_content do %> - <%= govuk_back_link( - href: request.query_parameters["check_answers"] ? "check-answers" : "primary-client-group", - ) %> + <%= govuk_back_link(href: :back) %> <% end %> <%= render partial: "organisations/headings", locals: { main: "Does this scheme provide for another client group?", sub: @scheme.service_name } %> @@ -22,7 +20,7 @@ legend: nil %> <%= f.hidden_field :page, value: "confirm-secondary" %> - <% if request.query_parameters["check_answers"] == "true" %> + <% if params[:referrer] == "check-answers" %> <%= f.hidden_field :check_answers, value: "true" %> <%= f.govuk_submit "Save changes" %> <% else %> diff --git a/app/views/schemes/details.html.erb b/app/views/schemes/details.html.erb index 71f924782..cb29a56dc 100644 --- a/app/views/schemes/details.html.erb +++ b/app/views/schemes/details.html.erb @@ -75,7 +75,7 @@ legend: { text: "Who provides the support services used by this scheme?", size: "m" } %> <%= f.hidden_field :page, value: "details" %> - <% if request.query_parameters["check_answers"] %> + <% if params[:referrer] == "check-answers" %> <%= f.hidden_field :check_answers, value: "true" %> <%= f.govuk_submit "Save changes" %> <% else %> diff --git a/app/views/schemes/edit_name.html.erb b/app/views/schemes/edit_name.html.erb index 030635b79..2cb0f791a 100644 --- a/app/views/schemes/edit_name.html.erb +++ b/app/views/schemes/edit_name.html.erb @@ -39,7 +39,7 @@ <%= f.hidden_field :page, value: "edit-name" %> - <% if request.query_parameters["check_answers"] %> + <% if params[:referrer] == "check-answers" %> <%= f.hidden_field :check_answers, value: "true" %> <% end %> diff --git a/app/views/schemes/primary_client_group.html.erb b/app/views/schemes/primary_client_group.html.erb index 85db05b55..55dc504b4 100644 --- a/app/views/schemes/primary_client_group.html.erb +++ b/app/views/schemes/primary_client_group.html.erb @@ -1,9 +1,9 @@ <% content_for :title, "What client group is this scheme intended for?" %> -<% if request.referer&.include?("new") || request.referer&.include?("details") %> +<% if request.referer&.include?("new") %> <% back_button_path = scheme_details_path(@scheme) %> -<% elsif request.query_parameters["check_answers"] %> - <% back_button_path = scheme_check_answers_path(@scheme) %> +<% else %> + <% back_button_path = :back %> <% end %> <% content_for :before_content do %> @@ -30,7 +30,7 @@ <%= f.hidden_field :check_answers, value: "true" %> <% end %> - <% if request.query_parameters["check_answers"] == "true" %> + <% if params[:referrer] == "check-answers" %> <%= f.hidden_field :check_answers, value: "true" %> <%= f.govuk_submit "Save changes" %> <% else %> diff --git a/app/views/schemes/secondary_client_group.html.erb b/app/views/schemes/secondary_client_group.html.erb index 1e70fd92b..cd50283e3 100644 --- a/app/views/schemes/secondary_client_group.html.erb +++ b/app/views/schemes/secondary_client_group.html.erb @@ -1,9 +1,7 @@ <% content_for :title, "What is the other client group?" %> <% content_for :before_content do %> - <%= govuk_back_link( - href: request.query_parameters["check_answers"] ? "check-answers" : "confirm-secondary-client-group", - ) %> + <%= govuk_back_link(href: :back) %> <% end %> <%= form_for(@scheme, method: :patch) do |f| %> @@ -22,7 +20,7 @@ legend: nil %> <%= f.hidden_field :page, value: "secondary-client-group" %> - <% if request.query_parameters["check_answers"] == "true" %> + <% if params[:referrer] == "check-answers" %> <%= f.hidden_field :check_answers, value: "true" %> <%= f.govuk_submit "Save changes" %> <% else %> diff --git a/app/views/schemes/support.html.erb b/app/views/schemes/support.html.erb index a004a3da9..a5d30ed7f 100644 --- a/app/views/schemes/support.html.erb +++ b/app/views/schemes/support.html.erb @@ -1,9 +1,7 @@ <% content_for :title, "What support does this scheme provide?" %> <% content_for :before_content do %> - <%= govuk_back_link( - href: request.query_parameters["check_answers"] ? "check-answers" : "secondary-client-group", - ) %> + <%= govuk_back_link(href: :back) %> <% end %> <%= form_for(@scheme, method: :patch) do |f| %> @@ -38,7 +36,7 @@ <%= f.hidden_field :page, value: "support" %> - <% if request.query_parameters["check_answers"] == "true" %> + <% if params[:referrer] == "check-answers" %> <%= f.hidden_field :check_answers, value: "true" %> <%= f.govuk_submit "Save changes" %> <% else %> diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 0706a0d7e..33ab00b34 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -644,8 +644,8 @@ RSpec.describe "Schemes scheme Features" do end it "allows changing details questions" do - click_link("Change", href: "/schemes/#{scheme.id}/details?check_answers=true", match: :first) - expect(page).to have_current_path("/schemes/#{scheme.id}/details?check_answers=true") + click_link("Change", href: "/schemes/#{scheme.id}/details?referrer=check-answers", match: :first) + expect(page).to have_current_path("/schemes/#{scheme.id}/details?referrer=check-answers") fill_in "Scheme name", with: "Example" click_button "Save changes" @@ -655,14 +655,14 @@ RSpec.describe "Schemes scheme Features" do end it "lets me select the support answers after navigating back" do - click_link("Change", href: "/schemes/#{scheme.id}/details?check_answers=true", match: :first) + click_link("Change", href: "/schemes/#{scheme.id}/details?referrer=check-answers", match: :first) click_link "Back" 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}/confirm-secondary-client-group?check_answers=true", match: :first) + click_link("Change", href: "/schemes/#{scheme.id}/confirm-secondary-client-group?referrer=check-answers", match: :first) choose "Yes" click_button "Save changes" visit("/schemes/#{scheme.id}/check-answers") @@ -710,8 +710,8 @@ RSpec.describe "Schemes scheme Features" do end it "allows changing details questions" do - click_link("Change", href: "/schemes/#{scheme.id}/details?check_answers=true", match: :first) - expect(page).to have_current_path("/schemes/#{scheme.id}/details?check_answers=true") + click_link("Change", href: "/schemes/#{scheme.id}/details?referrer=check-answers", match: :first) + expect(page).to have_current_path("/schemes/#{scheme.id}/details?referrer=check-answers") fill_in "Scheme name", with: "Example" click_button "Save changes" @@ -721,21 +721,14 @@ RSpec.describe "Schemes scheme Features" do end it "lets me select the support answers after navigating back" do - click_link("Change", href: "/schemes/#{scheme.id}/details?check_answers=true", match: :first) + click_link("Change", href: "/schemes/#{scheme.id}/details?referrer=check-answers", match: :first) click_link "Back" 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 switching between other provider options" do - click_link("Change", href: "/schemes/#{scheme.id}/confirm-secondary-client-group?check_answers=true", match: :first) - choose "Yes" - click_button "Save changes" - expect(find_field("Offenders and people at risk of offending")).to be_checked - 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) + click_link("Change", href: "/schemes/#{scheme.id}/details?referrer=check-answers", match: :first) choose "The same organisation that owns the housing stock" click_button "Save changes" expect(page).not_to have_content("Organisation providing support") @@ -787,11 +780,11 @@ RSpec.describe "Schemes scheme Features" do context "when I click to change scheme name" do before do - click_link("Change", href: "/schemes/#{scheme.id}/details?check_answers=true", match: :first) + click_link("Change", href: "/schemes/#{scheme.id}/details?referrer=check-answers", match: :first) end it "shows available fields to edit" do - expect(page).to have_current_path("/schemes/#{scheme.id}/details?check_answers=true") + expect(page).to have_current_path("/schemes/#{scheme.id}/details?referrer=check-answers") expect(page).to have_content "Scheme details" end diff --git a/spec/requests/schemes_controller_spec.rb b/spec/requests/schemes_controller_spec.rb index d93bc873d..19ede5cc4 100644 --- a/spec/requests/schemes_controller_spec.rb +++ b/spec/requests/schemes_controller_spec.rb @@ -1405,10 +1405,11 @@ RSpec.describe SchemesController, type: :request do expect(scheme_to_update.reload.has_other_client_group).to eq("Yes") end - context "when updating from check answers page with the answer YES" do + context "when updating from check answers page with the answer YES and no secondary client group set" do + let(:scheme_to_update) { create(:scheme, owning_organisation: user.organisation, confirmed: nil, secondary_client_group: nil) } let(:params) { { scheme: { has_other_client_group: "Yes", page: "confirm-secondary", check_answers: "true" } } } - it "renders check answers page after successful update" do + it "renders secondary client group page after successful update" do follow_redirect! expect(response).to have_http_status(:ok) expect(page).to have_content("What is the other client group?") @@ -1420,6 +1421,22 @@ RSpec.describe SchemesController, type: :request do end end + context "when updating from check answers page with the answer YES and a secondary client group set" do + let(:scheme_to_update) { create(:scheme, owning_organisation: user.organisation, confirmed: nil, secondary_client_group: "F") } + let(:params) { { scheme: { has_other_client_group: "Yes", page: "confirm-secondary", check_answers: "true" } } } + + it "renders check answers page after successful update" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content("Check your changes before creating this scheme") + end + + it "updates a scheme with valid params" do + follow_redirect! + expect(scheme_to_update.reload.has_other_client_group).to eq("Yes") + end + end + context "when updating from check answers page with the answer NO" do let(:params) { { scheme: { has_other_client_group: "No", page: "confirm-secondary", check_answers: "true" } } } @@ -1716,10 +1733,11 @@ RSpec.describe SchemesController, type: :request do expect(scheme_to_update.reload.has_other_client_group).to eq("Yes") end - context "when updating from check answers page with the answer YES" do + context "when updating from check answers page with the answer YES and no existing secondary client group set" do + let(:scheme_to_update) { create(:scheme, owning_organisation: user.organisation, confirmed: nil, secondary_client_group: nil) } let(:params) { { scheme: { has_other_client_group: "Yes", page: "confirm-secondary", check_answers: "true" } } } - it "renders check answers page after successful update" do + it "renders secondary client group page after successful update" do follow_redirect! expect(response).to have_http_status(:ok) expect(page).to have_content("What is the other client group?") @@ -1731,6 +1749,22 @@ RSpec.describe SchemesController, type: :request do end end + context "when updating from check answers page with the answer YES and an existing secondary client group set" do + let(:scheme_to_update) { create(:scheme, owning_organisation: user.organisation, confirmed: nil, secondary_client_group: "F") } + let(:params) { { scheme: { has_other_client_group: "Yes", page: "confirm-secondary", check_answers: "true" } } } + + it "renders check answers page after successful update" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content("Check your changes before creating this scheme") + end + + it "updates a scheme with valid params" do + follow_redirect! + expect(scheme_to_update.reload.has_other_client_group).to eq("Yes") + end + end + context "when updating from check answers page with the answer NO" do let(:params) { { scheme: { has_other_client_group: "No", page: "confirm-secondary", check_answers: "true" } } }