diff --git a/app/models/location.rb b/app/models/location.rb index 64272d549..bfe9f77a4 100644 --- a/app/models/location.rb +++ b/app/models/location.rb @@ -169,10 +169,6 @@ class Location < ApplicationRecord errors.add :postcode, error_message else self.postcode = PostcodeService.clean(postcode) - if postcode_changed? - self.location_admin_district = nil - self.location_code = nil - end end end @@ -200,9 +196,17 @@ private def lookup_postcode! result = PIO.lookup(postcode) + unless location_admin_district_changed? + self.location_admin_district = nil + self.location_code = nil + self.is_la_inferred = false + end if result self.location_code = result[:location_code] self.location_admin_district = result[:location_admin_district] + self.is_la_inferred = true + else + self.is_la_inferred = false end end end diff --git a/app/views/locations/check_answers.html.erb b/app/views/locations/check_answers.html.erb index fdc9796a5..7d45eb158 100644 --- a/app/views/locations/check_answers.html.erb +++ b/app/views/locations/check_answers.html.erb @@ -18,11 +18,20 @@
<%= govuk_summary_list do |summary_list| %> <% display_location_attributes_for_check_answers(@location).each do |attr| %> - <%= summary_list.row do |row| %> - <% row.key { attr[:name] } %> - <% row.value { details_html(attr) } %> - <% if LocationPolicy.new(current_user, @location).update? %> - <% row.action(text: action_text_helper(attr, @location), href: location_edit_path(@location, attr[:attribute])) %> + <% unless attr[:attribute].eql?("local_authority") && @location.is_la_inferred %> + <%= summary_list.row do |row| %> + <% row.key { attr[:name] } %> + <% if attr[:attribute].eql?("postcode") && @location.is_la_inferred %> + <% row.value do %> + <%= details_html(attr) %> + <%= formatted_local_authority_timeline(@location) %> + <% end %> + <% else %> + <% row.value { details_html(attr) } %> + <% end %> + <% if LocationPolicy.new(current_user, @location).update? %> + <% row.action(text: action_text_helper(attr, @location), href: location_edit_path(@location, attr[:attribute])) %> + <% end %> <% end %> <% end %> <% end %> diff --git a/app/views/locations/show.html.erb b/app/views/locations/show.html.erb index f4371213c..54758b00f 100644 --- a/app/views/locations/show.html.erb +++ b/app/views/locations/show.html.erb @@ -15,18 +15,29 @@ <%= govuk_summary_list do |summary_list| %> <% display_location_attributes(@location).each do |attr| %> - <%= summary_list.row do |row| %> - <% row.key { attr[:name] } %> - <% row.value { attr[:attribute].eql?("status") ? status_tag_from_resource(@location) : details_html(attr) } %> - <% if LocationPolicy.new(current_user, @location).update? %> - <% row.action(text: "Change", href: scheme_location_postcode_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "postcode" && current_user.support? %> - <% row.action(text: "Change", href: scheme_location_name_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "name" %> - <% row.action(text: "Change", href: scheme_location_units_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "units" && current_user.support? %> - <% row.action(text: "Change", href: scheme_location_type_of_unit_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "type_of_unit" && current_user.support? %> - <% row.action(text: "Change", href: scheme_location_mobility_standards_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "mobility_standards" && current_user.support? %> + <% unless attr[:attribute].eql?("local_authority") && @location.is_la_inferred %> + <%= summary_list.row do |row| %> + <% row.key { attr[:name] } %> + <% if attr[:attribute].eql?("status") %> + <%= row.value { status_tag_from_resource(@location) } %> + <% elsif attr[:attribute].eql?("postcode") && @location.is_la_inferred %> + <% row.value do %> + <%= details_html(attr) %> + <%= formatted_local_authority_timeline(@location) %> + <% end %> + <% else %> + <%= row.value { details_html(attr) } %> + <% end %> + <% if LocationPolicy.new(current_user, @location).update? %> + <% row.action(text: "Change", href: scheme_location_postcode_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "postcode" && current_user.support? %> + <% row.action(text: "Change", href: scheme_location_name_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "name" %> + <% row.action(text: "Change", href: scheme_location_units_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "units" && current_user.support? %> + <% row.action(text: "Change", href: scheme_location_type_of_unit_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "type_of_unit" && current_user.support? %> + <% row.action(text: "Change", href: scheme_location_mobility_standards_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "mobility_standards" && current_user.support? %> + <% end %> <% end %> <% end %> - <% end %> + <% end %> <% end %>
diff --git a/app/views/schemes/_scheme_summary_list_row.html.erb b/app/views/schemes/_scheme_summary_list_row.html.erb index cf82ccd95..6655c17be 100644 --- a/app/views/schemes/_scheme_summary_list_row.html.erb +++ b/app/views/schemes/_scheme_summary_list_row.html.erb @@ -1,4 +1,4 @@ -
"> +
">
<%= attribute[:name].to_s %>
diff --git a/app/views/schemes/show.html.erb b/app/views/schemes/show.html.erb index 9d20ab91c..fee4b8a83 100644 --- a/app/views/schemes/show.html.erb +++ b/app/views/schemes/show.html.erb @@ -31,7 +31,7 @@ <% end %>
- <% elsif attr[:id] != "secondary_client_group" || @scheme.has_other_client_group == "Yes" %> + <% elsif attr[:id] != "secondary_client_group" || @scheme.has_other_client_group == "Yes" %> <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: change_answer_link(@scheme, attr[:id], current_user) } %> <% end %> <% end %> diff --git a/db/migrate/20231218105226_add_la_inferred_to_locations.rb b/db/migrate/20231218105226_add_la_inferred_to_locations.rb new file mode 100644 index 000000000..781b528d9 --- /dev/null +++ b/db/migrate/20231218105226_add_la_inferred_to_locations.rb @@ -0,0 +1,5 @@ +class AddLaInferredToLocations < ActiveRecord::Migration[7.0] + def change + add_column :locations, :is_la_inferred, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 610f2f1a4..22d67bb9f 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_12_04_101105) do +ActiveRecord::Schema[7.0].define(version: 2023_12_18_105226) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -355,6 +355,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_12_04_101105) do t.datetime "startdate" t.string "location_admin_district" t.boolean "confirmed" + t.boolean "is_la_inferred" t.index ["old_id"], name: "index_locations_on_old_id", unique: true t.index ["scheme_id"], name: "index_locations_on_scheme_id" end diff --git a/spec/features/schemes_helpers.rb b/spec/features/schemes_helpers.rb index d5e5a8e22..552c2b9ad 100644 --- a/spec/features/schemes_helpers.rb +++ b/spec/features/schemes_helpers.rb @@ -62,7 +62,7 @@ module SchemesHelpers def fill_in_and_save_location click_link "Locations" click_button "Add a location" - fill_in with: "AA11AA" + fill_in with: "XX11XX" click_button "Save and continue" select "Adur" click_button "Save and continue" @@ -83,7 +83,7 @@ module SchemesHelpers def fill_in_and_save_second_location click_link "Locations" click_button "Add a location" - fill_in with: "AA12AA" + fill_in with: "XX12XX" click_button "Save and continue" select "Adur" click_button "Save and continue" diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 8cf2ab43d..fe9cbe232 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -218,7 +218,7 @@ RSpec.describe "Schemes scheme Features" do context "when there are locations that belong to the selected scheme" do let!(:schemes) { FactoryBot.create_list(:scheme, 5) } let(:scheme) { schemes.first } - let!(:locations) { FactoryBot.create_list(:location, 3, scheme:, postcode: "AA11AA", startdate: Time.utc(2022, 1, 1)) } + let!(:locations) { FactoryBot.create_list(:location, 3, scheme:, postcode: "XX11XX", startdate: Time.utc(2022, 1, 1)) } before do visit("schemes") @@ -336,7 +336,7 @@ RSpec.describe "Schemes scheme Features" do let(:location_name) { "Area 42" } before do - fill_in with: "AA12AA" + fill_in with: "XX12XX" click_button "Save and continue" select "Adur" click_button "Save and continue" @@ -378,7 +378,7 @@ RSpec.describe "Schemes scheme Features" do end it "lets you edit the saved location" do - click_link "AA1 2AA" + click_link "XX12XX" expect(page).to have_link("Change", href: /postcode/) end end @@ -577,7 +577,7 @@ RSpec.describe "Schemes scheme Features" do end it "displays information about the first created location" do - expect(page).to have_content "AA1 1AA" + expect(page).to have_content "XX11XX" expect(page).to have_content "Some name" expect(page).to have_content "Active" end @@ -591,7 +591,7 @@ RSpec.describe "Schemes scheme Features" do it "displays information about newly created location" do fill_in_and_save_second_location - expect(page).to have_content "AA1 2AA" + expect(page).to have_content "XX12XX" expect(page).to have_content "Other name" expect(page).to have_content "Self-contained house" end @@ -606,11 +606,11 @@ RSpec.describe "Schemes scheme Features" do end it "displays changed location" do - click_link "AA1 2AA" + click_link "XX12XX" click_link("Change", href: "/schemes/#{scheme.id}/locations/#{location.id}/name?referrer=details", match: :first) fill_in with: "new name" click_button "Save changes" - expect(page).to have_content "AA1 2AA" + expect(page).to have_content "XX12XX" expect(page).to have_content "new name" end end @@ -962,7 +962,7 @@ RSpec.describe "Schemes scheme Features" do let(:location_name) { "Area 42" } before do - fill_in with: "AA12AA" + fill_in with: "XX12XX" click_button "Save and continue" select "Adur" click_button "Save and continue" @@ -1004,7 +1004,7 @@ RSpec.describe "Schemes scheme Features" do end it "lets you edit the saved location" do - click_link "AA1 2AA" + click_link "XX12XX" expect(page).to have_link("Change", href: /postcode/) end end diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb index 1b9edb9bf..58a101496 100644 --- a/spec/models/location_spec.rb +++ b/spec/models/location_spec.rb @@ -24,6 +24,7 @@ RSpec.describe Location, type: :model do location.postcode = "M1 1AE" location.save! expect(location.location_code).to eq("E08000003") + expect(location.is_la_inferred).to eq(true) end it "infers and returns the list of local authorities" do @@ -33,9 +34,23 @@ RSpec.describe Location, type: :model do expect(location.linked_local_authorities.active(Time.zone.local(2023, 4, 1)).first.code).to eq("E06000063") end + context "when local authority cannot be inferred" do + before do + stub_request(:get, /api\.postcodes\.io\/postcodes\/CA101AA/) + .to_return(status: 200, body: '{"status":200}', headers: {}) + end + + it "does not set the local authority" do + location.update!(postcode: "CA10 1AA", location_code: nil, location_admin_district: nil) + expect(location.location_code).to eq(nil) + expect(location.linked_local_authorities.count).to eq(0) + expect(location.is_la_inferred).to eq(false) + end + end + context "when location_code is no in LocalAuthorities table" do before do - stub_request(:get, /api.postcodes.io\/postcodes\/CA101AA/) + stub_request(:get, /api\.postcodes\.io\/postcodes\/CA101AA/) .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Eden","codes":{"admin_district":"E01231231"}}}', headers: {}) end diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb index 68474124a..996dd3de0 100644 --- a/spec/requests/locations_controller_spec.rb +++ b/spec/requests/locations_controller_spec.rb @@ -466,7 +466,7 @@ RSpec.describe LocationsController, type: :request do it "redirects correctly when postcodes.io does return a local authority" do follow_redirect! - expect(page).to have_content("What is the local authority") + expect(page).to have_content("What is the name of this location") end end @@ -527,7 +527,7 @@ RSpec.describe LocationsController, type: :request do it "redirects correctly when postcodes.io does return a local authority" do follow_redirect! - expect(page).to have_content("What is the local authority") + expect(page).to have_content("What is the name of this location") end end @@ -1344,6 +1344,38 @@ RSpec.describe LocationsController, type: :request do expect(page).not_to have_content("added to this scheme") end end + + context "when local authority is inferred" do + let(:params) { { location: { postcode: "zz1 1zz" } } } + + before do + patch "/schemes/#{scheme.id}/locations/#{location.id}/postcode?referrer=check_answers", params: + end + + it "does not display local authority row" do + location.reload + follow_redirect! + expect(location.is_la_inferred).to eq(true) + expect(location.location_admin_district).to eq("Westminster") + expect(page).not_to have_content("Local authority") + expect(page).to have_content("Westminster") + end + end + + context "when local authority is not inferred" do + let(:params) { { location: { postcode: "a1 1aa" } } } + + before do + patch "/schemes/#{scheme.id}/locations/#{location.id}/postcode?referrer=check_answers", params: + end + + it "displays local authority row" do + location.reload + get "/schemes/#{scheme.id}/locations/#{location.id}/check-answers" + expect(location.is_la_inferred).to eq(false) + expect(page).to have_content("Local authority") + end + end end context "when trying to edit check_answers of location that belongs to another organisation" do diff --git a/spec/views/locations/check_answers.html.erb_spec.rb b/spec/views/locations/check_answers.html.erb_spec.rb index 3a5cedc15..426f9f9e0 100644 --- a/spec/views/locations/check_answers.html.erb_spec.rb +++ b/spec/views/locations/check_answers.html.erb_spec.rb @@ -41,6 +41,7 @@ RSpec.describe "locations/check_answers.html.erb" do active?: true, scheme:, startdate: 1.day.ago, + is_la_inferred: nil, ) end diff --git a/spec/views/locations/show.html.erb_spec.rb b/spec/views/locations/show.html.erb_spec.rb index 4164e6baa..0b6dee65e 100644 --- a/spec/views/locations/show.html.erb_spec.rb +++ b/spec/views/locations/show.html.erb_spec.rb @@ -42,6 +42,7 @@ RSpec.describe "locations/show.html.erb" do active?: true, scheme:, deactivates_in_a_long_time?: false, + is_la_inferred: nil, ) end