From 5193abdef4804cf8f888e2199cead9e740d43fae Mon Sep 17 00:00:00 2001 From: Jack S Date: Tue, 25 Apr 2023 16:45:25 +0100 Subject: [PATCH] Handle next page and previous_template with `page` --- app/controllers/merge_requests_controller.rb | 71 ++++++++++++------- .../absorbing_organisation.html.erb | 4 +- .../merge_requests/organisations.html.erb | 1 + .../merge_requests_controller_spec.rb | 6 +- 4 files changed, 51 insertions(+), 31 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index f0ff6e602..36a4f66c5 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -1,7 +1,20 @@ class MergeRequestsController < ApplicationController - before_action :find_resource, only: %i[update organisations update_organisations remove_merging_organisation absorbing_organisation confirm_telephone_number new_org_name] + before_action :find_resource, only: %i[ + update + organisations + update_organisations + remove_merging_organisation + absorbing_organisation + confirm_telephone_number + new_org_name + ] before_action :authenticate_user! before_action :authenticate_scope!, except: [:create] + before_action :redirect_to_next_page + + def absorbing_organisation; end + def confirm_telephone_number; end + def new_org_name; end def create ActiveRecord::Base.transaction do @@ -13,18 +26,12 @@ class MergeRequestsController < ApplicationController render_not_found end - def absorbing_organisation; end - def confirm_telephone_number; end - def new_org_name; end - def organisations @answer_options = organisations_answer_options end def update - if params.dig(:merge_request, :absorbing_organisation_id) == "other" - redirect_to next_page_path - elsif @merge_request.update(merge_request_params) + if @merge_request.update(merge_request_params) redirect_to next_page_path else render previous_template, status: :unprocessable_entity @@ -49,6 +56,35 @@ class MergeRequestsController < ApplicationController private + def page + params.dig(:merge_request, :page) + end + + def next_page_path + case page + when "absorbing_organisation" + if create_new_org? + new_org_name_merge_request_path(@merge_request) + else + confirm_telephone_number_merge_request_path(@merge_request) + end + when "organisations" + absorbing_organisation_merge_request_path(@merge_request) + end + end + + def previous_template + page + end + + def redirect_to_next_page + redirect_to next_page_path if create_new_org? + end + + def create_new_org? + params.dig(:merge_request, :absorbing_organisation_id) == "other" + end + def organisations_answer_options answer_options = { "" => "Select an option" } @@ -84,25 +120,6 @@ private @merge_request = MergeRequest.find(params[:id]) end - def next_page_path - if params.dig(:merge_request, :absorbing_organisation_id) == "other" - # TODO: flow to be implemented in follow up PR - new_org_name_merge_request_path(@merge_request) - elsif params.dig(:merge_request, :absorbing_organisation_id).present? - confirm_telephone_number_merge_request_path(@merge_request) - else - absorbing_organisation_merge_request_path(@merge_request) - end - end - - def previous_template - if params.dig(:merge_request, :absorbing_organisation_id).present? - :absorbing_organisation - else - :organisations - end - end - def authenticate_scope! if current_user.organisation != @merge_request.requesting_organisation && !current_user.support? render_not_found diff --git a/app/views/merge_requests/absorbing_organisation.html.erb b/app/views/merge_requests/absorbing_organisation.html.erb index 386dde311..8807dcdcd 100644 --- a/app/views/merge_requests/absorbing_organisation.html.erb +++ b/app/views/merge_requests/absorbing_organisation.html.erb @@ -15,7 +15,7 @@ <%= f.govuk_radio_buttons_fieldset( :absorbing_organisation_id, - hint: { text: "For example, if Skype and Yammer merged into Microsoft, you would select Microsoft. " }, + hint: { text: "For example, if Skype and Yammer merged into Microsoft, you would select Microsoft." }, legend: nil, ) do %> <% @merge_request.merging_organisations.order(:name).each do |org| %> @@ -30,6 +30,8 @@ <%= f.govuk_radio_button :absorbing_organisation_id, "other", label: { text: "These organisations are merging into a new one" } %> <% end %> + <%= f.hidden_field :page, value: "absorbing_organisation" %> + <%= f.govuk_submit %> <% end %> diff --git a/app/views/merge_requests/organisations.html.erb b/app/views/merge_requests/organisations.html.erb index 1cc5d3b03..221fbe87e 100644 --- a/app/views/merge_requests/organisations.html.erb +++ b/app/views/merge_requests/organisations.html.erb @@ -44,6 +44,7 @@ <%= f.govuk_text_area :other_merging_organisations, label: { text: "Other organisations" }, hint: { text: "List other organisations that are part of the merge but not registered on CORE." }, rows: 9 %> <% end %> <% if @merge_request.merging_organisations.count > 1 %> + <%= f.hidden_field :page, value: "organisations" %> <%= f.govuk_submit "Continue" %> <% end %> <% end %> diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index 602001fd6..2486c7925 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -238,7 +238,7 @@ RSpec.describe MergeRequestsController, type: :request do describe "#other_merging_organisations" do let(:other_merging_organisations) { "A list of other merging organisations" } - let(:params) { { merge_request: { other_merging_organisations: } } } + let(:params) { { merge_request: { other_merging_organisations:, page: "organisations" } } } let(:request) do patch "/merge-request/#{merge_request.id}", headers:, params: end @@ -262,7 +262,7 @@ RSpec.describe MergeRequestsController, type: :request do context "when absorbing_organisation_id set to other" do let(:params) do - { merge_request: { absorbing_organisation_id: "other" } } + { merge_request: { absorbing_organisation_id: "other", page: "absorbing_organisation" } } end before do @@ -276,7 +276,7 @@ RSpec.describe MergeRequestsController, type: :request do context "when absorbing_organisation_id set to id" do let(:params) do - { merge_request: { absorbing_organisation_id: other_organisation.id } } + { merge_request: { absorbing_organisation_id: other_organisation.id, page: "absorbing_organisation" } } end let(:request) do