From 896c720f82715d350f733f2b8ebccebee934fd8d Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Fri, 4 Oct 2024 17:21:50 -0700 Subject: [PATCH 1/8] Redefine scope, try to fix tests --- app/models/observation.rb | 13 +++++++------ test/models/observation_test.rb | 20 +++++++++++--------- test/models/project_test.rb | 3 ++- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/app/models/observation.rb b/app/models/observation.rb index 2d9d976b82..4959ecf361 100644 --- a/app/models/observation.rb +++ b/app/models/observation.rb @@ -499,12 +499,13 @@ class Observation < AbstractModel # rubocop:disable Metrics/ClassLength 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 + # should be + merge(Observation.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| diff --git a/test/models/observation_test.rb b/test/models/observation_test.rb index 06e75bf39c..7b6f25b345 100644 --- a/test/models/observation_test.rb +++ b/test/models/observation_test.rb @@ -1325,13 +1325,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), @@ -1339,7 +1333,6 @@ def test_scope_not_in_box lng: -78.4305382, where: "Quito, Ecuador" ) - wrangel = locations(:east_lt_west_location) wrangel_obs = Observation.create!( @@ -1347,14 +1340,23 @@ 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_includes(obss_not_in_cal_box, observations(:minimal_unknown_obs), + # "Observation without lat/lon should not 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..99ddf561bd 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,7 @@ def test_location_violations ] location_violations = proj.out_of_area_observations - + debugger assert_includes( location_violations, geoloc_outside_burbank, "Noncompliant Obss missing Obs with geoloc outside Proj location" From df75f2866bcb8157452d2f18ca03d3b8cb0df256 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sat, 5 Oct 2024 18:52:27 -0700 Subject: [PATCH 2/8] Update observation.rb --- app/models/observation.rb | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/app/models/observation.rb b/app/models/observation.rb index 4959ecf361..d6f1e37384 100644 --- a/app/models/observation.rb +++ b/app/models/observation.rb @@ -500,12 +500,23 @@ class Observation < AbstractModel # rubocop:disable Metrics/ClassLength return Observation.all unless box.valid? # should be - merge(Observation.in_box(**args).invert_where) - # if box.straddles_180_deg? - # not_in_box_straddling_dateline(**args) - # else - # not_in_box_regular(**args) - # end + # merge(Observation.in_box(**args).invert_where. + # or(Observation.where(lat: nil, location_lat: nil))) + if box.straddles_180_deg? + not_in_box_straddling_dateline(**args). + # merge( + joins(:location). + where(Location.contains_box(**args.except(:mappable)). + invert_where) + # ) + else + not_in_box_regular(**args). + # merge( + joins(:location). + where(Location.contains_box(**args.except(:mappable)). + invert_where) + # ) + end } scope :not_in_box_straddling_dateline, # helper for not_in_box lambda { |**args| From d6aecf06f86f7cce2c241caa55fe82c954e4574b Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sat, 5 Oct 2024 18:52:55 -0700 Subject: [PATCH 3/8] Update observation_test.rb --- test/models/observation_test.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/models/observation_test.rb b/test/models/observation_test.rb index 7b6f25b345..3e84106809 100644 --- a/test/models/observation_test.rb +++ b/test/models/observation_test.rb @@ -1344,11 +1344,8 @@ def test_scope_not_in_box 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 From 2424c3e2819b725ef8c2b44482783f5cf5df9a15 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sat, 5 Oct 2024 19:10:51 -0700 Subject: [PATCH 4/8] Update observation.rb --- app/models/observation.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/observation.rb b/app/models/observation.rb index d6f1e37384..e28a6e9366 100644 --- a/app/models/observation.rb +++ b/app/models/observation.rb @@ -506,8 +506,9 @@ class Observation < AbstractModel # rubocop:disable Metrics/ClassLength not_in_box_straddling_dateline(**args). # merge( joins(:location). - where(Location.contains_box(**args.except(:mappable)). - invert_where) + where.not( + Location[:id] = Location.contains_box(**args.except(:mappable)) + ) # ) else not_in_box_regular(**args). From 71323045d4622a865de324f9aa821304f3d0b0e1 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sat, 5 Oct 2024 20:54:08 -0700 Subject: [PATCH 5/8] Fix scopes and clean up --- app/models/location.rb | 5 ++- app/models/observation.rb | 54 +++------------------------------ test/models/observation_test.rb | 5 +-- test/models/project_test.rb | 1 - 4 files changed, 12 insertions(+), 53 deletions(-) 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 e28a6e9366..7666786118 100644 --- a/app/models/observation.rb +++ b/app/models/observation.rb @@ -492,60 +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 - # merge(Observation.in_box(**args).invert_where. - # or(Observation.where(lat: nil, location_lat: nil))) - if box.straddles_180_deg? - not_in_box_straddling_dateline(**args). - # merge( - joins(:location). - where.not( - Location[:id] = Location.contains_box(**args.except(:mappable)) - ) - # ) - else - not_in_box_regular(**args). - # merge( - joins(:location). - where(Location.contains_box(**args.except(:mappable)). - invert_where) - # ) - 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/test/models/observation_test.rb b/test/models/observation_test.rb index 3e84106809..e6cc5013d6 100644 --- a/test/models/observation_test.rb +++ b/test/models/observation_test.rb @@ -1352,8 +1352,9 @@ def test_scope_not_in_box 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 99ddf561bd..c7bf41a258 100644 --- a/test/models/project_test.rb +++ b/test/models/project_test.rb @@ -205,7 +205,6 @@ def test_location_violations ] location_violations = proj.out_of_area_observations - debugger assert_includes( location_violations, geoloc_outside_burbank, "Noncompliant Obss missing Obs with geoloc outside Proj location" From eea1db5b395782f120616fe57640765e94cfa39b Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 6 Oct 2024 01:08:08 -0700 Subject: [PATCH 6/8] Improve test Not passing but test is better. Scope may be wrong --- app/models/project.rb | 12 ++++++----- test/models/project_test.rb | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) 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/project_test.rb b/test/models/project_test.rb index c7bf41a258..a552f8e16e 100644 --- a/test/models/project_test.rb +++ b/test/models/project_test.rb @@ -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 From 09fe919a90c6c4f15165cc43ab8f8983fda5f184 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 6 Oct 2024 18:33:28 -0700 Subject: [PATCH 7/8] Redefine no_mushrooms_location, collection_location, display_location --- test/fixtures/locations.yml | 42 ++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/fixtures/locations.yml b/test/fixtures/locations.yml index 19188905a9..c33d6e2d3d 100644 --- a/test/fixtures/locations.yml +++ b/test/fixtures/locations.yml @@ -206,39 +206,39 @@ no_mushrooms_location: user_id: 0 name: No Mushrooms scientific_name: No Mushrooms - north: 90 - west: -180 - east: 180 - south: -90 - box_area: 510224605.17052704 - center_lat: 0 - center_lng: 0 + north: .0001 + west: 0 + east: .0001 + south: .00007 + box_area: 3.710458035376616e-05 + center_lat: 0.000085 + center_lng: 0.00005 collection_location: <<: *DEFAULTS user_id: 0 name: Collection Location scientific_name: Collection Location - north: 90 - west: -180 - east: 180 - south: -90 - box_area: 510224605.17052704 - center_lat: 0 - center_lng: 0 + north: .0001 + west: 3 + east: 3.0001 + south: .00007 + box_area: 3.710458035376616e-05 + center_lat: 0.000085 + center_lng: 3.00005 display_location: <<: *DEFAULTS user_id: 0 name: Display Location scientific_name: Display Location - north: 90 - west: -180 - east: 180 - south: -90 - box_area: 510224605.17052704 - center_lat: 0 - center_lng: 0 + north: .0001 + west: 6 + east: 6.0001 + south: .00007 + box_area: 3.710458035376616e-05 + center_lat: 0.000085 + center_lng: 6.00005 sortable_observation_user_location: <<: *DEFAULTS From 3d1efc1cd4bf1ee2cb6cfe3b7394acc96df17734 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 7 Oct 2024 00:13:20 -0700 Subject: [PATCH 8/8] Update locations.yml --- test/fixtures/locations.yml | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/fixtures/locations.yml b/test/fixtures/locations.yml index c33d6e2d3d..f13b20de2c 100644 --- a/test/fixtures/locations.yml +++ b/test/fixtures/locations.yml @@ -206,39 +206,39 @@ no_mushrooms_location: user_id: 0 name: No Mushrooms scientific_name: No Mushrooms - north: .0001 - west: 0 - east: .0001 - south: .00007 + north: 0.0001 + west: 3 + east: 3.0001 + south: 0.00007 box_area: 3.710458035376616e-05 center_lat: 0.000085 - center_lng: 0.00005 + center_lng: 3.00005 collection_location: <<: *DEFAULTS user_id: 0 name: Collection Location scientific_name: Collection Location - north: .0001 - west: 3 - east: 3.0001 - south: .00007 + north: 0.0001 + west: 6 + east: 6.0001 + south: 0.00007 box_area: 3.710458035376616e-05 center_lat: 0.000085 - center_lng: 3.00005 + center_lng: 6.00005 display_location: <<: *DEFAULTS user_id: 0 name: Display Location scientific_name: Display Location - north: .0001 - west: 6 - east: 6.0001 - south: .00007 + north: 0.0001 + west: 9 + east: 9.0001 + south: 0.00007 box_area: 3.710458035376616e-05 center_lat: 0.000085 - center_lng: 6.00005 + center_lng: 9.00005 sortable_observation_user_location: <<: *DEFAULTS