From: Nenad Vujicic <nenadus@gmail.com>
Date: Thu, 13 Jun 2024 16:06:31 +0000 (+0200)
Subject: Added link to nominatim results in searching results
X-Git-Tag: live~436^2
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/839d203d5121482e9bcd998749d410f541ce2f51?ds=sidebyside;hp=--cc

Added link to nominatim results in searching results

Fixes #3205. Added caching of nominatim URL query parameters in sources global variable (as parameters parameter) in GeocoderController#search for both direct and reverse geocoding. In app/views/geocoder/search.html.erb added displaying cached URL as forwarding link when clicked on "OpenStreetMap Nominatim" label. Updated GeocoderControllerTest to check only name (latlng, osm_nominatim, osm_nominatim_reverse) parameter of new sources variable.
---

839d203d5121482e9bcd998749d410f541ce2f51
diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb
index 912247c9c..0419fb3b8 100644
--- a/app/controllers/geocoder_controller.rb
+++ b/app/controllers/geocoder_controller.rb
@@ -13,10 +13,10 @@ class GeocoderController < ApplicationController
     @sources = []
 
     if @params[:lat] && @params[:lon]
-      @sources.push "latlon"
-      @sources.push "osm_nominatim_reverse"
+      @sources.push({ :name => "latlon", :parameters => "" })
+      @sources.push({ :name => "osm_nominatim_reverse", :parameters => "reverse?format=html&#{nominatim_reverse_url_parameters}" })
     elsif @params[:query]
-      @sources.push "osm_nominatim"
+      @sources.push({ :name => "osm_nominatim", :parameters => "search?format=html&#{nominatim_url_parameters}" })
     end
 
     if @sources.empty?
@@ -71,21 +71,8 @@ class GeocoderController < ApplicationController
   end
 
   def search_osm_nominatim
-    # get query parameters
-    query = params[:query]
-    minlon = params[:minlon]
-    minlat = params[:minlat]
-    maxlon = params[:maxlon]
-    maxlat = params[:maxlat]
-
-    # get view box
-    viewbox = "&viewbox=#{minlon},#{maxlat},#{maxlon},#{minlat}" if minlon && minlat && maxlon && maxlat
-
-    # get objects to excude
-    exclude = "&exclude_place_ids=#{params[:exclude]}" if params[:exclude]
-
     # ask nominatim
-    response = fetch_xml("#{Settings.nominatim_url}search?format=xml&extratags=1&q=#{escape_query(query)}#{viewbox}#{exclude}&accept-language=#{http_accept_language.user_preferred_languages.join(',')}")
+    response = fetch_xml("#{Settings.nominatim_url}search?format=xml&" + nominatim_url_parameters)
 
     # extract the results from the response
     results = response.elements["searchresults"]
@@ -138,15 +125,13 @@ class GeocoderController < ApplicationController
 
   def search_osm_nominatim_reverse
     # get query parameters
-    lat = params[:lat]
-    lon = params[:lon]
     zoom = params[:zoom]
 
     # create result array
     @results = []
 
     # ask nominatim
-    response = fetch_xml("#{Settings.nominatim_url}reverse?lat=#{lat}&lon=#{lon}&zoom=#{zoom}&accept-language=#{http_accept_language.user_preferred_languages.join(',')}")
+    response = fetch_xml("#{Settings.nominatim_url}reverse?" + nominatim_reverse_url_parameters)
 
     # parse the response
     response.elements.each("reversegeocode/result") do |result|
@@ -171,6 +156,32 @@ class GeocoderController < ApplicationController
 
   private
 
+  def nominatim_url_parameters
+    # get query parameters
+    query = params[:query]
+    minlon = params[:minlon]
+    minlat = params[:minlat]
+    maxlon = params[:maxlon]
+    maxlat = params[:maxlat]
+
+    # get view box
+    viewbox = "&viewbox=#{minlon},#{maxlat},#{maxlon},#{minlat}" if minlon && minlat && maxlon && maxlat
+
+    # get objects to excude
+    exclude = "&exclude_place_ids=#{params[:exclude]}" if params[:exclude]
+
+    "extratags=1&q=#{escape_query(query)}#{viewbox}#{exclude}&accept-language=#{http_accept_language.user_preferred_languages.join(',')}"
+  end
+
+  def nominatim_reverse_url_parameters
+    # get query parameters
+    lat = params[:lat]
+    lon = params[:lon]
+    zoom = params[:zoom]
+
+    "lat=#{lat}&lon=#{lon}&zoom=#{zoom}&accept-language=#{http_accept_language.user_preferred_languages.join(',')}"
+  end
+
   def fetch_text(url)
     response = OSM.http_client.get(URI.parse(url))
 
diff --git a/app/views/geocoder/search.html.erb b/app/views/geocoder/search.html.erb
index 86048b0ad..53e87b3fd 100644
--- a/app/views/geocoder/search.html.erb
+++ b/app/views/geocoder/search.html.erb
@@ -4,10 +4,10 @@
 
 <% @sources.each do |source| %>
   <h4>
-    <%= t(".title.results_from_html", :results_link => link_to(t(".title.#{source}"),
-                                                               t(".title.#{source}_url"))) %>
+    <%= t(".title.results_from_html", :results_link => link_to(t(".title.#{source[:name]}"),
+                                                               t(".title.#{source[:name]}_url").to_s + source[:parameters].to_s)) %>
   </h4>
-  <div class="search_results_entry mx-n3" data-href="<%= url_for @params.merge(:action => "search_#{source}") %>">
+  <div class="search_results_entry mx-n3" data-href="<%= url_for @params.merge(:action => "search_#{source[:name]}") %>">
     <div class="text-center loader">
       <div class="spinner-border" role="status">
         <span class="visually-hidden"><%= t("browse.start_rjs.loading") %></span>
diff --git a/test/controllers/geocoder_controller_test.rb b/test/controllers/geocoder_controller_test.rb
index d62eb35b7..029ccbbd5 100644
--- a/test/controllers/geocoder_controller_test.rb
+++ b/test/controllers/geocoder_controller_test.rb
@@ -364,7 +364,7 @@ class GeocoderControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
     assert_template :search
     assert_template :layout => "map"
-    assert_equal %w[latlon osm_nominatim_reverse], assigns(:sources)
+    assert_equal %w[latlon osm_nominatim_reverse], assigns(:sources).pluck(:name)
     assert_nil @controller.params[:query]
     assert_in_delta lat, @controller.params[:lat]
     assert_in_delta lon, @controller.params[:lon]
@@ -373,7 +373,7 @@ class GeocoderControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
     assert_template :search
     assert_template :layout => "xhr"
-    assert_equal %w[latlon osm_nominatim_reverse], assigns(:sources)
+    assert_equal %w[latlon osm_nominatim_reverse], assigns(:sources).pluck(:name)
     assert_nil @controller.params[:query]
     assert_in_delta lat, @controller.params[:lat]
     assert_in_delta lon, @controller.params[:lon]
@@ -384,13 +384,13 @@ class GeocoderControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
     assert_template :search
     assert_template :layout => "map"
-    assert_equal sources, assigns(:sources)
+    assert_equal sources, assigns(:sources).pluck(:name)
 
     get search_path(:query => query), :xhr => true
     assert_response :success
     assert_template :search
     assert_template :layout => "xhr"
-    assert_equal sources, assigns(:sources)
+    assert_equal sources, assigns(:sources).pluck(:name)
   end
 
   def results_check(*results)