From 01e65617edd935056938de9bdf1558216d91e1c7 Mon Sep 17 00:00:00 2001 From: Kat Date: Tue, 13 Aug 2024 13:43:11 +0100 Subject: [PATCH] Remove unused paths and update merge date --- app/controllers/merge_requests_controller.rb | 65 +-- app/models/merge_request.rb | 8 - .../confirm_telephone_number.html.erb | 41 -- .../merge_requests/helpdesk_ticket.html.erb | 5 + app/views/merge_requests/merge_date.html.erb | 22 +- .../new_organisation_address.html.erb | 33 -- .../new_organisation_name.html.erb | 19 - ...new_organisation_telephone_number.html.erb | 20 - .../new_organisation_type.html.erb | 5 - config/locales/en.yml | 13 +- config/routes.rb | 6 +- ...119_remove_new_org_merge_request_fields.rb | 27 ++ db/schema.rb | 10 +- .../merge_requests_controller_spec.rb | 383 ++---------------- 14 files changed, 110 insertions(+), 547 deletions(-) delete mode 100644 app/views/merge_requests/confirm_telephone_number.html.erb create mode 100644 app/views/merge_requests/helpdesk_ticket.html.erb delete mode 100644 app/views/merge_requests/new_organisation_address.html.erb delete mode 100644 app/views/merge_requests/new_organisation_name.html.erb delete mode 100644 app/views/merge_requests/new_organisation_telephone_number.html.erb delete mode 100644 app/views/merge_requests/new_organisation_type.html.erb create mode 100644 db/migrate/20240813112119_remove_new_org_merge_request_fields.rb diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 46798d261..883a35003 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -6,12 +6,8 @@ class MergeRequestsController < ApplicationController def absorbing_organisation; end def merging_organisations; end - def confirm_telephone_number; end - def new_organisation_name; end - def new_organisation_address; end - def new_organisation_telephone_number; end - def new_organisation_type; end def merge_date; end + def helpdesk_ticket; end def create ActiveRecord::Base.transaction do @@ -59,19 +55,9 @@ private when "absorbing_organisation" merging_organisations_merge_request_path(@merge_request) when "merging_organisations" - if create_new_organisation? - new_organisation_name_merge_request_path(@merge_request) - else - confirm_telephone_number_merge_request_path(@merge_request) - end - when "confirm_telephone_number" merge_date_merge_request_path(@merge_request) - when "new_organisation_name" - new_organisation_address_merge_request_path(@merge_request) - when "new_organisation_address" - new_organisation_telephone_number_merge_request_path(@merge_request) - when "new_organisation_telephone_number" - new_organisation_type_merge_request_path(@merge_request) + when "merge_date" + helpdesk_ticket_merge_request_path(@merge_request) end end @@ -79,10 +65,6 @@ private page end - def create_new_organisation? - params.dig(:merge_request, :absorbing_organisation_id) == "other" - end - def set_organisations_answer_options answer_options = { "" => "Select an option" } @@ -100,48 +82,31 @@ private :requesting_organisation_id, :status, :absorbing_organisation_id, - :telephone_number_correct, - :new_telephone_number, - :new_organisation_name, - :new_organisation_address_line1, - :new_organisation_address_line2, - :new_organisation_postcode, - :new_organisation_telephone_number, + :merge_date ) merge_params[:requesting_organisation_id] = current_user.organisation.id - if merge_params[:absorbing_organisation_id].present? - if create_new_organisation? - merge_params[:new_absorbing_organisation] = true - merge_params[:absorbing_organisation_id] = nil - else - merge_params[:new_absorbing_organisation] = false - end - end - - if merge_params[:telephone_number_correct] == "true" - merge_params[:new_telephone_number] = nil - end - merge_params end def validate_response case page when "absorbing_organisation" - if merge_request_params[:absorbing_organisation_id].blank? && merge_request_params[:new_absorbing_organisation].blank? + if merge_request_params[:absorbing_organisation_id].blank? @merge_request.errors.add(:absorbing_organisation_id, :blank) end - when "confirm_telephone_number" - if merge_request_params[:telephone_number_correct].blank? - @merge_request.errors.add(:telephone_number_correct, :blank) if @merge_request.absorbing_organisation.phone.present? - @merge_request.errors.add(:new_telephone_number, :blank) if @merge_request.absorbing_organisation.phone.blank? + when "merge_date" + day = merge_request_params["merge_date(3i)"] + month = merge_request_params["merge_date(2i)"] + year = merge_request_params["merge_date(1i)"] + + return @merge_request.errors.add(:merge_date, :blank) if [day, month, year].all?(&:blank?) + if [day, month, year].none?(&:blank?) && Date.valid_date?(year.to_i, month.to_i, day.to_i) + merge_request_params["merge_date"] = Time.zone.local(year.to_i, month.to_i, day.to_i) + else + @merge_request.errors.add(:merge_date, :invalid) end - when "new_organisation_name" - @merge_request.errors.add(:new_organisation_name, :blank) if merge_request_params[:new_organisation_name].blank? - when "new_organisation_telephone_number" - @merge_request.errors.add(:new_organisation_telephone_number, :blank) if merge_request_params[:new_organisation_telephone_number].blank? end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index ec844576b..2abea9d7f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -4,8 +4,6 @@ class MergeRequest < ApplicationRecord belongs_to :absorbing_organisation, class_name: "Organisation", optional: true has_many :merging_organisations, through: :merge_request_organisations, source: :merging_organisation belongs_to :requester, class_name: "User", optional: true - validate :organisation_name_uniqueness, if: :new_organisation_name - validates :new_telephone_number, presence: true, if: -> { telephone_number_correct == false } STATUS = { "merge_issues" => 0, @@ -23,12 +21,6 @@ class MergeRequest < ApplicationRecord merged.where("merge_requests.merge_date >= ?", open_collection_period_start_date).or(not_merged) } - def organisation_name_uniqueness - if Organisation.where("lower(name) = ?", new_organisation_name&.downcase).exists? - errors.add(:new_organisation_name, :invalid) - end - end - def absorbing_organisation_name absorbing_organisation&.name || "" end diff --git a/app/views/merge_requests/confirm_telephone_number.html.erb b/app/views/merge_requests/confirm_telephone_number.html.erb deleted file mode 100644 index 8b3a354b4..000000000 --- a/app/views/merge_requests/confirm_telephone_number.html.erb +++ /dev/null @@ -1,41 +0,0 @@ -<% content_for :before_content do %> - <% title = "Tell us if your organisation is merging" %> - <% content_for :title, title %> - <%= govuk_back_link href: absorbing_organisation_merge_request_path(id: @merge_request) %> -<% end %> - -<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> - <%= f.govuk_error_summary %> - -

