Browse Source

Refactor flow to store value when selecting new org option

pull/1580/head
Jack S 3 years ago
parent
commit
b4158184a4
  1. 14
      app/controllers/merge_requests_controller.rb
  2. 3
      app/views/merge_requests/absorbing_organisation.html.erb
  3. 5
      db/migrate/20230421124536_add_absorbing_organisation_to_merge_request.rb
  4. 1
      db/schema.rb
  5. 29
      spec/requests/merge_requests_controller_spec.rb

14
app/controllers/merge_requests_controller.rb

@ -10,7 +10,6 @@ class MergeRequestsController < ApplicationController
] ]
before_action :authenticate_user! before_action :authenticate_user!
before_action :authenticate_scope!, except: [:create] before_action :authenticate_scope!, except: [:create]
before_action :redirect_to_next_page
def absorbing_organisation; end def absorbing_organisation; end
def confirm_telephone_number; end def confirm_telephone_number; end
@ -77,10 +76,6 @@ private
page page
end end
def redirect_to_next_page
redirect_to next_page_path if create_new_org?
end
def create_new_org? def create_new_org?
params.dig(:merge_request, :absorbing_organisation_id) == "other" params.dig(:merge_request, :absorbing_organisation_id) == "other"
end end
@ -106,6 +101,15 @@ private
merge_params[:requesting_organisation_id] = current_user.organisation.id merge_params[:requesting_organisation_id] = current_user.organisation.id
end end
if merge_params[:absorbing_organisation_id].present?
if create_new_org?
merge_params[:new_absorbing_organisation] = true
merge_params[:absorbing_organisation_id] = nil
else
merge_params[:new_absorbing_organisation] = false
end
end
merge_params merge_params
end end

3
app/views/merge_requests/absorbing_organisation.html.erb

@ -23,11 +23,10 @@
:absorbing_organisation_id, :absorbing_organisation_id,
org.id, org.id,
label: { text: org.name }, label: { text: org.name },
link_errors: true,
) %> ) %>
<% end %> <% end %>
<%= f.govuk_radio_divider %> <%= f.govuk_radio_divider %>
<%= f.govuk_radio_button :absorbing_organisation_id, "other", label: { text: "These organisations are merging into a new one" } %> <%= f.govuk_radio_button :absorbing_organisation_id, "other", checked: @merge_request.new_absorbing_organisation?, label: { text: "These organisations are merging into a new one" } %>
<% end %> <% end %>
<%= f.hidden_field :page, value: "absorbing_organisation" %> <%= f.hidden_field :page, value: "absorbing_organisation" %>

5
db/migrate/20230421124536_add_absorbing_organisation_to_merge_request.rb

@ -1,5 +1,8 @@
class AddAbsorbingOrganisationToMergeRequest < ActiveRecord::Migration[7.0] class AddAbsorbingOrganisationToMergeRequest < ActiveRecord::Migration[7.0]
def change def change
add_column :merge_requests, :absorbing_organisation_id, :integer change_table :merge_requests, bulk: true do |t|
t.column :absorbing_organisation_id, :integer
t.column :new_absorbing_organisation, :boolean
end
end end
end end

1
db/schema.rb

@ -368,6 +368,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_21_124536) do
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.integer "status" t.integer "status"
t.integer "absorbing_organisation_id" t.integer "absorbing_organisation_id"
t.boolean "new_absorbing_organisation"
end end
create_table "organisation_relationships", force: :cascade do |t| create_table "organisation_relationships", force: :cascade do |t|

29
spec/requests/merge_requests_controller_spec.rb

@ -112,7 +112,7 @@ RSpec.describe MergeRequestsController, type: :request do
end end
context "when the user selects an organisation that is a part of another merge" do context "when the user selects an organisation that is a part of another merge" do
let(:another_organisation) { create(:organisation, name: "Other Test Org") } let(:another_organisation) { create(:organisation) }
let(:params) { { merge_request: { merging_organisation: another_organisation.id } } } let(:params) { { merge_request: { merging_organisation: another_organisation.id } } }
before do before do
@ -130,7 +130,7 @@ RSpec.describe MergeRequestsController, type: :request do
end end
context "when the user selects an organisation that is a part of another unsubmitted merge" do context "when the user selects an organisation that is a part of another unsubmitted merge" do
let(:another_organisation) { create(:organisation, name: "Other Test Org") } let(:another_organisation) { create(:organisation) }
let(:params) { { merge_request: { merging_organisation: another_organisation.id } } } let(:params) { { merge_request: { merging_organisation: another_organisation.id } } }
before do before do
@ -147,7 +147,7 @@ RSpec.describe MergeRequestsController, type: :request do
end end
context "when the user selects an organisation that is a part of current merge" do context "when the user selects an organisation that is a part of current merge" do
let(:another_organisation) { create(:organisation, name: "Other Test Org") } let(:another_organisation) { create(:organisation) }
let(:params) { { merge_request: { merging_organisation: another_organisation.id } } } let(:params) { { merge_request: { merging_organisation: another_organisation.id } } }
before do before do
@ -261,20 +261,31 @@ RSpec.describe MergeRequestsController, type: :request do
end end
context "when absorbing_organisation_id set to other" do context "when absorbing_organisation_id set to other" do
let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
let(:params) do let(:params) do
{ merge_request: { absorbing_organisation_id: "other", page: "absorbing_organisation" } } { merge_request: { absorbing_organisation_id: "other", page: "absorbing_organisation" } }
end end
let(:request) do
before do
patch "/merge-request/#{merge_request.id}", headers:, params: patch "/merge-request/#{merge_request.id}", headers:, params:
end end
it "redirects to new org path" do it "redirects to new org path" do
request
expect(response).to redirect_to(new_org_name_merge_request_path(merge_request)) expect(response).to redirect_to(new_org_name_merge_request_path(merge_request))
end end
it "resets absorbing_organisation and sets new_absorbing_organisation to true" do
expect { request }.to change {
merge_request.reload.absorbing_organisation
}.from(other_organisation).to(nil).and change {
merge_request.reload.new_absorbing_organisation
}.from(nil).to(true)
end
end end
context "when absorbing_organisation_id set to id" do context "when absorbing_organisation_id set to id" do
let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_absorbing_organisation: true) }
let(:params) do let(:params) do
{ merge_request: { absorbing_organisation_id: other_organisation.id, page: "absorbing_organisation" } } { merge_request: { absorbing_organisation_id: other_organisation.id, page: "absorbing_organisation" } }
end end
@ -289,8 +300,12 @@ RSpec.describe MergeRequestsController, type: :request do
expect(response).to redirect_to(confirm_telephone_number_merge_request_path(merge_request)) expect(response).to redirect_to(confirm_telephone_number_merge_request_path(merge_request))
end end
it "updates the merge request" do it "updates absorbing_organisation_id and sets new_absorbing_organisation to false" do
expect { request }.to change { merge_request.reload.absorbing_organisation_id }.from(nil).to(other_organisation.id) expect { request }.to change {
merge_request.reload.absorbing_organisation
}.from(nil).to(other_organisation).and change {
merge_request.reload.new_absorbing_organisation
}.from(true).to(false)
end end
end end
end end

Loading…
Cancel
Save