[openstreetmap/openstreetmap-website] Added /notes/mine url path (#2006)

Tom Hughes notifications at github.com
Wed Sep 26 14:22:18 UTC 2018


tomhughes requested changes on this pull request.



> @@ -121,7 +121,11 @@ def test_routes
 
     assert_routing(
       { :path => "/user/username/notes", :method => :get },
-      { :controller => "notes", :action => "mine", :display_name => "username" }
+      { :controller => "notes", :action => "user", :display_name => "username" }
+    )
+    assert_routing(
+      { :path => "/traces/mine", :method => :get },
+      { :controller => "traces", :action => "mine" }

I think you mean `notes` here ;-)

>      if params[:display_name]
       if @user = User.active.find_by(:display_name => params[:display_name])
         @params = params.permit(:display_name)
-        @title = t "notes.mine.title", :user => @user.display_name
-        @heading = t "notes.mine.heading", :user => @user.display_name
+
+        # set title/heading
+        if current_user && current_user.display_name == @user.display_name
+          @title = t "user.show.my notes"
+          @heading = t "user.show.my notes"

I know that traces does something similar, but I wonder if this is sensible/worthwhile...

I mean `/mine` is basically a shortcut to asking for a particular user that isn't even preserved in the URL because we redirect, so giving this page a different title if you're logged in seems a little odd and not really necessary?

-- 
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/2006#pullrequestreview-159019716
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20180926/02e3f588/attachment.html>


More information about the rails-dev mailing list