Simplify browse routes and make routes more consistent
authorTom Hughes <tom@compton.nu>
Thu, 28 Nov 2013 00:14:07 +0000 (00:14 +0000)
committerTom Hughes <tom@compton.nu>
Thu, 28 Nov 2013 00:14:07 +0000 (00:14 +0000)
This gets rid of the /browse/ prefix and uses /history consistently
for all routes that show a list of changesets.

app/assets/javascripts/index.js
app/assets/javascripts/index/browse.js
app/assets/javascripts/index/new_note.js.erb
app/assets/javascripts/index/notes.js.erb
app/assets/javascripts/router.js
config/routes.rb
test/functional/browse_controller_test.rb
test/functional/changeset_controller_test.rb
test/functional/user_controller_test.rb

index 5ddd1bb..3ce1a02 100644 (file)
@@ -261,13 +261,13 @@ $(document).ready(function () {
     "/":                           OSM.Index(map),
     "/search":                     OSM.Search(map),
     "/export":                     OSM.Export(map),
-    "/new_note":                   OSM.NewNote(map),
+    "/note/new":                   OSM.NewNote(map),
+    "/history/friends":            history,
+    "/history/nearby":             history,
     "/history":                    history,
-    "/user/:display_name/edits":   history,
-    "/browse/friends":             history,
-    "/browse/nearby":              history,
-    "/browse/note/:id":            OSM.Note(map),
-    "/browse/:type/:id(/history)": OSM.Browse(map)
+    "/user/:display_name/history": history,
+    "/note/:id":                  OSM.Note(map),
+    "/:type/:id(/history)":       OSM.Browse(map)
   });
 
   OSM.router.load();
index 2133857..21f0ae7 100644 (file)
@@ -124,7 +124,7 @@ function initializeBrowse(map) {
     layer.originalStyle = layer.options;
     layer.setStyle({color: '#0000ff', weight: 8});
 
-    OSM.router.route('/browse/' + layer.feature.type + '/' + layer.feature.id);
+    OSM.router.route('/' + layer.feature.type + '/' + layer.feature.id);
 
     // Stash the currently drawn feature
     selectedLayer = layer;
index 0541ba6..6f734a3 100644 (file)
@@ -30,7 +30,7 @@ OSM.NewNote = function(map) {
 
     if ($(this).hasClass('disabled')) return;
 
-    OSM.router.route('/new_note');
+    OSM.router.route('/note/new');
   });
 
   function createNote(marker, form, url) {
@@ -61,7 +61,7 @@ OSM.NewNote = function(map) {
       newNote = null;
       noteLayer.removeLayer(marker);
       addNoteButton.removeClass("active");
-      OSM.route('/browse/note/' + feature.properties.id);
+      OSM.route('/note/' + feature.properties.id);
     }
   }
 
