Cldc 1292 permitted user can edit location name (#719)

* added test to check locations

* added test to click to change location

* added edit name to routes and front

* edit name request specs

* finilised

* tab nav helper test

* specs for update

* rubo

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

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), "<span class=\"govuk-visually-hidden\">Location </span><span class=\"govuk-!-font-weight-regular app-!-colour-muted\">#{location.name}</span>"].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), "<span class=\"govuk-visually-hidden\">Location </span><span class=\"govuk-!-font-weight-regular app-!-colour-muted\">#{location.name}</span>"].join("\n")
+  end
+
   def scheme_cell(scheme)
     link_text = scheme.service_name
     [govuk_link_to(link_text, scheme), "<span class=\"govuk-visually-hidden\">Scheme </span><span class=\"govuk-!-font-weight-regular app-!-colour-muted\">#{scheme.primary_client_group}</span>"].join("\n")

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" %>
     </div>
   </div>

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| %>
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds">
+      <%= 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" %>
+    </div>
+  </div>
+<% end %>

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("<span>#{location.type_of_unit}</span>#{location.wheelchair_adaptation == 'Yes' ? "\n<span class=\"govuk-!-font-weight-regular app-!-colour-muted\">With wheelchair adaptations</span>" : ''}")) %>
           <% end %>

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
 

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",

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

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 = "<a class=\"govuk-link\" rel=\"nofollow\" data-method=\"patch\" href=\"/schemes/#{scheme.id}/locations/#{location.id}/edit-name\">#{location.postcode}</a>\n<span class=\"govuk-visually-hidden\">Location </span><span class=\"govuk-!-font-weight-regular app-!-colour-muted\">#{location.name}</span>"
+      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 = "<a class=\"govuk-link\" rel=\"nofollow\" data-method=\"patch\" href=\"/schemes/#{scheme.id}/locations/#{location.id}/edit\">#{location.postcode}</a>\n<span class=\"govuk-visually-hidden\">Location </span><span class=\"govuk-!-font-weight-regular app-!-colour-muted\">#{location.name}</span>"

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