Fix numericality validation to use correct integer only constraint
authorTom Hughes <tom@compton.nu>
Thu, 22 Aug 2019 09:36:03 +0000 (10:36 +0100)
committerTom Hughes <tom@compton.nu>
Thu, 22 Aug 2019 11:14:06 +0000 (12:14 +0100)
Fixes #2355

12 files changed:
app/controllers/api/amf_controller.rb
app/controllers/api/changesets_controller.rb
app/models/changeset.rb
app/models/changeset_comment.rb
app/models/node.rb
app/models/note.rb
app/models/note_comment.rb
app/models/old_node.rb
app/models/relation.rb
app/models/way.rb
test/controllers/api/changesets_controller_test.rb
test/models/node_test.rb

index ca46726..e564898 100644 (file)
@@ -569,7 +569,7 @@ module Api
           new_relation.members = typedmembers
           new_relation.tags = tags
           new_relation.visible = visible
-          new_relation.changeset_id = changeset_id
+          new_relation.changeset_id = changeset_id.to_i
           new_relation.version = version
 
           if relid <= 0
@@ -653,7 +653,7 @@ module Api
             id = renumberednodes[id] if renumberednodes[id]
 
             node = Node.new
-            node.changeset_id = changeset_id
+            node.changeset_id = changeset_id.to_i
             node.lat = lat
             node.lon = lon
             node.tags = a[4]
@@ -687,7 +687,7 @@ module Api
           new_way = Way.new
           new_way.tags = attributes
           new_way.nds = pointlist
-          new_way.changeset_id = changeset_id
+          new_way.changeset_id = changeset_id.to_i
           new_way.version = wayversion
           if originalway <= 0
             new_way.create_with_history(user)
@@ -705,7 +705,7 @@ module Api
           deletednodes.each do |id, v|
             node = Node.find(id.to_i)
             new_node = Node.new
-            new_node.changeset_id = changeset_id
+            new_node.changeset_id = changeset_id.to_i
             new_node.version = v.to_i
             new_node.id = id.to_i
             begin
@@ -758,7 +758,7 @@ module Api
           # We always need a new node, based on the data that has been sent to us
           new_node = Node.new
 
-          new_node.changeset_id = changeset_id
+          new_node.changeset_id = changeset_id.to_i
           new_node.version = version
           new_node.lat = lat
           new_node.lon = lon
@@ -834,7 +834,7 @@ module Api
           old_way = Way.find(way_id)
           delete_way = Way.new
           delete_way.version = way_version
-          delete_way.changeset_id = changeset_id
+          delete_way.changeset_id = changeset_id.to_i
           delete_way.id = way_id
           old_way.delete_with_history!(delete_way, user)
 
@@ -843,7 +843,7 @@ module Api
           deletednodes.each do |id, v|
             node = Node.find(id.to_i)
             new_node = Node.new
-            new_node.changeset_id = changeset_id
+            new_node.changeset_id = changeset_id.to_i
             new_node.version = v.to_i
             new_node.id = id.to_i
             begin
index ffd9a11..5f87324 100644 (file)
@@ -93,10 +93,10 @@ module Api
       lat << cs.max_lat unless cs.max_lat.nil?
 
       # collapse the arrays to minimum and maximum
-      cs.min_lon = lon.min
-      cs.min_lat = lat.min
-      cs.max_lon = lon.max
-      cs.max_lat = lat.max
+      cs.min_lon = lon.min.round
+      cs.min_lat = lat.min.round
+      cs.max_lon = lon.max.round
+      cs.max_lat = lat.max.round
 
       # save the larger bounding box and return the changeset, which
       # will include the bigger bounding box.
index d57086a..47f03c7 100644 (file)
@@ -43,15 +43,15 @@ class Changeset < ActiveRecord::Base
   has_and_belongs_to_many :subscribers, :class_name => "User", :join_table => "changesets_subscribers", :association_foreign_key => "subscriber_id"
 
   validates :id, :uniqueness => true, :presence => { :on => :update },
-                 :numericality => { :on => :update, :integer_only => true }
+                 :numericality => { :on => :update, :only_integer => true }
   validates :user_id, :presence => true,
