Add extra tests for API and redaction controllers

diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index caa2df9d81f622e92c4b0f01656968e91579bb3b..379bb7e8650c3577588e4954d8a731c721425b99 100644 (file)
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -125,17 +125,13 @@ class ApiController < ApplicationController
       return
     end
 
       return
     end
 

-    @nodes = Node.bbox(bbox).where(:visible => true).includes(:node_tags).limit(MAX_NUMBER_OF_NODES + 1)
+    nodes = Node.bbox(bbox).where(:visible => true).includes(:node_tags).limit(MAX_NUMBER_OF_NODES + 1)

 
 

-    node_ids = @nodes.collect(&:id)
+    node_ids = nodes.collect(&:id)

     if node_ids.length > MAX_NUMBER_OF_NODES
       report_error("You requested too many nodes (limit is #{MAX_NUMBER_OF_NODES}). Either request a smaller area, or use planet.osm")
       return
     end
     if node_ids.length > MAX_NUMBER_OF_NODES
       report_error("You requested too many nodes (limit is #{MAX_NUMBER_OF_NODES}). Either request a smaller area, or use planet.osm")
       return
     end

-    if node_ids.length == 0
-      render :text => "<osm version='#{API_VERSION}' generator='#{GENERATOR}'></osm>", :content_type => "text/xml"
-      return
-    end

 
     doc = OSM::API.new.get_xml_doc
 
 
     doc = OSM::API.new.get_xml_doc
 


@@ -163,14 +159,14 @@ class ApiController < ApplicationController
     nodes_to_fetch = (list_of_way_nodes.uniq - node_ids) - [0]
 
     if nodes_to_fetch.length > 0
     nodes_to_fetch = (list_of_way_nodes.uniq - node_ids) - [0]
 
     if nodes_to_fetch.length > 0

-      @nodes += Node.includes(:node_tags).find(nodes_to_fetch)
+      nodes += Node.includes(:node_tags).find(nodes_to_fetch)

     end
 
     visible_nodes = {}
     changeset_cache = {}
     user_display_name_cache = {}
 
     end
 
     visible_nodes = {}
     changeset_cache = {}
     user_display_name_cache = {}
 

-    @nodes.each do |node|
+    nodes.each do |node|

       if node.visible?
         doc.root << node.to_xml_node(changeset_cache, user_display_name_cache)
         visible_nodes[node.id] = node
       if node.visible?
         doc.root << node.to_xml_node(changeset_cache, user_display_name_cache)
         visible_nodes[node.id] = node

diff --git a/app/models/redaction.rb b/app/models/redaction.rb
index fda59f33bcd10e5098af8d210faf5f11715237e0..49de3177a127af1a0bbe260aaf1feaa15b716109 100644 (file)
--- a/app/models/redaction.rb
+++ b/app/models/redaction.rb
@@ -14,6 +14,9 @@ class Redaction < ActiveRecord::Base
   has_many :old_ways
   has_many :old_relations
 
   has_many :old_ways
   has_many :old_relations
 

+  validates :description, :presence => true
+  validates :description_format, :inclusion => { :in => %w(text html markdown) }
+

   # this method overrides the AR default to provide the rich
   # text object for the description field.
   def description
   # this method overrides the AR default to provide the rich
   # text object for the description field.
   def description

diff --git a/test/controllers/api_controller_test.rb b/test/controllers/api_controller_test.rb
index 5437e24221d46a1c9c63fc3e474e13f1e36780a6..178eecc6542c8cdebd67f034fdabadf99943e020 100644 (file)
--- a/test/controllers/api_controller_test.rb
+++ b/test/controllers/api_controller_test.rb
@@ -72,7 +72,11 @@ class ApiControllerTest < ActionController::TestCase
         # This should really be more generic
         assert_select "tag[k='test'][v='yes']"
       end
         # This should really be more generic
         assert_select "tag[k='test'][v='yes']"
       end

-      # Should also test for the ways and relation
+      assert_select "way", :count => 2
+      assert_select "way[id='1']", :count => 1
+      assert_select "way[id='3']", :count => 1
+      assert_select "relation", :count => 1
+      assert_select "relation[id='1']", :count => 1

     end
   end
 
     end
   end
 


@@ -89,7 +93,41 @@ class ApiControllerTest < ActionController::TestCase
         # This should really be more generic
         assert_select "tag[k='test'][v='yes']"
       end
         # This should really be more generic
         assert_select "tag[k='test'][v='yes']"
       end

