From 6b660535953eba06753badf8e80cc5063068b432 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 19 Apr 2023 10:22:14 +0100 Subject: [PATCH] CLDC-2055 Add merging organisations merge request page (#1535) * Create merge_request table and paths * Save merging organisations wip * Add update organisations * Add merging organisations validation * merge fixes * Update schema to have merge_request_organisations * Add relationships between merge request and organisations * Update validations and saving organisations * Add ability to remove merging orgs from the list * Allow support users to create merge request for any organisation * Update wording in organisations view * Allow adding other merging organisations * Add back button, update content * Add validation if the organisation is not selected * Fix path * Use generic update method * Update validations * fix remove organisation * remove reloads * Update routes * Authenticate scope * PR review changes * Add status, run validations unless the status is unsubmitted and save requesting organisation as a merging organisation as well * PR comments * Display continue button when there are at least 2 merging organisations --- app/controllers/merge_requests_controller.rb | 86 ++++++ app/controllers/organisations_controller.rb | 4 + app/models/merge_request.rb | 11 + app/models/merge_request_organisation.rb | 34 +++ .../merge_requests/organisations.html.erb | 49 +++ ...{merge.html.erb => merge_request.html.erb} | 9 +- app/views/organisations/show.html.erb | 2 +- config/locales/en.yml | 3 + config/routes.rb | 11 +- ...20230412111338_add_merge_requests_table.rb | 9 + .../20230413135407_add_merge_organisations.rb | 9 + ...30418095819_add_status_to_merge_request.rb | 5 + db/schema.rb | 31 +- .../merge_requests_controller_spec.rb | 278 ++++++++++++++++++ .../requests/organisations_controller_spec.rb | 10 +- 15 files changed, 532 insertions(+), 19 deletions(-) create mode 100644 app/controllers/merge_requests_controller.rb create mode 100644 app/models/merge_request.rb create mode 100644 app/models/merge_request_organisation.rb create mode 100644 app/views/merge_requests/organisations.html.erb rename app/views/organisations/{merge.html.erb => merge_request.html.erb} (89%) create mode 100644 db/migrate/20230412111338_add_merge_requests_table.rb create mode 100644 db/migrate/20230413135407_add_merge_organisations.rb create mode 100644 db/migrate/20230418095819_add_status_to_merge_request.rb create mode 100644 spec/requests/merge_requests_controller_spec.rb diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb new file mode 100644 index 000000000..3d1aa3f5f --- /dev/null +++ b/app/controllers/merge_requests_controller.rb @@ -0,0 +1,86 @@ +class MergeRequestsController < ApplicationController + before_action :find_resource, only: %i[update organisations update_organisations remove_merging_organisation] + before_action :authenticate_user! + before_action :authenticate_scope!, except: [:create] + + def create + ActiveRecord::Base.transaction do + @merge_request = MergeRequest.create!(merge_request_params.merge(status: :unsubmitted)) + MergeRequestOrganisation.create!({ merge_request: @merge_request, merging_organisation: @merge_request.requesting_organisation }) + end + redirect_to organisations_merge_request_path(@merge_request) + rescue ActiveRecord::RecordInvalid + render_not_found + end + + def organisations + @answer_options = organisations_answer_options + end + + def update + if @merge_request.update(merge_request_params) + redirect_to next_page_path + else + render previous_template, status: :unprocessable_entity + end + end + + def update_organisations + merge_request_organisation = MergeRequestOrganisation.new(merge_request_organisation_params) + @answer_options = organisations_answer_options + if merge_request_organisation.save + render :organisations + else + render :organisations, status: :unprocessable_entity + end + end + + def remove_merging_organisation + MergeRequestOrganisation.find_by(merge_request_organisation_params)&.destroy! + @answer_options = organisations_answer_options + render :organisations + end + +private + + def organisations_answer_options + answer_options = { "" => "Select an option" } + + Organisation.all.pluck(:id, :name).each do |organisation| + answer_options[organisation[0]] = organisation[1] + end + answer_options + end + + def merge_request_params + merge_params = params.fetch(:merge_request, {}).permit(:requesting_organisation_id, :other_merging_organisations, :status) + + if merge_params[:requesting_organisation_id].present? && (current_user.data_coordinator? || current_user.data_provider?) + merge_params[:requesting_organisation_id] = current_user.organisation.id + end + + merge_params + end + + def merge_request_organisation_params + { merge_request: @merge_request, merging_organisation_id: params[:merge_request][:merging_organisation] } + end + + def find_resource + @merge_request = MergeRequest.find(params[:id]) + end + + def next_page_path + absorbing_organisation_merge_request_path(@merge_request) + end + + def previous_template + :organisations + end + + def authenticate_scope! + if current_user.organisation != @merge_request.requesting_organisation && !current_user.support? + render_not_found + end + end +end diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 1bd4694ec..70943063f 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -137,6 +137,10 @@ class OrganisationsController < ApplicationController end end + def merge_request + @merge_request = MergeRequest.new + end + private def org_params diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb new file mode 100644 index 000000000..111d25321 --- /dev/null +++ b/app/models/merge_request.rb @@ -0,0 +1,11 @@ +class MergeRequest < ApplicationRecord + belongs_to :requesting_organisation, class_name: "Organisation" + has_many :merge_request_organisations + has_many :merging_organisations, through: :merge_request_organisations, source: :merging_organisation + + STATUS = { + "unsubmitted" => 0, + "submitted" => 1, + }.freeze + enum status: STATUS +end diff --git a/app/models/merge_request_organisation.rb b/app/models/merge_request_organisation.rb new file mode 100644 index 000000000..08c1d6d45 --- /dev/null +++ b/app/models/merge_request_organisation.rb @@ -0,0 +1,34 @@ +class MergeRequestOrganisation < ApplicationRecord + belongs_to :merge_request, class_name: "MergeRequest" + belongs_to :merging_organisation, class_name: "Organisation" + validates :merge_request, presence: { message: I18n.t("validations.merge_request.merge_request_id.blank") } + validates :merging_organisation, presence: { message: I18n.t("validations.merge_request.merging_organisation_id.blank") } + validate :validate_merging_organisations + + scope :not_unsubmitted, -> { joins(:merge_request).where.not(merge_requests: { status: "unsubmitted" }) } + scope :with_merging_organisation, ->(merging_organisation) { where(merging_organisation:) } + + has_paper_trail + +private + + def validate_merging_organisations + if MergeRequestOrganisation.where(merge_request:, merging_organisation:).count.positive? + errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge")) + end + + if MergeRequestOrganisation.not_unsubmitted.with_merging_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 MergeRequest.not_unsubmitted.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 + end +end diff --git a/app/views/merge_requests/organisations.html.erb b/app/views/merge_requests/organisations.html.erb new file mode 100644 index 000000000..14b70c17c --- /dev/null +++ b/app/views/merge_requests/organisations.html.erb @@ -0,0 +1,49 @@ +<% 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 %> + +

Which organisations are merging?

+ +
+
+

Add all organisations to be merged - we have already added your own.

+ +<%= form_with model: @merge_request, url: organisations_merge_request_path, method: :patch do |f| %> + <%= f.govuk_error_summary %> +

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.each do |merging_organisation| %> + <%= table.body do |body| %> + <%= body.row do |row| %> + <% row.cell(text: merging_organisation.name) %> + <% row.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.govuk_submit "Continue" %> + <% end %> +<% end %> +
diff --git a/app/views/organisations/merge.html.erb b/app/views/organisations/merge_request.html.erb similarity index 89% rename from app/views/organisations/merge.html.erb rename to app/views/organisations/merge_request.html.erb index 7d72d08b7..d8602b46c 100644 --- a/app/views/organisations/merge.html.erb +++ b/app/views/organisations/merge_request.html.erb @@ -40,9 +40,10 @@ <%= govuk_warning_text text: "You will not be able to submit your request without the above information. Do not start the form until you have obtained all of the information. " %> - <%= govuk_start_button( - text: "Start now", - href: "#", - ) %> + + <%= form_for @merge_request, url: merge_requests_path do |f| %> + <%= f.hidden_field :requesting_organisation_id, value: @organisation.id %> + <%= f.submit "Start now", class: "govuk-button govuk-button--start" %> + <% end %>
diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index 4561e985b..e2dbb9862 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -36,7 +36,7 @@ <% end %> <% end %> <% if FeatureToggle.merge_organisations_enabled? %> -

Is your organisation merging with another? <%= govuk_link_to "Let us know using this form", merge_organisation_path %>

+

Is your organisation merging with another? <%= govuk_link_to "Let us know using this form", merge_request_organisation_path %>

<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 16583a911..34dc4651e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -504,6 +504,9 @@ en: discounted_ownership_value: "Mortgage, deposit, and grant total must equal %{value_with_discount}" monthly_rent: higher_than_expected: "Basic monthly rent must be between £0.00 and £9,999.00" + merge_request: + organisation_part_of_another_merge: "This organisation is part of another merge - select a different one" + organisation_not_selected: "Select an organisation from the search list" soft_validations: net_income: diff --git a/config/routes.rb b/config/routes.rb index f6e00083b..95dcb21a3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -119,7 +119,16 @@ Rails.application.routes.draw do get "managing-agents/remove", to: "organisation_relationships#remove_managing_agent" post "managing-agents", to: "organisation_relationships#create_managing_agent" delete "managing-agents", to: "organisation_relationships#delete_managing_agent" - get "merge", to: "organisations#merge" + get "merge-request", to: "organisations#merge_request" + end + end + + 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 "absorbing-organisation" end end diff --git a/db/migrate/20230412111338_add_merge_requests_table.rb b/db/migrate/20230412111338_add_merge_requests_table.rb new file mode 100644 index 000000000..2aaeb2989 --- /dev/null +++ b/db/migrate/20230412111338_add_merge_requests_table.rb @@ -0,0 +1,9 @@ +class AddMergeRequestsTable < ActiveRecord::Migration[7.0] + def change + create_table :merge_requests do |t| + t.integer :requesting_organisation_id + t.text :other_merging_organisations + t.timestamps + end + end +end diff --git a/db/migrate/20230413135407_add_merge_organisations.rb b/db/migrate/20230413135407_add_merge_organisations.rb new file mode 100644 index 000000000..97178f973 --- /dev/null +++ b/db/migrate/20230413135407_add_merge_organisations.rb @@ -0,0 +1,9 @@ +class AddMergeOrganisations < ActiveRecord::Migration[7.0] + def change + create_table :merge_request_organisations do |t| + t.integer :merge_request_id, foreign_key: true + t.integer :merging_organisation_id, foreign_key: true + t.timestamps + end + end +end diff --git a/db/migrate/20230418095819_add_status_to_merge_request.rb b/db/migrate/20230418095819_add_status_to_merge_request.rb new file mode 100644 index 000000000..a6a3b01b9 --- /dev/null +++ b/db/migrate/20230418095819_add_status_to_merge_request.rb @@ -0,0 +1,5 @@ +class AddStatusToMergeRequest < ActiveRecord::Migration[7.0] + def change + add_column :merge_requests, :status, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index e121fda09..e87364403 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do +ActiveRecord::Schema[7.0].define(version: 2023_04_18_095819) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -354,6 +354,21 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do t.string "collection" end + create_table "merge_request_organisations", force: :cascade do |t| + t.integer "merge_request_id" + t.integer "merging_organisation_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + 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" + end + create_table "organisation_relationships", force: :cascade do |t| t.integer "child_organisation_id" t.integer "parent_organisation_id" @@ -519,6 +534,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do t.integer "prevten" t.integer "mortgageused" t.integer "wchair" + t.integer "income2_value_check" t.integer "armedforcesspouse" t.datetime "hodate", precision: nil t.integer "hoday" @@ -543,14 +559,13 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do t.integer "retirement_value_check" t.integer "hodate_check" t.integer "extrabor_value_check" - t.integer "grant_value_check" - t.integer "staircase_bought_value_check" t.integer "deposit_and_mortgage_value_check" t.integer "shared_ownership_deposit_value_check" + t.integer "grant_value_check" + t.integer "value_value_check" t.integer "old_persons_shared_ownership_value_check" - t.integer "income2_value_check" + t.integer "staircase_bought_value_check" t.integer "monthly_charges_value_check" - t.integer "value_value_check" t.integer "details_known_5" t.integer "details_known_6" t.integer "saledate_check" @@ -560,10 +575,9 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do t.integer "ethnicbuy2" t.integer "proplen_asked" t.string "old_id" - t.integer "pregblank" t.integer "buy2living" t.integer "prevtenbuy2" - t.integer "nationalbuy2" + t.integer "pregblank" t.string "uprn" t.integer "uprn_known" t.integer "uprn_confirmed" @@ -571,10 +585,11 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do t.string "address_line2" t.string "town_or_city" t.string "county" + t.integer "nationalbuy2" t.integer "discounted_sale_value_check" t.integer "student_not_child_value_check" - t.integer "buyer_livein_value_check" t.integer "percentage_discount_value_check" + t.integer "buyer_livein_value_check" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" t.index ["old_id"], name: "index_sales_logs_on_old_id", unique: true diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb new file mode 100644 index 000000000..ac7094df6 --- /dev/null +++ b/spec/requests/merge_requests_controller_spec.rb @@ -0,0 +1,278 @@ +require "rails_helper" + +RSpec.describe MergeRequestsController, type: :request do + let(:organisation) { user.organisation } + let(:other_organisation) { FactoryBot.create(:organisation, name: "Other Test Org") } + let(:headers) { { "Accept" => "text/html" } } + let(:page) { Capybara::Node::Simple.new(response.body) } + let(:user) { FactoryBot.create(:user, :data_coordinator) } + let(:support_user) { FactoryBot.create(:user, :support, organisation:) } + let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } + let(:other_merge_request) { MergeRequest.create!(requesting_organisation: other_organisation) } + + context "when user is signed in with a data coordinator user" do + before do + sign_in user + end + + describe "#organisations" do + let(:params) { { merge_request: { requesting_organisation_id: "9", status: "unsubmitted" } } } + + context "when creating a new merge request" do + before do + organisation.update!(name: "Test Org") + post "/merge-request", headers:, params: + end + + it "creates merge request with requesting organisation" do + follow_redirect! + expect(page).to have_content("Which organisations are merging?") + expect(page).to have_content("Test Org") + expect(page).not_to have_link("Remove") + end + + context "when passing a different requesting organisation id" do + let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "unsubmitted" } } } + + 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(organisation.id) + expect(MergeRequest.first.merging_organisations.count).to eq(1) + expect(MergeRequest.first.merging_organisations.first.id).to eq(organisation.id) + end + end + end + + context "when viewing existing merge request" do + before do + organisation.update!(name: "Test Org") + get "/merge-request/#{merge_request.id}/organisations", headers:, params: + end + + it "shows merge request with requesting organisation" do + expect(page).to have_content("Which organisations are merging?") + expect(page).to have_content("Test Org") + end + end + + context "when viewing existing merge request of a different (unauthorised) organisation" do + before do + get "/merge-request/#{other_merge_request.id}/organisations", headers:, params: + end + + it "shows merge request with requesting organisation" do + expect(response).to have_http_status(:not_found) + end + end + end + + describe "#update_organisations" do + let(:params) { { merge_request: { merging_organisation: other_organisation.id } } } + + context "when updating a merge request with a new organisation" do + before do + patch "/merge-request/#{merge_request.id}/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("Other Test Org") + expect(page).to have_link("Remove") + end + end + + context "when the user selects an organisation that requested another merge" do + let(:params) { { merge_request: { merging_organisation: other_organisation.id } } } + + before do + MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "submitted") + 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: "unsubmitted") + patch "/merge-request/#{merge_request.id}/organisations", headers:, params: + end + + 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")) + end + end + + context "when the user selects an organisation that is a part of another merge" do + let(:another_organisation) { FactoryBot.create(:organisation, name: "Other Test Org") } + let(:params) { { merge_request: { merging_organisation: another_organisation.id } } } + + before do + existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "submitted") + MergeRequestOrganisation.create!(merge_request_id: existing_merge_request.id, merging_organisation_id: another_organisation.id) + 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 is a part of another unsubmitted merge" do + let(:another_organisation) { FactoryBot.create(:organisation, name: "Other Test Org") } + let(:params) { { merge_request: { merging_organisation: another_organisation.id } } } + + before do + existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "unsubmitted") + MergeRequestOrganisation.create!(merge_request_id: existing_merge_request.id, merging_organisation_id: another_organisation.id) + 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(1) + expect(page).not_to have_content(I18n.t("validations.merge_request.organisation_part_of_another_merge")) + end + end + + context "when the user selects an organisation that is a part of current merge" do + let(:another_organisation) { FactoryBot.create(:organisation, name: "Other Test Org") } + let(:params) { { merge_request: { merging_organisation: another_organisation.id } } } + + before do + merge_request.merging_organisations << another_organisation + 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(1) + 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: + 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_not_selected")) + end + end + + context "when the user selects non existent id" do + let(:params) { { merge_request: { merging_organisation: "clearly_not_an_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(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_not_selected")) + end + end + end + + describe "#remove_organisation" do + let(:params) { { merge_request: { merging_organisation: other_organisation.id } } } + + 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: + end + + it "updates the merge request" do + expect(merge_request.merging_organisations.count).to eq(0) + expect(page).not_to have_link("Remove") + end + end + + 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: + end + + it "does not throw an error" do + expect(merge_request.merging_organisations.count).to eq(0) + expect(page).not_to have_link("Remove") + end + end + end + + describe "#other_merging_organisations" do + let(:params) { { merge_request: { other_merging_organisations: "A list of other merging organisations" } } } + + context "when adding other merging organisations" do + before do + MergeRequestOrganisation.create!(merge_request_id: merge_request.id, merging_organisation_id: other_organisation.id) + patch "/merge-request/#{merge_request.id}", headers:, params: + end + + it "updates the merge request" do + merge_request.reload + expect(merge_request.other_merging_organisations).to eq("A list of other merging organisations") + end + end + end + end + + context "when user is signed in as a support user" do + before do + allow(support_user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in support_user + end + + describe "#organisations" do + let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "unsubmitted" } } } + + before do + organisation.update!(name: "Test Org") + post "/merge-request", headers:, params: + end + + it "creates merge request with requesting organisation" do + follow_redirect! + expect(MergeRequest.count).to eq(1) + expect(MergeRequest.first.requesting_organisation_id).to eq(other_organisation.id) + end + end + end +end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 459987fdc..d526f218f 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -230,7 +230,7 @@ RSpec.describe OrganisationsController, type: :request do it "displays a link to merge organisations" do expect(page).to have_content("Is your organisation merging with another?") - expect(page).to have_link("Let us know using this form", href: "/organisations/#{organisation.id}/merge") + expect(page).to have_link("Let us know using this form", href: "/organisations/#{organisation.id}/merge-request") end end @@ -444,7 +444,7 @@ RSpec.describe OrganisationsController, type: :request do describe "#merge" do context "with an organisation that the user belongs to" do before do - get "/organisations/#{organisation.id}/merge", headers:, params: {} + get "/organisations/#{organisation.id}/merge-request", headers:, params: {} end it "shows the correct content" do @@ -455,14 +455,14 @@ RSpec.describe OrganisationsController, type: :request do expect(page).to have_link("Back", href: "/organisations/#{organisation.id}") end - it "has a correct start no button" do - expect(page).to have_link("Start now", href: "#") + it "has a correct start now button" do + expect(page).to have_button("Start now") end end context "with organisation that are not in scope for the user, i.e. that they do not belong to" do before do - get "/organisations/#{unauthorised_organisation.id}/merge", headers:, params: {} + get "/organisations/#{unauthorised_organisation.id}/merge-request", headers:, params: {} end it "returns not found 404 from org details route" do