-                      :numericality => { :integer_only => true }
+                      :numericality => { :only_integer => true }
   validates :num_changes, :presence => true,
-                          :numericality => { :integer_only => true,
+                          :numericality => { :only_integer => true,
                                              :greater_than_or_equal_to => 0 }
   validates :created_at, :closed_at, :presence => true
   validates :min_lat, :max_lat, :min_lon, :max_lat, :allow_nil => true,
-                                                    :numericality => { :integer_only => true }
+                                                    :numericality => { :only_integer => true }
 
   before_save :update_closed_at
 
@@ -131,7 +131,7 @@ class Changeset < ActiveRecord::Base
 
     # update active record. rails 2.1's dirty handling should take care of
     # whether this object needs saving or not.
-    self.min_lon, self.min_lat, self.max_lon, self.max_lat = @bbox.to_a if bbox.complete?
+    self.min_lon, self.min_lat, self.max_lon, self.max_lat = @bbox.to_a.collect(&:round) if bbox.complete?
   end
 
   ##
index 529641c..75b1a05 100644 (file)
@@ -24,7 +24,7 @@ class ChangesetComment < ActiveRecord::Base
   belongs_to :author, :class_name => "User"
 
   validates :id, :uniqueness => true, :presence => { :on => :update },
-                 :numericality => { :on => :update, :integer_only => true }
+                 :numericality => { :on => :update, :only_integer => true }
   validates :changeset, :presence => true, :associated => true
   validates :author, :presence => true, :associated => true
   validates :visible, :inclusion => [true, false]
index 91a1dbc..c5df1f7 100644 (file)
@@ -47,15 +47,15 @@ class Node < ActiveRecord::Base
   has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation
 
   validates :id, :uniqueness => true, :presence => { :on => :update },
-                 :numericality => { :on => :update, :integer_only => true }
+                 :numericality => { :on => :update, :only_integer => true }
   validates :version, :presence => true,
-                      :numericality => { :integer_only => true }
+                      :numericality => { :only_integer => true }
   validates :changeset_id, :presence => true,
-                           :numericality => { :integer_only => true }
+                           :numericality => { :only_integer => true }
   validates :latitude, :presence => true,
-                       :numericality => { :integer_only => true }
+                       :numericality => { :only_integer => true }
   validates :longitude, :presence => true,
-                        :numericality => { :integer_only => true }
+                        :numericality => { :only_integer => true }
   validates :timestamp, :presence => true
   validates :changeset, :associated => true
   validates :visible, :inclusion => [true, false]
index d4f9a80..fa9d0b1 100644 (file)
@@ -24,7 +24,7 @@ class Note < ActiveRecord::Base
   has_many :comments, -> { left_joins(:author).where(:visible => true, :users => { :status => [nil, "active", "confirmed"] }).order(:created_at) }, :class_name => "NoteComment", :foreign_key => :note_id
 
   validates :id, :uniqueness => true, :presence => { :on => :update },
-                 :numericality => { :on => :update, :integer_only => true }
+                 :numericality => { :on => :update, :only_integer => true }
   validates :latitude, :longitude, :numericality => { :only_integer => true }
   validates :closed_at, :presence => true, :if => proc { :status == "closed" }
   validates :status, :inclusion => %w[open closed hidden]
index 388f890..448703f 100644 (file)
@@ -28,7 +28,7 @@ class NoteComment < ActiveRecord::Base
   belongs_to :author, :class_name => "User", :foreign_key => :author_id
 
   validates :id, :uniqueness => true, :presence => { :on => :update },
-                 :numericality => { :on => :update, :integer_only => true }
+                 :numericality => { :on => :update, :only_integer => true }
   validates :note, :presence => true, :associated => true
   validates :visible, :inclusion => [true, false]
   validates :author, :associated => true
index cc2327d..bdf8cb4 100644 (file)
@@ -38,9 +38,9 @@ class OldNode < ActiveRecord::Base
 
   validates :changeset, :presence => true, :associated => true
   validates :latitude, :presence => true,
-                       :numericality => { :integer_only => true }
+                       :numericality => { :only_integer => true }
   validates :longitude, :presence => true,
-                        :numericality => { :integer_only => true }
+                        :numericality => { :only_integer => true }
   validates :timestamp, :presence => true
   validates :visible, :inclusion => [true, false]
 
index bcac9d0..dc2b71a 100644 (file)
@@ -37,11 +37,11 @@ class Relation < ActiveRecord::Base
   has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation
 
   validates :id, :uniqueness => true, :presence => { :on => :update },
-                 :numericality => { :on => :update, :integer_only => true }
+                 :numericality => { :on => :update, :only_integer => true }
   validates :version, :presence => true,
-                      :numericality => { :integer_only => true }
+                      :numericality => { :only_integer => true }
   validates :changeset_id, :presence => true,
-                           :numericality => { :integer_only => true }
+                           :numericality => { :only_integer => true }
   validates :timestamp, :presence => true
   validates :changeset, :associated => true
   validates :visible, :inclusion => [true, false]
index 6fcaf39..b346621 100644 (file)
@@ -39,11 +39,11 @@ class Way < ActiveRecord::Base
   has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation
 
   validates :id, :uniqueness => true, :presence => { :on => :update },
-                 :numericality => { :on => :update, :integer_only => true }
+                 :numericality => { :on => :update, :only_integer => true }
   validates :version, :presence => true,
-                      :numericality => { :integer_only => true }
+                      :numericality => { :only_integer => true }
   validates :changeset_id, :presence => true,
-                           :numericality => { :integer_only => true }
+                           :numericality => { :only_integer => true }
   validates :timestamp, :presence => true
   validates :changeset, :associated => true
   validates :visible, :inclusion => [true, false]
index 9f2b64c..3716ae7 100644 (file)
@@ -1561,8 +1561,8 @@ CHANGESET
       user = create(:user)
       changeset = create(:changeset, :user => user)
       closed_changeset = create(:changeset, :closed, :user => user, :created_at => Time.utc(2008, 1, 1, 0, 0, 0), :closed_at => Time.utc(2008, 1, 2, 0, 0, 0))
-      changeset2 = create(:changeset, :min_lat => 5 * GeoRecord::SCALE, :min_lon => 5 * GeoRecord::SCALE, :max_lat => 15 * GeoRecord::SCALE, :max_lon => 15 * GeoRecord::SCALE)
-      changeset3 = create(:changeset, :min_lat => 4.5 * GeoRecord::SCALE, :min_lon => 4.5 * GeoRecord::SCALE, :max_lat => 5 * GeoRecord::SCALE, :max_lon => 5 * GeoRecord::SCALE)
+      changeset2 = create(:changeset, :min_lat => (5 * GeoRecord::SCALE).round, :min_lon => (5 * GeoRecord::SCALE).round, :max_lat => (15 * GeoRecord::SCALE).round, :max_lon => (15 * GeoRecord::SCALE).round)
+      changeset3 = create(:changeset, :min_lat => (4.5 * GeoRecord::SCALE).round, :min_lon => (4.5 * GeoRecord::SCALE).round, :max_lat => (5 * GeoRecord::SCALE).round, :max_lon => (5 * GeoRecord::SCALE).round)
 
       get :query, :params => { :bbox => "-10,-10, 10, 10" }
       assert_response :success, "can't get changesets in bbox"
index 5f63251..d6fa1cd 100644 (file)
@@ -80,8 +80,8 @@ class NodeTest < ActiveSupport::TestCase
   def test_create
     changeset = create(:changeset)
     node_template = Node.new(
-      :latitude => 12.3456,
-      :longitude => 65.4321,
+      :lat => 12.3456,
+      :lon => 65.4321,
       :changeset_id => changeset.id,
       :visible => 1,
       :version => 1
@@ -116,8 +116,8 @@ class NodeTest < ActiveSupport::TestCase
     assert_equal OldNode.where(:node_id => node_template.id).count, 1
     assert_not_nil node
 
-    node_template.latitude = 12.3456
-    node_template.longitude = 65.4321
+    node_template.lat = 12.3456
+    node_template.lon = 65.4321
     # node_template.tags = "updated=yes"
     assert node.update_from(node_template, node.changeset.user)