feat: enriched favorites list + Following→Favorite rename + shared BackButton (#437)#459
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Button (#437) My Valley Favorites tab (resolves #437): - Enrich /api/favorites with trail_status + news/events counts (one LATERAL query, tz-whitelisted). New public /api/pois/summary?ids= batch endpoint so anonymous (localStorage) favorites get the same status/counts — local-first parity. - Rename 'Following' → 'Favorite' across UI + MyValley internals (backend tables unchanged). Favorites is now the first tab and the default view. - Rows show a StatusBadge + green clickable name (opens the POI on the map) + green news/events count chips. Sort (recent/name/activity) + type-filter chips. - Clicking a count opens that POI's news/events INSIDE My Valley (reuses sidebar PoiNews/PoiEvents), and clicking an item opens the article in-panel (ContentDetail), so focus never leaves the modal — works the same on mobile and desktop. - App router now resolves organization (virtual) POIs for /<slug>/<subtab> paths, so org favorites open the sidebar. Shared BackButton: - New BackButton.jsx standardizes every navigational back control to '← Back' (action, not destination — fixes the misleading 'Back to <POI>'). Swapped in ContentDetail, MyValley, News/EventPermalink, TripsManager, PrivacyPolicy; one shared .back-button style. My Valley owns a single fixed back button across all detail levels (PoiNews/PoiEvents/ContentDetail gain opt-out flags). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements the enriched favorites list and renames the 'Following' feature to 'Favorites' across the application. It introduces a new public batch enrichment endpoint /api/pois/summary for anonymous users, standardizes navigation with a new BackButton component, and updates the favorites tab to support sorting, filtering, and in-modal detail views. The reviewer feedback highlights critical issues in the backend: the timezone validation regex is too restrictive because it lacks hyphen support, which will break valid IANA timezones; the new /api/pois/summary endpoint is missing name and poi_roles fields required for anonymous user features; and applying AT TIME ZONE to the DATE column e.start_date can cause incorrect date shifts, which should be resolved by comparing the date directly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const summary = await pool.query(` | ||
| SELECT p.id, | ||
| ts.status AS trail_status, | ||
| COALESCE(nc.cnt, 0)::int AS news_count, | ||
| COALESCE(ec.cnt, 0)::int AS events_count | ||
| FROM pois p | ||
| LEFT JOIN LATERAL ( | ||
| SELECT status FROM trail_status | ||
| WHERE poi_id = p.id ORDER BY created_at DESC LIMIT 1 | ||
| ) ts ON true | ||
| LEFT JOIN LATERAL ( | ||
| SELECT COUNT(*) AS cnt FROM poi_news n | ||
| WHERE n.poi_id = p.id | ||
| AND n.moderation_status IN ('published', 'auto_approved') | ||
| ) nc ON true | ||
| LEFT JOIN LATERAL ( | ||
| SELECT COUNT(*) AS cnt FROM poi_events e | ||
| WHERE e.poi_id = p.id | ||
| AND e.moderation_status IN ('published', 'auto_approved') | ||
| AND (e.start_date AT TIME ZONE $2)::date >= (CURRENT_TIMESTAMP AT TIME ZONE $2)::date | ||
| ) ec ON true | ||
| WHERE p.id = ANY($1) AND p.deleted IS NOT TRUE`, | ||
| [ids, tz] | ||
| ); |
There was a problem hiding this comment.
The /api/pois/summary endpoint is used to enrich anonymous favorites (stored in localStorage). However, it currently does not return p.name or p.poi_roles.\n\nBecause MyValley only receives destinations (point POIs) as a prop, any favorited linear features (trails/rivers) or virtual POIs (organizations) will fail to resolve their names in toRows and default to 'Saved place'. Furthermore, because poi_roles is missing, anonymous users will not be able to use the type-filtering or sorting features at all.\n\nIncluding p.name and p.poi_roles in the summary query solves both issues cleanly.\n\nAdditionally, e.start_date is of type DATE (without timezone). Applying AT TIME ZONE to a DATE converts it to a timestamp with time zone at midnight in that zone, and casting it back to ::date can shift the date by +/- 1 day depending on the database session's timezone setting. Comparing e.start_date directly to the timezone-adjusted current date is safer, simpler, and more correct.
const summary = await pool.query(`
SELECT p.id, p.name, p.poi_roles,
ts.status AS trail_status,
COALESCE(nc.cnt, 0)::int AS news_count,
COALESCE(ec.cnt, 0)::int AS events_count
FROM pois p
LEFT JOIN LATERAL (
SELECT status FROM trail_status
WHERE poi_id = p.id ORDER BY created_at DESC LIMIT 1
) ts ON true
LEFT JOIN LATERAL (
SELECT COUNT(*) AS cnt FROM poi_news n
WHERE n.poi_id = p.id
AND n.moderation_status IN ('published', 'auto_approved')
) nc ON true
LEFT JOIN LATERAL (
SELECT COUNT(*) AS cnt FROM poi_events e
WHERE e.poi_id = p.id
AND e.moderation_status IN ('published', 'auto_approved')
AND e.start_date >= (CURRENT_TIMESTAMP AT TIME ZONE $2)::date
) ec ON true
WHERE p.id = ANY($1) AND p.deleted IS NOT TRUE`,
[ids, tz]
);| const tz = (typeof rawTz === 'string' && /^[A-Za-z_]+\/[A-Za-z_]+(?:\/[A-Za-z_]+)?$/.test(rawTz)) | ||
| ? rawTz | ||
| : 'America/New_York'; |
There was a problem hiding this comment.
The regex used to validate the timezone whitelisting is too restrictive because it does not allow hyphens (-). Hyphens are perfectly valid and common in IANA timezone names (e.g., America/Port-au-Prince, America/Blanc-Sablon, Africa/Porto-Novo, Asia/Ust-Nera). If a user's browser is in one of these timezones, the validation will fail and fallback to America/New_York.\n\nUpdating the regex to allow hyphens resolves this issue.
| const tz = (typeof rawTz === 'string' && /^[A-Za-z_]+\/[A-Za-z_]+(?:\/[A-Za-z_]+)?$/.test(rawTz)) | |
| ? rawTz | |
| : 'America/New_York'; | |
| const tz = (typeof rawTz === 'string' && /^[A-Za-z_-]+\/[A-Za-z_-]+(?:\/[A-Za-z_-]+)?$/.test(rawTz)) | |
| ? rawTz | |
| : 'America/New_York'; |
| const tz = (typeof rawTz === 'string' && /^[A-Za-z_]+\/[A-Za-z_]+(?:\/[A-Za-z_]+)?$/.test(rawTz)) | ||
| ? rawTz | ||
| : 'America/New_York'; |
There was a problem hiding this comment.
The regex used to validate the timezone whitelisting is too restrictive because it does not allow hyphens (-). Hyphens are perfectly valid and common in IANA timezone names (e.g., America/Port-au-Prince, America/Blanc-Sablon, Africa/Porto-Novo, Asia/Ust-Nera). If a user's browser is in one of these timezones, the validation will fail and fallback to America/New_York.\n\nUpdating the regex to allow hyphens resolves this issue.
| const tz = (typeof rawTz === 'string' && /^[A-Za-z_]+\/[A-Za-z_]+(?:\/[A-Za-z_]+)?$/.test(rawTz)) | |
| ? rawTz | |
| : 'America/New_York'; | |
| const tz = (typeof rawTz === 'string' && /^[A-Za-z_-]+\/[A-Za-z_-]+(?:\/[A-Za-z_-]+)?$/.test(rawTz)) | |
| ? rawTz | |
| : 'America/New_York'; |
| const favorites = await pool.query( | ||
| `SELECT p.id, p.name, p.poi_roles, p.brief_description, p.has_primary_image, | ||
| f.created_at AS favorited_at | ||
| f.created_at AS favorited_at, | ||
| ts.status AS trail_status, | ||
| COALESCE(nc.cnt, 0)::int AS news_count, | ||
| COALESCE(ec.cnt, 0)::int AS events_count | ||
| FROM user_poi_favorites f | ||
| JOIN pois p ON p.id = f.poi_id | ||
| LEFT JOIN LATERAL ( | ||
| SELECT status FROM trail_status | ||
| WHERE poi_id = p.id ORDER BY created_at DESC LIMIT 1 | ||
| ) ts ON true | ||
| LEFT JOIN LATERAL ( | ||
| SELECT COUNT(*) AS cnt FROM poi_news n | ||
| WHERE n.poi_id = p.id | ||
| AND n.moderation_status IN ('published', 'auto_approved') | ||
| ) nc ON true | ||
| LEFT JOIN LATERAL ( | ||
| SELECT COUNT(*) AS cnt FROM poi_events e | ||
| WHERE e.poi_id = p.id | ||
| AND e.moderation_status IN ('published', 'auto_approved') | ||
| AND (e.start_date AT TIME ZONE $2)::date >= (CURRENT_TIMESTAMP AT TIME ZONE $2)::date | ||
| ) ec ON true | ||
| WHERE f.user_id = $1 AND p.deleted IS NOT TRUE | ||
| ORDER BY f.created_at DESC`, | ||
| [req.user.id] | ||
| [req.user.id, tz] | ||
| ); |
There was a problem hiding this comment.
e.start_date is of type DATE (without timezone). Applying AT TIME ZONE to a DATE converts it to a timestamp with time zone at midnight in that zone, and casting it back to ::date can shift the date by +/- 1 day depending on the database session's timezone setting. Comparing e.start_date directly to the timezone-adjusted current date is safer, simpler, and more correct.
const favorites = await pool.query(
`SELECT p.id, p.name, p.poi_roles, p.brief_description, p.has_primary_image,
f.created_at AS favorited_at,
ts.status AS trail_status,
COALESCE(nc.cnt, 0)::int AS news_count,
COALESCE(ec.cnt, 0)::int AS events_count
FROM user_poi_favorites f
JOIN pois p ON p.id = f.poi_id
LEFT JOIN LATERAL (
SELECT status FROM trail_status
WHERE poi_id = p.id ORDER BY created_at DESC LIMIT 1
) ts ON true
LEFT JOIN LATERAL (
SELECT COUNT(*) AS cnt FROM poi_news n
WHERE n.poi_id = p.id
AND n.moderation_status IN ('published', 'auto_approved')
) nc ON true
LEFT JOIN LATERAL (
SELECT COUNT(*) AS cnt FROM poi_events e
WHERE e.poi_id = p.id
AND e.moderation_status IN ('published', 'auto_approved')
AND e.start_date >= (CURRENT_TIMESTAMP AT TIME ZONE $2)::date
) ec ON true
WHERE f.user_id = $1 AND p.deleted IS NOT TRUE
ORDER BY f.created_at DESC`,
[req.user.id, tz]
);…rflow fix: My Valley row name overlapping chips on mobile (#459 follow-up)
fix: smaller My Valley titles on mobile (#459 follow-up)
Summary
Turns the My Valley "Following" list into the enriched "My List" view from #437, renames the feature to Favorite, and standardizes back buttons app-wide.
Favorites (#437)
/api/favoritesenriched withtrail_status+ news/upcoming-events counts (oneLATERALquery, tz-whitelisted). New publicGET /api/pois/summary?ids=&tz=batch endpoint gives anonymous favorites the same status/counts (local-first parity; mirrors the notification?pois=precedent).MyValleyinternals (backendfavorites/user_poi_favoritesunchanged). Favorites is now the first tab and default view.StatusBadge, green clickable name (opens POI on map), green 📰/📅 count chips, sort (recent/name/activity) + type-filter chips.PoiNews/PoiEvents), and an item opens the article in-panel (ContentDetail) — focus never leaves the modal; identical on mobile + desktop./<slug>/<subtab>paths, so org favorites open the sidebar.Shared BackButton
BackButton.jsx→ every navigational back control reads "← Back" (action, not destination — fixes the misleading "Back to "). Swapped inContentDetail,MyValley,News/EventPermalink,TripsManager,PrivacyPolicy; one.back-buttonstyle. My Valley keeps a single fixed back button across all detail levels.Frontend + two additive backend endpoints. No DB changes. Spec/plan:
.specify/specs/033-favorites-list/.Closes #437
Test plan
./run.sh buildpasses🤖 Generated with Claude Code