What is <%= @merge_request.absorbing_organisation.name %>'s telephone number?

- -
-
- <% if @merge_request.absorbing_organisation.phone.present? %> -

Confirm the telephone number on file, or enter a new one.

-
- <%= @merge_request.absorbing_organisation.phone %> -
- <% end %> - - <% if @merge_request.absorbing_organisation.phone.present? %> - <%= f.govuk_radio_buttons_fieldset(:telephone_number_correct, legend: nil) do %> - <%= f.govuk_radio_button :telephone_number_correct, true, checked: @merge_request.telephone_number_correct?, label: { text: "This telephone number is correct" }, link_errors: true %> - <%= f.govuk_radio_button( - :telephone_number_correct, - false, - checked: (@merge_request.new_telephone_number.present? || @merge_request.errors.key?(:new_telephone_number)), - label: { text: "Enter a new phone number" }, - ) do %> - <%= f.govuk_text_field :new_telephone_number, label: { text: "Telephone number" }, width: "two-thirds" %> - <% end %> - <%= f.hidden_field :page, value: "confirm_telephone_number" %> - <% end %> - <% else %> - <%= f.govuk_text_field :new_telephone_number, label: nil, width: "two-thirds" %> - <%= f.hidden_field :page, value: "confirm_telephone_number" %> - <% end %> - <%= f.govuk_submit %> - <% end %> -
-
diff --git a/app/views/merge_requests/helpdesk_ticket.html.erb b/app/views/merge_requests/helpdesk_ticket.html.erb new file mode 100644 index 000000000..3cd6b047f --- /dev/null +++ b/app/views/merge_requests/helpdesk_ticket.html.erb @@ -0,0 +1,5 @@ +<% content_for :before_content do %> + <% title = "Helpdesk ticket" %> + <% content_for :title, title %> + <%= govuk_back_link href: merge_date_merge_request_path(@merge_request) %> +<% end %> diff --git a/app/views/merge_requests/merge_date.html.erb b/app/views/merge_requests/merge_date.html.erb index 35d9ef36d..0a36a6bdd 100644 --- a/app/views/merge_requests/merge_date.html.erb +++ b/app/views/merge_requests/merge_date.html.erb @@ -1,8 +1,24 @@ <% content_for :before_content do %> <% title = "Tell us if your organisation is merging" %> <% content_for :title, title %> - <%# TODO: Update this backlink to also work with the create org flow %> - <%= govuk_back_link href: confirm_telephone_number_merge_request_path(@merge_request) %> + <%= govuk_back_link href: merging_organisations_merge_request_path(@merge_request) %> <% end %> +
+
+ <%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> + <%= f.govuk_error_summary %> -

