Optimize Map Performance, Bounding, and Dive Trips Visualization#198
Merged
Optimize Map Performance, Bounding, and Dive Trips Visualization#198
Conversation
- Refactor `/newsletters/trips` API to use a paginated response (`ParsedDiveTripListResponse`) for consistency with other endpoints. - Add `latitude` and `longitude` to the trip response schema, eliminating the need for N+1 queries to resolve map marker coordinates on the frontend. - Short-circuit map data fetching in `useViewportData.js` when no trips match the current filters, saving bandwidth. - Add top pagination controls to the Dive Trips list page. - Fix state initialization in `IndependentMapView` to prevent duplicate API requests on mount. - Fix MySQL sorting error caused by `DISTINCT` and `ORDER BY` execution order.
- Add `latitude` and `longitude` to nested `ParsedDiveResponse` schema to support multi-site trip mapping. - Update trip serialization to assign fallback coordinates from the diving center to the parent trip object. - Modify `LeafletMapView` to plot individual markers for every dive site within a trip, using the diving center coordinates only as a fallback. - Implement dynamic marker color-coding for dive trips based on schedule status (past, today, future). - Overhaul map popups for dive trips: replace raw coordinates with linked Dive Site names, formatted trip details, and brief descriptions. - Add dive trips map URL to performance measuring scripts.
kargig
force-pushed
the
feature/optimize-dive-trips-map-pagination
branch
from
56c9b33 to
4da7169
Compare
- Replace slow Python-side distance sorting with native MySQL `ST_Distance_Sphere` in the dive trips API, massively speeding up spatial queries. - Defer loading of the heavy `content` column in the `newsletters` list endpoint to reduce memory and bandwidth overhead. - Set `ORJSONResponse` as the default FastAPI response class globally for much faster JSON serialization. - Fix a frontend caching regression in `useViewportData` by omitting map bounds from the React Query cache key for dive trips, stopping redundant API requests when panning.
- Sync the debounced viewport immediately on the initial load in `useViewportData` to bypass the 1.5-second debounce delay. - Require `bounds` to be calculated before allowing API fetches in `useViewportData` to prevent querying 1000 random items globally. - Calculate and set initial map bounds quickly after the Leaflet instance loads in `IndependentMapView` to ensure data fetching triggers without requiring user interaction.
- Implement deferred DOMPurify HTML sanitization and deferred HTML string generation for Leaflet popups (`LeafletMapView`). Popups are now only generated when explicitly clicked, massively unblocking the main thread and eliminating a 10s hang when loading thousands of points. - Implement explicit map bounding box filtering for Diving Centers and Dives in both the frontend `useViewportData` hook and the backend SQLAlchemy endpoints (`diving_centers.py`, `dives_crud.py`), mirroring the logic for Dive Sites. - Add `defer` to the massive `description` column in `diving_centers.py` for map payloads, dropping bandwidth usage by 90%. - Exempt dive trips from the `useViewportData` initial load bounds requirement, fixing a flicker and a massive 6-second double-fetch React race condition regression.
kargig
changed the title
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
1 participant
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.
Overview
This PR delivers sweeping performance optimizations and architectural improvements to the Map visualization across all entity types (
dive-trips,diving-centers,dives, anddive-sites). It standardizes API responses, optimizes SQL queries with spatial functions, eliminates N+1 queries, defers expensive HTML sanitization to unblock the main thread, and implements strict geo-bounding filters to slash network payload sizes.Changes Made
Backend
/api/v1/newsletters/tripsendpoint to return a standardized paginated dictionary response (ParsedDiveTripListResponse), bringing it in line with other list APIs.latitudeandlongitudefields directly into theParsedDiveTripResponseand nestedParsedDiveResponseschemas. The backend now calculates coordinate fallbacks (Diving Center vs. Dive Site), eliminating the need for the frontend to fetch thousands of external entities as a lookup dictionary.ST_Distance_Spheresorting in the dive trips API.defer()to massivecontentanddescriptioncolumns in thenewslettersanddiving-centerslist endpoints. Map payload sizes dropped by ~90%.ORJSONResponseas the default FastAPI response class globally for significantly faster JSON serialization.north,south,east, andwestquery parameters to thediving_centersanddivesendpoints to allow database-level spatial filtering.OperationalError 3065sorting error caused by the execution order ofDISTINCTandORDER BY.Frontend
DOMPurify.sanitize()and HTML string generation for Leaflet popups. Popups are now only generated and sanitized when a user explicitly clicks a marker, completely unblocking the main JavaScript thread and eliminating severe browser hangs.useViewportDatato requireboundsto be calculated by Leaflet before allowing API fetches. This prevents the map from blindly querying 1,000 random items globally on initial load.dive-tripsfrom the bounds-fetching requirement (since trips are globally filtered by date), which fixed a massive 6-second double-fetch race condition.LeafletMapViewto plot an individual map marker for every dive site within a single dive trip. The parent diving center coordinates are only used as a fallback if no sites have coordinates./dive-tripslist page.Breaking Changes
/api/v1/newsletters/tripshas been updated from a flat JSON array to a paginated dictionary structure ({ items: [...], total: X, page: Y, ... }). All frontend consumers have been updated to reflect this.Testing
scripts/measure_performance2.js. Achieved massive gains:-58.1%faster sequential load times for/map?type=dive-trips.-87.7%reduction in payload size for/map?type=diving-centers(from ~340KB down to ~41KB).test_newsletters.py,test_sorting.py,test_dives.py,test_diving_centers.py) run successfully in the isolated GitHub Actions Docker environment.npx eslint --fix.Related Issues
Additional Notes
ST_Distance_Sphere). Ensure the production database environment correctly supports these spatial operations. The global shift toORJSONResponseis fully backwards compatible but offers significant throughput gains.wind=trueoverlay and thousands oftype=divesmarkers have been resolved by the deferred popup rendering optimization.