From: Tom Hughes <tom@compton.nu>
Date: Sun, 3 Sep 2023 17:54:18 +0000 (+0100)
Subject: Merge remote-tracking branch 'upstream/pull/4222'
X-Git-Tag: live~1534
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/be3baea4de2522eae935db262b29490d35c011ee?hp=-c

Merge remote-tracking branch 'upstream/pull/4222'
---

be3baea4de2522eae935db262b29490d35c011ee
diff --combined app/controllers/api/notes_controller.rb
index 0ce5450a7,46b91d445..95466781f
--- a/app/controllers/api/notes_controller.rb
+++ b/app/controllers/api/notes_controller.rb
@@@ -18,10 -18,13 +18,10 @@@ module Ap
        # support the old, deprecated, method with four arguments
        if params[:bbox]
          bbox = BoundingBox.from_bbox_params(params)
 -      else
 -        raise OSM::APIBadUserInput, "No l was given" unless params[:l]
 -        raise OSM::APIBadUserInput, "No r was given" unless params[:r]
 -        raise OSM::APIBadUserInput, "No b was given" unless params[:b]
 -        raise OSM::APIBadUserInput, "No t was given" unless params[:t]
 -
 +      elsif params[:l] && params[:r] && params[:b] && params[:t]
          bbox = BoundingBox.from_lrbt_params(params)
 +      else
 +        raise OSM::APIBadUserInput, "The parameter bbox is required"
        end
  
        # Get any conditions that need to be applied
@@@ -232,20 -235,7 +232,7 @@@
      def feed
        # Get any conditions that need to be applied
        notes = closed_condition(Note.all)
- 
-       # Process any bbox
-       if params[:bbox]
-         bbox = BoundingBox.from_bbox_params(params)
- 
-         bbox.check_boundaries
-         bbox.check_size(Settings.max_note_request_area)
- 
-         notes = notes.bbox(bbox)
-         @min_lon = bbox.min_lon
-         @min_lat = bbox.min_lat
-         @max_lon = bbox.max_lon
-         @max_lat = bbox.max_lat
-       end
+       notes = bbox_condition(notes)
  
        # Find the comments we want to return
        @comments = NoteComment.where(:note => notes)
@@@ -263,6 -253,7 +250,7 @@@
      def search
        # Get the initial set of notes
        @notes = closed_condition(Note.all)
+       @notes = bbox_condition(@notes)
  
        # Add any user filter
        if params[:display_name] || params[:user]
@@@ -375,6 -366,27 +363,27 @@@
        end
      end
  
+     ##
+     # Generate a condition to choose which notes we want based
+     # on the user's bounding box request parameters
+     def bbox_condition(notes)
+       if params[:bbox]
+         bbox = BoundingBox.from_bbox_params(params)
+ 
+         bbox.check_boundaries
+         bbox.check_size(Settings.max_note_request_area)
+ 
+         @min_lon = bbox.min_lon
+         @min_lat = bbox.min_lat
+         @max_lon = bbox.max_lon
+         @max_lat = bbox.max_lat
+ 
+         notes.bbox(bbox)
+       else
+         notes
+       end
+     end
+ 
      ##
      # Add a comment to a note
      def add_comment(note, text, event, notify: true)
diff --combined test/controllers/api/notes_controller_test.rb
index 4d4e53cb9,a03b84ae8..874ac3ccd
--- a/test/controllers/api/notes_controller_test.rb
+++ b/test/controllers/api/notes_controller_test.rb
@@@ -786,10 -786,6 +786,10 @@@ module Ap
      end
  
      def test_index_bad_params
 +      get api_notes_path
 +      assert_response :bad_request
 +      assert_equal "The parameter bbox is required", @response.body
 +
        get api_notes_path(:bbox => "-2.5,-2.5,2.5")
        assert_response :bad_request
  
@@@ -931,6 -927,28 +931,28 @@@
        end
      end
  
+     def test_search_by_bbox_success
+       notes = Array.new(5) do |i|
+         position = ((1.0 + (i * 0.1)) * GeoRecord::SCALE).to_i
+         create(:note_with_comments, :created_at => Time.parse("2020-01-01T00:00:00Z") + i.day, :latitude => position, :longitude => position)
+       end
+ 
+       get search_api_notes_path(:bbox => "1.0,1.0,1.6,1.6", :sort => "created_at", :order => "oldest", :format => "xml")
+       assert_response :success
+       assert_equal "application/xml", @response.media_type
+       assert_notes_in_order notes
+ 
+       get search_api_notes_path(:bbox => "1.25,1.25,1.45,1.45", :sort => "created_at", :order => "oldest", :format => "xml")
+       assert_response :success
+       assert_equal "application/xml", @response.media_type
+       assert_notes_in_order [notes[3], notes[4]]
+ 
+       get search_api_notes_path(:bbox => "2.0,2.0,2.5,2.5", :sort => "created_at", :order => "oldest", :format => "xml")
+       assert_response :success
+       assert_equal "application/xml", @response.media_type
+       assert_notes_in_order []
+     end
+ 
      def test_search_no_match
        create(:note_with_comments)
  
@@@ -1065,5 -1083,14 +1087,14 @@@
        get feed_api_notes_path(:bbox => "1,1,1.2,1.2", :limit => Settings.max_note_query_limit + 1, :format => "rss")
        assert_response :bad_request
      end
+ 
+     private
+ 
+     def assert_notes_in_order(notes)
+       assert_select "osm>note", notes.size
+       notes.each_with_index do |note, index|
+         assert_select "osm>note:nth-child(#{index + 1})>id", :text => note.id.to_s, :count => 1
+       end
+     end
    end
  end