From a293355efeb1c7b8c8d412e3d17121ea11868288 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 18 Mar 2025 10:07:50 +0000 Subject: [PATCH] Update address logic for queries with non-digit characters (#2998) * Update condition for queries with non-digit characters * Remove redundant route * Add and update address search tests * Update js disabled message --- app/controllers/address_search_controller.rb | 4 +- .../form/_address_search_question.html.erb | 2 +- config/routes.rb | 1 - .../address_search_controller_spec.rb | 88 ++++++++++++++++--- 4 files changed, 79 insertions(+), 16 deletions(-) diff --git a/app/controllers/address_search_controller.rb b/app/controllers/address_search_controller.rb index 74cda65c2..f1d834a11 100644 --- a/app/controllers/address_search_controller.rb +++ b/app/controllers/address_search_controller.rb @@ -18,8 +18,8 @@ class AddressSearchController < ApplicationController presenter = UprnDataPresenter.new(service.result) render json: [{ text: presenter.address, value: presenter.uprn }] end - elsif query.match?(/[a-zA-Z]/) - # Query contains letters, assume it's an address + elsif query.match?(/\D/) + # Query contains any non-digit characters, assume it's an address service = AddressClient.new(query, { minmatch: 0.2 }) service.call diff --git a/app/views/form/_address_search_question.html.erb b/app/views/form/_address_search_question.html.erb index ea30be718..76b39709a 100644 --- a/app/views/form/_address_search_question.html.erb +++ b/app/views/form/_address_search_question.html.erb @@ -17,7 +17,7 @@ <%= question.answer_selected?(@log, answer) ? "selected" : "" %>><%= answer.name || answer.resource %> <% end %> <% else %> - + <% end %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 304d54ef0..6b1b6458b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -40,7 +40,6 @@ Rails.application.routes.draw do get "/service-moved", to: "maintenance#service_moved" get "/service-unavailable", to: "maintenance#service_unavailable" get "/address-search", to: "address_search#index" - get "/address-search/current", to: "address_search#current" get "/address-search/manual-input/:log_type/:log_id", to: "address_search#manual_input", as: "address_manual_input" get "/address-search/search-input/:log_type/:log_id", to: "address_search#search_input", as: "address_search_input" diff --git a/spec/requests/address_search_controller_spec.rb b/spec/requests/address_search_controller_spec.rb index 7c8523abe..1dffdf8cf 100644 --- a/spec/requests/address_search_controller_spec.rb +++ b/spec/requests/address_search_controller_spec.rb @@ -25,7 +25,7 @@ RSpec.describe AddressSearchController, type: :request do expect(sales_log.town_or_city).to eq(nil) expect(sales_log.la).to eq(nil) - get "/address-search/manual-input/sales_log/#{sales_log.id}" + get address_manual_input_path(log_type: "sales_log", log_id: sales_log.id) sales_log.reload expect(sales_log.manual_address_entry_selected).to eq(true) @@ -58,7 +58,7 @@ RSpec.describe AddressSearchController, type: :request do expect(lettings_log.town_or_city).to eq("London") expect(lettings_log.la).to eq("E09000033") - get "/address-search/manual-input/lettings_log/#{lettings_log.id}" + get address_manual_input_path(log_type: "lettings_log", log_id: lettings_log.id) lettings_log.reload expect(lettings_log.manual_address_entry_selected).to eq(true) @@ -94,7 +94,7 @@ RSpec.describe AddressSearchController, type: :request do expect(lettings_log.town_or_city).to eq(nil) expect(lettings_log.la).to eq(nil) - get "/address-search/search-input/lettings_log/#{lettings_log.id}" + get address_search_input_path(log_type: "lettings_log", log_id: lettings_log.id) lettings_log.reload expect(lettings_log.manual_address_entry_selected).to eq(false) @@ -128,7 +128,7 @@ RSpec.describe AddressSearchController, type: :request do expect(sales_log.town_or_city).to eq("Test Town") expect(sales_log.la).to eq("E09000033") - get "/address-search/search-input/sales_log/#{sales_log.id}" + get address_search_input_path(log_type: "sales_log", log_id: sales_log.id) sales_log.reload expect(sales_log.manual_address_entry_selected).to eq(false) @@ -150,7 +150,7 @@ RSpec.describe AddressSearchController, type: :request do context "and theres no uprn returned" do before do - body = { results: [{ DPA: { "ADDRESS": "1, Test Street", "UPRN": "123" } }] }.to_json + body = { results: [{ DPA: { "ADDRESS": "100, Test Street", "UPRN": "100" } }] }.to_json uprn_body = { results: [{ DPA: nil }] }.to_json WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/find?key=OS_DATA_KEY&maxresults=10&minmatch=0.2&query=100") .to_return(status: 200, body:, headers: {}) @@ -159,30 +159,94 @@ RSpec.describe AddressSearchController, type: :request do end it "returns the address results" do - get "/address-search?query=100" + get address_search_path, params: { query: "100" } expect(response).to have_http_status(:ok) - expect(response.body).to eq([{ text: "1, Test Street", value: "123" }].to_json) + expect(response.body).to eq([{ text: "100, Test Street", value: "100" }].to_json) end end context "and theres no address returned" do before do body = { results: [{ DPA: nil }] }.to_json - uprn_body = { results: [{ DPA: { "ADDRESS": "2, Test Street", UPRN: "321" } }] }.to_json - WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/find?key=OS_DATA_KEY&maxresults=10&minmatch=0.2&query=100") + uprn_body = { results: [{ DPA: { "ADDRESS": "321, Test Street", UPRN: "321" } }] }.to_json + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/find?key=OS_DATA_KEY&maxresults=10&minmatch=0.2&query=321") .to_return(status: 200, body:, headers: {}) - WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/uprn?dataset=DPA,LPI&key=OS_DATA_KEY&uprn=100") + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/uprn?dataset=DPA,LPI&key=OS_DATA_KEY&uprn=321") .to_return(status: 200, body: uprn_body, headers: {}) end it "returns the address results" do - get "/address-search?query=100" + get address_search_path, params: { query: "321" } expect(response).to have_http_status(:ok) - expect(response.body).to eq([{ text: "2, Test Street", value: "321" }].to_json) + expect(response.body).to eq([{ text: "321, Test Street", value: "321" }].to_json) end end end end + + describe "GET #index" do + context "when query is nil" do + it "returns a bad request error" do + get address_search_path, params: { query: nil } + expect(response).to have_http_status(:bad_request) + expect(response.body).to include("Query cannot be blank.") + end + end + + context "when query is all numbers and greater than 5 digits" do + before do + address_body = { results: [{ DPA: { "ADDRESS": "Path not taken", UPRN: "111" } }] }.to_json + uprn_body = { results: [{ DPA: { "ADDRESS": "2, Test Street", UPRN: "123456" } }] }.to_json + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/find?key=OS_DATA_KEY&maxresults=10&minmatch=0.2&query=123456") + .to_return(status: 200, body: address_body, headers: {}) + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/uprn?dataset=DPA,LPI&key=OS_DATA_KEY&uprn=123456") + .to_return(status: 200, body: uprn_body, headers: {}) + end + + it "assumes it's a UPRN and returns the address" do + get address_search_path, params: { query: "123456" } + expect(response).to have_http_status(:ok) + expect(response.body).to include("2, Test Street") + expect(response.body).not_to include("Path not taken") + end + end + + context "when query contains any non-digit characters" do + before do + address_body = { results: [{ DPA: { "ADDRESS": "70, Test Street", UPRN: "123777" } }] }.to_json + uprn_body = { results: [{ DPA: { "ADDRESS": "Path not taken", UPRN: "111" } }] }.to_json + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/find?key=OS_DATA_KEY&maxresults=10&minmatch=0.2&query=70,") + .to_return(status: 200, body: address_body, headers: {}) + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/uprn?dataset=DPA,LPI&key=OS_DATA_KEY&uprn=70,") + .to_return(status: 200, body: uprn_body, headers: {}) + end + + it "assumes it's an address and returns the address results" do + get address_search_path, params: { query: "70," } + expect(response).to have_http_status(:ok) + expect(response.body).to include("70, Test Street") + expect(response.body).not_to include("Path not taken") + end + end + + context "when query is ambiguous" do + before do + address_body = { results: [{ DPA: { "ADDRESS": "111, Test Street", UPRN: "123777" } }] }.to_json + uprn_body = { results: [{ DPA: { "ADDRESS": "70 Bean Road", UPRN: "111" } }] }.to_json + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/find?key=OS_DATA_KEY&maxresults=10&minmatch=0.2&query=111") + .to_return(status: 200, body: address_body, headers: {}) + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/uprn?dataset=DPA,LPI&key=OS_DATA_KEY&uprn=111") + .to_return(status: 200, body: uprn_body, headers: {}) + end + + it "uses both APIs and merges results" do + get address_search_path, params: { query: "111" } + expect(response).to have_http_status(:ok) + expect(response.body).to include("111, Test Street") + expect(response.body).to include("70 Bean Road") + end + end + end end