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.
This commit is contained in:
Tom Hughes 2013-11-28 00:14:07 +00:00
parent 6b236ec95b
commit 3cd5f45e08
9 changed files with 62 additions and 46 deletions

View file

@ -261,13 +261,13 @@ $(document).ready(function () {
"/": OSM.Index(map), "/": OSM.Index(map),
"/search": OSM.Search(map), "/search": OSM.Search(map),
"/export": OSM.Export(map), "/export": OSM.Export(map),
"/new_note": OSM.NewNote(map), "/note/new": OSM.NewNote(map),
"/history/friends": history,
"/history/nearby": history,
"/history": history, "/history": history,
"/user/:display_name/edits": history, "/user/:display_name/history": history,
"/browse/friends": history, "/note/:id": OSM.Note(map),
"/browse/nearby": history, "/:type/:id(/history)": OSM.Browse(map)
"/browse/note/:id": OSM.Note(map),
"/browse/:type/:id(/history)": OSM.Browse(map)
}); });
OSM.router.load(); OSM.router.load();

View file

@ -124,7 +124,7 @@ function initializeBrowse(map) {
layer.originalStyle = layer.options; layer.originalStyle = layer.options;
layer.setStyle({color: '#0000ff', weight: 8}); 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 // Stash the currently drawn feature
selectedLayer = layer; selectedLayer = layer;

View file

@ -30,7 +30,7 @@ OSM.NewNote = function(map) {
if ($(this).hasClass('disabled')) return; if ($(this).hasClass('disabled')) return;
OSM.router.route('/new_note'); OSM.router.route('/note/new');
}); });
function createNote(marker, form, url) { function createNote(marker, form, url) {
@ -61,7 +61,7 @@ OSM.NewNote = function(map) {
newNote = null; newNote = null;
noteLayer.removeLayer(marker); noteLayer.removeLayer(marker);
addNoteButton.removeClass("active"); addNoteButton.removeClass("active");
OSM.route('/browse/note/' + feature.properties.id); OSM.route('/note/' + feature.properties.id);
} }
} }

View file

