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

@@ -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]/)
+    elsif query.match?(/\D/)
-      # Query contains letters, assume it's an address
+      # Query contains any non-digit characters, assume it's an address
       service = AddressClient.new(query, { minmatch: 0.2 })
       service.call
 

app/views/form/_address_search_question.html.erb

@@ -17,7 +17,7 @@
               <%= question.answer_selected?(@log, answer) ? "selected" : "" %>><%= answer.name || answer.resource %></option>
     <% end %>
   <% else %>
-    <option value="" disabled>Javascript is disabled. Please enter the address manually.</option>
+    <option value="" disabled>Javascript is disabled, please enter the address manually.</option>
   <% end %>
 <% end %>
 

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"
 

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
+          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=100")
+          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