]> git.openstreetmap.org Git - rails.git/commitdiff
Tidy up some of the map bugs code
authorTom Hughes <tom@compton.nu>
Sat, 7 May 2011 11:49:38 +0000 (12:49 +0100)
committerTom Hughes <tom@compton.nu>
Sat, 7 May 2011 12:05:39 +0000 (13:05 +0100)
app/views/browse/_map.html.erb
app/views/site/index.html.erb
config/routes.rb
db/migrate/053_add_map_bug_tables.rb
db/migrate/054_refactor_map_bug_tables.rb
db/migrate/055_change_map_bug_comment_type.rb
db/migrate/056_add_date_closed.rb
db/migrate/057_add_map_bug_comment_event.rb
lib/geo_record.rb
public/stylesheets/large.css
test/functional/map_bugs_controller_test.rb

index 2e20f07f38fc0a80ab7d1f1ca10a8f7d0676c379..d8980f2a2bd9eed59221874887bc188826a2fb9c 100644 (file)
 
         $("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;
index 681924444a7fefaf5aaa6d830fb00c5292b3008c..c0f783499ec037b9055e06408f93f156022066ce 100644 (file)
@@ -25,7 +25,7 @@
 <div id="permalink">
   <a href="/" id="permalinkanchor"><%= t 'site.index.permalink' %></a><br/>
   <a href="/" id="shortlinkanchor"><%= t 'site.index.shortlink' %></a><br/>
-  <a href="javascript:void();" id="ReportBug" class="reportProblem">Report a problem</a>       
+  <a href="#" id="reportbuganchor">Report a problem</a>        
 </div>
 
 <div id="attribution">
@@ -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);
index e648f2a198c7a49da7e7590d723d0d4d32fb2940..62c43ee031cfe40e283dc10e3656a5cfa642e324 100644 (file)
@@ -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
index 2d3b7348bd3d62bc953a51f1dc0ba935b667f488..8d444a49fdb1bdc45f65b326ef3e66a1a47eaa19 100644 (file)
@@ -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
index b45f935cc32c7db4daa847579ec0894a4bc0ab42..6d259d20aae11e86d55d8b66d35174eefc7c8543 100644 (file)
@@ -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
index bf009c3dd2721443d9cf4719bfe70558b2eae08a..2a64bf216477ac818630a1f19f55ce9e89bb271a 100644 (file)
@@ -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
index 7f609cade402653801823e4ebf4d67351c653ac3..c5aa2c2f5feed40d96d6f351cf440c1b9a1195e2 100644 (file)
@@ -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
index ff5f0b352e27b44ec4bc60ea78fa88dc29c2ab6d..c13c1f9d514bf5e025f2bce5671888c73e2ba19a 100644 (file)
@@ -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
index d44227dd85d77d07ec30db995041595d6dea3c92..4aa45531f37a6719c43aa7277896d358c96eda7c 100644 (file)
@@ -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
index cb2fe29b6e3b433ee1ae8e0bf3eca359a320cea3..5d25e7fbadfd39349cc27a509a483e12f92c483b 100644 (file)
@@ -19,6 +19,8 @@
   display: none;
 }
 
-a.reportProblem { 
-  font-size:150%;
+/* Rules for map bug reporting */
+
+#reportbuganchor { 
+  font-size: 150%;
 }
index c3335d2b10ec8f257523c4e8fb9edd73a0f09812..9aa155c3eb907502710dcd72db3c65e3ae00889c 100644 (file)
@@ -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