[openstreetmap/openstreetmap-website] Sort and order notes (#2381)

Tom Hughes notifications at github.com
Tue Feb 18 17:35:38 UTC 2020


tomhughes requested changes on this pull request.



> @@ -294,11 +294,25 @@ def search
           raise OSM::APIBadUserInput, "Date #{params[:to]} is in a wrong format"
         end
 
-        @notes = @notes.where(:created_at => from..to)
+        @notes = params[:sort] == "created_at" ? @notes.where(:created_at => from..to) : @notes.where(:updated_at => from..to)

This will change the behaviour of existing API calls - you should make `created_at` the default to preserve the existing behaviour when no sort is specified.

I'd also prefer it if this was written like the sort order code with a multiline if rather than using the ternary operator, which is much less readable.

> @@ -294,11 +294,25 @@ def search
           raise OSM::APIBadUserInput, "Date #{params[:to]} is in a wrong format"
         end
 
-        @notes = @notes.where(:created_at => from..to)
+        @notes = params[:sort] == "created_at" ? @notes.where(:created_at => from..to) : @notes.where(:updated_at => from..to)
       end
 
       # Find the notes we want to return

This comment should be moved down to stay with the line it was originally attached to as it's incorrect where it is now.

If you want a comment here then it should say `Chose the sort order` or something.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/2381#pullrequestreview-360535847
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20200218/fb6cac73/attachment.htm>


More information about the rails-dev mailing list