-
Notifications
You must be signed in to change notification settings - Fork 1
feat: geofenced & associated news and events rollup (#406) #424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
ea030c2
spec: geofenced and associated news & events (#406)
fatherlinux 2a18ea1
feat: geofenced and associated news & events rollup (#406)
fatherlinux 4eff19a
fix: index-backed containment + empty rollup for missing POI (PR #424…
fatherlinux fa13a6e
fix: use ST_PointOnSurface for contained linear features (PR #424 rev…
fatherlinux ee5f838
perf: pass jsonb directly to ST_GeomFromGeoJSON (PR #424 review)
fatherlinux File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,170 @@ | ||
| # Implementation Plan: GeoFenced and Associated News & Events | ||
|
|
||
| > **Spec ID:** 026-geofenced-news | ||
| > **Status:** Planning | ||
| > **Last Updated:** 2026-05-27 | ||
| > **Estimated Effort:** M | ||
|
|
||
| ## Summary | ||
|
|
||
| Add a backend helper that expands a target POI id into the set of POI ids whose | ||
| news/events should be shown — `[id]` for a normal point, the contained POIs for a | ||
| boundary, and the owned/associated POIs (plus POIs inside owned park boundaries) | ||
| for an organization. Three existing read endpoints query `poi_id = ANY(ids)` and | ||
| return each item's source POI name; the two sidebar tab components label rolled-up | ||
| items and link them to their own permalink. | ||
|
|
||
| --- | ||
|
|
||
| ## Architecture | ||
|
|
||
| ### Data Flow | ||
|
|
||
| 1. User selects a boundary or org in the map → Sidebar fetches | ||
| `/api/pois/:id/tab-counts`, then `PoiNews`/`PoiEvents` fetch | ||
| `/api/pois/:id/news` and `/api/pois/:id/events` (already keyed by `displayItem.id`). | ||
| 2. Each endpoint calls `getRollupPoiIds(pool, id)` → array of POI ids. | ||
| 3. The news/events/count SQL filters on `poi_id = ANY($ids)` and joins `pois` | ||
| for the source name. | ||
| 4. Frontend renders items; a source-POI label appears when the item's POI ≠ the | ||
| page POI, and clicks build the permalink from the item's source POI slug. | ||
|
|
||
| ### POI-id expansion (`getRollupPoiIds`) | ||
|
|
||
| ``` | ||
| ids = { targetId } | ||
| roles = target.poi_roles | ||
|
|
||
| if 'organization' in roles: | ||
| owned ∪= SELECT id FROM pois WHERE owner_id = targetId AND not deleted | ||
| owned ∪= SELECT physical_poi_id FROM poi_associations WHERE virtual_poi_id = targetId | ||
| ids ∪= owned | ||
| ids ∪= POIs whose point ⊂ boundary_geom of any owned boundary (spatial) | ||
|
|
||
| if 'boundary' in roles AND target has boundary_geom: | ||
| ids ∪= POIs whose point ⊂ target.boundary_geom (spatial) | ||
|
|
||
| return distinct ids | ||
| ``` | ||
|
|
||
| Spatial steps are wrapped so a PostGIS failure logs a warning and returns the | ||
| non-spatial ids (NFR-026-1). Point geometry per POI reuses the `CASE` expression | ||
| from `getContainingBoundaries` (point `geom`, else start point of `geometry`). | ||
|
|
||
| --- | ||
|
|
||
| ## Technology Choices | ||
|
|
||
| | Component | Technology | Rationale | | ||
| |-----------|------------|-----------| | ||
| | Containment | PostGIS `ST_Contains` | Same pattern already in `geoService.js` | | ||
| | Expansion location | `backend/services/geoService.js` | Co-located with `getContainingBoundaries`; shared by all three endpoints | | ||
|
|
||
| --- | ||
|
|
||
| ## Implementation Steps | ||
|
|
||
| ### Phase 1: Backend rollup helper | ||
|
|
||
| - [ ] Add `getRollupPoiIds(pool, poiId)` to `backend/services/geoService.js` | ||
| returning a de-duplicated array of POI ids (always includes `poiId`). | ||
| - [ ] Non-spatial expansion (org owned + associated) runs first; spatial | ||
| containment wrapped in try/catch with `[Geo]` warning + fallback. | ||
|
|
||
| ### Phase 2: Wire endpoints | ||
|
|
||
| - [ ] `/api/pois/:id/news`: use `getRollupPoiIds`, `WHERE n.poi_id = ANY($1)`, | ||
| `JOIN pois src ON src.id = n.poi_id`, return `n.poi_id`, `src.name AS poi_name`. | ||
| - [ ] `/api/pois/:id/events`: same expansion + `src.name AS poi_name`, `e.poi_id`. | ||
| - [ ] `/api/pois/:id/tab-counts`: count over `poi_id = ANY($ids)`. | ||
|
|
||
| ### Phase 3: Frontend labeling + permalink | ||
|
|
||
| - [ ] `PoiNews.jsx`: show source-POI label when `Number(item.poi_id) !== Number(poiId)`; | ||
| build slug from `item.poi_name || poiName`. | ||
| - [ ] `PoiEvents.jsx`: same. | ||
| - [ ] Minimal CSS for the source-POI label. | ||
|
|
||
| --- | ||
|
|
||
| ## File Changes | ||
|
|
||
| ### Modified Files | ||
|
|
||
| | File | Changes | | ||
| |------|---------| | ||
| | `backend/services/geoService.js` | Add `getRollupPoiIds` helper | | ||
| | `backend/server.js` | Rollup in `/news`, `/events`, `/tab-counts` handlers; return source POI name | | ||
| | `frontend/src/components/sidebar/PoiNews.jsx` | Source-POI label + permalink-by-source-slug | | ||
| | `frontend/src/components/sidebar/PoiEvents.jsx` | Source-POI label + permalink-by-source-slug | | ||
| | `frontend/src/App.css` (or component CSS) | `.poi-item-source` label style | | ||
|
|
||
| No new files, no migrations. | ||
|
|
||
| --- | ||
|
|
||
| ## API Implementation | ||
|
|
||
| ### `GET /api/pois/:id/news` (boundary/org example) | ||
|
|
||
| **Response (rolled-up item gains `poi_id` + `poi_name`):** | ||
| ```json | ||
| [ | ||
| { | ||
| "id": 123, | ||
| "title": "Trail reopens after storm", | ||
| "summary": "...", | ||
| "source_url": "https://...", | ||
| "publication_date": "2026-05-20", | ||
| "poi_id": 5536, | ||
| "poi_name": "Old Portage Trail", | ||
| "additional_urls": [] | ||
| } | ||
| ] | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Testing Strategy | ||
|
|
||
| ### Manual Testing | ||
|
|
||
| 1. Click Brecksville Reservation (boundary) → News/Events tabs list items from | ||
| POIs inside the boundary, each labeled with its POI name. | ||
| 2. Click Cleveland Metroparks (org) → tabs list items from owned parks and POIs | ||
| inside them. | ||
| 3. Click a rolled-up item → opens that item's own permalink/detail. | ||
| 4. Click a regular point POI → behaves exactly as before (own content only). | ||
| 5. Boundary/org with zero rolled-up content → tabs hidden as before. | ||
|
|
||
| ### Build / Quality Gates | ||
|
|
||
| - [ ] `./run.sh build` passes | ||
| - [ ] Human verification in browser (Phase 5) | ||
| - [ ] `gourmand --full .` clean | ||
| - [ ] Gatehouse review clean | ||
|
|
||
| --- | ||
|
|
||
| ## Rollback Plan | ||
|
|
||
| 1. Revert the PR. No migrations or schema changes, so rollback is code-only. | ||
|
|
||
| --- | ||
|
|
||
| ## Risks and Mitigations | ||
|
|
||
| | Risk | Impact | Mitigation | | ||
| |------|--------|------------| | ||
| | Large boundary (county/state) rolls up many items | Low | Existing `LIMIT` caps results; ordered newest-first | | ||
| | PostGIS query failure | Med | try/catch fallback to non-spatial expansion (NFR-026-1) | | ||
| | Rolled-up item permalink fails to resolve | Med | Navigate using item's source POI slug; App.jsx already resolves destinations/virtual/linear POIs (#412) | | ||
| | Spatial cost on every POI click | Low | Helper short-circuits to `[id]` for point POIs — no spatial query | | ||
|
|
||
| --- | ||
|
|
||
| ## Changelog | ||
|
|
||
| | Date | Changes | | ||
| |------|---------| | ||
| | 2026-05-27 | Initial plan | |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| # Specification: GeoFenced and Associated News & Events | ||
|
|
||
| > **Spec ID:** 026-geofenced-news | ||
| > **Status:** Draft | ||
| > **Version:** 0.1.0 | ||
| > **Author:** Scott McCarty | ||
| > **Date:** 2026-05-27 | ||
|
|
||
| ## Overview | ||
|
|
||
| When a user clicks a geographic boundary (e.g. Sand Run) or an organization (e.g. | ||
| Summit County Metro Parks), the News and Events sub-tabs should show content for | ||
| everything that boundary contains or that organization owns — not just content | ||
| collected against that single POI. This rolls up the news/events that already | ||
| exist on the contained/owned POIs so a park or org page becomes a true digest of | ||
| its area, with each item labeled by the POI it came from. | ||
|
|
||
| Closes #406. | ||
|
|
||
| --- | ||
|
|
||
| ## User Stories | ||
|
|
||
| ### Geo-fenced rollup | ||
|
|
||
| **US-026-1: News & events for everything inside a boundary** | ||
| > As a visitor, I want to click a park boundary like Sand Run and see all of the | ||
| > News and Events for every point of interest contained within that park, so that | ||
| > I get one consolidated view of what's happening in the park. | ||
|
|
||
| Acceptance Criteria: | ||
| - [ ] Clicking a boundary POI shows news/events whose owning POI's location falls | ||
| inside that boundary's polygon (`ST_Contains`), plus any collected against | ||
| the boundary itself. | ||
| - [ ] Applies to all boundary types (park, municipal, county, state). | ||
| - [ ] Each rolled-up item shows the name of the POI it came from. | ||
| - [ ] The News/Events tabs appear for a boundary that has rolled-up content even | ||
| if the boundary POI itself has none. | ||
|
|
||
| ### Organization rollup | ||
|
|
||
| **US-026-2: News & events for everything an organization owns** | ||
| > As a visitor, I want to click an organization like Summit County Metro Parks and | ||
| > see news for every park and point of interest within those parks, so that I can | ||
| > follow an entire agency from one place. | ||
|
|
||
| Acceptance Criteria: | ||
| - [ ] Clicking an organization POI shows its own news/events, plus content from | ||
| POIs it owns (`owner_id`) or is associated with (`poi_associations`), plus | ||
| content from POIs geographically contained within any park boundary it owns. | ||
| - [ ] Each rolled-up item shows the name of the POI it came from. | ||
| - [ ] Tabs appear when the org has rolled-up content even with none of its own. | ||
|
|
||
| ### Navigation integrity | ||
|
|
||
| **US-026-3: Rolled-up items link to their own POI's permalink** | ||
| > As a visitor, I want clicking a rolled-up news/event item to open that item's | ||
| > real detail page, so that the "read more" / permalink works. | ||
|
|
||
| Acceptance Criteria: | ||
| - [ ] Clicking a rolled-up item navigates to `/{sourcePoiSlug}/{news|events}/{titleSlug}` | ||
| (the item's owning POI), not the boundary/org slug. | ||
|
|
||
| --- | ||
|
|
||
| ## Data Model | ||
|
|
||
| No schema changes. Uses existing structures: | ||
|
|
||
| | Structure | Role | | ||
| |-----------|------| | ||
| | `pois.poi_roles` | distinguishes `boundary` / `organization` / `point` targets | | ||
| | `pois.boundary_geom` | polygon used for `ST_Contains` containment | | ||
| | `pois.geom` / `pois.geometry` | POI point used as containment test point | | ||
| | `pois.owner_id` | direct org ownership FK | | ||
| | `poi_associations(virtual_poi_id, physical_poi_id)` | org ↔ POI association | | ||
| | `poi_news.poi_id` / `poi_events.poi_id` | one-to-one item → POI link (unchanged) | | ||
|
|
||
| --- | ||
|
|
||
| ## API Endpoints | ||
|
|
||
| No new endpoints. Three existing endpoints gain rollup behavior when the target | ||
| POI is a boundary or organization (regular point POIs are unchanged): | ||
|
|
||
| | Method | Path | Change | | ||
| |--------|------|--------| | ||
| | GET | `/api/pois/:id/news` | Returns news for the expanded POI-id set; adds `poi_id` + `poi_name` per item | | ||
| | GET | `/api/pois/:id/events` | Returns events for the expanded POI-id set; adds `poi_id` + `poi_name` per item | | ||
| | GET | `/api/pois/:id/tab-counts` | Counts over the expanded POI-id set | | ||
|
|
||
| Expansion is automatic and role-driven — no new query parameter. | ||
|
|
||
| --- | ||
|
|
||
| ## UI/UX Requirements | ||
|
|
||
| - `PoiNews` / `PoiEvents`: when an item's `poi_id` differs from the page POI, | ||
| render a small source-POI label (the originating POI name) on the item. | ||
| - Clicks use the item's source POI name to build the permalink slug. | ||
|
|
||
| --- | ||
|
|
||
| ## Non-Functional Requirements | ||
|
|
||
| **NFR-026-1: Graceful PostGIS fallback** | ||
| - If a spatial query fails (PostGIS unavailable), rollup degrades to the | ||
| non-spatial expansion (org's own + owned/associated) and never errors the | ||
| endpoint — mirrors `getContainingBoundaries`' existing fallback. | ||
|
|
||
| **NFR-026-2: No regression for point POIs** | ||
| - A regular point POI's endpoints behave exactly as today (id set is just `[id]`), | ||
| paying no spatial-query cost. | ||
|
|
||
| --- | ||
|
|
||
| ## Dependencies | ||
|
|
||
| - Depends on: PostGIS support (migration 021) and boundary geometry (spec 005 / | ||
| issue #198 imports). Builds on the same `ST_Contains` pattern as | ||
| `backend/services/geoService.js`. | ||
|
|
||
| --- | ||
|
|
||
| ## Open Questions | ||
|
|
||
| None — scope decisions resolved: all boundary types roll up; org rollup includes | ||
| POIs inside owned parks; rolled-up items are labeled with their source POI. | ||
|
|
||
| --- | ||
|
|
||
| ## Changelog | ||
|
|
||
| | Version | Date | Changes | | ||
| |---------|------|---------| | ||
| | 0.1.0 | 2026-05-27 | Initial draft | |
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
idparameter fromreq.paramsis not validated before being passed togetRollupPoiIds. If a non-integer or invalid ID is provided, it will propagate to the database query and could result in unnecessary database roundtrips or errors.Adding an upfront validation check (similar to the one implemented in
/api/pois/:id/tab-counts) ensures consistency and improves defensive programming.Note: The same validation should also be applied to the
/api/pois/:id/eventsendpoint.