@ -34,7 +34,7 @@ function initializeNotes(map) {
}); });
noteLayer.on('click', function(e) { noteLayer.on('click', function(e) {
OSM.router.route('/browse/note/' + e.layer.id); OSM.router.route('/note/' + e.layer.id);
}); });
function updateMarker(marker, feature) { function updateMarker(marker, feature) {

View file

@ -8,7 +8,7 @@
The router is initialized with a set of routes: a mapping of URL path templates The router is initialized with a set of routes: a mapping of URL path templates
to route controller objects. Path templates can contain placeholders 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 Route controller objects can define four methods that are called at defined
times during routing: times during routing:
@ -34,7 +34,7 @@
An instance of OSM.Router is assigned to `OSM.router`. To navigate to a new page 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`: 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 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. 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 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 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 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.moveListenerOff` and `OSM.router.moveListenerOn`.
*/ */
OSM.Router = function(map, rts) { OSM.Router = function(map, rts) {

View file

@ -102,24 +102,36 @@ OpenStreetMap::Application.routes.draw do
end end
# Data browsing # Data browsing
match '/browse/way/:id' => 'browse#way', :via => :get, :id => /\d+/, :as => :way match '/way/:id' => 'browse#way', :via => :get, :id => /\d+/, :as => :way
match '/browse/way/:id/history' => 'browse#way_history', :via => :get, :id => /\d+/ match '/way/:id/history' => 'browse#way_history', :via => :get, :id => /\d+/
match '/browse/node/:id' => 'browse#node', :via => :get, :id => /\d+/, :as => :node match '/node/:id' => 'browse#node', :via => :get, :id => /\d+/, :as => :node
match '/browse/node/:id/history' => 'browse#node_history', :via => :get, :id => /\d+/ match '/node/:id/history' => 'browse#node_history', :via => :get, :id => /\d+/
match '/browse/relation/:id' => 'browse#relation', :via => :get, :id => /\d+/, :as => :relation match '/relation/:id' => 'browse#relation', :via => :get, :id => /\d+/, :as => :relation
match '/browse/relation/:id/history' => 'browse#relation_history', :via => :get, :id => /\d+/ match '/relation/:id/history' => 'browse#relation_history', :via => :get, :id => /\d+/
match '/browse/changeset/:id' => 'browse#changeset', :via => :get, :as => :changeset, :id => /\d+/ match '/changeset/:id' => 'browse#changeset', :via => :get, :as => :changeset, :id => /\d+/
match '/browse/note/:id' => 'browse#note', :via => :get, :id => /\d+/, :as => "browse_note" match '/note/:id' => 'browse#note', :via => :get, :id => /\d+/, :as => "browse_note"
match '/new_note' => 'browse#new_note', :via => :get match '/note/new' => 'browse#new_note', :via => :get
match '/user/:display_name/edits' => 'changeset#list', :via => :get match '/user/:display_name/history' => 'changeset#list', :via => :get
match '/user/:display_name/edits/feed' => 'changeset#feed', :via => :get, :defaults => { :format => :atom } match '/user/:display_name/history/feed' => 'changeset#feed', :via => :get, :defaults => { :format => :atom }
match '/user/:display_name/notes' => 'notes#mine', :via => :get match '/user/:display_name/notes' => 'notes#mine', :via => :get
match '/browse/friends' => 'changeset#list', :via => :get, :friends => true, :as => "friend_changesets" match '/history/friends' => 'changeset#list', :via => :get, :friends => true, :as => "friend_changesets"
match '/browse/nearby' => 'changeset#list', :via => :get, :nearby => true, :as => "nearby_changesets" match '/history/nearby' => 'changeset#list', :via => :get, :nearby => true, :as => "nearby_changesets"
get '/browse/changesets/feed', :to => redirect('/history/feed') get '/browse/way/:id', :to => redirect('/way/%{id}')
get '/browse/changesets', :to => redirect('/history') get '/browse/way/:id/history', :to => redirect('/way/%{id}/history')
get '/browse', :to => redirect('/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 # web site
root :to => 'site#index', :via => [:get, :post] root :to => 'site#index', :via => [:get, :post]

View file

@ -8,37 +8,41 @@ class BrowseControllerTest < ActionController::TestCase
# test all routes which lead to this controller # test all routes which lead to this controller
def test_routes def test_routes
assert_routing( assert_routing(
{ :path => "/browse/node/1", :method => :get }, { :path => "/node/1", :method => :get },
{ :controller => "browse", :action => "node", :id => "1" } { :controller => "browse", :action => "node", :id => "1" }
) )
assert_routing( assert_routing(
{ :path => "/browse/node/1/history", :method => :get }, { :path => "/node/1/history", :method => :get },
{ :controller => "browse", :action => "node_history", :id => "1" } { :controller => "browse", :action => "node_history", :id => "1" }
) )
assert_routing( assert_routing(
{ :path => "/browse/way/1", :method => :get }, { :path => "/way/1", :method => :get },
{ :controller => "browse", :action => "way", :id => "1" } { :controller => "browse", :action => "way", :id => "1" }
) )
assert_routing( assert_routing(
{ :path => "/browse/way/1/history", :method => :get }, { :path => "/way/1/history", :method => :get },
{ :controller => "browse", :action => "way_history", :id => "1" } { :controller => "browse", :action => "way_history", :id => "1" }
) )
assert_routing( assert_routing(
{ :path => "/browse/relation/1", :method => :get }, { :path => "/relation/1", :method => :get },
{ :controller => "browse", :action => "relation", :id => "1" } { :controller => "browse", :action => "relation", :id => "1" }
) )
assert_routing( assert_routing(
{ :path => "/browse/relation/1/history", :method => :get }, { :path => "/relation/1/history", :method => :get },
{ :controller => "browse", :action => "relation_history", :id => "1" } { :controller => "browse", :action => "relation_history", :id => "1" }
) )
assert_routing( assert_routing(
{ :path => "/browse/changeset/1", :method => :get }, { :path => "/changeset/1", :method => :get },
{ :controller => "browse", :action => "changeset", :id => "1" } { :controller => "browse", :action => "changeset", :id => "1" }
) )
assert_routing( assert_routing(
{ :path => "/browse/note/1", :method => :get }, { :path => "/note/1", :method => :get },
{ :controller => "browse", :action => "note", :id => "1" } { :controller => "browse", :action => "note", :id => "1" }
) )
assert_routing(
{ :path => "/note/new", :method => :get },
{ :controller => "browse", :action => "new_note" }
)
end end
def test_read_relation def test_read_relation

View file

@ -40,19 +40,19 @@ class ChangesetControllerTest < ActionController::TestCase
{ :controller => "changeset", :action => "query" } { :controller => "changeset", :action => "query" }
) )
assert_routing( assert_routing(
{ :path => "/user/name/edits", :method => :get }, { :path => "/user/name/history", :method => :get },
{ :controller => "changeset", :action => "list", :display_name => "name" } { :controller => "changeset", :action => "list", :display_name => "name" }
) )
assert_routing( 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 } { :controller => "changeset", :action => "feed", :display_name => "name", :format => :atom }
) )
assert_routing( assert_routing(
{ :path => "/browse/friends", :method => :get }, { :path => "/history/friends", :method => :get },
{ :controller => "changeset", :action => "list", :friends => true } { :controller => "changeset", :action => "list", :friends => true }
) )
assert_routing( assert_routing(
{ :path => "/browse/nearby", :method => :get }, { :path => "/history/nearby", :method => :get },
{ :controller => "changeset", :action => "list", :nearby => true } { :controller => "changeset", :action => "list", :nearby => true }
) )
assert_routing( assert_routing(

View file

@ -548,7 +548,7 @@ class UserControllerTest < ActionController::TestCase
get :view, {:display_name => "test"} get :view, {:display_name => "test"}
assert_response :success assert_response :success
assert_select "div#userinformation" do 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/traces]", 1
assert_select "a[href=/user/test/diary]", 1 assert_select "a[href=/user/test/diary]", 1
assert_select "a[href=/user/test/diary/comments]", 1 assert_select "a[href=/user/test/diary/comments]", 1
@ -562,7 +562,7 @@ class UserControllerTest < ActionController::TestCase
get :view, {:display_name => "blocked"} get :view, {:display_name => "blocked"}
assert_response :success assert_response :success
assert_select "div#userinformation" do 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/traces]", 1
assert_select "a[href=/user/blocked/diary]", 1 assert_select "a[href=/user/blocked/diary]", 1
assert_select "a[href=/user/blocked/diary/comments]", 1 assert_select "a[href=/user/blocked/diary/comments]", 1
@ -576,7 +576,7 @@ class UserControllerTest < ActionController::TestCase
get :view, {:display_name => "moderator"} get :view, {:display_name => "moderator"}
assert_response :success assert_response :success
assert_select "div#userinformation" do 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/traces]", 1
assert_select "a[href=/user/moderator/diary]", 1 assert_select "a[href=/user/moderator/diary]", 1
assert_select "a[href=/user/moderator/diary/comments]", 1 assert_select "a[href=/user/moderator/diary/comments]", 1
@ -593,7 +593,7 @@ class UserControllerTest < ActionController::TestCase
get :view, {:display_name => "test"} get :view, {:display_name => "test"}
assert_response :success assert_response :success
assert_select "div#userinformation" do 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=/traces/mine]", 1
assert_select "a[href=/user/test/diary]", 1 assert_select "a[href=/user/test/diary]", 1
assert_select "a[href=/user/test/diary/comments]", 1 assert_select "a[href=/user/test/diary/comments]", 1
@ -610,7 +610,7 @@ class UserControllerTest < ActionController::TestCase
get :view, {:display_name => "test"} get :view, {:display_name => "test"}
assert_response :success assert_response :success
assert_select "div#userinformation" do 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/traces]", 1
assert_select "a[href=/user/test/diary]", 1 assert_select "a[href=/user/test/diary]", 1
assert_select "a[href=/user/test/diary/comments]", 1 assert_select "a[href=/user/test/diary/comments]", 1