From: ENT8R <info.ent8r@gmail.com>
Date: Tue, 9 Oct 2018 09:41:22 +0000 (+0200)
Subject: Merge branch 'master' into notes-search
X-Git-Tag: live~2814^2~4
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/083500f056f5b995f1979f88b37793046d9e4d37?hp=1c59d4a776c6dacb472889a3c68b374697b4cb4b

Merge branch 'master' into notes-search
---

diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index fc7a9e3c7..ba2df3680 100644
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -255,15 +255,48 @@ class NotesController < ApplicationController
   ##
   # Return a list of notes matching a given string
   def search
-    # Check the arguments are sane
-    raise OSM::APIBadUserInput, "No query string was given" unless params[:q]
+    # Filter either by the name or the id of the user
+    if params[:display_name]
+      @user = User.find_by(:display_name => params[:display_name])
+    elsif params[:id]
+      @user = User.find_by(:id => params[:id])
+    end
 
-    # Get any conditions that need to be applied
-    @notes = closed_condition(Note.all)
-    @notes = @notes.joins(:comments).where("to_tsvector('english', note_comments.body) @@ plainto_tsquery('english', ?)", params[:q])
+    if @user
+      @notes = @user.notes
+      @notes = closed_condition(@notes)
+    elsif params[:display_name] || params[:id]
+      # Return an error message because obviously the user could not be found
+      raise OSM::APIBadUserInput, "The user could not be found"
+    else
+      @notes = closed_condition(Note.all)
+    end
+
+    # Filter by a given string
+    if params[:q]
+      # TODO: why doesn't this work if we want to filter the notes of a given user?
+      @notes = @notes.joins(:comments).where("to_tsvector('english', note_comments.body) @@ plainto_tsquery('english', ?)", params[:q]) unless params[:display_name] || params[:id]
+    end
+
+    # Filter by a given start date and an optional end date
+    if params[:from]
+      begin
+        from = Time.parse(params[:from])
+        to = if params[:to]
+               Time.parse(params[:to])
+             else
+               Time.now
+             end
+      rescue ArgumentError
+        # return a more generic error so that everybody knows what is wrong
+        raise OSM::APIBadUserInput, "The date is in a wrong format"
+      end
+
+      @notes = @notes.where("(created_at > '#{from}' AND created_at < '#{to}')")
+    end
 
     # Find the notes we want to return
-    @notes = @notes.order("updated_at DESC").limit(result_limit).preload(:comments)
+    @notes = @notes.order("updated_at DESC").distinct.limit(result_limit).preload(:comments)
 
     # Render the result
     respond_to do |format|
diff --git a/test/controllers/notes_controller_test.rb b/test/controllers/notes_controller_test.rb
index 1ebce60b5..c2f44801c 100644
--- a/test/controllers/notes_controller_test.rb
+++ b/test/controllers/notes_controller_test.rb
@@ -876,6 +876,84 @@ class NotesControllerTest < ActionController::TestCase
     end
   end
 
