diff --git a/app/models/location.rb b/app/models/location.rb index d69582290d..e0bca1953c 100644 --- a/app/models/location.rb +++ b/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? diff --git a/app/models/observation.rb b/app/models/observation.rb index 3ebdb526f8..8f5a8323f3 100644 --- a/app/models/observation.rb +++ b/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, diff --git a/app/models/project.rb b/app/models/project.rb index 2bb58120d5..26aaaaa0fd 100644 --- a/app/models/project.rb +++ b/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 diff --git a/test/models/observation_test.rb b/test/models/observation_test.rb index 3431222679..62160f3a2e 100644 --- a/test/models/observation_test.rb +++ b/test/models/observation_test.rb @@ -1342,13 +1342,7 @@ 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), @@ -1356,7 +1350,6 @@ def test_scope_not_in_box lng: -78.4305382, where: "Quito, Ecuador" ) - wrangel = locations(:east_lt_west_location) wrangel_obs = Observation.create!( @@ -1364,14 +1357,21 @@ def test_scope_not_in_box 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) diff --git a/test/models/project_test.rb b/test/models/project_test.rb index 0ca066d58c..a552f8e16e 100644 --- a/test/models/project_test.rb +++ b/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