From 0c34ff7968a10ffda48f314010a1135b2dc330dd Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 13 Aug 2024 13:39:33 +0100
Subject: [PATCH] CLDC-3584 Update merging orgs question (#2567)
* Update merging organisations question
* Update rebase tests
* Update routing between CYA and questions
---
app/controllers/merge_requests_controller.rb | 34 +++--
app/helpers/merge_requests_helper.rb | 22 ++-
app/models/merge_request_organisation.rb | 5 -
.../merging_organisations.html.erb | 45 ++++++
.../merge_requests/organisations.html.erb | 52 -------
...ated_organisation_select_question.html.erb | 2 +-
.../add_managing_agent.html.erb | 1 +
.../add_stock_owner.html.erb | 1 +
config/routes.rb | 6 +-
...13072041_remove_other_merging_org_field.rb | 5 +
db/schema.rb | 1 -
.../merge_requests_controller_spec.rb | 136 ++++++------------
.../merge_requests/show.html.erb_spec.rb | 9 --
13 files changed, 141 insertions(+), 178 deletions(-)
create mode 100644 app/views/merge_requests/merging_organisations.html.erb
delete mode 100644 app/views/merge_requests/organisations.html.erb
create mode 100644 db/migrate/20240813072041_remove_other_merging_org_field.rb
diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb
index 6d7738204..46798d261 100644
--- a/app/controllers/merge_requests_controller.rb
+++ b/app/controllers/merge_requests_controller.rb
@@ -2,10 +2,10 @@ class MergeRequestsController < ApplicationController
before_action :find_resource, exclude: %i[create new]
before_action :authenticate_user!
before_action :authenticate_scope!
- before_action :set_organisations_answer_options, only: %i[organisations absorbing_organisation update_organisations remove_merging_organisation update]
+ before_action :set_organisations_answer_options, only: %i[merging_organisations absorbing_organisation update_merging_organisations remove_merging_organisation update]
def absorbing_organisation; end
- def organisations; end
+ def merging_organisations; end
def confirm_telephone_number; end
def new_organisation_name; end
def new_organisation_address; end
@@ -32,18 +32,18 @@ class MergeRequestsController < ApplicationController
end
end
- def update_organisations
+ def update_merging_organisations
merge_request_organisation = MergeRequestOrganisation.new(merge_request_organisation_params)
if merge_request_organisation.save
- render :organisations
+ render :merging_organisations
else
- render :organisations, status: :unprocessable_entity
+ render :merging_organisations, status: :unprocessable_entity
end
end
def remove_merging_organisation
MergeRequestOrganisation.find_by(merge_request_organisation_params)&.destroy!
- render :organisations
+ render :merging_organisations
end
private
@@ -53,10 +53,12 @@ private
end
def next_page_path
+ return merge_request_path if is_referrer_type?("check_answers")
+
case page
when "absorbing_organisation"
- organisations_merge_request_path(@merge_request)
- when "organisations"
+ merging_organisations_merge_request_path(@merge_request)
+ when "merging_organisations"
if create_new_organisation?
new_organisation_name_merge_request_path(@merge_request)
else
@@ -96,7 +98,6 @@ private
def merge_request_params
merge_params = params.fetch(:merge_request, {}).permit(
:requesting_organisation_id,
- :other_merging_organisations,
:status,
:absorbing_organisation_id,
:telephone_number_correct,
@@ -162,4 +163,19 @@ private
render_not_found
end
end
+
+ def is_referrer_type?(referrer_type)
+ from_referrer_query("referrer") == referrer_type
+ end
+
+ def from_referrer_query(query_param)
+ referrer = request.headers["HTTP_REFERER"]
+ return unless referrer
+
+ query_params = URI.parse(referrer).query
+ return unless query_params
+
+ parsed_params = CGI.parse(query_params)
+ parsed_params[query_param]&.first
+ end
end
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index 9f6afe1df..4975a238f 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -13,9 +13,9 @@ module MergeRequestsHelper
def merge_details(merge_request)
[
- { label: "Absorbing organisation", value: display_value_or_placeholder(merge_request.absorbing_organisation_name), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: absorbing_organisation_merge_request_path(merge_request), visually_hidden_text: "absorbing organisation" } },
- { label: "Merging organisations", value: merge_request.merge_request_organisations.any? ? merge_request.merge_request_organisations.map(&:merging_organisation_name).join("
").html_safe : display_value_or_placeholder(nil), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: organisations_merge_request_path(merge_request), visually_hidden_text: "merging organisations" } },
- { label: "Merge date", value: display_value_or_placeholder(merge_request.merge_date), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: merge_date_merge_request_path(merge_request), visually_hidden_text: "merge date" } },
+ { label: "Absorbing organisation", value: display_value_or_placeholder(merge_request.absorbing_organisation_name), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: absorbing_organisation_merge_request_path(merge_request, referrer: "check_answers"), visually_hidden_text: "absorbing organisation" } },
+ { label: "Merging organisations", value: merge_request.merge_request_organisations.any? ? merge_request.merge_request_organisations.map(&:merging_organisation_name).join("
").html_safe : display_value_or_placeholder(nil), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: merging_organisations_merge_request_path(merge_request, referrer: "check_answers"), visually_hidden_text: "merging organisations" } },
+ { label: "Merge date", value: display_value_or_placeholder(merge_request.merge_date), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: merge_date_merge_request_path(merge_request, referrer: "check_answers"), visually_hidden_text: "merge date" } },
]
end
@@ -27,4 +27,20 @@ module MergeRequestsHelper
{ label: "Total stock owners & managing agents after merge", value: merge_request.total_stock_owners.present? || merge_request.total_managing_agents.present? ? "#{merge_request.total_stock_owners} stock owners
#{merge_request.total_managing_agents} managing agents".html_safe : display_value_or_placeholder(nil), action: { text: "View", href: "#", visually_hidden_text: "total stock owners & managing agents after merge" } },
]
end
+
+ def ordered_merging_organisations(merge_request)
+ merge_request.merge_request_organisations.order(created_at: :desc).map(&:merging_organisation)
+ end
+
+ def submit_merge_request_button_text(referrer)
+ if accessed_from_check_answers?(referrer)
+ "Save changes"
+ else
+ "Save and continue"
+ end
+ end
+
+ def accessed_from_check_answers?(referrer)
+ %w[check_answers].include?(referrer)
+ end
end
diff --git a/app/models/merge_request_organisation.rb b/app/models/merge_request_organisation.rb
index fd57b5e51..e181ca304 100644
--- a/app/models/merge_request_organisation.rb
+++ b/app/models/merge_request_organisation.rb
@@ -26,11 +26,6 @@ private
merge_request.errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
end
- if MergeRequest.merged.where.not(id: merge_request_id).where(requesting_organisation: merging_organisation).count.positive?
- errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
- merge_request.errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
- end
-
if merging_organisation_id.blank? || !Organisation.where(id: merging_organisation_id).exists?
merge_request.errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_not_selected"))
end
diff --git a/app/views/merge_requests/merging_organisations.html.erb b/app/views/merge_requests/merging_organisations.html.erb
new file mode 100644
index 000000000..f7dbeb378
--- /dev/null
+++ b/app/views/merge_requests/merging_organisations.html.erb
@@ -0,0 +1,45 @@
+<% content_for :before_content do %>
+ <% title = "Tell us if your organisation is merging" %>
+ <% content_for :title, title %>
+ <%= govuk_back_link href: merge_request_organisation_path(id: @merge_request.requesting_organisation_id) %>
+<% end %>
+
+<%= form_with model: @merge_request, url: merging_organisations_merge_request_path, method: :patch do |f| %>
+ <%= f.govuk_error_summary %>
+
Which organisations are merging into <%= @merge_request.absorbing_organisation&.name %>?
+
+
+
+
Add all organisations that are merging.
+
+ <%= render partial: "organisation_relationships/related_organisation_select_question", locals: {
+ label: { text: "Select an organisation", class: "govuk-label--m" },
+ field: :merging_organisation,
+ question: Form::Question.new("", { "answer_options" => @answer_options.reject { |id, _org_name| id != "" && id == @merge_request.absorbing_organisation_id } }, nil),
+ f:,
+ } %>
+ <%= f.govuk_submit "Add organisation", secondary: true, classes: "govuk-button--secondary" %>
+ <%= govuk_table do |table| %>
+ <% ordered_merging_organisations(@merge_request).each do |merging_organisation| %>
+ <%= table.with_body do |body| %>
+ <%= body.with_row do |row| %>
+ <% row.with_cell(text: merging_organisation.name) %>
+ <% row.with_cell(html_attributes: {
+ scope: "row",
+ class: "govuk-!-text-align-right",
+ }) do %>
+ <%= govuk_link_to("Remove", merging_organisations_remove_merge_request_path(merge_request: { merging_organisation: merging_organisation.id })) %>
+ <% end %>
+ <% end %>
+ <% end %>
+ <% end %>
+ <% end %>
+ <% end %>
+ <%= form_with model: @merge_request, url: merge_request_path(id: @merge_request.id), method: :patch do |f| %>
+ <% if @merge_request.merging_organisations.count.positive? %>
+ <%= f.hidden_field :page, value: "merging_organisations" %>
+ <%= f.govuk_submit submit_merge_request_button_text(request.query_parameters["referrer"]) %>
+ <% end %>
+ <% end %>
+
+
diff --git a/app/views/merge_requests/organisations.html.erb b/app/views/merge_requests/organisations.html.erb
deleted file mode 100644
index 6b6a0783c..000000000
--- a/app/views/merge_requests/organisations.html.erb
+++ /dev/null
@@ -1,52 +0,0 @@
-<% content_for :before_content do %>
- <% title = "Tell us if your organisation is merging" %>
- <% content_for :title, title %>
- <%= govuk_back_link href: merge_request_organisation_path(id: @merge_request.requesting_organisation_id) %>
-<% end %>
-
-<%= form_with model: @merge_request, url: organisations_merge_request_path, method: :patch do |f| %>
- <%= f.govuk_error_summary %>
- Which organisations are merging?
-
-
-
-
- Add all organisations to be merged - we have already added your own.
-
-
-
Start typing to search
- <%= render partial: "organisation_relationships/related_organisation_select_question", locals: {
- field: :merging_organisation,
- question: Form::Question.new("", { "answer_options" => @answer_options }, nil),
- f:,
- } %>
- <%= f.govuk_submit "Add organisation", classes: "govuk-button--secondary" %>
- <%= govuk_table do |table| %>
- <% @merge_request.merging_organisations.order(:name).each do |merging_organisation| %>
- <%= table.with_body do |body| %>
- <%= body.with_row do |row| %>
- <% row.with_cell(text: merging_organisation.name) %>
- <% row.with_cell(html_attributes: {
- scope: "row",
- class: "govuk-!-text-align-right",
- }) do %>
- <% if @merge_request.requesting_organisation != merging_organisation %>
- <%= govuk_link_to("Remove", organisations_remove_merge_request_path(merge_request: { merging_organisation: merging_organisation.id })) %>
- <% end %>
- <% end %>
- <% end %>
- <% end %>
- <% end %>
- <% end %>
- <% end %>
- <%= form_with model: @merge_request, url: merge_request_path(id: @merge_request.id), method: :patch do |f| %>
- <%= govuk_details(summary_text: "I cannot find an organisation on the list") do %>
- <%= 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/app/views/organisation_relationships/_related_organisation_select_question.html.erb b/app/views/organisation_relationships/_related_organisation_select_question.html.erb
index 551bce5ed..f0d1aa2eb 100644
--- a/app/views/organisation_relationships/_related_organisation_select_question.html.erb
+++ b/app/views/organisation_relationships/_related_organisation_select_question.html.erb
@@ -1,3 +1,3 @@
<% answers = question.answer_options.map { |key, value| OpenStruct.new(id: key, name: value) } %>
-<%= f.govuk_collection_select field, answers, :id, :name, label: { hidden: true }, "data-controller": "accessible-autocomplete" do %>
+<%= f.govuk_collection_select field, answers, :id, :name, label:, "data-controller": "accessible-autocomplete" do %>
<% end %>
diff --git a/app/views/organisation_relationships/add_managing_agent.html.erb b/app/views/organisation_relationships/add_managing_agent.html.erb
index b3e3a4a42..25c7c53a2 100644
--- a/app/views/organisation_relationships/add_managing_agent.html.erb
+++ b/app/views/organisation_relationships/add_managing_agent.html.erb
@@ -19,6 +19,7 @@
<% end %>
<%= render partial: "organisation_relationships/related_organisation_select_question", locals: {
field: :child_organisation_id,
+ label: { hidden: true },
question: Form::Question.new("", { "answer_options" => answer_options }, nil),
f:,
} %>
diff --git a/app/views/organisation_relationships/add_stock_owner.html.erb b/app/views/organisation_relationships/add_stock_owner.html.erb
index b74d812ec..042442125 100644
--- a/app/views/organisation_relationships/add_stock_owner.html.erb
+++ b/app/views/organisation_relationships/add_stock_owner.html.erb
@@ -19,6 +19,7 @@
<% end %>
<%= render partial: "organisation_relationships/related_organisation_select_question", locals: {
field: :parent_organisation_id,
+ label: { hidden: true },
question: Form::Question.new("", { "answer_options" => answer_options }, nil),
f:,
} %>
diff --git a/config/routes.rb b/config/routes.rb
index c5748d419..1e9e0dbf1 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -203,9 +203,9 @@ Rails.application.routes.draw do
resources :merge_requests, path: "/merge-request" do
member do
- get "organisations"
- patch "organisations", to: "merge_requests#update_organisations"
- get "organisations/remove", to: "merge_requests#remove_merging_organisation"
+ get "merging-organisations"
+ patch "merging-organisations", to: "merge_requests#update_merging_organisations"
+ get "merging-organisations/remove", to: "merge_requests#remove_merging_organisation"
get "absorbing-organisation"
get "confirm-telephone-number"
get "new-organisation-name"
diff --git a/db/migrate/20240813072041_remove_other_merging_org_field.rb b/db/migrate/20240813072041_remove_other_merging_org_field.rb
new file mode 100644
index 000000000..eb1ddc2e4
--- /dev/null
+++ b/db/migrate/20240813072041_remove_other_merging_org_field.rb
@@ -0,0 +1,5 @@
+class RemoveOtherMergingOrgField < ActiveRecord::Migration[7.0]
+ def change
+ remove_column :merge_requests, :other_merging_organisations, :string
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 761ebf60f..47ec3c35f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -429,7 +429,6 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_19_143150) do
create_table "merge_requests", force: :cascade do |t|
t.integer "requesting_organisation_id"
- t.text "other_merging_organisations"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "status"
diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb
index 2ae5dce42..805f91f1c 100644
--- a/spec/requests/merge_requests_controller_spec.rb
+++ b/spec/requests/merge_requests_controller_spec.rb
@@ -16,56 +16,56 @@ RSpec.describe MergeRequestsController, type: :request do
sign_in support_user
end
- describe "#organisations" do
+ context "when creating a new merge request" do
let(:params) { { merge_request: { requesting_organisation_id: support_user.organisation_id, status: "incomplete" } } }
- context "when creating a new merge request" do
- before do
- post "/merge-request", headers:, params:
- end
+ before do
+ post "/merge-request", headers:, params:
+ end
- it "creates merge request with requesting organisation" do
- follow_redirect!
- expect(page).to have_content("Which organisation is absorbing the others?")
- expect(page).to have_content(support_user.organisation.name)
- end
+ it "creates merge request with requesting organisation" do
+ follow_redirect!
+ expect(page).to have_content("Which organisation is absorbing the others?")
+ expect(MergeRequest.first.requesting_organisation_id).to eq(support_user.organisation_id)
+ end
- context "when passing a different requesting organisation id" do
- let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "incomplete" } } }
+ context "when passing a different requesting organisation id" do
+ let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "incomplete" } } }
- it "creates merge request with current user organisation" do
- follow_redirect!
- expect(MergeRequest.count).to eq(1)
- expect(MergeRequest.first.requesting_organisation_id).to eq(support_user.organisation_id)
- expect(MergeRequest.first.merging_organisations.count).to eq(0)
- end
+ it "creates merge request with current user organisation" do
+ follow_redirect!
+ expect(MergeRequest.count).to eq(1)
+ expect(MergeRequest.first.requesting_organisation_id).to eq(support_user.organisation_id)
+ expect(MergeRequest.first.merging_organisations.count).to eq(0)
end
end
+ end
- context "when viewing existing merge request" do
+ describe "#merging-organisations" do
+ context "when viewing merging organisations page" do
before do
- get "/merge-request/#{merge_request.id}/organisations", headers:, params:
+ merge_request.update!(absorbing_organisation_id: organisation.id)
+ get "/merge-request/#{merge_request.id}/merging-organisations", headers:
end
- it "shows merge request with requesting organisation" do
- expect(page).to have_content("Which organisations are merging?")
- expect(page).to have_content(organisation.name)
+ it "shows the correct content" do
+ expect(page).to have_content("Which organisations are merging into MHCLG?")
end
end
end
- describe "#update_organisations" do
+ describe "#update_merging_organisations" do
let(:params) { { merge_request: { merging_organisation: other_organisation.id } } }
- context "when updating a merge request with a new organisation" do
+ context "when updating a merge request with a new merging organisation" do
before do
- patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
+ patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
it "updates the merge request" do
merge_request.reload
expect(merge_request.merging_organisations.count).to eq(1)
- expect(page).to have_content("Test Org")
+ expect(page).to have_content("MHCLG")
expect(page).to have_content("Other Test Org")
expect(page).to have_link("Remove")
end
@@ -76,23 +76,7 @@ RSpec.describe MergeRequestsController, type: :request do
before do
MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "request_merged")
- patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
- end
-
- it "does not update the merge request" do
- merge_request.reload
- expect(merge_request.merging_organisations.count).to eq(0)
- expect(response).to have_http_status(:unprocessable_entity)
- expect(page).to have_content(I18n.t("validations.merge_request.organisation_part_of_another_merge"))
- end
- end
-
- context "when the user selects an organisation that has another non submitted merge" do
- let(:params) { { merge_request: { merging_organisation: other_organisation.id } } }
-
- before do
- MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "incomplete")
- patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
+ patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
it "updates the merge request" do
@@ -109,7 +93,7 @@ RSpec.describe MergeRequestsController, type: :request do
before do
existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "request_merged")
MergeRequestOrganisation.create!(merge_request_id: existing_merge_request.id, merging_organisation_id: another_organisation.id)
- patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
+ patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
it "does not update the merge request" do
@@ -120,17 +104,17 @@ 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 incomplete merge" do
let(:another_organisation) { create(:organisation) }
let(:params) { { merge_request: { merging_organisation: another_organisation.id } } }
before do
existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "incomplete")
MergeRequestOrganisation.create!(merge_request_id: existing_merge_request.id, merging_organisation_id: another_organisation.id)
- patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
+ patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
- it "does not update the merge request" do
+ it "updates the merge request" do
merge_request.reload
expect(merge_request.merging_organisations.count).to eq(1)
expect(page).not_to have_content(I18n.t("validations.merge_request.organisation_part_of_another_merge"))
@@ -143,7 +127,7 @@ RSpec.describe MergeRequestsController, type: :request do
before do
merge_request.merging_organisations << another_organisation
- patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
+ patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
it "does not update the merge request" do
@@ -152,25 +136,11 @@ RSpec.describe MergeRequestsController, type: :request do
end
end
- context "when the user selects an organisation that is requesting this merge" do
- let(:params) { { merge_request: { merging_organisation: merge_request.requesting_organisation_id } } }
-
- before do
- patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
- end
-
- it "does not update the merge request" do
- merge_request.reload
- expect(page).not_to have_content(I18n.t("validations.merge_request.organisation_part_of_another_merge"))
- expect(merge_request.merging_organisations.count).to eq(1)
- end
- end
-
context "when the user does not select an organisation" do
let(:params) { { merge_request: { merging_organisation: nil } } }
before do
- patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
+ patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
it "does not update the merge request" do
@@ -185,7 +155,7 @@ RSpec.describe MergeRequestsController, type: :request do
let(:params) { { merge_request: { merging_organisation: "clearly_not_an_id" } } }
before do
- patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
+ patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
it "does not update the merge request" do
@@ -203,7 +173,7 @@ RSpec.describe MergeRequestsController, type: :request do
context "when removing an organisation from merge request" do
before do
MergeRequestOrganisation.create!(merge_request_id: merge_request.id, merging_organisation_id: other_organisation.id)
- get "/merge-request/#{merge_request.id}/organisations/remove", headers:, params:
+ get "/merge-request/#{merge_request.id}/merging-organisations/remove", headers:, params:
end
it "updates the merge request" do
@@ -214,7 +184,7 @@ RSpec.describe MergeRequestsController, type: :request do
context "when removing an organisation that is not part of a merge from merge request" do
before do
- get "/merge-request/#{merge_request.id}/organisations/remove", headers:, params:
+ get "/merge-request/#{merge_request.id}/merging-organisations/remove", headers:, params:
end
it "does not throw an error" do
@@ -306,7 +276,7 @@ RSpec.describe MergeRequestsController, type: :request do
it "redirects to merging organisations path" do
request
- expect(response).to redirect_to(organisations_merge_request_path(merge_request))
+ expect(response).to redirect_to(merging_organisations_merge_request_path(merge_request))
end
it "updates absorbing_organisation_id and sets new_absorbing_organisation to false" do
@@ -319,30 +289,6 @@ RSpec.describe MergeRequestsController, type: :request do
end
end
- describe "#other_merging_organisations" do
- let(:other_merging_organisations) { "A list of other merging organisations" }
- let(:params) { { merge_request: { other_merging_organisations:, page: "organisations" } } }
- let(:request) do
- patch "/merge-request/#{merge_request.id}", headers:, params:
- end
-
- context "when adding other merging organisations" do
- before do
- MergeRequestOrganisation.create!(merge_request_id: merge_request.id, merging_organisation_id: other_organisation.id)
- end
-
- it "updates the merge request" do
- expect { request }.to change { merge_request.reload.other_merging_organisations }.from(nil).to(other_merging_organisations)
- end
-
- it "redirects telephone number path" do
- request
-
- expect(response).to redirect_to(confirm_telephone_number_merge_request_path(merge_request))
- end
- end
- end
-
describe "from confirm_telephone_number page" do
context "when confirming the number" do
let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_absorbing_organisation: true, new_telephone_number: "123") }
@@ -694,7 +640,7 @@ RSpec.describe MergeRequestsController, type: :request do
context "when viewing existing merge request" do
before do
- get "/merge-request/#{merge_request.id}/organisations", headers:, params:
+ get "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
it "does not allow viewing a merge request" do
@@ -708,7 +654,7 @@ RSpec.describe MergeRequestsController, type: :request do
context "when updating a merge request with a new organisation" do
before do
- patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
+ patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
it "does not allow updaing a merge request" do
@@ -723,7 +669,7 @@ RSpec.describe MergeRequestsController, type: :request do
context "when removing an organisation from merge request" do
before do
MergeRequestOrganisation.create!(merge_request_id: merge_request.id, merging_organisation_id: other_organisation.id)
- get "/merge-request/#{merge_request.id}/organisations/remove", headers:, params:
+ get "/merge-request/#{merge_request.id}/merging-organisations/remove", headers:, params:
end
it "does not allow removing an organisation" do
diff --git a/spec/views/merge_requests/show.html.erb_spec.rb b/spec/views/merge_requests/show.html.erb_spec.rb
index 0f0e42b2d..72974fc9e 100644
--- a/spec/views/merge_requests/show.html.erb_spec.rb
+++ b/spec/views/merge_requests/show.html.erb_spec.rb
@@ -46,15 +46,6 @@ RSpec.describe "merge_requests/show.html.erb", type: :view do
expect(rendered).to have_selector("dd", text: merge_request.absorbing_organisation_name)
end
- it "displays the merging organisations details" do
- expect(rendered).to have_selector("dt", text: "Merging organisations")
- if merge_request.other_merging_organisations.present?
- merge_request.other_merging_organisations.split(",").each do |organisation|
- expect(rendered).to have_selector("dd", text: organisation.strip)
- end
- end
- end
-
it "displays the merge date details" do
expect(rendered).to have_selector("dt", text: "Merge date")
expect(rendered).to have_selector("dd", text: merge_request.merge_date || "You didn't answer this question")