index 928d91f..841ae1f 100644 (file)
@@ -34,7 +34,7 @@ function initializeNotes(map) {
   });
 
   noteLayer.on('click', function(e) {
-    OSM.router.route('/browse/note/' + e.layer.id);
+    OSM.router.route('/note/' + e.layer.id);
   });
 
   function updateMarker(marker, feature) {
index 0c855d2..8661f95 100644 (file)
@@ -8,7 +8,7 @@
 
   The router is initialized with a set of routes: a mapping of URL path templates
   to route controller objects. Path templates can contain placeholders
-  (`/browse/note/:id`) and optional segments (`/browse/:type/:id(/history)`).
+  (`/note/:id`) and optional segments (`/:type/:id(/history)`).
 
   Route controller objects can define four methods that are called at defined
   times during routing:
@@ -34,7 +34,7 @@
    An instance of OSM.Router is assigned to `OSM.router`. To navigate to a new page
    via pushState (with automatic full-page load fallback), call `OSM.router.route`:
 
-       OSM.router.route('/browse/way/1234');
+       OSM.router.route('/way/1234');
 
    If `route` is passed a path that matches one of the path templates, it performs
    the appropriate actions and returns true. Otherwise it returns false.
@@ -42,7 +42,7 @@
    OSM.Router also handles updating the hash portion of the URL containing transient
    map state such as the position and zoom level. Some route controllers may wish to
    temporarily suppress updating the hash (for example, to omit the hash on pages
-   such as `/browse/way/1234` unless the map is moved). This can be done by calling
+   such as `/way/1234` unless the map is moved). This can be done by calling
    `OSM.router.moveListenerOff` and `OSM.router.moveListenerOn`.
  */
 OSM.Router = function(map, rts) {
index 59c5b68..5551c7c 100644 (file)
@@ -102,24 +102,36 @@ OpenStreetMap::Application.routes.draw do
   end
 
   # Data browsing
-  match '/browse/way/:id' => 'browse#way', :via => :get, :id => /\d+/, :as => :way
-  match '/browse/way/:id/history' => 'browse#way_history', :via => :get, :id => /\d+/
-  match '/browse/node/:id' => 'browse#node', :via => :get, :id => /\d+/, :as => :node
-  match '/browse/node/:id/history' => 'browse#node_history', :via => :get, :id => /\d+/
-  match '/browse/relation/:id' => 'browse#relation', :via => :get, :id => /\d+/, :as => :relation
-  match '/browse/relation/:id/history' => 'browse#relation_history', :via => :get, :id => /\d+/
-  match '/browse/changeset/:id' => 'browse#changeset', :via => :get, :as => :changeset, :id => /\d+/
-  match '/browse/note/:id' => 'browse#note', :via => :get, :id => /\d+/, :as => "browse_note"
-  match '/new_note' => 'browse#new_note', :via => :get
-  match '/user/:display_name/edits' => 'changeset#list', :via => :get
-  match '/user/:display_name/edits/feed' => 'changeset#feed', :via => :get, :defaults => { :format => :atom }
+  match '/way/:id' => 'browse#way', :via => :get, :id => /\d+/, :as => :way
+  match '/way/:id/history' => 'browse#way_history', :via => :get, :id => /\d+/
+  match '/node/:id' => 'browse#node', :via => :get, :id => /\d+/, :as => :node
+  match '/node/:id/history' => 'browse#node_history', :via => :get, :id => /\d+/
+  match '/relation/:id' => 'browse#relation', :via => :get, :id => /\d+/, :as => :relation
+  match '/relation/:id/history' => 'browse#relation_history', :via => :get, :id => /\d+/
+  match '/changeset/:id' => 'browse#changeset', :via => :get, :as => :changeset, :id => /\d+/
+  match '/note/:id' => 'browse#note', :via => :get, :id => /\d+/, :as => "browse_note"
+  match '/note/new' => 'browse#new_note', :via => :get
+  match '/user/:display_name/history' => 'changeset#list', :via => :get
+  match '/user/:display_name/history/feed' => 'changeset#feed', :via => :get, :defaults => { :format => :atom }
   match '/user/:display_name/notes' => 'notes#mine', :via => :get
-  match '/browse/friends' => 'changeset#list', :via => :get, :friends => true, :as => "friend_changesets"
-  match '/browse/nearby' => 'changeset#list', :via => :get, :nearby => true, :as => "nearby_changesets"
+  match '/history/friends' => 'changeset#list', :via => :get, :friends => true, :as => "friend_changesets"
+  match '/history/nearby' => 'changeset#list', :via => :get, :nearby => true, :as => "nearby_changesets"
 
-  get '/browse/changesets/feed', :to => redirect('/history/feed')
-  get '/browse/changesets',      :to => redirect('/history')
-  get '/browse',                 :to => redirect('/history')
+  get '/browse/way/:id',                :to => redirect('/way/%{id}')
+  get '/browse/way/:id/history',        :to => redirect('/way/%{id}/history')
+  get '/browse/node/:id',               :to => redirect('/node/%{id}')
+  get '/browse/node/:id/history',       :to => redirect('/node/%{id}/history')
+  get '/browse/relation/:id',           :to => redirect('/relation/%{id}')
+  get '/browse/relation/:id/history',   :to => redirect('/relation/%{id}/history')
+  get '/browse/changset/:id',           :to => redirect('/changeset/%{id}')
+  get '/browse/note/:id',               :to => redirect('/note/%{id}')
+  get '/user/:display_name/edits',      :to => redirect('/user/:display_name/history')
+  get '/user/:display_name/edits/feed', :to => redirect('/user/:display_name/history/feed')
+  get '/browse/friends',                :to => redirect('/history/friends')
+  get '/browse/nearby',                 :to => redirect('/history/nearby')
+  get '/browse/changesets/feed',        :to => redirect('/history/feed')
+  get '/browse/changesets',             :to => redirect('/history')
+  get '/browse',                        :to => redirect('/history')
 
   # web site
   root :to => 'site#index', :via => [:get, :post]
index 2dcf6b3..5c48a5d 100644 (file)
@@ -8,37 +8,41 @@ class BrowseControllerTest < ActionController::TestCase
   # test all routes which lead to this controller
   def test_routes
     assert_routing(
-      { :path => "/browse/node/1", :method => :get },
+      { :path => "/node/1", :method => :get },
       { :controller => "browse", :action => "node", :id => "1" }
     )
     assert_routing(
-      { :path => "/browse/node/1/history", :method => :get },
+      { :path => "/node/1/history", :method => :get },
       { :controller => "browse", :action => "node_history", :id => "1" }
     )
     assert_routing(
-      { :path => "/browse/way/1", :method => :get },
+      { :path => "/way/1", :method => :get },
       { :controller => "browse", :action => "way", :id => "1" }
     )
     assert_routing(
-      { :path => "/browse/way/1/history", :method => :get },
+      { :path => "/way/1/history", :method => :get },
       { :controller => "browse", :action => "way_history", :id => "1" }
     )
     assert_routing(
-      { :path => "/browse/relation/1", :method => :get },
+      { :path => "/relation/1", :method => :get },
       { :controller => "browse", :action => "relation", :id => "1" }
     )
     assert_routing(
-      { :path => "/browse/relation/1/history", :method => :get },
+      { :path => "/relation/1/history", :method => :get },
       { :controller => "browse", :action => "relation_history", :id => "1" }
     )
     assert_routing(
-      { :path => "/browse/changeset/1", :method => :get },
+      { :path => "/changeset/1", :method => :get },
       { :controller => "browse", :action => "changeset", :id => "1" }
     )
     assert_routing(
-      { :path => "/browse/note/1", :method => :get },
+      { :path => "/note/1", :method => :get },
       { :controller => "browse", :action => "note", :id => "1" }
     )
+    assert_routing(
+      { :path => "/note/new", :method => :get },
+      { :controller => "browse", :action => "new_note" }
+    )
   end
 
   def test_read_relation
index 69811ad..0db84a9 100644 (file)
@@ -40,19 +40,19 @@ class ChangesetControllerTest < ActionController::TestCase
       { :controller => "changeset", :action => "query" }
     )
     assert_routing(
-      { :path => "/user/name/edits", :method => :get },
+      { :path => "/user/name/history", :method => :get },
       { :controller => "changeset", :action => "list", :display_name => "name" }
     )
     assert_routing(
-      { :path => "/user/name/edits/feed", :method => :get },
+      { :path => "/user/name/history/feed", :method => :get },
       { :controller => "changeset", :action => "feed", :display_name => "name", :format => :atom }
     )
     assert_routing(
-      { :path => "/browse/friends", :method => :get },
+      { :path => "/history/friends", :method => :get },
       { :controller => "changeset", :action => "list", :friends => true }
     )
     assert_routing(
-      { :path => "/browse/nearby", :method => :get },
+      { :path => "/history/nearby", :method => :get },
       { :controller => "changeset", :action => "list", :nearby => true }
     )
     assert_routing(
index 1cfb1c8..873becc 100644 (file)
@@ -548,7 +548,7 @@ class UserControllerTest < ActionController::TestCase
     get :view, {:display_name => "test"}
     assert_response :success
     assert_select "div#userinformation" do
-      assert_select "a[href^=/user/test/edits]", 1
+      assert_select "a[href^=/user/test/history]", 1
       assert_select "a[href=/user/test/traces]", 1
       assert_select "a[href=/user/test/diary]", 1
       assert_select "a[href=/user/test/diary/comments]", 1
@@ -562,7 +562,7 @@ class UserControllerTest < ActionController::TestCase
     get :view, {:display_name => "blocked"}
     assert_response :success
     assert_select "div#userinformation" do
-      assert_select "a[href^=/user/blocked/edits]", 1
+      assert_select "a[href^=/user/blocked/history]", 1
       assert_select "a[href=/user/blocked/traces]", 1
       assert_select "a[href=/user/blocked/diary]", 1
       assert_select "a[href=/user/blocked/diary/comments]", 1
@@ -576,7 +576,7 @@ class UserControllerTest < ActionController::TestCase
     get :view, {:display_name => "moderator"}
     assert_response :success
     assert_select "div#userinformation" do
-      assert_select "a[href^=/user/moderator/edits]", 1
+      assert_select "a[href^=/user/moderator/history]", 1
       assert_select "a[href=/user/moderator/traces]", 1
       assert_select "a[href=/user/moderator/diary]", 1
       assert_select "a[href=/user/moderator/diary/comments]", 1
@@ -593,7 +593,7 @@ class UserControllerTest < ActionController::TestCase
     get :view, {:display_name => "test"}
     assert_response :success
     assert_select "div#userinformation" do
-      assert_select "a[href^=/user/test/edits]", 1
+      assert_select "a[href^=/user/test/history]", 1
       assert_select "a[href=/traces/mine]", 1
       assert_select "a[href=/user/test/diary]", 1
       assert_select "a[href=/user/test/diary/comments]", 1
@@ -610,7 +610,7 @@ class UserControllerTest < ActionController::TestCase
     get :view, {:display_name => "test"}
     assert_response :success
     assert_select "div#userinformation" do
-      assert_select "a[href^=/user/test/edits]", 1
+      assert_select "a[href^=/user/test/history]", 1
       assert_select "a[href=/user/test/traces]", 1
       assert_select "a[href=/user/test/diary]", 1
       assert_select "a[href=/user/test/diary/comments]", 1