What is the merge date?

+

What is the merge date?

+

+ Enter the official merge date. Log and organisation page data will show the new organisation name from this date.

+ For example, 27 3 2024

+ <%= f.govuk_date_field :merge_date, + legend: {hidden: true}, + width: 20 do %> + <% end %> + <%= f.hidden_field :page, value: "merge_date" %> + + <%= f.govuk_submit %> + <% end %> +
+
\ No newline at end of file diff --git a/app/views/merge_requests/new_organisation_address.html.erb b/app/views/merge_requests/new_organisation_address.html.erb deleted file mode 100644 index 7dd331507..000000000 --- a/app/views/merge_requests/new_organisation_address.html.erb +++ /dev/null @@ -1,33 +0,0 @@ -<% content_for :before_content do %> - <% title = "New organisation address" %> - <% content_for :title, title %> - <%= govuk_back_link href: new_organisation_name_merge_request_path(@merge_request) %> -<% end %> - -<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> - <%= f.govuk_error_summary %> - -

What is <%= @merge_request.new_organisation_name.possessive %> address?

-
-
- <%= f.govuk_text_field :new_organisation_address_line1, - label: { text: "Address line 1", size: "m" }, - autocomplete: "address-line1" %> - - <%= f.govuk_text_field :new_organisation_address_line2, - label: { text: "Address line 2", size: "m" }, - autocomplete: "address-line2" %> - - <%= f.govuk_text_field :new_organisation_postcode, - label: { text: "Postcode", size: "m" }, - autocomplete: "postal-code", - width: 10 %> - - <%= f.hidden_field :page, value: "new_organisation_address" %> -
- <%= f.govuk_submit %> - <%= govuk_link_to("Skip for now", new_organisation_telephone_number_merge_request_path(@merge_request)) %> -
-
-
-<% end %> diff --git a/app/views/merge_requests/new_organisation_name.html.erb b/app/views/merge_requests/new_organisation_name.html.erb deleted file mode 100644 index 8b391e735..000000000 --- a/app/views/merge_requests/new_organisation_name.html.erb +++ /dev/null @@ -1,19 +0,0 @@ -<% content_for :before_content do %> - <% title = "New organisation name" %> - <% content_for :title, title %> - <%= govuk_back_link href: absorbing_organisation_merge_request_path(id: @merge_request) %> -<% end %> - -<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> - <%= f.govuk_error_summary %> - -

What is the new organisation called?

- -
-
- <%= f.govuk_text_field :new_organisation_name, label: nil %> - <%= f.hidden_field :page, value: "new_organisation_name" %> - <%= f.govuk_submit %> - <% end %> -
-
diff --git a/app/views/merge_requests/new_organisation_telephone_number.html.erb b/app/views/merge_requests/new_organisation_telephone_number.html.erb deleted file mode 100644 index 6c0c1d81e..000000000 --- a/app/views/merge_requests/new_organisation_telephone_number.html.erb +++ /dev/null @@ -1,20 +0,0 @@ -<% content_for :before_content do %> - <% title = "New organisation telephone number" %> - <% content_for :title, title %> - <%= govuk_back_link href: new_organisation_address_merge_request_path(@merge_request) %> -<% end %> - -<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> - <%= f.govuk_error_summary %> - -

