Merge pull request #4922 from tomhughes/geocoder-urls

Simplify handling of geocoder URLs

app/controllers/geocoder_controller.rb

@@ -13,10 +13,10 @@ class GeocoderController < ApplicationController
     @sources = []
 
     if @params[:lat] && @params[:lon]
-      @sources.push({ :name => "latlon", :parameters => "" })
-      @sources.push({ :name => "osm_nominatim_reverse", :parameters => "reverse?format=html&#{nominatim_reverse_url_parameters}" })
+      @sources.push(:name => "latlon", :url => root_path)
+      @sources.push(:name => "osm_nominatim_reverse", :url => nominatim_reverse_url(:format => "html"))
     elsif @params[:query]
-      @sources.push({ :name => "osm_nominatim", :parameters => "search?format=html&#{nominatim_url_parameters}" })
+      @sources.push(:name => "osm_nominatim", :url => nominatim_url(:format => "html"))
     end
 
     if @sources.empty?
@@ -72,7 +72,7 @@ class GeocoderController < ApplicationController
 
   def search_osm_nominatim
     # ask nominatim
-    response = fetch_xml("#{Settings.nominatim_url}search?format=xml&" + nominatim_url_parameters)
+    response = fetch_xml(nominatim_url(:format => "xml"))
 
     # extract the results from the response
     results = response.elements["searchresults"]
@@ -131,7 +131,7 @@ class GeocoderController < ApplicationController
     @results = []
 
     # ask nominatim
-    response = fetch_xml("#{Settings.nominatim_url}reverse?" + nominatim_reverse_url_parameters)
+    response = fetch_xml(nominatim_reverse_url(:format => "xml"))
 
     # parse the response
     response.elements.each("reversegeocode/result") do |result|
@@ -156,7 +156,7 @@ class GeocoderController < ApplicationController
 
   private
 
-  def nominatim_url_parameters
+  def nominatim_url(format: nil)
     # get query parameters
     query = params[:query]
     minlon = params[:minlon]
@@ -170,16 +170,18 @@ class GeocoderController < ApplicationController
     # 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(',')}"
+    # build url
+    "#{Settings.nominatim_url}search?format=#{format}&extratags=1&q=#{escape_query(query)}#{viewbox}#{exclude}&accept-language=#{http_accept_language.user_preferred_languages.join(',')}"
   end
 
-  def nominatim_reverse_url_parameters
+  def nominatim_reverse_url(format: nil)
     # 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(',')}"
+    # build url
+    "#{Settings.nominatim_url}reverse?format=#{format}&lat=#{lat}&lon=#{lon}&zoom=#{zoom}&accept-language=#{http_accept_language.user_preferred_languages.join(',')}"
   end
 
   def fetch_text(url)

app/views/geocoder/search.html.erb

@@ -4,8 +4,7 @@
 
 <% @sources.each do |source| %>
   <h4>
-    <%= t(".title.results_from_html", :results_link => link_to(t(".title.#{source[:name]}"),
-                                                               t(".title.#{source[:name]}_url").to_s + source[:parameters].to_s)) %>
+    <%= t(".title.results_from_html", :results_link => link_to(t(".title.#{source[:name]}"), source[:url].to_s)) %>
   </h4>
   <div class="search_results_entry mx-n3" data-href="<%= url_for @params.merge(:action => "search_#{source[:name]}") %>">
     <div class="text-center loader">

config/locales/en.yml

@@ -671,11 +671,8 @@ en:
       title:
         results_from_html: 'Results from %{results_link}'
         latlon: Internal
-        latlon_url: https://openstreetmap.org/
         osm_nominatim: OpenStreetMap Nominatim
-        osm_nominatim_url: https://nominatim.openstreetmap.org/
         osm_nominatim_reverse: OpenStreetMap Nominatim
-        osm_nominatim_reverse_url: https://nominatim.openstreetmap.org/
     search_osm_nominatim:
       prefix_format: "%{name}"
       prefix:

test/http/nominatim.yml

@@ -16,7 +16,7 @@
       <place place_id='109724' osm_type='node' osm_id='17044599' place_rank='30' boundingbox="51.7418469,51.7518469,-0.0156773,-0.0056773" lat='51.7468469' lon='-0.0106773' display_name='Broxbourne, Stafford Drive, Broxbourne, Hertfordshire, East of England, England, United Kingdom' class='railway' type='station' importance='0.111' icon='http://nominatim.openstreetmap.org/images/mapicons/transport_train_station2.p.20.png'><extratags></extratags></place>
     </searchresults>
 
-/reverse?accept-language=&lat=51.7632&lon=-0.0076&zoom=15:
+/reverse?accept-language=&format=xml&lat=51.7632&lon=-0.0076&zoom=15:
   code: 200
   body: |
     <?xml version="1.0" encoding="UTF-8"?>
@@ -33,7 +33,7 @@
       </addressparts>
     </reversegeocode>
 
-/reverse?accept-language=&lat=51.7632&lon=-0.0076&zoom=17:
+/reverse?accept-language=&format=xml&lat=51.7632&lon=-0.0076&zoom=17:
   code: 200
   body: |
     <?xml version="1.0" encoding="UTF-8"?>
@@ -52,7 +52,7 @@
       </addressparts>
     </reversegeocode>
 
-/reverse?accept-language=&lat=13.7709&lon=100.50507&zoom=19:
+/reverse?accept-language=&format=xml&lat=13.7709&lon=100.50507&zoom=19:
   code: 200
   body: |
     <?xml version="1.0" encoding="UTF-8" ?>