Fixed bug in changeset closing and querying where the number of elements exceeded...

diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb
index c820aa70528fb7daf7dfe1307e6caf07a06c5002..b2ff42711db55a59f35056993647417494252be5 100644 (file)
--- a/app/controllers/changeset_controller.rb
+++ b/app/controllers/changeset_controller.rb
@@ -395,8 +395,14 @@ private
 
   ##
   # restrict changes to those which are open
 
   ##
   # restrict changes to those which are open

+  #
+  # at the moment this code assumes we're only interested in open
+  # changesets and gives no facility to query closed changesets. this
+  # would be reasonably simple to implement if anyone actually wants
+  # it?

   def conditions_open(open)
   def conditions_open(open)

-    return open.nil? ? nil : ['closed_at >= ?', DateTime.now]
+    return open.nil? ? nil : ['closed_at >= ? and num_changes <= ?', 
+                              DateTime.now, Changeset::MAX_ELEMENTS]

   end
 
 end
   end
 
 end

diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index 7b90659d841ef4436f315a5fb38e0c68690f0d81..31d927e9a6649094b7ff9825519bad8ccff26365 100644 (file)
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -46,7 +46,7 @@ class Changeset < ActiveRecord::Base
   end
 
   def set_closed_time_now
   end
 
   def set_closed_time_now

-    unless is_open?
+    if is_open?

       self.closed_at = Time.now
     end
   end
       self.closed_at = Time.now
     end
   end

diff --git a/test/fixtures/changesets.yml b/test/fixtures/changesets.yml
index defd691d2c773d67a4579cb37273fa72660fcdf0..9fe5bc6d8f8902eedf9aedbf97575d51ab52dfc9 100644 (file)
--- a/test/fixtures/changesets.yml
+++ b/test/fixtures/changesets.yml
@@ -48,3 +48,17 @@ invalid_changeset:
   created_at: "2008-01-01 00:00:00"
   closed_at: "2008-01-02 00:00:00"
   num_changes: 9
   created_at: "2008-01-01 00:00:00"
   closed_at: "2008-01-02 00:00:00"
   num_changes: 9

+
+# changeset which still has time remaining, but has been closed
+# by containing too many elements.
+too_many_elements_changeset:
+  id: 6
+  user_id: 1
+  created_at: "2008-01-01 00:00:00"
+  closed_at: <%= DateTime.now + Rational(1,24) %>
+  min_lon: <%= 1 * SCALE %>
+  min_lat: <%= 1 * SCALE %>
+  max_lon: <%= 4 * SCALE %>
+  max_lat: <%= 4 * SCALE %>
+  num_changes: <%= Changeset::MAX_ELEMENTS + 1 %>
+

diff --git a/test/functional/changeset_controller_test.rb b/test/functional/changeset_controller_test.rb
index e8648e5c3a8c9058c4db9fe10dd1e5f6d454e33f..524fad91b922894f2c3f3c98240d35deac921936 100644 (file)
--- a/test/functional/changeset_controller_test.rb
+++ b/test/functional/changeset_controller_test.rb
@@ -65,8 +65,14 @@ class ChangesetControllerTest < ActionController::TestCase
   def test_close
     basic_authorization "test@openstreetmap.org", "test"
 
   def test_close
     basic_authorization "test@openstreetmap.org", "test"
 

-    put :close, :id => changesets(:normal_user_first_change).id
+    cs_id = changesets(:normal_user_first_change).id
+    put :close, :id => cs_id

     assert_response :success
     assert_response :success

+
+    # test that it really is closed now
+    cs = Changeset.find(cs_id)
+    assert(!cs.is_open?, 
+           "changeset should be closed now (#{cs.closed_at} > #{Time.now}.")

   end
 
   ##
   end
 
   ##


@@ -669,7 +675,7 @@ EOF
   def test_query
     get :query, :bbox => "-10,-10, 10, 10"
     assert_response :success, "can't get changesets in bbox"
   def test_query
     get :query, :bbox => "-10,-10, 10, 10"
     assert_response :success, "can't get changesets in bbox"

-    assert_changesets [1,4]
+    assert_changesets [1,4,6]

 
     get :query, :bbox => "4.5,4.5,4.6,4.6"
     assert_response :success, "can't get changesets in bbox"
 
     get :query, :bbox => "4.5,4.5,4.6,4.6"
     assert_response :success, "can't get changesets in bbox"


@@ -683,7 +689,7 @@ EOF
     basic_authorization "test@openstreetmap.org", "test"
     get :query, :user => users(:normal_user).id
     assert_response :success, "can't get changesets by user"
     basic_authorization "test@openstreetmap.org", "test"
     get :query, :user => users(:normal_user).id
     assert_response :success, "can't get changesets by user"

-    assert_changesets [1,3,4]
+    assert_changesets [1,3,4,6]

 
     get :query, :user => users(:normal_user).id, :open => true
     assert_response :success, "can't get changesets by user and open"
 
     get :query, :user => users(:normal_user).id, :open => true
     assert_response :success, "can't get changesets by user and open"


@@ -691,15 +697,15 @@ EOF
 
     get :query, :time => '2007-12-31'
     assert_response :success, "can't get changesets by time-since"
 
     get :query, :time => '2007-12-31'
     assert_response :success, "can't get changesets by time-since"

-    assert_changesets [1,2,4,5]
+    assert_changesets [1,2,4,5,6]

 
     get :query, :time => '2008-01-01T12:34Z'
     assert_response :success, "can't get changesets by time-since with hour"
 
     get :query, :time => '2008-01-01T12:34Z'
     assert_response :success, "can't get changesets by time-since with hour"

-    assert_changesets [1,2,4,5]
+    assert_changesets [1,2,4,5,6]

 
     get :query, :time => '2007-12-31T23:59Z,2008-01-01T00:01Z'
     assert_response :success, "can't get changesets by time-range"
 
     get :query, :time => '2007-12-31T23:59Z,2008-01-01T00:01Z'
     assert_response :success, "can't get changesets by time-range"

-    assert_changesets [1,4,5]
+    assert_changesets [1,4,5,6]

 
     get :query, :open => 'true'
     assert_response :success, "can't get changesets by open-ness"
 
     get :query, :open => 'true'
     assert_response :success, "can't get changesets by open-ness"


@@ -840,6 +846,11 @@ EOF
 
     changeset = Changeset.find(cs_id)
     assert_equal Changeset::MAX_ELEMENTS + 1, changeset.num_changes
 
     changeset = Changeset.find(cs_id)
     assert_equal Changeset::MAX_ELEMENTS + 1, changeset.num_changes

+
+    # check that the changeset is now closed as well
+    assert(!changeset.is_open?, 
+           "changeset should have been auto-closed by exceeding " + 
+           "element limit.")

   end
   
   #------------------------------------------------------------
   end
   
   #------------------------------------------------------------

diff --git a/test/unit/changeset_test.rb b/test/unit/changeset_test.rb
index ee9d0925a382c6d53797f9bb4839ce8434b6dfd7..4550ffba57700238b97d73d28d9c05948292390c 100644 (file)
--- a/test/unit/changeset_test.rb
+++ b/test/unit/changeset_test.rb
@@ -5,7 +5,7 @@ class ChangesetTest < Test::Unit::TestCase
   
   
   def test_changeset_count
   
   
   def test_changeset_count

-    assert_equal 5, Changeset.count
+    assert_equal 6, Changeset.count

   end
   
 end
   end
   
 end