diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb index d12968cc7..d08a14209 100644 --- a/app/controllers/locations_controller.rb +++ b/app/controllers/locations_controller.rb @@ -27,9 +27,18 @@ class LocationsController < ApplicationController def edit; end + def edit_name; end + def update + page = params[:location][:page] + if @location.update(location_params) - location_params[:add_another_location] == "Yes" ? redirect_to(new_location_path(@location.scheme)) : redirect_to(scheme_check_answers_path(@scheme, anchor: "locations")) + case page + when "edit" + location_params[:add_another_location] == "Yes" ? redirect_to(new_location_path(@location.scheme)) : redirect_to(scheme_check_answers_path(@scheme, anchor: "locations")) + when "edit-name" + redirect_to(locations_path(@scheme)) + end else render :edit, status: :unprocessable_entity end @@ -38,7 +47,7 @@ class LocationsController < ApplicationController private def find_scheme - @scheme = if %w[new create index].include?(action_name) + @scheme = if %w[new create index edit_name].include?(action_name) Scheme.find(params[:id]) else @location.scheme @@ -46,7 +55,7 @@ private end def find_location - @location = Location.find(params[:id]) + @location = params[:location_id].present? ? Location.find(params[:location_id]) : Location.find(params[:id]) end def authenticate_scope! @@ -54,7 +63,7 @@ private end def authenticate_action! - if %w[new edit update create index].include?(action_name) && !((current_user.organisation == @scheme.owning_organisation) || current_user.support?) + if %w[new edit update create index edit_name].include?(action_name) && !((current_user.organisation == @scheme.owning_organisation) || current_user.support?) render_not_found and return end end diff --git a/app/helpers/tab_nav_helper.rb b/app/helpers/tab_nav_helper.rb index 3ef211401..0cba9f8e4 100644 --- a/app/helpers/tab_nav_helper.rb +++ b/app/helpers/tab_nav_helper.rb @@ -11,6 +11,11 @@ module TabNavHelper [govuk_link_to(link_text, "/schemes/#{location.scheme.id}/locations/#{location.id}/edit", method: :patch), "Location #{location.name}"].join("\n") end + def edit_location_name_cell(location) + link_text = location.postcode + [govuk_link_to(link_text, "/schemes/#{location.scheme.id}/locations/#{location.id}/edit-name", method: :patch), "Location #{location.name}"].join("\n") + end + def scheme_cell(scheme) link_text = scheme.service_name [govuk_link_to(link_text, scheme), "Scheme #{scheme.primary_client_group}"].join("\n") diff --git a/app/views/locations/edit.html.erb b/app/views/locations/edit.html.erb index bbcccdef6..e8a38ecb3 100644 --- a/app/views/locations/edit.html.erb +++ b/app/views/locations/edit.html.erb @@ -57,6 +57,8 @@ inline: true, legend: { text: "Do you want to add another location?", size: "m" } %> + <%= f.hidden_field :page, value: "edit" %> + <%= f.govuk_submit "Save and continue" %> diff --git a/app/views/locations/edit_name.html.erb b/app/views/locations/edit_name.html.erb new file mode 100644 index 000000000..22ce4be1f --- /dev/null +++ b/app/views/locations/edit_name.html.erb @@ -0,0 +1,26 @@ +<% content_for :title, "Location name for #{@location.postcode}" %> + +<% content_for :before_content do %> + <%= govuk_back_link( + text: "Back", + href: "/schemes/#{@scheme.id}/locations", + ) %> +<% end %> + +<%= render partial: "organisations/headings", locals: { main: "Location name for #{@location.postcode}", sub: @scheme.service_name } %> + +<%= form_for(@location, method: :patch, url: location_path(location_id: @location.id)) do |f| %> +
+
+ <%= f.govuk_error_summary %> + + <%= f.govuk_text_field :name, + label: { hidden: true }, + hint: { text: "This is how you refer to this location within your organisation" } %> + + <%= f.hidden_field :page, value: "edit-name" %> + + <%= f.govuk_submit "Save and continue" %> +
+
+<% end %> diff --git a/app/views/locations/index.html.erb b/app/views/locations/index.html.erb index 6d3cc3de3..88bff9ca4 100644 --- a/app/views/locations/index.html.erb +++ b/app/views/locations/index.html.erb @@ -36,7 +36,7 @@ <%= table.body do |body| %> <%= body.row do |row| %> <% row.cell(text: location.id) %> - <% row.cell(text: location.postcode) %> + <% row.cell(text: simple_format(edit_location_name_cell(location), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %> <% row.cell(text: location.total_units) %> <% row.cell(text: simple_format("#{location.type_of_unit}#{location.wheelchair_adaptation == 'Yes' ? "\nWith wheelchair adaptations" : ''}")) %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 7b18a6fce..de42be41a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -45,7 +45,9 @@ Rails.application.routes.draw do get "edit-name", to: "schemes#edit_name" member do - resources :locations + resources :locations do + get "edit-name", to: "locations#edit_name" + end end end diff --git a/db/seeds.rb b/db/seeds.rb index 175922921..be13a937c 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -79,7 +79,7 @@ unless Rails.env.test? intended_stay: "M", primary_client_group: "O", secondary_client_group: "H", - organisation: org, + owning_organisation: org, created_at: Time.zone.now, ) @@ -92,7 +92,7 @@ unless Rails.env.test? intended_stay: "S", primary_client_group: "D", secondary_client_group: "E", - organisation: org, + owning_organisation: org, created_at: Time.zone.now, ) @@ -105,14 +105,14 @@ unless Rails.env.test? intended_stay: "X", primary_client_group: "G", secondary_client_group: "R", - organisation: dummy_org, + owning_organisation: dummy_org, created_at: Time.zone.now, ) Location.create!( scheme: scheme1, location_code: "S254-CU193AA", - postcode: "CU19 3AA", + postcode: "CU193AA", name: "Rectory Road", type_of_unit: 4, type_of_building: "Purpose-built", @@ -123,7 +123,7 @@ unless Rails.env.test? Location.create!( scheme: scheme1, location_code: "S254-DM250DC", - postcode: "DM25 0DC", + postcode: "DM250DC", name: "Smithy Lane", type_of_unit: 1, type_of_building: "Converted from previous residential or non-residential property", @@ -134,7 +134,7 @@ unless Rails.env.test? Location.create!( scheme: scheme2, location_code: "S254-YX130WP", - postcode: "YX13 0WP", + postcode: "YX130WP", name: "Smithy Lane", type_of_unit: 2, type_of_building: "Converted from previous residential or non-residential property", diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 5999d09a0..43f966c1c 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -681,6 +681,7 @@ RSpec.describe "Schemes scheme Features" do context "when I click to see individual scheme" do let(:scheme) { schemes.first } + let!(:location) { FactoryBot.create(:location, scheme:) } before do click_link(scheme.service_name) @@ -721,6 +722,52 @@ RSpec.describe "Schemes scheme Features" do end end end + + context "when I click to see locations" do + before do + click_link "1 location" + end + + it "I see location details" do + expect(page).to have_content scheme.locations.first.id + expect(page).to have_current_path("/schemes/#{scheme.id}/locations") + end + + context "when I click to change location name" do + before do + click_link(location.postcode) + end + + it "shows available fields to edit" do + expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}/edit-name") + expect(page).to have_content "Location name for #{location.postcode}" + end + + context "when I press the back button" do + before do + click_link "Back" + end + + it "I see location details" do + expect(page).to have_content scheme.locations.first.id + expect(page).to have_current_path("/schemes/#{scheme.id}/locations") + end + end + + context "and I change the location name" do + before do + fill_in "location-name-field", with: "NewName" + click_button "Save and continue" + end + + it "returns to locations page and shows the new name" do + expect(page).to have_content location.id + expect(page).to have_content "NewName" + expect(page).to have_current_path("/schemes/#{scheme.id}/locations") + end + end + end + end end end end diff --git a/spec/helpers/tab_nav_helper_spec.rb b/spec/helpers/tab_nav_helper_spec.rb index 3403ce490..8afc45193 100644 --- a/spec/helpers/tab_nav_helper_spec.rb +++ b/spec/helpers/tab_nav_helper_spec.rb @@ -20,6 +20,13 @@ RSpec.describe TabNavHelper do end end + describe "#edit_location_name_cell" do + it "returns the location link to the postcode with optional name" do + expected_html = "#{location.postcode}\nLocation #{location.name}" + expect(edit_location_name_cell(location)).to match(expected_html) + end + end + describe "#location_cell" do it "returns the location link to the postcode with optional name" do expected_html = "#{location.postcode}\nLocation #{location.name}" diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb index cba28a541..e3909e754 100644 --- a/spec/requests/locations_controller_spec.rb +++ b/spec/requests/locations_controller_spec.rb @@ -390,7 +390,7 @@ RSpec.describe LocationsController, type: :request do let(:user) { FactoryBot.create(:user, :data_coordinator) } let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } let!(:location) { FactoryBot.create(:location, scheme:) } - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "ZZ1 1ZZ" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "ZZ1 1ZZ", page: "edit" } } } before do sign_in user @@ -412,8 +412,22 @@ RSpec.describe LocationsController, type: :request do expect(Location.last.wheelchair_adaptation).to eq("No") end + context "when updating from edit-name page" do + let(:params) { { location: { name: "Test", page: "edit-name" } } } + + it "updates existing location for scheme with valid params and redirects to correct page" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content("1 location") + end + + it "updates existing location for scheme with valid params" do + expect(Location.last.name).to eq("Test") + end + end + context "when postcode is submitted with lower case" do - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "zz1 1zz" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "zz1 1zz", page: "edit" } } } it "updates existing location for scheme with postcode " do expect(Location.last.postcode).to eq("ZZ11ZZ") @@ -423,7 +437,7 @@ RSpec.describe LocationsController, type: :request do context "when trying to update location for a scheme that belongs to another organisation" do let(:another_scheme) { FactoryBot.create(:scheme) } let(:another_location) { FactoryBot.create(:location) } - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "ZZ1 1ZZ" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "ZZ1 1ZZ", page: "edit" } } } it "displays the new page with an error message" do patch "/schemes/#{another_scheme.id}/locations/#{another_location.id}", params: params @@ -432,7 +446,7 @@ RSpec.describe LocationsController, type: :request do end context "when required postcode param is invalid" do - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "invalid" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "invalid", page: "edit" } } } it "displays the new page with an error message" do expect(response).to have_http_status(:unprocessable_entity) @@ -441,7 +455,7 @@ RSpec.describe LocationsController, type: :request do end context "when do you want to add another location is selected as yes" do - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "Yes", postcode: "ZZ1 1ZZ" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "Yes", postcode: "ZZ1 1ZZ", page: "edit" } } } it "updates existing location for scheme with valid params and redirects to correct page" do follow_redirect! @@ -459,7 +473,7 @@ RSpec.describe LocationsController, type: :request do end context "when do you want to add another location is selected as no" do - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "ZZ1 1ZZ" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "ZZ1 1ZZ", page: "edit" } } } it "updates existing location for scheme with valid params and redirects to correct page" do follow_redirect! @@ -477,7 +491,7 @@ RSpec.describe LocationsController, type: :request do end context "when do you want to add another location is not selected" do - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", postcode: "ZZ1 1ZZ" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", postcode: "ZZ1 1ZZ", page: "edit" } } } it "updates existing location for scheme with valid params and redirects to correct page" do follow_redirect! @@ -499,7 +513,7 @@ RSpec.describe LocationsController, type: :request do let(:user) { FactoryBot.create(:user, :data_coordinator) } let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } let!(:location) { FactoryBot.create(:location, scheme:) } - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "ZZ1 1ZZ" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "ZZ1 1ZZ", page: "edit" } } } before do allow(user).to receive(:need_two_factor_authentication?).and_return(false) @@ -521,8 +535,22 @@ RSpec.describe LocationsController, type: :request do expect(Location.last.wheelchair_adaptation).to eq("No") end + context "when updating from edit-name page" do + let(:params) { { location: { name: "Test", page: "edit-name" } } } + + it "updates existing location for scheme with valid params and redirects to correct page" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content("1 location") + end + + it "updates existing location for scheme with valid params" do + expect(Location.last.name).to eq("Test") + end + end + context "when postcode is submitted with lower case" do - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "zz1 1zz" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "zz1 1zz", page: "edit" } } } it "updates a location for scheme with postcode " do expect(Location.last.postcode).to eq("ZZ11ZZ") @@ -530,7 +558,7 @@ RSpec.describe LocationsController, type: :request do end context "when required postcode param is missing" do - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "invalid" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "invalid", page: "edit" } } } it "displays the new page with an error message" do expect(response).to have_http_status(:unprocessable_entity) @@ -539,7 +567,7 @@ RSpec.describe LocationsController, type: :request do end context "when do you want to add another location is selected as yes" do - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "Yes", postcode: "ZZ1 1ZZ" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "Yes", postcode: "ZZ1 1ZZ", page: "edit" } } } it "updates location for scheme with valid params and redirects to correct page" do follow_redirect! @@ -556,7 +584,7 @@ RSpec.describe LocationsController, type: :request do end context "when do you want to add another location is selected as no" do - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "ZZ1 1ZZ" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", add_another_location: "No", postcode: "ZZ1 1ZZ", page: "edit" } } } it "updates a location for scheme with valid params and redirects to correct page" do follow_redirect! @@ -573,7 +601,7 @@ RSpec.describe LocationsController, type: :request do end context "when do you want to add another location is not selected" do - let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", postcode: "ZZ1 1ZZ" } } } + let(:params) { { location: { name: "Test", total_units: "5", type_of_unit: "Bungalow", wheelchair_adaptation: "No", postcode: "ZZ1 1ZZ", page: "edit" } } } it "updates a location for scheme with valid params and redirects to correct page" do follow_redirect! @@ -781,4 +809,70 @@ RSpec.describe LocationsController, type: :request do end end end + + describe "#edit-name" do + context "when not signed in" do + it "redirects to the sign in page" do + get "/schemes/1/locations/1/edit-name" + expect(response).to redirect_to("/account/sign-in") + end + end + + context "when signed in as a data provider" do + let(:user) { FactoryBot.create(:user) } + + before do + sign_in user + get "/schemes/1/locations/1/edit-name" + end + + it "returns 401 unauthorized" do + request + expect(response).to have_http_status(:unauthorized) + end + end + + context "when signed in as a data coordinator" do + let(:user) { FactoryBot.create(:user, :data_coordinator) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } + let!(:location) { FactoryBot.create(:location, scheme:) } + + before do + sign_in user + get "/schemes/#{scheme.id}/locations/#{location.id}/edit-name" + end + + it "returns a template for a edit-name" do + expect(response).to have_http_status(:ok) + expect(page).to have_content("Location name for #{location.postcode}") + end + + context "when trying to edit location name of location that belongs to another organisation" do + let(:another_scheme) { FactoryBot.create(:scheme) } + let(:another_location) { FactoryBot.create(:location, scheme: another_scheme) } + + it "displays the new page with an error message" do + get "/schemes/#{another_scheme.id}/locations/#{another_location.id}/edit-name" + expect(response).to have_http_status(:not_found) + end + end + end + + context "when signed in as a support user" do + let(:user) { FactoryBot.create(:user, :support) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } + let!(:location) { FactoryBot.create(:location, scheme:) } + + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + get "/schemes/#{scheme.id}/locations/#{location.id}/edit-name" + end + + it "returns a template for a new location" do + expect(response).to have_http_status(:ok) + expect(page).to have_content("Location name for #{location.postcode}") + end + end + end end