Fix the not_in_box scope

app/models/location.rb

@@ -201,7 +201,10 @@ class Location < AbstractModel # rubocop:disable Metrics/ClassLength
             and(Location[:west] <= Location[:east])
           )
         }
-  scope :not_in_box, # Pass kwargs (:north, :south, :east, :west), any order
+  # Returns locations whose bounding box is not entirely within the given box.
+  # Some locations may overlap with the box, even mostly.
+  # Pass kwargs (:north, :south, :east, :west), any order
+  scope :not_in_box,
         lambda { |**args|
           box = Mappable::Box.new(**args)
           return none unless box.valid?

app/models/observation.rb

@@ -492,47 +492,16 @@ class Observation < AbstractModel # rubocop:disable Metrics/ClassLength
               )
           end
         }
-
-  scope :not_in_box, # Pass kwargs (:north, :south, :east, :west), any order
-        lambda { |**args|
-          args[:mappable] ||= false
-          box = Mappable::Box.new(**args.except(:mappable))
-          return Observation.all unless box.valid?
-
-          # should be in_box(**args).invert_where
-          if box.straddles_180_deg?
-            not_in_box_straddling_dateline(**args)
-          else
-            not_in_box_regular(**args)
-          end
-        }
-  scope :not_in_box_straddling_dateline, # helper for not_in_box
-        lambda { |**args|
-          args[:mappable] ||= false
-          box = Mappable::Box.new(**args.except(:mappable))
-          return Observation.all unless box.valid?
-
-          where(
-            Observation[:lat].eq(nil).
-            or(Observation[:lat] < box.south).
-            or(Observation[:lat] > box.north).
-            or((Observation[:lng] < box.west).
-                and(Observation[:lng] > box.east))
-          )
-        }
-  scope :not_in_box_regular, # helper for not_in_box
+  # Returns observations outside the box, plus observations with no location.
+  # Pass kwargs (:north, :south, :east, :west), any order
+  scope :not_in_box,
         lambda { |**args|
           args[:mappable] ||= false
           box = Mappable::Box.new(**args.except(:mappable))
           return Observation.all unless box.valid?
 
-          where(
-            Observation[:lat].eq(nil).
-            or(Observation[:lat] < box.south).
-            or(Observation[:lat] > box.north).
-            or(Observation[:lng] < box.west).
-            or(Observation[:lng] > box.east)
-          )
+          merge(Observation.where.not(id: Observation.in_box(**args)).
+            or(Observation.where(location_id: nil, lat: nil)))
         }
 
   scope :is_collection_location,

app/models/project.rb

@@ -497,10 +497,12 @@ def obs_geoloc_outside_project_location
   end
 
   def obs_without_geoloc_location_not_contained_in_location
-    observations.where(lat: nil).joins(:location).
-      merge(
-        # invert_where is safe (doesn't invert observations.where(lat: nil))
-        Location.not_in_box(**location.bounding_box)
-      )
+    # observations.where(lat: nil).joins(:location).
+    #   merge(
+    #     # invert_where is safe (doesn't invert observations.where(lat: nil))
+    #     Location.not_in_box(**location.bounding_box)
+    #   )
+    observations.where(lat: nil).
+      where.not(location_id: Location.not_in_box(**location.bounding_box))
   end
 end

test/models/observation_test.rb

@@ -1342,36 +1342,36 @@ def test_scope_in_box
 
   def test_scope_not_in_box
     cal = locations(:california)
-    obss_not_in_cal_box = Observation.not_in_box(**cal.bounding_box)
-    obs_with_burbank_geoloc = observations(:unknown_with_lat_lng)
-
     nybg = locations(:nybg_location)
-    obss_not_in_nybg_box = Observation.not_in_box(**nybg.bounding_box)
-
-    obss_not_in_ecuador_box = Observation.not_in_box(**ecuador_box)
     quito_obs =
       Observation.create!(
         user: users(:rolf),
         lat: -0.1865944,
         lng: -78.4305382,
         where: "Quito, Ecuador"
       )
-
     wrangel = locations(:east_lt_west_location)
     wrangel_obs =
       Observation.create!(
         user: users(:rolf),
         lat: (wrangel.north + wrangel.south) / 2,
         lng: (wrangel.east + wrangel.west) / 2 + wrangel.west
       )
+    Location.update_box_area_and_center_columns
+
+    obss_not_in_cal_box = Observation.not_in_box(**cal.bounding_box)
+    obs_with_burbank_geoloc = observations(:unknown_with_lat_lng)
+    obss_not_in_nybg_box = Observation.not_in_box(**nybg.bounding_box)
+    obss_not_in_ecuador_box = Observation.not_in_box(**ecuador_box)
     obss_not_in_wrangel_box = Observation.not_in_box(**wrangel.bounding_box)
 
     # boxes not straddling 180 deg
     assert_not_includes(obss_not_in_cal_box, obs_with_burbank_geoloc)
     assert_not_includes(obss_not_in_ecuador_box, quito_obs)
     assert_includes(obss_not_in_nybg_box, obs_with_burbank_geoloc)
-    assert_includes(obss_not_in_cal_box, observations(:minimal_unknown_obs),
-                    "Observation without lat/lon should not be in box")
+    assert_not_includes(obss_not_in_cal_box, observations(:minimal_unknown_obs),
+                        "Observation without lat/lon but has location_lat " \
+                        "in box should be in box")
 
     # box straddling 180 deg
     assert_not_includes(obss_not_in_wrangel_box, wrangel_obs)

test/models/project_test.rb

@@ -189,6 +189,7 @@ def test_location_violations
       title: "With Location Violations",
       open_membership: true
     )
+    Location.update_box_area_and_center_columns
     geoloc_in_burbank = observations(:unknown_with_lat_lng)
     geoloc_outside_burbank =
       observations(:trusted_hidden) # lat/lon in Falmouth
@@ -204,7 +205,6 @@ def test_location_violations
     ]
 
     location_violations = proj.out_of_area_observations
-
     assert_includes(
       location_violations, geoloc_outside_burbank,
       "Noncompliant Obss missing Obs with geoloc outside Proj location"
@@ -224,4 +224,44 @@ def test_location_violations
       "whose Loc is contained in Proj location"
     )
   end
+
+  def test_location_violations_with_box_overlap
+    proj = Project.create(
+      location: locations(:california),
+      title: "With Location Violations",
+      open_membership: true
+    )
+    # Slightly outside the box of California, but small enough to count!
+    mesquite = Location.create!(
+      name: "Mesquite, Clark Co., Nevada, USA",
+      north: 36.8433951,
+      south: 36.73912,
+      east: -114.0499929,
+      west: -114.198545,
+      center_lat: 36.791258,
+      center_lng: -114.124269,
+      user: users(:rolf)
+    )
+    obs_in_mesquite = Observation.create!(
+      location_id: mesquite.id,
+      when: Time.zone.now,
+      location_lat: 36.791258,
+      location_lng: -114.124269,
+      user: users(:rolf)
+    )
+    Location.update_box_area_and_center_columns
+
+    obs_in_burbank = observations(:unknown_with_lat_lng)
+    proj.observations = [obs_in_mesquite, obs_in_burbank]
+    location_violations = proj.out_of_area_observations
+
+    assert_includes(
+      location_violations, obs_in_mesquite,
+      "Noncompliant Obss missing Obs with geoloc outside Proj location"
+    )
+    assert_not_includes(
+      location_violations, obs_in_burbank,
+      "Noncompliant Obss wrongly includes Obs with geoloc inside Proj location"
+    )
+  end
 end