-      # Should also test for the ways and relation
+      assert_select "way", :count => 2
+      assert_select "way[id='1']", :count => 1
+      assert_select "way[id='3']", :count => 1
+      assert_select "relation", :count => 1
+      assert_select "relation[id='1']", :count => 1
+    end
+  end
+
+  def test_map_complete_way
+    node = current_nodes(:used_node_2)
+    bbox = "#{node.lon},#{node.lat},#{node.lon},#{node.lat}"
+    get :map, :bbox => bbox
+    assert_response :success, "The map call should have succeeded"
+    assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+      assert_select "bounds[minlon='#{node.lon}'][minlat='#{node.lat}'][maxlon='#{node.lon}'][maxlat='#{node.lat}']", :count => 1
+      assert_select "node", :count => 3
+      assert_select "node[id='4']", :count => 1
+      assert_select "node[id='11']", :count => 1
+      assert_select "node[id='15']", :count => 1
+      assert_select "way", :count => 2
+      assert_select "way[id='5']", :count => 1
+      assert_select "way[id='7']", :count => 1
+      assert_select "relation", :count => 1
+      assert_select "relation[id='8']", :count => 1
+    end
+  end
+
+  def test_map_empty
+    get :map, :bbox => "179.998,89.998,179.999.1,89.999"
+    assert_response :success, "The map call should have succeeded"
+    assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+      assert_select "bounds[minlon='179.998'][minlat='89.998'][maxlon='179.999'][maxlat='89.999']", :count => 1
+      assert_select "node", :count => 0
+      assert_select "way", :count => 0
+      assert_select "relation", :count => 0

     end
   end
 
     end
   end
 


@@ -240,7 +278,24 @@ class ApiControllerTest < ActionController::TestCase
     now = Time.now.getutc
     hourago = now - 1.hour
     assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
     now = Time.now.getutc
     hourago = now - 1.hour
     assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do

-      assert_select "changes[starttime='#{hourago.xmlschema}'][endtime='#{now.xmlschema}']", :count => 1
+      assert_select "changes[starttime='#{hourago.xmlschema}'][endtime='#{now.xmlschema}']", :count => 1 do
+        assert_select "tile", :count => 0
+      end
+    end
+    Timecop.return
+
+    Timecop.freeze(Time.parse("2007-01-01 00:30:00"))
+    get :changes
+    assert_response :success
+    # print @response.body
+    # As we have loaded the fixtures, we can assume that there are some
+    # changes at the time we have frozen at
+    now = Time.now.getutc
+    hourago = now - 1.hour
+    assert_select "osm[version='#{API_VERSION}'][generator='#{GENERATOR}']", :count => 1 do
+      assert_select "changes[starttime='#{hourago.xmlschema}'][endtime='#{now.xmlschema}']", :count => 1 do
+        assert_select "tile", :count => 10
+      end

     end
     Timecop.return
   end
     end
     Timecop.return
   end


@@ -266,7 +321,7 @@ class ApiControllerTest < ActionController::TestCase
     end
   end
 
     end
   end
 

-  def test_hours_invalid
+  def test_changes_hours_invalid

     invalid = %w(-21 335 -1 0 25 26 100 one two three ping pong :)
     invalid.each do |hour|
       get :changes, :hours => hour
     invalid = %w(-21 335 -1 0 25 26 100 one two three ping pong :)
     invalid.each do |hour|
       get :changes, :hours => hour


@@ -275,13 +330,24 @@ class ApiControllerTest < ActionController::TestCase
     end
   end
 
     end
   end
 

-  def test_hours_valid
+  def test_changes_hours_valid

     1.upto(24) do |hour|
       get :changes, :hours => hour
       assert_response :success
     end
   end
 
     1.upto(24) do |hour|
       get :changes, :hours => hour
       assert_response :success
     end
   end
 

+  def test_changes_start_end_invalid
+    get :changes, :start => "2010-04-03 10:55:00", :end => "2010-04-03 09:55:00"
+    assert_response :bad_request
+    assert_equal @response.body, "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours"
+  end
+
+  def test_changes_start_end_valid
+    get :changes, :start => "2010-04-03 09:55:00", :end => "2010-04-03 10:55:00"
+    assert_response :success
+  end
+

   def test_capabilities
     get :capabilities
     assert_response :success
   def test_capabilities
     get :capabilities
     assert_response :success

diff --git a/test/controllers/redactions_controller_test.rb b/test/controllers/redactions_controller_test.rb
index 7e9170a2c192985370ea1551c2a91493070693e7..1df2a6dff9e8e613fadb8569843874c229cf7913 100644 (file)
--- a/test/controllers/redactions_controller_test.rb
+++ b/test/controllers/redactions_controller_test.rb
@@ -37,7 +37,38 @@ class RedactionsControllerTest < ActionController::TestCase
     )
   end
 
     )
   end
 

