From: Tom Hughes Date: Sat, 7 May 2011 11:49:38 +0000 (+0100) Subject: Tidy up some of the map bugs code X-Git-Tag: live~5068^2~180 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/fb4d003ebee8842ace3807bb1a60b9b36d94723e Tidy up some of the map bugs code --- diff --git a/app/views/browse/_map.html.erb b/app/views/browse/_map.html.erb index 2e20f07f3..d8980f2a2 100644 --- a/app/views/browse/_map.html.erb +++ b/app/views/browse/_map.html.erb @@ -38,13 +38,15 @@ $("area_larger_map").href = '/?minlon='+minlon+'&minlat='+minlat+'&maxlon='+maxlon+'&maxlat='+maxlat+'&box=yes'; $("area_larger_map").innerHTML = "<%= t 'browse.map.larger.area' %>"; - <% else if map.instance_of? MapBug %> - $("loading").innerHTML = ""; - var centre = new OpenLayers.LonLat(<%= map.lon %>, <%= map.lat %>); - var zoom = 16; - setMapCenter(centre, zoom); - marker = addMarkerToMap(centre); - $("area_larger_map").href = '/?mlon=<%= map.lon %>&mlat=<%=map.lat %>'; + <% elsif map.instance_of? MapBug %> + var centre = new OpenLayers.LonLat(<%= map.lon %>, <%= map.lat %>); + + setMapCenter(centre, 16); + addMarkerToMap(centre); + + $("loading").innerHTML = ""; + + $("area_larger_map").href = '/?mlon=<%= map.lon %>&mlat=<%=map.lat %>'; $("area_larger_map").innerHTML = "<%= t 'browse.map.larger.area' %>"; <% else %> var obj_type = "<%= map.class.name.downcase %>"; @@ -77,7 +79,7 @@ $("small_map").style.display = "none"; } }); - <% end end %> + <% end %> } window.onload = init; diff --git a/app/views/site/index.html.erb b/app/views/site/index.html.erb index 681924444..c0f783499 100644 --- a/app/views/site/index.html.erb +++ b/app/views/site/index.html.erb @@ -25,7 +25,7 @@
@@ -142,34 +142,23 @@ end map.addLayer(map.dataLayer); map.osbLayer = new OpenLayers.Layer.OpenStreetBugs("OpenStreetBugs", { - serverURL : "/api/0.6/", - iconOpen : new OpenLayers.Icon("/images/open_bug_marker.png", new OpenLayers.Size(22, 22), new OpenLayers.Pixel(-11, -11)), - iconClosed : new OpenLayers.Icon("/images/closed_bug_marker.png", new OpenLayers.Size(22, 22), new OpenLayers.Pixel(-11, -11)), - readonly : false, - setCookie : false, - cookieLifetime : 1000, - cookiePath : "/my/map/", - permalinkURL : "http://www.openstreetmap.org/", - theme : "/stylesheets/openstreetbugs.css", - visibility : false + serverURL: "/api/0.6/", + iconOpen: new OpenLayers.Icon("<%= image_path "open_bug_marker.png" %>", new OpenLayers.Size(22, 22), new OpenLayers.Pixel(-11, -11)), + iconClosed: new OpenLayers.Icon("<%= image_path "closed_bug_marker.png" %>", new OpenLayers.Size(22, 22), new OpenLayers.Pixel(-11, -11)), + readonly: false, + setCookie: false, + permalinkURL: "http://www.openstreetmap.org/", + theme: "<%= stylesheet_path "openstreetbugs" %>", + visibility: false }); - map.addLayer(map.osbLayer); map.osbControl = new OpenLayers.Control.OpenStreetBugs(map.osbLayer); - map.addControl(map.osbControl); - - var lBug = document.getElementById('ReportBug'); - /* lBug.addEventListener('click',function (e) { - map.osbControl.activate(); document.getElementById("OpenLayers.Map_18_OpenLayers_Container").style.cursor = "crosshair" },false); */ - lBug.addEventListener('click',function (e) { - map.osbControl.activate(); map.osbControl.addTemporaryMarker(map.getCenter());},false); - - map.events.register("zoomend",map,function () { var zoom = map.getZoom(); var lBug = document.getElementById('ReportBug') - if (zoom > 11) { lBug.style.visibility = 'visible';} else {lBug.style.visibility = "hidden";}}); + $("reportbuganchor").observe("click", addBug); + map.events.register("zoomend", map, allowBugReports); <% end %> <% unless object_zoom %> @@ -333,6 +322,19 @@ end <% end %> } + function addBug() { + map.osbControl.activate(); + map.osbControl.addTemporaryMarker(map.getCenter()); + } + + function allowBugReports() { + if (map.getZoom() > 11) { + $("reportbuganchor").style.visibility = "visible"; + } else { + $("reportbuganchor").style.visibility = "hidden"; + } + } + mapInit(); Event.observe(window, "load", installEditHandler); diff --git a/config/routes.rb b/config/routes.rb index e648f2a19..62c43ee03 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -76,25 +76,20 @@ ActionController::Routing::Routes.draw do |map| map.connect "api/#{API_VERSION}/swf/trackpoints", :controller =>'swf', :action =>'trackpoints' # Map Bugs API - map.connect "api/#{API_VERSION}/bugs/getBugs", :controller =>'map_bugs', :action =>'get_bugs' - map.connect "api/#{API_VERSION}/bugs/addPOIexec", :controller =>'map_bugs', :action =>'add_bug' - map.connect "api/#{API_VERSION}/bugs/closePOIexec", :controller =>'map_bugs', :action =>'close_bug' - map.connect "api/#{API_VERSION}/bugs/editPOIexec", :controller =>'map_bugs', :action =>'edit_bug' - map.connect "api/#{API_VERSION}/bugs/getGPX", :controller =>'map_bugs', :action =>'gpx_bugs' - map.connect "api/#{API_VERSION}/bugs/getRSSfeed", :controller =>'map_bugs', :action =>'rss' - + map.connect "api/#{API_VERSION}/bugs/getBugs", :controller => 'map_bugs', :action => 'get_bugs' + map.connect "api/#{API_VERSION}/bugs/addPOIexec", :controller => 'map_bugs', :action => 'add_bug' + map.connect "api/#{API_VERSION}/bugs/closePOIexec", :controller => 'map_bugs', :action => 'close_bug' + map.connect "api/#{API_VERSION}/bugs/editPOIexec", :controller => 'map_bugs', :action => 'edit_bug' + map.connect "api/#{API_VERSION}/bugs/getGPX", :controller => 'map_bugs', :action => 'gpx_bugs' + map.connect "api/#{API_VERSION}/bugs/getRSSfeed", :controller => 'map_bugs', :action => 'rss' map.connect "api/#{API_VERSION}/bugs", :controller => 'map_bugs', :action => 'get_bugs' map.connect "api/#{API_VERSION}/bugs/search", :controller => 'map_bugs', :action => 'search' - map.connect "api/#{API_VERSION}/bugs/rss", :controller =>'map_bugs', :action =>'rss' + map.connect "api/#{API_VERSION}/bugs/rss", :controller =>'map_bugs', :action => 'rss' map.connect "api/#{API_VERSION}/bug/create", :controller => 'map_bugs', :action => 'add_bug' map.connect "api/#{API_VERSION}/bug/:id/comment", :controller => 'map_bugs', :action => 'edit_bug', :id => /\d+/ map.connect "api/#{API_VERSION}/bug/:id/close", :controller => 'map_bugs', :action => 'close_bug', :id => /\d+/ map.connect "api/#{API_VERSION}/bug/:id", :controller => 'map_bugs', :action => 'read', :id => /\d+/, :conditions => { :method => :get } map.connect "api/#{API_VERSION}/bug/:id", :controller => 'map_bugs', :action => 'delete', :id => /\d+/, :conditions => { :method => :delete } - - map.connect '/user/:display_name/bugs', :controller => 'map_bugs', :action => 'my_bugs' - - # Data browsing map.connect '/browse/start', :controller => 'browse', :action => 'start' @@ -110,6 +105,7 @@ ActionController::Routing::Routes.draw do |map| map.connect '/browse/changesets/feed', :controller => 'changeset', :action => 'list', :format => :atom map.connect '/browse/changesets', :controller => 'changeset', :action => 'list' map.connect '/browse/bug/:id', :controller => 'browse', :action => 'bug', :id => /\d+/ + map.connect '/user/:display_name/bugs', :controller => 'map_bugs', :action => 'my_bugs' map.connect '/browse', :controller => 'changeset', :action => 'list' # web site diff --git a/db/migrate/053_add_map_bug_tables.rb b/db/migrate/053_add_map_bug_tables.rb index 2d3b7348b..8d444a49f 100644 --- a/db/migrate/053_add_map_bug_tables.rb +++ b/db/migrate/053_add_map_bug_tables.rb @@ -2,8 +2,7 @@ require 'lib/migrate' class AddMapBugTables < ActiveRecord::Migration def self.up - - create_enumeration :map_bug_status_enum, ["open", "closed","hidden"] + create_enumeration :map_bug_status_enum, ["open", "closed", "hidden"] create_table :map_bugs do |t| t.column :id, :bigint, :null => false @@ -14,20 +13,21 @@ class AddMapBugTables < ActiveRecord::Migration t.datetime :date_created, :null => false t.string :nearby_place t.string :text - t.column :status, :map_bug_status_enum, :null => false - + t.column :status, :map_bug_status_enum, :null => false end - add_index :map_bugs, [:tile,:status], :name => "map_bugs_tile_idx" - add_index :map_bugs, [:last_changed], :name => "map_bugs_changed_idx" - add_index :map_bugs, [:date_created], :name => "map_bugs_created_idx" + add_index :map_bugs, [:tile, :status], :name => "map_bugs_tile_idx" + add_index :map_bugs, [:last_changed], :name => "map_bugs_changed_idx" + add_index :map_bugs, [:date_created], :name => "map_bugs_created_idx" end def self.down remove_index :map_bugs, :name => "map_bugs_tile_idx" - remove_index :map_bugs, :name => "map_bugs_changed_idx" - remove_index :map_bugs, :name => "map_bugs_created_idx" + remove_index :map_bugs, :name => "map_bugs_changed_idx" + remove_index :map_bugs, :name => "map_bugs_created_idx" + drop_table :map_bugs - drop_enumeration :map_bug_status_enum + + drop_enumeration :map_bug_status_enum end end diff --git a/db/migrate/054_refactor_map_bug_tables.rb b/db/migrate/054_refactor_map_bug_tables.rb index b45f935cc..6d259d20a 100644 --- a/db/migrate/054_refactor_map_bug_tables.rb +++ b/db/migrate/054_refactor_map_bug_tables.rb @@ -2,35 +2,33 @@ require 'lib/migrate' class RefactorMapBugTables < ActiveRecord::Migration def self.up - - create_table :map_bug_comment do |t| t.column :id, :bigint, :null => false - t.column :bug_id, :bigint, :null => false + t.column :bug_id, :bigint, :null => false t.boolean :visible, :null => false t.datetime :date_created, :null => false - t.string :commenter_name - t.string :commenter_ip - t.column :commenter_id, :bigint + t.string :commenter_name + t.string :commenter_ip + t.column :commenter_id, :bigint t.string :comment end - remove_column :map_bugs, :text + remove_column :map_bugs, :text add_index :map_bug_comment, [:bug_id], :name => "map_bug_comment_id_idx" - add_foreign_key :map_bug_comment, [:bug_id], :map_bugs, [:id] - add_foreign_key :map_bug_comment, [:commenter_id], :users, [:id] + add_foreign_key :map_bug_comment, [:bug_id], :map_bugs, [:id] + add_foreign_key :map_bug_comment, [:commenter_id], :users, [:id] end def self.down - - add_column :map_bugs, :text, :string + remove_foreign_key :map_bug_comment, [:commenter_id] + remove_foreign_key :map_bug_comment, [:bug_id] remove_index :map_bugs, :name => "map_bug_comment_id_idx" - remove_foreign_key :map_bug_comment, [:bug_id] - remove_foreign_key :map_bug_comment, [:commenter_id] - drop_table :map_bugs_comment + add_column :map_bugs, :text, :string + + drop_table :map_bug_comment end end diff --git a/db/migrate/055_change_map_bug_comment_type.rb b/db/migrate/055_change_map_bug_comment_type.rb index bf009c3dd..2a64bf216 100644 --- a/db/migrate/055_change_map_bug_comment_type.rb +++ b/db/migrate/055_change_map_bug_comment_type.rb @@ -2,10 +2,10 @@ require 'lib/migrate' class ChangeMapBugCommentType < ActiveRecord::Migration def self.up - change_column :map_bug_comment, :comment, :text + change_column :map_bug_comment, :comment, :text end def self.down - change_column :map_bug_comment, :comment, :string + change_column :map_bug_comment, :comment, :string end end diff --git a/db/migrate/056_add_date_closed.rb b/db/migrate/056_add_date_closed.rb index 7f609cade..c5aa2c2f5 100644 --- a/db/migrate/056_add_date_closed.rb +++ b/db/migrate/056_add_date_closed.rb @@ -2,12 +2,10 @@ require 'lib/migrate' class AddDateClosed < ActiveRecord::Migration def self.up - - add_column :map_bugs, :date_closed, :timestamp + add_column :map_bugs, :date_closed, :timestamp end def self.down - - remove_column :map_bugs, :date_closed + remove_column :map_bugs, :date_closed end end diff --git a/db/migrate/057_add_map_bug_comment_event.rb b/db/migrate/057_add_map_bug_comment_event.rb index ff5f0b352..c13c1f9d5 100644 --- a/db/migrate/057_add_map_bug_comment_event.rb +++ b/db/migrate/057_add_map_bug_comment_event.rb @@ -2,13 +2,14 @@ require 'lib/migrate' class AddMapBugCommentEvent < ActiveRecord::Migration def self.up - create_enumeration :map_bug_event_enum, ["opened", "closed","reopened","commented","hidden"] - add_column :map_bug_comment, :event, :map_bug_event_enum + create_enumeration :map_bug_event_enum, ["opened", "closed", "reopened", "commented", "hidden"] + + add_column :map_bug_comment, :event, :map_bug_event_enum end def self.down + remove_column :map_bug_comment, :event - remove_column :map_bug_comment, :event - drop_enumeration :map_bug_event_enum + drop_enumeration :map_bug_event_enum end end diff --git a/lib/geo_record.rb b/lib/geo_record.rb index d44227dd8..4aa45531f 100644 --- a/lib/geo_record.rb +++ b/lib/geo_record.rb @@ -54,7 +54,8 @@ private return self.find(:all, options) end end - def find_by_area_no_quadtile(minlat, minlon, maxlat, maxlon, options) + + def find_by_area_no_quadtile(minlat, minlon, maxlat, maxlon, options) self.with_scope(:find => {:conditions => OSM.sql_for_area_no_quadtile(minlat, minlon, maxlat, maxlon)}) do return self.find(:all, options) end diff --git a/public/stylesheets/large.css b/public/stylesheets/large.css index cb2fe29b6..5d25e7fba 100644 --- a/public/stylesheets/large.css +++ b/public/stylesheets/large.css @@ -19,6 +19,8 @@ display: none; } -a.reportProblem { - font-size:150%; +/* Rules for map bug reporting */ + +#reportbuganchor { + font-size: 150%; } diff --git a/test/functional/map_bugs_controller_test.rb b/test/functional/map_bugs_controller_test.rb index c3335d2b1..9aa155c3e 100644 --- a/test/functional/map_bugs_controller_test.rb +++ b/test/functional/map_bugs_controller_test.rb @@ -5,34 +5,33 @@ class MapBugsControllerTest < ActionController::TestCase def test_map_bug_create_success assert_difference('MapBug.count') do - assert_difference('MapBugComment.count') do + assert_difference('MapBugComment.count') do post :add_bug, {:lat => -1.0, :lon => -1.0, :name => "new_tester", :text => "This is a comment"} - end - end + end + end assert_response :success - id = @response.body.sub(/ok/,"").to_i - get :read, {:id => id, :format => 'json'} + id = @response.body.sub(/ok/,"").to_i + get :read, {:id => id, :format => 'json'} assert_response :success - js = @response.body - assert_match "\"status\":\"open\"", js - assert_match "\"comment\":\"This is a comment\"", js - assert_match "\"commenter_name\":\"new_tester (a)\"", js + js = @response.body + assert_match "\"status\":\"open\"", js + assert_match "\"comment\":\"This is a comment\"", js + assert_match "\"commenter_name\":\"new_tester (a)\"", js end def test_map_bug_comment_create_success assert_difference('MapBugComment.count') do - post :edit_bug, {:id => 2, :name => "new_tester2", :text => "This is an additional comment"} + post :edit_bug, {:id => 2, :name => "new_tester2", :text => "This is an additional comment"} end assert_response :success - get :read, {:id => 2, :format => 'json'} + get :read, {:id => 2, :format => 'json'} assert_response :success - js = @response.body - assert_match "\"id\":2", js - assert_match "\"status\":\"open\"", js - assert_match "\"comment\":\"This is an additional comment\"", js - assert_match "\"commenter_name\":\"new_tester2 (a)\"", js - + js = @response.body + assert_match "\"id\":2", js + assert_match "\"status\":\"open\"", js + assert_match "\"comment\":\"This is an additional comment\"", js + assert_match "\"commenter_name\":\"new_tester2 (a)\"", js end def test_map_bug_read_success @@ -53,122 +52,118 @@ class MapBugsControllerTest < ActionController::TestCase end def test_map_bug_close_success - post :close_bug, {:id => 2} + post :close_bug, {:id => 2} assert_response :success - get :read, {:id => 2, :format => 'json'} - js = @response.body - assert_match "\"id\":2", js - assert_match "\"status\":\"closed\"", js + get :read, {:id => 2, :format => 'json'} + js = @response.body + assert_match "\"id\":2", js + assert_match "\"status\":\"closed\"", js end def test_get_bugs_success - get :get_bugs, {:bbox=>'1,1,1.2,1.2'} - assert_response :success + get :get_bugs, {:bbox=>'1,1,1.2,1.2'} + assert_response :success - get :get_bugs, {:bbox=>'1,1,1.2,1.2', :format => 'rss'} - assert_response :success + get :get_bugs, {:bbox=>'1,1,1.2,1.2', :format => 'rss'} + assert_response :success - get :get_bugs, {:bbox=>'1,1,1.2,1.2', :format => 'json'} - assert_response :success + get :get_bugs, {:bbox=>'1,1,1.2,1.2', :format => 'json'} + assert_response :success - get :get_bugs, {:bbox=>'1,1,1.2,1.2', :format => 'xml'} - assert_response :success + get :get_bugs, {:bbox=>'1,1,1.2,1.2', :format => 'xml'} + assert_response :success - get :get_bugs, {:bbox=>'1,1,1.2,1.2', :format => 'gpx'} - assert_response :success + get :get_bugs, {:bbox=>'1,1,1.2,1.2', :format => 'gpx'} + assert_response :success end def test_get_bugs_large_area_success - get :get_bugs, {:bbox=>'-10,-10,12,12'} - assert_response :success + get :get_bugs, {:bbox=>'-10,-10,12,12'} + assert_response :success end def test_get_bugs_closed_7_success - get :get_bugs, {:bbox=>'1,1,1.2,1.2', :closed => '7'} - assert_response :success + get :get_bugs, {:bbox=>'1,1,1.2,1.2', :closed => '7'} + assert_response :success end def test_get_bugs_closed_0_success - get :get_bugs, {:bbox=>'1,1,1.2,1.2', :closed => '0'} - assert_response :success + get :get_bugs, {:bbox=>'1,1,1.2,1.2', :closed => '0'} + assert_response :success end def test_get_bugs_closed_n1_success - get :get_bugs, {:bbox=>'1,1,1.2,1.2', :closed => '-1'} - assert_response :success + get :get_bugs, {:bbox=>'1,1,1.2,1.2', :closed => '-1'} + assert_response :success end def test_search_success - get :search, {:bbox=>'1,1,1.2,1.2', :q => 'bug 1'} - assert_response :success + get :search, {:bbox=>'1,1,1.2,1.2', :q => 'bug 1'} + assert_response :success - get :search, {:bbox=>'1,1,1.2,1.2', :q => 'bug 1', :format => 'xml'} - assert_response :success + get :search, {:bbox=>'1,1,1.2,1.2', :q => 'bug 1', :format => 'xml'} + assert_response :success - get :search, {:bbox=>'1,1,1.2,1.2', :q => 'bug 1', :format => 'json'} - assert_response :success + get :search, {:bbox=>'1,1,1.2,1.2', :q => 'bug 1', :format => 'json'} + assert_response :success - get :search, {:bbox=>'1,1,1.2,1.2', :q => 'bug 1', :format => 'rss'} - assert_response :success + get :search, {:bbox=>'1,1,1.2,1.2', :q => 'bug 1', :format => 'rss'} + assert_response :success - get :search, {:bbox=>'1,1,1.2,1.2', :q => 'bug 1', :format => 'gpx'} - assert_response :success + get :search, {:bbox=>'1,1,1.2,1.2', :q => 'bug 1', :format => 'gpx'} + assert_response :success end def test_rss_success - get :rss, {:bbox=>'1,1,1.2,1.2'} - assert_response :success + get :rss, {:bbox=>'1,1,1.2,1.2'} + assert_response :success - get :rss - assert_response :success + get :rss + assert_response :success end def test_user_bugs_success - get :my_bugs, {:display_name=>'test'} - assert_response :success + get :my_bugs, {:display_name=>'test'} + assert_response :success - get :my_bugs, {:display_name=>'pulibc_test2'} - assert_response :success + get :my_bugs, {:display_name=>'pulibc_test2'} + assert_response :success - get :my_bugs, {:display_name=>'non-existent'} - assert_response :not_found - + get :my_bugs, {:display_name=>'non-existent'} + assert_response :not_found end def test_map_bug_comment_create_not_found assert_no_difference('MapBugComment.count') do - post :edit_bug, {:id => 12345, :name => "new_tester", :text => "This is an additional comment"} + post :edit_bug, {:id => 12345, :name => "new_tester", :text => "This is an additional comment"} end assert_response :not_found end def test_map_bug_close_not_found - post :close_bug, {:id => 12345} + post :close_bug, {:id => 12345} assert_response :not_found end def test_map_bug_read_not_found - get :read, {:id => 12345} + get :read, {:id => 12345} assert_response :not_found end def test_map_bug_read_gone - get :read, {:id => 4} + get :read, {:id => 4} assert_response :gone end def test_map_bug_hidden_comment - get :read, {:id => 5, :format => 'json'} - assert_response :success - js = @response.body - assert_match "\"id\":5", js - assert_match "\"comment\":\"Valid comment for bug 5\"", js - assert_match "\"comment\":\"Another valid comment for bug 5\"", js - assert_no_match /\"comment\":\"Spam for bug 5\"/, js + get :read, {:id => 5, :format => 'json'} + assert_response :success + js = @response.body + assert_match "\"id\":5", js + assert_match "\"comment\":\"Valid comment for bug 5\"", js + assert_match "\"comment\":\"Another valid comment for bug 5\"", js + assert_no_match /\"comment\":\"Spam for bug 5\"/, js end - - - end