Group sub-locations under target locations#4127
Merged
Conversation
Switch from GPS bounding box to name-suffix matching for candidate observations and location grouping. Target locations now appear with collapsible chevron toggles showing aggregated observation counts. Locations not matching any target appear in a separate section below. Fixes #4126 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
|
coverage: 96.276% (+0.01%) from 96.266% — njw-4126-group-sub-locations into main |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates project location handling to group observed sub-locations under configured target locations using name-suffix matching (instead of GPS bounding boxes) and updates the Locations tab UI to support collapsible target groups with aggregated observation counts, plus a checklist option to include sub-locations.
Changes:
- Switch candidate observation location matching from bounding-box matching to location-name suffix matching (with optional project bounding-box constraint).
- Add grouped/collapsible target-location rendering on the Project “Locations” tab with aggregated per-location observation counts.
- Extend project checklist generation to optionally include sub-locations when viewing a target location.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/models/location_test.rb | Adds coverage for new Location.sub_locations_of scope. |
| test/controllers/projects/locations_controller_test.rb | Adds controller tests for presence/absence of target-location grouping UI. |
| test/components/projects/locations_table_test.rb | Updates component tests for grouped/ungrouped data + chevron/count behavior. |
| app/views/controllers/projects/target_locations/_locations_update.erb | Updates Turbo Stream partial to render grouped/ungrouped location tables and counts. |
| app/views/controllers/projects/locations/index.html.erb | Updates index view to use grouped/ungrouped data and counts. |
| app/models/project.rb | Updates candidate location matching to use name-suffix conditions (plus optional project box). |
| app/models/location/scopes.rb | Introduces sub_locations_of suffix-matching scope. |
| app/controllers/projects/target_locations_controller.rb | Refactors update rendering to use shared grouping concern. |
| app/controllers/projects/locations_controller.rb | Refactors index to use shared grouping concern + observation counts. |
| app/controllers/concerns/projects/location_grouping.rb | New shared grouping/counting logic for target/sub-location organization. |
| app/controllers/checklists_controller.rb | Adds request param handling to include sub-locations in project checklists. |
| app/components/projects/locations_table.rb | Implements grouped/collapsible rendering + observation count column. |
| app/classes/checklist.rb | Adds include_sub_locations option for project checklists. |
- Escape SQL LIKE wildcards with sanitize_sql_like in all suffix-matching queries (project.rb, scopes.rb, checklist.rb) - Add text-decoration:none to .panel-collapse-trigger CSS - Add CSS override for tbody.collapse.in to use table-row-group display instead of Bootstrap's default block - Assign sub-locations to most specific (longest name) target to prevent duplicates when targets overlap - Add test for include_sub_locations checklist behavior - Add global outline-offset:2px for focus outlines on interactive elements so they don't clip text/icons Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Narrow the inset box-shadow focus override from all .panel-body to just .rss-box-details so that checklist and other panel pages use the outline-offset approach like the project locations tab. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test exercises assign_to_targets, most_specific_target, and ungrouped filtering by creating a project with California as a target, Albion as a sub-location, and NYBG as an ungrouped location. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates how project locations and target locations are matched and displayed by switching target matching from GPS bounding boxes to name-suffix matching, and adds a grouped/collapsible UI on the Project “Locations” tab to nest observed sub-locations under their target locations (with aggregated observation counts).
Changes:
- Add name-suffix matching for “sub-locations” (e.g.,
..., <Target Location Name>) and reuse it for candidate observation matching and checklists. - Introduce shared
Projects::LocationGroupingconcern to build grouped/ungrouped location collections and observation counts for controllers/views. - Update the Locations tab UI to render collapsible target groups with aggregated counts, plus an ungrouped section.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/models/project.rb | Switch candidate location matching from GPS-box-based to location-name-suffix matching (with optional project bounding box constraint). |
| app/models/location/scopes.rb | Add sub_locations_of scope based on location-name suffix. |
| app/classes/checklist.rb | Add include_sub_locations option for project checklists using suffix matching. |
| app/controllers/concerns/projects/location_grouping.rb | New concern to group observed locations under targets and compute per-location observation counts. |
| app/controllers/projects/locations_controller.rb | Use LocationGrouping to supply grouped/ungrouped data + counts to the Locations view. |
| app/controllers/projects/target_locations_controller.rb | Use LocationGrouping for Turbo updates after target location changes. |
| app/controllers/checklists_controller.rb | Wire request param to Checklist::ForProject#include_sub_locations. |
| app/components/projects/locations_table.rb | Render grouped target rows with collapsible sub-location <tbody> sections and an observations-count column. |
| app/views/controllers/projects/locations/index.html.erb | Pass grouped/ungrouped datasets and counts into the LocationsTable component. |
| app/views/controllers/projects/target_locations/_locations_update.erb | Update Turbo-streamed table render to the new component API. |
| app/assets/stylesheets/mo/_icons.scss | Add collapse/table display overrides and trigger styling for the chevron toggle UI. |
| app/assets/stylesheets/mo/_elements.scss | Improve focus outline spacing and provide visible focus styling inside .rss-box-details. |
| test/models/location_test.rb | Add coverage for Location.sub_locations_of. |
| test/models/checklist_test.rb | Add coverage for project checklist include_sub_locations option. |
| test/controllers/projects/locations_controller_test.rb | Add coverage for grouping behavior and target/no-target rendering. |
| test/components/projects/locations_table_test.rb | Update component tests for grouped/ungrouped inputs, chevrons, and aggregated counts. |
Memoize is_admin? result in LocationsTable component to eliminate redundant queries (one per table row). Add controller test that exercises location grouping with sub-locations and ungrouped locations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test creates an observation at a Nevada location with GPS coords inside California's bounding box, then verifies it appears with GPS-box matching but is correctly excluded by name-suffix matching. This is the core behavior fix from #4126. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
include_sub_locationsparam to Checklist so target location links show species from all sub-locationsProjects::LocationGroupingconcern for reuse between LocationsController and TargetLocationsControllerFixes #4126
Test plan
🤖 Generated with Claude Code