+  def test_search_by_display_name_success
+    user = create(:user)
+
+    create(:note) do |note|
+      create(:note_comment, :note => note, :author => user)
+    end
+
+    get :search, :params => { :display_name => user.display_name, :format => "xml" }
+    assert_response :success
+    assert_equal "application/xml", @response.content_type
+    assert_select "osm", :count => 1 do
+      assert_select "note", :count => 1
+    end
+
+    get :search, :params => { :display_name => user.display_name, :format => "json" }
+    assert_response :success
+    assert_equal "application/json", @response.content_type
+    js = ActiveSupport::JSON.decode(@response.body)
+    assert_not_nil js
+    assert_equal "FeatureCollection", js["type"]
+    assert_equal 1, js["features"].count
+
+    get :search, :params => { :display_name => user.display_name, :format => "rss" }
+    assert_response :success
+    assert_equal "application/rss+xml", @response.content_type
+    assert_select "rss", :count => 1 do
+      assert_select "channel", :count => 1 do
+        assert_select "item", :count => 1
+      end
+    end
+
+    get :search, :params => { :display_name => user.display_name, :format => "gpx" }
+    assert_response :success
+    assert_equal "application/gpx+xml", @response.content_type
+    assert_select "gpx", :count => 1 do
+      assert_select "wpt", :count => 1
+    end
+  end
+
+  def test_search_by_id_success
+    user = create(:user)
+
+    create(:note) do |note|
+      create(:note_comment, :note => note, :author => user)
+    end
+
+    get :search, :params => { :id => user.id, :format => "xml" }
+    assert_response :success
+    assert_equal "application/xml", @response.content_type
+    assert_select "osm", :count => 1 do
+      assert_select "note", :count => 1
+    end
+
+    get :search, :params => { :id => user.id, :format => "json" }
+    assert_response :success
+    assert_equal "application/json", @response.content_type
+    js = ActiveSupport::JSON.decode(@response.body)
+    assert_not_nil js
+    assert_equal "FeatureCollection", js["type"]
+    assert_equal 1, js["features"].count
+
+    get :search, :params => { :id => user.id, :format => "rss" }
+    assert_response :success
+    assert_equal "application/rss+xml", @response.content_type
+    assert_select "rss", :count => 1 do
+      assert_select "channel", :count => 1 do
+        assert_select "item", :count => 1
+      end
+    end
+
+    get :search, :params => { :id => user.id, :format => "gpx" }
+    assert_response :success
+    assert_equal "application/gpx+xml", @response.content_type
+    assert_select "gpx", :count => 1 do
+      assert_select "wpt", :count => 1
+    end
+  end
+
   def test_search_no_match
     create(:note_with_comments)
 
@@ -911,15 +989,59 @@ class NotesControllerTest < ActionController::TestCase
     end
   end
 
-  def test_search_bad_params
-    get :search
-    assert_response :bad_request
+  def test_search_by_time_no_match
+    create(:note_with_comments)
+
+    get :search, :params => { :from => "01.01.2010", :to => "01.10.2010", :format => "xml" }
+    assert_response :success
+    assert_equal "application/xml", @response.content_type
+    assert_select "osm", :count => 1 do
+      assert_select "note", :count => 0
+    end
+
+    get :search, :params => { :from => "01.01.2010", :to => "01.10.2010", :format => "json" }
+    assert_response :success
+    assert_equal "application/json", @response.content_type
+    js = ActiveSupport::JSON.decode(@response.body)
+    assert_not_nil js
+    assert_equal "FeatureCollection", js["type"]
+    assert_equal 0, js["features"].count
+
+    get :search, :params => { :from => "01.01.2010", :to => "01.10.2010", :format => "rss" }
+    assert_response :success
+    assert_equal "application/rss+xml", @response.content_type
+    assert_select "rss", :count => 1 do
+      assert_select "channel", :count => 1 do
+        assert_select "item", :count => 0
+      end
+    end
 
+    get :search, :params => { :from => "01.01.2010", :to => "01.10.2010", :format => "gpx" }
+    assert_response :success
+    assert_equal "application/gpx+xml", @response.content_type
+    assert_select "gpx", :count => 1 do
+      assert_select "wpt", :count => 0
+    end
+  end
+
+  def test_search_bad_params
     get :search, :params => { :q => "no match", :limit => "0", :format => "json" }
     assert_response :bad_request
 
     get :search, :params => { :q => "no match", :limit => "10001", :format => "json" }
     assert_response :bad_request
+
+    get :search, :params => { :display_name => "non-existent" }
+    assert_response :bad_request
+
+    get :search, :params => { :id => "-1" }
+    assert_response :bad_request
+
+    get :search, :params => { :from => "wrong-date", :to => "wrong-date" }
+    assert_response :bad_request
+
+    get :search, :params => { :from => "01.01.2010", :to => "2010.01.2010" }
+    assert_response :bad_request
   end
 
   def test_feed_success