What is <%= @merge_request.new_organisation_name.possessive %> telephone number?

-
-
- <%= f.govuk_text_field :new_organisation_telephone_number, label: nil, width: "two-thirds" %> - <%= f.hidden_field :page, value: "new_organisation_telephone_number" %> -
- <%= f.govuk_submit %> -
-
-
-<% end %> diff --git a/app/views/merge_requests/new_organisation_type.html.erb b/app/views/merge_requests/new_organisation_type.html.erb deleted file mode 100644 index 2e22071e7..000000000 --- a/app/views/merge_requests/new_organisation_type.html.erb +++ /dev/null @@ -1,5 +0,0 @@ -<% content_for :before_content do %> - <% title = "New organisation type" %> - <% content_for :title, title %> - <%= govuk_back_link href: new_organisation_telephone_number_merge_request_path(@merge_request) %> -<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index a53b375fe..8ac3f0d53 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -176,16 +176,9 @@ en: attributes: absorbing_organisation_id: blank: "Select the organisation absorbing the others" - telephone_number_correct: - blank: "Select to confirm or enter a new telephone number" - invalid: "Enter a valid telephone number" - new_telephone_number: - blank: "Enter a valid telephone number" - new_organisation_name: - blank: "Enter an organisation name" - invalid: "An organisation with this name already exists" - new_organisation_telephone_number: - blank: "Enter a valid telephone number" + merge_date: + blank: "Enter a merge date" + invalid: "Enter a valid merge date" notification: logs_deleted: diff --git a/config/routes.rb b/config/routes.rb index b304ecb10..8920e1587 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -199,12 +199,8 @@ Rails.application.routes.draw do 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" - get "new-organisation-address" - get "new-organisation-telephone-number" - get "new-organisation-type" get "merge-date" + get "helpdesk-ticket" get "details", to: "merge_requests#details" end end diff --git a/db/migrate/20240813112119_remove_new_org_merge_request_fields.rb b/db/migrate/20240813112119_remove_new_org_merge_request_fields.rb new file mode 100644 index 000000000..c7050b54e --- /dev/null +++ b/db/migrate/20240813112119_remove_new_org_merge_request_fields.rb @@ -0,0 +1,27 @@ +class RemoveNewOrgMergeRequestFields < ActiveRecord::Migration[7.0] + def up + change_table :merge_requests, bulk: true do |t| + t.remove :new_absorbing_organisation + t.remove :telephone_number_correct + t.remove :new_telephone_number + t.remove :new_organisation_name + t.remove :new_organisation_address_line1 + t.remove :new_organisation_address_line2 + t.remove :new_organisation_postcode + t.remove :new_organisation_telephone_number + end + end + + def down + change_table :merge_requests, bulk: true do |t| + t.column :new_absorbing_organisation, :boolean + t.column :telephone_number_correct, :boolean + t.column :new_telephone_number, :string + t.column :new_organisation_name, :string + t.column :new_organisation_address_line1, :string + t.column :new_organisation_address_line2, :string + t.column :new_organisation_postcode, :string + t.column :new_organisation_telephone_number, :string + end + end +end diff --git a/db/schema.rb b/db/schema.rb index ca0241179..2d728a72d 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: 2024_08_13_072041) do +ActiveRecord::Schema[7.0].define(version: 2024_08_13_112119) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -421,14 +421,6 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_13_072041) do t.datetime "updated_at", null: false t.integer "status" t.integer "absorbing_organisation_id" - t.boolean "new_absorbing_organisation" - t.boolean "telephone_number_correct" - t.string "new_telephone_number" - t.string "new_organisation_name" - t.string "new_organisation_address_line1" - t.string "new_organisation_address_line2" - t.string "new_organisation_postcode" - t.string "new_organisation_telephone_number" t.datetime "merge_date" t.integer "requester_id" t.string "helpdesk_ticket" diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index 805f91f1c..8bbd419f7 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -194,38 +194,6 @@ RSpec.describe MergeRequestsController, type: :request do end end - describe "#confirm_telephone_number" do - let(:merge_request) do - MergeRequest.create!( - absorbing_organisation: create(:organisation, phone: phone_number), - requesting_organisation: organisation, - ) - end - - before { get "/merge-request/#{merge_request.id}/confirm-telephone-number", headers: } - - context "when org has phone number" do - let(:phone_number) { 123 } - - it "asks to confirm or provide new number" do - expect(page).to have_content("This telephone number is correct") - expect(page).to have_content("Confirm the telephone number on file, or enter a new one.") - expect(page).to have_content(phone_number) - expect(page).to have_content("What is #{merge_request.absorbing_organisation.name}'s telephone number?") - end - end - - context "when org does not have a phone number set" do - let(:phone_number) { nil } - - it "asks provide new number" do - expect(page).not_to have_content("This telephone number is correct") - expect(page).not_to have_content("Confirm the telephone number on file, or enter a new one.") - expect(page).to have_content("What is #{merge_request.absorbing_organisation.name}'s telephone number?") - end - end - end - describe "#absorbing_organisation" do let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } @@ -241,6 +209,19 @@ RSpec.describe MergeRequestsController, type: :request do end end + describe "#merge_date" do + context "when viewing merge date page" do + before do + merge_request.update!(absorbing_organisation_id: organisation.id) + get "/merge-request/#{merge_request.id}/merge-date", headers: + end + + it "shows the correct content" do + expect(page).to have_content("What is the merge date?") + end + end + end + describe "#update" do describe "from absorbing_organisation page" do context "when not answering the question" do @@ -264,7 +245,7 @@ RSpec.describe MergeRequestsController, type: :request do end context "when absorbing_organisation_id set to id" do - let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_absorbing_organisation: true) } + let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } let(:params) do { merge_request: { absorbing_organisation_id: other_organisation.id, page: "absorbing_organisation" } } end @@ -279,109 +260,19 @@ RSpec.describe MergeRequestsController, type: :request do 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 + it "updates absorbing_organisation_id" do 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) + }.from(nil).to(other_organisation) 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") } - let(:params) do - { merge_request: { telephone_number_correct: true, page: "confirm_telephone_number" } } - end - - let(:request) do - patch "/merge-request/#{merge_request.id}", headers:, params: - end - - it "redirects merge date path" do - request - - expect(response).to redirect_to(merge_date_merge_request_path(merge_request)) - end - - it "updates telephone_number_correct and sets new_telephone_number to nil" do - expect { request }.to change { - merge_request.reload.telephone_number_correct - }.from(nil).to(true).and change { - merge_request.reload.new_telephone_number - }.from("123").to(nil) - end - end - - context "when setting new number" do - let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_absorbing_organisation: true) } - let(:params) do - { merge_request: { telephone_number_correct: false, new_telephone_number: "123", page: "confirm_telephone_number" } } - end - - let(:request) do - patch "/merge-request/#{merge_request.id}", headers:, params: - end - - it "redirects merge date path" do - request - - expect(response).to redirect_to(merge_date_merge_request_path(merge_request)) - end - - it "updates telephone_number_correct and sets new_telephone_number to nil" do - expect { request }.to change { - merge_request.reload.new_telephone_number - }.from(nil).to("123") - end - end - - context "when not answering the question and the org has phone number" do - let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: create(:organisation, phone: "123")) } - let(:params) do - { merge_request: { page: "confirm_telephone_number" } } - end - let(:request) do - patch "/merge-request/#{merge_request.id}", headers:, params: - end - - it "renders the error" do - request - - expect(page).to have_content("Select to confirm or enter a new telephone number") - end - - it "does not update the request" do - expect { request }.not_to(change { merge_request.reload.attributes }) - end - end - - context "when not answering the question and the org does not have a phone number" do - let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) } - let(:params) do - { merge_request: { page: "confirm_telephone_number" } } - end - let(:request) do - patch "/merge-request/#{merge_request.id}", headers:, params: - end - - it "renders the error" do - request - - expect(page).to have_content("Enter a valid telephone number") - end - - it "does not update the request" do - expect { request }.not_to(change { merge_request.reload.attributes }) - end - end - - context "when not answering the phone number" do + describe "from merge_date page" do + context "when not answering the question" do let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) } let(:params) do - { merge_request: { page: "confirm_telephone_number", telephone_number_correct: false } } + { merge_request: { page: "merge_date" } } end let(:request) do patch "/merge-request/#{merge_request.id}", headers:, params: @@ -390,230 +281,53 @@ RSpec.describe MergeRequestsController, type: :request do it "renders the error" do request - expect(page).to have_content("Enter a valid telephone number") + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content("Enter a merge date") end it "does not update the request" do expect { request }.not_to(change { merge_request.reload.attributes }) end end - end - - describe "#new_organisation_name" do - let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_absorbing_organisation: true) } - - context "when viewing the new organisation name page" do - before do - get "/merge-request/#{merge_request.id}/new-organisation-name", headers: - end - - it "displays the correct question" do - expect(page).to have_content("What is the new organisation called?") - end - - it "has the correct back button" do - expect(page).to have_link("Back", href: absorbing_organisation_merge_request_path(merge_request)) - end - end - - context "when updating the new organisation name" do - let(:params) do - { merge_request: { new_organisation_name: "new org name", page: "new_organisation_name" } } - end - - let(:request) do - patch "/merge-request/#{merge_request.id}", headers:, params: - end - - it "redirects to new organisation address path" do - request - expect(response).to redirect_to(new_organisation_address_merge_request_path(merge_request)) - end - - it "updates new organisation name to the correct name" do - expect { request }.to change { - merge_request.reload.new_organisation_name - }.from(nil).to("new org name") - end - end - - context "when the new organisation name is not answered" do - let(:params) do - { merge_request: { new_organisation_name: nil, page: "new_organisation_name" } } - end - - let(:request) do - patch "/merge-request/#{merge_request.id}", headers:, params: - end - - it "renders the error" do - request - expect(page).to have_content("Enter an organisation name") - end - - it "does not update the organisation name" do - expect { request }.not_to(change { merge_request.reload.attributes }) - end - end - - context "when the new organisation name already exists" do - before do - create(:organisation, name: "new org name") - end + context "when merge date set to an invalid date" do + let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } let(:params) do - { merge_request: { new_organisation_name: "New org name", page: "new_organisation_name" } } + { merge_request: { page: "merge_date", "merge_date(3i)": "10", "merge_date(2i)": "44", "merge_date(1i)": "2022" } } end let(:request) do patch "/merge-request/#{merge_request.id}", headers:, params: end - it "renders the error" do + it "displays the page with an error message" do request - expect(page).to have_content("An organisation with this name already exists") - end - it "does not update the organisation name" do - expect { request }.not_to(change { merge_request.reload.attributes }) + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content("Enter a valid merge date") end end - end - - describe "#new_organisation_address" do - let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_organisation_name: "New name", new_absorbing_organisation: true) } - context "when viewing the new organisation name page" do - before do - get "/merge-request/#{merge_request.id}/new-organisation-address", headers: - end - - it "displays the correct question" do - expect(page).to have_content("What is New name’s address?") - end - - it "has the correct back button" do - expect(page).to have_link("Back", href: new_organisation_name_merge_request_path(merge_request)) - end - - it "has a skip link" do - expect(page).to have_link("Skip for now", href: new_organisation_telephone_number_merge_request_path(merge_request)) - end - end - - context "when updating the new organisation address" do + context "when merge date set to a valid date" do + let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } let(:params) do - { merge_request: { - new_organisation_address_line1: "first address line", - new_organisation_address_line2: "second address line", - new_organisation_postcode: "new postcode", - page: "new_organisation_address", - } } + { merge_request: { page: "merge_date", "merge_date(3i)": "10", "merge_date(2i)": "4", "merge_date(1i)": "2022" } } end let(:request) do patch "/merge-request/#{merge_request.id}", headers:, params: end - it "redirects to new organisation telephone path" do + it "redirects to helpdesk ticket path" do request - expect(response).to redirect_to(new_organisation_telephone_number_merge_request_path(merge_request)) - end - - it "updates new organisation address line 1 to correct address line" do - expect { request }.to change { - merge_request.reload.new_organisation_address_line1 - }.from(nil).to("first address line") - end - it "updates new organisation address line 2 to correct address line" do - expect { request }.to change { - merge_request.reload.new_organisation_address_line2 - }.from(nil).to("second address line") + expect(response).to redirect_to(helpdesk_ticket_merge_request_path(merge_request)) end - it "updates new organisation postcode to correct address line" do + it "updates merge_date" do expect { request }.to change { - merge_request.reload.new_organisation_postcode - }.from(nil).to("new postcode") - end - end - - context "when address is not provided" do - let(:params) do - { merge_request: { - new_organisation_address_line1: nil, - new_organisation_address_line2: nil, - new_organisation_postcode: nil, - page: "new_organisation_address", - } } - end - - let(:request) do - patch "/merge-request/#{merge_request.id}", headers:, params: - end - - it "does not throw an error" do - request - expect(response).to redirect_to(new_organisation_telephone_number_merge_request_path(merge_request)) - end - end - end - - describe "#new_organisation_telephone_number" do - let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_organisation_name: "New name", new_absorbing_organisation: true) } - - context "when viewing the new organisation telephone number page" do - before do - get "/merge-request/#{merge_request.id}/new-organisation-telephone-number", headers: - end - - it "displays the correct question" do - expect(page).to have_content("What is New name’s telephone number?") - end - - it "has the correct back button" do - expect(page).to have_link("Back", href: new_organisation_address_merge_request_path(merge_request)) - end - end - - context "when updating the new organisation telephone number" do - let(:params) do - { merge_request: { new_organisation_telephone_number: "1234", page: "new_organisation_telephone_number" } } - end - - let(:request) do - patch "/merge-request/#{merge_request.id}", headers:, params: - end - - it "redirects to new organisation type path" do - request - expect(response).to redirect_to(new_organisation_type_merge_request_path(merge_request)) - end - - it "updates new organisation name to the correct telephone number" do - expect { request }.to change { - merge_request.reload.new_organisation_telephone_number - }.from(nil).to("1234") - end - end - - context "when the new organisation telephone number is not answered" do - let(:params) do - { merge_request: { new_organisation_telephone_number: nil, page: "new_organisation_telephone_number" } } - end - - let(:request) do - patch "/merge-request/#{merge_request.id}", headers:, params: - end - - it "renders the error" do - request - expect(page).to have_content("Enter a valid telephone number") - end - - it "does not update the organisation telephone number" do - expect { request }.not_to(change { merge_request.reload.attributes }) + merge_request.reload.merge_date + }.from(nil).to(Time.zone.local(2022, 4, 10)) end end end @@ -625,7 +339,7 @@ RSpec.describe MergeRequestsController, type: :request do sign_in user end - describe "#organisations" do + describe "#merging_organisations" do let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "incomplete" } } } context "when creating a new merge request" do @@ -649,7 +363,7 @@ RSpec.describe MergeRequestsController, type: :request do 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 @@ -663,7 +377,7 @@ RSpec.describe MergeRequestsController, type: :request do end end - describe "#remove_organisation" do + describe "#remove_merging_organisation" do let(:params) { { merge_request: { merging_organisation: other_organisation.id } } } context "when removing an organisation from merge request" do @@ -681,7 +395,7 @@ RSpec.describe MergeRequestsController, type: :request do describe "#update" do describe "from absorbing_organisation page" do context "when absorbing_organisation_id set to id" do - let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, new_absorbing_organisation: true) } + let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } let(:params) do { merge_request: { absorbing_organisation_id: other_organisation.id, page: "absorbing_organisation" } } end @@ -695,25 +409,6 @@ RSpec.describe MergeRequestsController, type: :request do end 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) - request - end - - it "does not allow updating merging organisations" do - expect(response).to have_http_status(:not_found) - end - end - end end end end