From: Tom Hughes Date: Thu, 28 Nov 2013 00:14:07 +0000 (+0000) Subject: Simplify browse routes and make routes more consistent X-Git-Tag: live~5234^2~1 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/3cd5f45e08d977d04a778ab8802f71df85edc314?ds=sidebyside Simplify browse routes and make routes more consistent This gets rid of the /browse/ prefix and uses /history consistently for all routes that show a list of changesets. --- diff --git a/app/assets/javascripts/index.js b/app/assets/javascripts/index.js index 5ddd1bba1..3ce1a02da 100644 --- a/app/assets/javascripts/index.js +++ b/app/assets/javascripts/index.js @@ -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(); diff --git a/app/assets/javascripts/index/browse.js b/app/assets/javascripts/index/browse.js index 213385790..21f0ae71a 100644 --- a/app/assets/javascripts/index/browse.js +++ b/app/assets/javascripts/index/browse.js @@ -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; diff --git a/app/assets/javascripts/index/new_note.js.erb b/app/assets/javascripts/index/new_note.js.erb index 0541ba6e9..6f734a35b 100644 --- a/app/assets/javascripts/index/new_note.js.erb +++ b/app/assets/javascripts/index/new_note.js.erb @@ -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); } } diff --git a/app/assets/javascripts/index/notes.js.erb b/app/assets/javascripts/index/notes.js.erb index 928d91fe2..841ae1f7b 100644 --- a/app/assets/javascripts/index/notes.js.erb +++ b/app/assets/javascripts/index/notes.js.erb @@ -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) { diff --git a/app/assets/javascripts/router.js b/app/assets/javascripts/router.js index 0c855d2d3..8661f95dc 100644 --- a/app/assets/javascripts/router.js +++ b/app/assets/javascripts/router.js @@ -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) { diff --git a/config/routes.rb b/config/routes.rb index 59c5b6803..5551c7c55 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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] diff --git a/test/functional/browse_controller_test.rb b/test/functional/browse_controller_test.rb index 2dcf6b330..5c48a5dc7 100644 --- a/test/functional/browse_controller_test.rb +++ b/test/functional/browse_controller_test.rb @@ -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 diff --git a/test/functional/changeset_controller_test.rb b/test/functional/changeset_controller_test.rb index 69811ad59..0db84a90b 100644 --- a/test/functional/changeset_controller_test.rb +++ b/test/functional/changeset_controller_test.rb @@ -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( diff --git a/test/functional/user_controller_test.rb b/test/functional/user_controller_test.rb index 1cfb1c864..873beccad 100644 --- a/test/functional/user_controller_test.rb +++ b/test/functional/user_controller_test.rb @@ -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