-  def test_moderators_can_create
+  def test_index
+    get :index
+    assert_response :success
+    assert_template :index
+    assert_select "ul#redaction_list", 1 do
+      assert_select "li", Redaction.count
+    end
+  end
+
+  def test_new
+    get :new
+    assert_response :redirect
+    assert_redirected_to login_path(:referer => new_redaction_path)
+  end
+
+  def test_new_moderator
+    session[:user] = users(:moderator_user).id
+
+    get :new
+    assert_response :success
+    assert_template :new
+  end
+
+  def test_new_non_moderator
+    session[:user] = users(:public_user).id
+
+    get :new
+    assert_response :redirect
+    assert_redirected_to redactions_path
+  end
+
+  def test_create_moderator

     session[:user] = users(:moderator_user).id
 
     post :create, :redaction => { :title => "Foo", :description => "Description here." }
     session[:user] = users(:moderator_user).id
 
     post :create, :redaction => { :title => "Foo", :description => "Description here." }


@@ -45,14 +76,22 @@ class RedactionsControllerTest < ActionController::TestCase
     assert_redirected_to(redaction_path(Redaction.find_by_title("Foo")))
   end
 
     assert_redirected_to(redaction_path(Redaction.find_by_title("Foo")))
   end
 

-  def test_non_moderators_cant_create
+  def test_create_moderator_invalid
+    session[:user] = users(:moderator_user).id
+
+    post :create, :redaction => { :title => "Foo", :description => "" }
+    assert_response :success
+    assert_template :new
+  end
+
+  def test_create_non_moderator

     session[:user] = users(:public_user).id
 
     post :create, :redaction => { :title => "Foo", :description => "Description here." }
     assert_response :forbidden
   end
 
     session[:user] = users(:public_user).id
 
     post :create, :redaction => { :title => "Foo", :description => "Description here." }
     assert_response :forbidden
   end
 

-  def test_moderators_can_delete_empty
+  def test_destroy_moderator_empty

     session[:user] = users(:moderator_user).id
 
     # remove all elements from the redaction
     session[:user] = users(:moderator_user).id
 
     # remove all elements from the redaction


@@ -66,7 +105,7 @@ class RedactionsControllerTest < ActionController::TestCase
     assert_redirected_to(redactions_path)
   end
 
     assert_redirected_to(redactions_path)
   end
 

-  def test_moderators_cant_delete_nonempty
+  def test_destroy_moderator_non_empty

     session[:user] = users(:moderator_user).id
 
     # leave elements in the redaction
     session[:user] = users(:moderator_user).id
 
     # leave elements in the redaction


@@ -78,21 +117,27 @@ class RedactionsControllerTest < ActionController::TestCase
     assert_match /^Redaction is not empty/, flash[:error]
   end
 
     assert_match /^Redaction is not empty/, flash[:error]
   end
 

-  def test_non_moderators_cant_delete
+  def test_delete_non_moderator

     session[:user] = users(:public_user).id
 
     delete :destroy, :id => redactions(:example).id
     assert_response :forbidden
   end
 
     session[:user] = users(:public_user).id
 
     delete :destroy, :id => redactions(:example).id
     assert_response :forbidden
   end
 

-  def test_moderators_can_edit
+  def test_edit
+    get :edit, :id => redactions(:example).id
+    assert_response :redirect
+    assert_redirected_to login_path(:referer => edit_redaction_path(redactions(:example)))
+  end
+
+  def test_edit_moderator

     session[:user] = users(:moderator_user).id
 
     get :edit, :id => redactions(:example).id
     assert_response :success
   end
 
     session[:user] = users(:moderator_user).id
 
     get :edit, :id => redactions(:example).id
     assert_response :success
   end
 

-  def test_non_moderators_cant_edit
+  def test_edit_non_moderator

     session[:user] = users(:public_user).id
 
     get :edit, :id => redactions(:example).id
     session[:user] = users(:public_user).id
 
     get :edit, :id => redactions(:example).id


@@ -100,7 +145,7 @@ class RedactionsControllerTest < ActionController::TestCase
     assert_redirected_to(redactions_path)
   end
 
     assert_redirected_to(redactions_path)
   end
 

-  def test_moderators_can_update
+  def test_update_moderator

     session[:user] = users(:moderator_user).id
 
     redaction = redactions(:example)
     session[:user] = users(:moderator_user).id
 
     redaction = redactions(:example)


@@ -110,7 +155,17 @@ class RedactionsControllerTest < ActionController::TestCase
     assert_redirected_to(redaction_path(redaction))
   end
 
     assert_redirected_to(redaction_path(redaction))
   end
 

-  def test_non_moderators_cant_update
+  def test_update_moderator_invalid
+    session[:user] = users(:moderator_user).id
+
+    redaction = redactions(:example)
+
+    put :update, :id => redaction.id, :redaction => { :title => "Foo", :description => "" }
+    assert_response :success
+    assert_template :edit
+  end
+
+  def test_updated_non_moderator

     session[:user] = users(:public_user).id
 
     redaction = redactions(:example)
     session[:user] = users(:public_user).id
 
     redaction = redactions(:example)