From ce8a7cd4efd4c8f9969581988ffe54c0141926b9 Mon Sep 17 00:00:00 2001 From: Evgenii Burmakin Date: Wed, 7 Jan 2026 19:48:14 +0100 Subject: [PATCH] =?UTF-8?q?Implement=20some=20performance=20improvements?= =?UTF-8?q?=20and=20caching=20for=20various=20featu=E2=80=A6=20(#2133)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Implement some performance improvements and caching for various features. * Fix failing tests * Implement routes behaviour in map v2 to match map v1 * Fix route highlighting * Add fallbacks when retrieving full route features to handle cases where source data access methods vary. * Fix some e2e tests --- CHANGELOG.md | 13 + CLAUDE.md | 41 ++ app/controllers/api/v1/places_controller.rb | 22 +- app/controllers/api/v1/visits_controller.rb | 11 + app/controllers/map/leaflet_controller.rb | 32 +- app/controllers/stats_controller.rb | 12 +- .../controllers/maps/maplibre/data_loader.js | 2 +- .../maps/maplibre/event_handlers.js | 242 +++++++ .../maps/maplibre/layer_manager.js | 58 +- .../maps/maplibre/map_data_manager.js | 25 +- .../controllers/maps/maplibre_controller.js | 48 +- app/javascript/maps/marker_factory.js | 2 +- .../maps_maplibre/layers/routes_layer.js | 152 ++++- .../maps_maplibre/services/api_client.js | 182 +++++- .../maps_maplibre/utils/settings_manager.js | 2 +- app/jobs/cache/preheating_job.rb | 8 + app/jobs/users/mailer_sending_job.rb | 20 +- app/models/user.rb | 6 +- app/services/cache/clean.rb | 7 + app/services/cache/invalidate_user_caches.rb | 5 + app/services/stats/calculate_month.rb | 14 +- app/services/visits/find_in_time.rb | 2 +- .../visits/find_within_bounding_box.rb | 2 +- .../map/maplibre/_settings_panel.html.erb | 30 + e2e/v2/map/interactions.spec.js | 598 ++++++++++++++++++ e2e/v2/map/layers/family.spec.js | 49 +- e2e/v2/map/search.spec.js | 11 +- spec/requests/api/v1/users_spec.rb | 2 +- 28 files changed, 1472 insertions(+), 126 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45b4e351..906dd1b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +# [0.37.3] - Unreleased + +## Fixed + +- Routes are now being drawn the very same way on Map V2 as in Map V1. #2132 #2086 + +## Changed + +- Map V2 points loading is significantly sped up. +- Points size on Map V2 was reduced to prevent overlapping. + # [0.37.2] - 2026-01-04 ## Fixed @@ -12,6 +23,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Time spent in a country and city is now calculated correctly for the year-end digest email. #2104 - Updated Trix to fix a XSS vulnerability. #2102 - Map v2 UI no longer blocks when Immich/Photoprism integration has a bad URL or is unreachable. Added 10-second timeout to photo API requests and improved error handling to prevent UI freezing during initial load. #2085 + +## Added - In Map v2 settings, you can now enable map to be rendered as a globe. # [0.37.1] - 2025-12-30 diff --git a/CLAUDE.md b/CLAUDE.md index bea64b39..c40f2e9c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -238,6 +238,47 @@ bundle exec bundle-audit # Dependency security - Respect expiration settings and disable sharing when expired - Only expose minimal necessary data in public sharing contexts +### Route Drawing Implementation (Critical) + +⚠️ **IMPORTANT: Unit Mismatch in Route Splitting Logic** + +Both Map v1 (Leaflet) and Map v2 (MapLibre) contain an **intentional unit mismatch** in route drawing that must be preserved for consistency: + +**The Issue**: +- `haversineDistance()` function returns distance in **kilometers** (e.g., 0.5 km) +- Route splitting threshold is stored and compared as **meters** (e.g., 500) +- The code compares them directly: `0.5 > 500` = always **FALSE** + +**Result**: +- The distance threshold (`meters_between_routes` setting) is **effectively disabled** +- Routes only split on **time gaps** (default: 60 minutes between points) +- This creates longer, more continuous routes that users expect + +**Code Locations**: +- **Map v1**: `app/javascript/maps/polylines.js:390` + - Uses `haversineDistance()` from `maps/helpers.js` (returns km) + - Compares to `distanceThresholdMeters` variable (value in meters) + +- **Map v2**: `app/javascript/maps_maplibre/layers/routes_layer.js:82-104` + - Has built-in `haversineDistance()` method (returns km) + - Intentionally skips `/1000` conversion to replicate v1 behavior + - Comment explains this is matching v1's unit mismatch + +**Critical Rules**: +1. ❌ **DO NOT "fix" the unit mismatch** - this would break user expectations +2. ✅ **Keep both versions synchronized** - they must behave identically +3. ✅ **Document any changes** - route drawing changes affect all users +4. ⚠️ If you ever fix this bug: + - You MUST update both v1 and v2 simultaneously + - You MUST migrate user settings (multiply existing values by 1000 or divide by 1000 depending on direction) + - You MUST communicate the breaking change to users + +**Additional Route Drawing Details**: +- **Time threshold**: 60 minutes (default) - actually functional +- **Distance threshold**: 500 meters (default) - currently non-functional due to unit bug +- **Sorting**: Map v2 sorts points by timestamp client-side; v1 relies on backend ASC order +- **API ordering**: Map v2 must request `order: 'asc'` to match v1's chronological data flow + ## Contributing - **Main Branch**: `master` diff --git a/app/controllers/api/v1/places_controller.rb b/app/controllers/api/v1/places_controller.rb index 97035526..41d4dac8 100644 --- a/app/controllers/api/v1/places_controller.rb +++ b/app/controllers/api/v1/places_controller.rb @@ -16,11 +16,11 @@ module Api include_untagged = tag_ids.include?('untagged') if numeric_tag_ids.any? && include_untagged - # Both tagged and untagged: return union (OR logic) - tagged = current_api_user.places.includes(:tags, :visits).with_tags(numeric_tag_ids) - untagged = current_api_user.places.includes(:tags, :visits).without_tags - @places = Place.from("(#{tagged.to_sql} UNION #{untagged.to_sql}) AS places") - .includes(:tags, :visits) + # Both tagged and untagged: use OR logic to preserve eager loading + tagged_ids = current_api_user.places.with_tags(numeric_tag_ids).pluck(:id) + untagged_ids = current_api_user.places.without_tags.pluck(:id) + combined_ids = (tagged_ids + untagged_ids).uniq + @places = current_api_user.places.includes(:tags, :visits).where(id: combined_ids) elsif numeric_tag_ids.any? # Only tagged places with ANY of the selected tags (OR logic) @places = @places.with_tags(numeric_tag_ids) @@ -30,6 +30,16 @@ module Api end end + # Support optional pagination (backward compatible - returns all if no page param) + if params[:page].present? + per_page = [params[:per_page]&.to_i || 100, 500].min + @places = @places.page(params[:page]).per(per_page) + + response.set_header('X-Current-Page', @places.current_page.to_s) + response.set_header('X-Total-Pages', @places.total_pages.to_s) + response.set_header('X-Total-Count', @places.total_count.to_s) + end + render json: @places.map { |place| serialize_place(place) } end @@ -120,7 +130,7 @@ module Api note: place.note, icon: place.tags.first&.icon, color: place.tags.first&.color, - visits_count: place.visits.count, + visits_count: place.visits.size, created_at: place.created_at, tags: place.tags.map do |tag| { diff --git a/app/controllers/api/v1/visits_controller.rb b/app/controllers/api/v1/visits_controller.rb index 1002536d..0110a97f 100644 --- a/app/controllers/api/v1/visits_controller.rb +++ b/app/controllers/api/v1/visits_controller.rb @@ -3,6 +3,17 @@ class Api::V1::VisitsController < ApiController def index visits = Visits::Finder.new(current_api_user, params).call + + # Support optional pagination (backward compatible - returns all if no page param) + if params[:page].present? + per_page = [params[:per_page]&.to_i || 100, 500].min + visits = visits.page(params[:page]).per(per_page) + + response.set_header('X-Current-Page', visits.current_page.to_s) + response.set_header('X-Total-Pages', visits.total_pages.to_s) + response.set_header('X-Total-Count', visits.total_count.to_s) + end + serialized_visits = visits.map do |visit| Api::VisitSerializer.new(visit).call end diff --git a/app/controllers/map/leaflet_controller.rb b/app/controllers/map/leaflet_controller.rb index 660b9615..0c425a57 100644 --- a/app/controllers/map/leaflet_controller.rb +++ b/app/controllers/map/leaflet_controller.rb @@ -41,19 +41,31 @@ class Map::LeafletController < ApplicationController end def calculate_distance - return 0 if @coordinates.size < 2 + return 0 if @points.count(:id) < 2 - total_distance = 0 + # Use PostGIS window function for efficient distance calculation + # This is O(1) database operation vs O(n) Ruby iteration + sql = <<~SQL.squish + SELECT COALESCE(SUM(distance_m) / 1000.0, 0) as total_km FROM ( + SELECT ST_Distance( + lonlat::geography, + LAG(lonlat::geography) OVER (ORDER BY timestamp) + ) as distance_m + FROM points + WHERE user_id = :user_id + AND timestamp >= :start_at + AND timestamp <= :end_at + ) distances + SQL - @coordinates.each_cons(2) do - distance_km = Geocoder::Calculations.distance_between( - [_1[0], _1[1]], [_2[0], _2[1]], units: :km - ) + result = Point.connection.select_value( + ActiveRecord::Base.sanitize_sql_array([ + sql, + { user_id: current_user.id, start_at: start_at, end_at: end_at } + ]) + ) - total_distance += distance_km - end - - total_distance.round + result&.to_f&.round || 0 end def parsed_start_at diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index 8d735acf..e12c7e9d 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -80,8 +80,14 @@ class StatsController < ApplicationController end def build_stats - current_user.stats.group_by(&:year).transform_values do |stats| - stats.sort_by(&:updated_at).reverse - end.sort.reverse + # Select only needed columns - avoid loading large JSONB fields + # daily_distance and h3_hex_ids are never needed on index page + columns = [:id, :year, :month, :distance, :updated_at, :user_id] + columns << :toponyms if DawarichSettings.reverse_geocoding_enabled? + + current_user.stats + .select(columns) + .order(year: :desc, updated_at: :desc) + .group_by(&:year) end end diff --git a/app/javascript/controllers/maps/maplibre/data_loader.js b/app/javascript/controllers/maps/maplibre/data_loader.js index f4e266fb..141bb148 100644 --- a/app/javascript/controllers/maps/maplibre/data_loader.js +++ b/app/javascript/controllers/maps/maplibre/data_loader.js @@ -39,7 +39,7 @@ export class DataLoader { performanceMonitor.mark('transform-geojson') data.pointsGeoJSON = pointsToGeoJSON(data.points) data.routesGeoJSON = RoutesLayer.pointsToRoutes(data.points, { - distanceThresholdMeters: this.settings.metersBetweenRoutes || 1000, + distanceThresholdMeters: this.settings.metersBetweenRoutes || 500, timeThresholdMinutes: this.settings.minutesBetweenRoutes || 60 }) performanceMonitor.measure('transform-geojson') diff --git a/app/javascript/controllers/maps/maplibre/event_handlers.js b/app/javascript/controllers/maps/maplibre/event_handlers.js index be214d13..4c450822 100644 --- a/app/javascript/controllers/maps/maplibre/event_handlers.js +++ b/app/javascript/controllers/maps/maplibre/event_handlers.js @@ -1,4 +1,6 @@ import { formatTimestamp } from 'maps_maplibre/utils/geojson_transformers' +import { formatDistance, formatSpeed, minutesToDaysHoursMinutes } from 'maps/helpers' +import maplibregl from 'maplibre-gl' /** * Handles map interaction events (clicks, info display) @@ -7,6 +9,8 @@ export class EventHandlers { constructor(map, controller) { this.map = map this.controller = controller + this.selectedRouteFeature = null + this.routeMarkers = [] // Store start/end markers for routes } /** @@ -126,4 +130,242 @@ export class EventHandlers { this.controller.showInfo(properties.name || 'Area', content, actions) } + + /** + * Handle route hover + */ + handleRouteHover(e) { + const clickedFeature = e.features[0] + if (!clickedFeature) return + + const routesLayer = this.controller.layerManager.getLayer('routes') + if (!routesLayer) return + + // Get the full feature from source (not the clipped tile version) + // Fallback to clipped feature if full feature not found + const fullFeature = this._getFullRouteFeature(clickedFeature.properties) || clickedFeature + + // If a route is selected and we're hovering over a different route, show both + if (this.selectedRouteFeature) { + // Check if we're hovering over the same route that's selected + const isSameRoute = this._areFeaturesSame(this.selectedRouteFeature, fullFeature) + + if (!isSameRoute) { + // Show both selected and hovered routes + const features = [this.selectedRouteFeature, fullFeature] + routesLayer.setHoverRoute({ + type: 'FeatureCollection', + features: features + }) + // Create markers for both routes + this._createRouteMarkers(features) + } + } else { + // No selection, just show hovered route + routesLayer.setHoverRoute(fullFeature) + // Create markers for hovered route + this._createRouteMarkers(fullFeature) + } + } + + /** + * Handle route mouse leave + */ + handleRouteMouseLeave(e) { + const routesLayer = this.controller.layerManager.getLayer('routes') + if (!routesLayer) return + + // If a route is selected, keep showing only the selected route + if (this.selectedRouteFeature) { + routesLayer.setHoverRoute(this.selectedRouteFeature) + // Keep markers for selected route only + this._createRouteMarkers(this.selectedRouteFeature) + } else { + // No selection, clear hover and markers + routesLayer.setHoverRoute(null) + this._clearRouteMarkers() + } + } + + /** + * Get full route feature from source data (not clipped tile version) + * MapLibre returns clipped geometries from queryRenderedFeatures() + * We need the full geometry from the source for proper highlighting + */ + _getFullRouteFeature(properties) { + const routesLayer = this.controller.layerManager.getLayer('routes') + if (!routesLayer) return null + + const source = this.map.getSource(routesLayer.sourceId) + if (!source) return null + + // Get the source data (GeoJSON FeatureCollection) + // Try multiple ways to access the data + let sourceData = null + + // Method 1: Internal _data property (most common) + if (source._data) { + sourceData = source._data + } + // Method 2: Serialize and deserialize (fallback) + else if (source.serialize) { + const serialized = source.serialize() + sourceData = serialized.data + } + // Method 3: Use cached data from layer + else if (routesLayer.data) { + sourceData = routesLayer.data + } + + if (!sourceData || !sourceData.features) return null + + // Find the matching feature by properties + return sourceData.features.find(feature => { + const props = feature.properties + return props.startTime === properties.startTime && + props.endTime === properties.endTime && + props.pointCount === properties.pointCount + }) + } + + /** + * Compare two features to see if they represent the same route + */ + _areFeaturesSame(feature1, feature2) { + if (!feature1 || !feature2) return false + + // Compare by start/end times and point count (unique enough for routes) + const props1 = feature1.properties + const props2 = feature2.properties + + return props1.startTime === props2.startTime && + props1.endTime === props2.endTime && + props1.pointCount === props2.pointCount + } + + /** + * Create start/end markers for route(s) + * @param {Array|Object} features - Single feature or array of features + */ + _createRouteMarkers(features) { + // Clear existing markers first + this._clearRouteMarkers() + + // Ensure we have an array + const featureArray = Array.isArray(features) ? features : [features] + + featureArray.forEach(feature => { + if (!feature || !feature.geometry || feature.geometry.type !== 'LineString') return + + const coords = feature.geometry.coordinates + if (coords.length < 2) return + + // Start marker (🚥) + const startCoord = coords[0] + const startMarker = this._createEmojiMarker('🚥') + startMarker.setLngLat(startCoord).addTo(this.map) + this.routeMarkers.push(startMarker) + + // End marker (🏁) + const endCoord = coords[coords.length - 1] + const endMarker = this._createEmojiMarker('🏁') + endMarker.setLngLat(endCoord).addTo(this.map) + this.routeMarkers.push(endMarker) + }) + } + + /** + * Create an emoji marker + * @param {String} emoji - The emoji to display + * @returns {maplibregl.Marker} + */ + _createEmojiMarker(emoji) { + const el = document.createElement('div') + el.className = 'route-emoji-marker' + el.textContent = emoji + el.style.fontSize = '24px' + el.style.cursor = 'pointer' + el.style.userSelect = 'none' + + return new maplibregl.Marker({ element: el, anchor: 'center' }) + } + + /** + * Clear all route markers + */ + _clearRouteMarkers() { + this.routeMarkers.forEach(marker => marker.remove()) + this.routeMarkers = [] + } + + /** + * Handle route click + */ + handleRouteClick(e) { + const clickedFeature = e.features[0] + const properties = clickedFeature.properties + + // Get the full feature from source (not the clipped tile version) + // Fallback to clipped feature if full feature not found + const fullFeature = this._getFullRouteFeature(properties) || clickedFeature + + // Store selected route (use full feature) + this.selectedRouteFeature = fullFeature + + // Update hover layer to show selected route + const routesLayer = this.controller.layerManager.getLayer('routes') + if (routesLayer) { + routesLayer.setHoverRoute(fullFeature) + } + + // Create markers for selected route + this._createRouteMarkers(fullFeature) + + // Calculate duration + const durationSeconds = properties.endTime - properties.startTime + const durationMinutes = Math.floor(durationSeconds / 60) + const durationFormatted = minutesToDaysHoursMinutes(durationMinutes) + + // Calculate average speed + let avgSpeed = properties.speed + if (!avgSpeed && properties.distance > 0 && durationSeconds > 0) { + avgSpeed = (properties.distance / durationSeconds) * 3600 // km/h + } + + // Get user preferences + const distanceUnit = this.controller.settings.distance_unit || 'km' + + // Prepare route data object + const routeData = { + startTime: formatTimestamp(properties.startTime, this.controller.timezoneValue), + endTime: formatTimestamp(properties.endTime, this.controller.timezoneValue), + duration: durationFormatted, + distance: formatDistance(properties.distance, distanceUnit), + speed: avgSpeed ? formatSpeed(avgSpeed, distanceUnit) : null, + pointCount: properties.pointCount + } + + // Call controller method to display route info + this.controller.showRouteInfo(routeData) + } + + /** + * Clear route selection + */ + clearRouteSelection() { + if (!this.selectedRouteFeature) return + + this.selectedRouteFeature = null + + const routesLayer = this.controller.layerManager.getLayer('routes') + if (routesLayer) { + routesLayer.setHoverRoute(null) + } + + // Clear markers + this._clearRouteMarkers() + + // Close info panel + this.controller.closeInfo() + } } diff --git a/app/javascript/controllers/maps/maplibre/layer_manager.js b/app/javascript/controllers/maps/maplibre/layer_manager.js index 2968713e..d6d31abb 100644 --- a/app/javascript/controllers/maps/maplibre/layer_manager.js +++ b/app/javascript/controllers/maps/maplibre/layer_manager.js @@ -21,6 +21,7 @@ export class LayerManager { this.settings = settings this.api = api this.layers = {} + this.eventHandlersSetup = false } /** @@ -30,7 +31,7 @@ export class LayerManager { performanceMonitor.mark('add-layers') // Layer order matters - layers added first render below layers added later - // Order: scratch (bottom) -> heatmap -> areas -> tracks -> routes -> visits -> places -> photos -> family -> points -> recent-point (top) -> fog (canvas overlay) + // Order: scratch (bottom) -> heatmap -> areas -> tracks -> routes (visual) -> visits -> places -> photos -> family -> points -> routes-hit (interaction) -> recent-point (top) -> fog (canvas overlay) await this._addScratchLayer(pointsGeoJSON) this._addHeatmapLayer(pointsGeoJSON) @@ -49,6 +50,7 @@ export class LayerManager { this._addFamilyLayer() this._addPointsLayer(pointsGeoJSON) + this._addRoutesHitLayer() // Add hit target layer after points for better interactivity this._addRecentPointLayer() this._addFogLayer(pointsGeoJSON) @@ -57,8 +59,13 @@ export class LayerManager { /** * Setup event handlers for layer interactions + * Only sets up handlers once to prevent duplicates */ setupLayerEventHandlers(handlers) { + if (this.eventHandlersSetup) { + return + } + // Click handlers this.map.on('click', 'points', handlers.handlePointClick) this.map.on('click', 'visits', handlers.handleVisitClick) @@ -69,6 +76,11 @@ export class LayerManager { this.map.on('click', 'areas-outline', handlers.handleAreaClick) this.map.on('click', 'areas-labels', handlers.handleAreaClick) + // Route handlers - use routes-hit layer for better interactivity + this.map.on('click', 'routes-hit', handlers.handleRouteClick) + this.map.on('mouseenter', 'routes-hit', handlers.handleRouteHover) + this.map.on('mouseleave', 'routes-hit', handlers.handleRouteMouseLeave) + // Cursor change on hover this.map.on('mouseenter', 'points', () => { this.map.getCanvas().style.cursor = 'pointer' @@ -94,6 +106,13 @@ export class LayerManager { this.map.on('mouseleave', 'places', () => { this.map.getCanvas().style.cursor = '' }) + // Route cursor handlers - use routes-hit layer + this.map.on('mouseenter', 'routes-hit', () => { + this.map.getCanvas().style.cursor = 'pointer' + }) + this.map.on('mouseleave', 'routes-hit', () => { + this.map.getCanvas().style.cursor = '' + }) // Areas hover handlers for all sub-layers const areaLayers = ['areas-fill', 'areas-outline', 'areas-labels'] areaLayers.forEach(layerId => { @@ -107,6 +126,16 @@ export class LayerManager { }) } }) + + // Map-level click to deselect routes + this.map.on('click', (e) => { + const routeFeatures = this.map.queryRenderedFeatures(e.point, { layers: ['routes-hit'] }) + if (routeFeatures.length === 0) { + handlers.clearRouteSelection() + } + }) + + this.eventHandlersSetup = true } /** @@ -132,6 +161,7 @@ export class LayerManager { */ clearLayerReferences() { this.layers = {} + this.eventHandlersSetup = false } // Private methods for individual layer management @@ -197,6 +227,32 @@ export class LayerManager { } } + _addRoutesHitLayer() { + // Add invisible hit target layer for routes after points layer + // This ensures route interactions work even when points are on top + if (!this.map.getLayer('routes-hit') && this.map.getSource('routes-source')) { + this.map.addLayer({ + id: 'routes-hit', + type: 'line', + source: 'routes-source', + layout: { + 'line-join': 'round', + 'line-cap': 'round' + }, + paint: { + 'line-color': 'transparent', + 'line-width': 20, // Much wider for easier clicking/hovering + 'line-opacity': 0 + } + }) + // Match visibility with routes layer + const routesLayer = this.layers.routesLayer + if (routesLayer && !routesLayer.visible) { + this.map.setLayoutProperty('routes-hit', 'visibility', 'none') + } + } + } + _addVisitsLayer(visitsGeoJSON) { if (!this.layers.visitsLayer) { this.layers.visitsLayer = new VisitsLayer(this.map, { diff --git a/app/javascript/controllers/maps/maplibre/map_data_manager.js b/app/javascript/controllers/maps/maplibre/map_data_manager.js index 88d13462..507f152b 100644 --- a/app/javascript/controllers/maps/maplibre/map_data_manager.js +++ b/app/javascript/controllers/maps/maplibre/map_data_manager.js @@ -90,22 +90,31 @@ export class MapDataManager { data.placesGeoJSON ) + // Setup event handlers after layers are added this.layerManager.setupLayerEventHandlers({ handlePointClick: this.eventHandlers.handlePointClick.bind(this.eventHandlers), handleVisitClick: this.eventHandlers.handleVisitClick.bind(this.eventHandlers), handlePhotoClick: this.eventHandlers.handlePhotoClick.bind(this.eventHandlers), handlePlaceClick: this.eventHandlers.handlePlaceClick.bind(this.eventHandlers), - handleAreaClick: this.eventHandlers.handleAreaClick.bind(this.eventHandlers) + handleAreaClick: this.eventHandlers.handleAreaClick.bind(this.eventHandlers), + handleRouteClick: this.eventHandlers.handleRouteClick.bind(this.eventHandlers), + handleRouteHover: this.eventHandlers.handleRouteHover.bind(this.eventHandlers), + handleRouteMouseLeave: this.eventHandlers.handleRouteMouseLeave.bind(this.eventHandlers), + clearRouteSelection: this.eventHandlers.clearRouteSelection.bind(this.eventHandlers) }) } - if (this.map.loaded()) { - await addAllLayers() - } else { - this.map.once('load', async () => { - await addAllLayers() - }) - } + // Always use Promise-based approach for consistent timing + await new Promise((resolve) => { + if (this.map.loaded()) { + addAllLayers().then(resolve) + } else { + this.map.once('load', async () => { + await addAllLayers() + resolve() + }) + } + }) } /** diff --git a/app/javascript/controllers/maps/maplibre_controller.js b/app/javascript/controllers/maps/maplibre_controller.js index cb19ca30..4924a07c 100644 --- a/app/javascript/controllers/maps/maplibre_controller.js +++ b/app/javascript/controllers/maps/maplibre_controller.js @@ -79,7 +79,16 @@ export default class extends Controller { 'infoDisplay', 'infoTitle', 'infoContent', - 'infoActions' + 'infoActions', + // Route info template + 'routeInfoTemplate', + 'routeStartTime', + 'routeEndTime', + 'routeDuration', + 'routeDistance', + 'routeSpeed', + 'routeSpeedContainer', + 'routePoints' ] async connect() { @@ -467,9 +476,46 @@ export default class extends Controller { this.switchToToolsTab() } + showRouteInfo(routeData) { + if (!this.hasRouteInfoTemplateTarget) return + + // Clone the template + const template = this.routeInfoTemplateTarget.content.cloneNode(true) + + // Populate the template with data + const fragment = document.createDocumentFragment() + fragment.appendChild(template) + + fragment.querySelector('[data-maps--maplibre-target="routeStartTime"]').textContent = routeData.startTime + fragment.querySelector('[data-maps--maplibre-target="routeEndTime"]').textContent = routeData.endTime + fragment.querySelector('[data-maps--maplibre-target="routeDuration"]').textContent = routeData.duration + fragment.querySelector('[data-maps--maplibre-target="routeDistance"]').textContent = routeData.distance + fragment.querySelector('[data-maps--maplibre-target="routePoints"]').textContent = routeData.pointCount + + // Handle optional speed field + const speedContainer = fragment.querySelector('[data-maps--maplibre-target="routeSpeedContainer"]') + if (routeData.speed) { + fragment.querySelector('[data-maps--maplibre-target="routeSpeed"]').textContent = routeData.speed + speedContainer.style.display = '' + } else { + speedContainer.style.display = 'none' + } + + // Convert fragment to HTML string for showInfo + const div = document.createElement('div') + div.appendChild(fragment) + + this.showInfo('Route Information', div.innerHTML) + } + closeInfo() { if (!this.hasInfoDisplayTarget) return this.infoDisplayTarget.classList.add('hidden') + + // Clear route selection when info panel is closed + if (this.eventHandlers) { + this.eventHandlers.clearRouteSelection() + } } /** diff --git a/app/javascript/maps/marker_factory.js b/app/javascript/maps/marker_factory.js index b4c257d5..8b7338b5 100644 --- a/app/javascript/maps/marker_factory.js +++ b/app/javascript/maps/marker_factory.js @@ -28,7 +28,7 @@ const MARKER_DATA_INDICES = { * @param {number} size - Icon size in pixels (default: 8) * @returns {L.DivIcon} Leaflet divIcon instance */ -export function createStandardIcon(color = 'blue', size = 8) { +export function createStandardIcon(color = 'blue', size = 4) { return L.divIcon({ className: 'custom-div-icon', html: `
`, diff --git a/app/javascript/maps_maplibre/layers/routes_layer.js b/app/javascript/maps_maplibre/layers/routes_layer.js index 0539114e..5c241e0e 100644 --- a/app/javascript/maps_maplibre/layers/routes_layer.js +++ b/app/javascript/maps_maplibre/layers/routes_layer.js @@ -8,6 +8,7 @@ export class RoutesLayer extends BaseLayer { constructor(map, options = {}) { super(map, { id: 'routes', ...options }) this.maxGapHours = options.maxGapHours || 5 // Max hours between points to connect + this.hoverSourceId = 'routes-hover-source' } getSourceConfig() { @@ -20,6 +21,36 @@ export class RoutesLayer extends BaseLayer { } } + /** + * Override add() to create both main and hover sources + */ + add(data) { + this.data = data + + // Add main source + if (!this.map.getSource(this.sourceId)) { + this.map.addSource(this.sourceId, this.getSourceConfig()) + } + + // Add hover source (initially empty) + if (!this.map.getSource(this.hoverSourceId)) { + this.map.addSource(this.hoverSourceId, { + type: 'geojson', + data: { type: 'FeatureCollection', features: [] } + }) + } + + // Add layers + const layers = this.getLayerConfigs() + layers.forEach(layerConfig => { + if (!this.map.getLayer(layerConfig.id)) { + this.map.addLayer(layerConfig) + } + }) + + this.setVisibility(this.visible) + } + getLayerConfigs() { return [ { @@ -41,10 +72,93 @@ export class RoutesLayer extends BaseLayer { 'line-width': 3, 'line-opacity': 0.8 } + }, + { + id: 'routes-hover', + type: 'line', + source: this.hoverSourceId, + layout: { + 'line-join': 'round', + 'line-cap': 'round' + }, + paint: { + 'line-color': '#ffff00', // Yellow highlight + 'line-width': 8, + 'line-opacity': 1.0 + } } + // Note: routes-hit layer is added separately in LayerManager after points layer + // for better interactivity (see _addRoutesHitLayer method) ] } + /** + * Override setVisibility to also control routes-hit layer + * @param {boolean} visible - Show/hide layer + */ + setVisibility(visible) { + // Call parent to handle main routes and routes-hover layers + super.setVisibility(visible) + + // Also control routes-hit layer if it exists + if (this.map.getLayer('routes-hit')) { + const visibility = visible ? 'visible' : 'none' + this.map.setLayoutProperty('routes-hit', 'visibility', visibility) + } + } + + /** + * Update hover layer with route geometry + * @param {Object|null} feature - Route feature, FeatureCollection, or null to clear + */ + setHoverRoute(feature) { + const hoverSource = this.map.getSource(this.hoverSourceId) + if (!hoverSource) return + + if (feature) { + // Handle both single feature and FeatureCollection + if (feature.type === 'FeatureCollection') { + hoverSource.setData(feature) + } else { + hoverSource.setData({ + type: 'FeatureCollection', + features: [feature] + }) + } + } else { + hoverSource.setData({ type: 'FeatureCollection', features: [] }) + } + } + + /** + * Override remove() to clean up hover source and hit layer + */ + remove() { + // Remove layers + this.getLayerIds().forEach(layerId => { + if (this.map.getLayer(layerId)) { + this.map.removeLayer(layerId) + } + }) + + // Remove routes-hit layer if it exists + if (this.map.getLayer('routes-hit')) { + this.map.removeLayer('routes-hit') + } + + // Remove main source + if (this.map.getSource(this.sourceId)) { + this.map.removeSource(this.sourceId) + } + + // Remove hover source + if (this.map.getSource(this.hoverSourceId)) { + this.map.removeSource(this.hoverSourceId) + } + + this.data = null + } + /** * Calculate haversine distance between two points in kilometers * @param {number} lat1 - First point latitude @@ -67,6 +181,7 @@ export class RoutesLayer extends BaseLayer { /** * Convert points to route LineStrings with splitting * Matches V1's route splitting logic for consistency + * Also handles International Date Line (IDL) crossings * @param {Array} points - Points from API * @param {Object} options - Splitting options * @returns {Object} GeoJSON FeatureCollection @@ -77,7 +192,9 @@ export class RoutesLayer extends BaseLayer { } // Default thresholds (matching V1 defaults from polylines.js) - const distanceThresholdKm = (options.distanceThresholdMeters || 500) / 1000 + // Note: V1 has a unit mismatch bug where it compares km to meters directly + // We replicate this behavior for consistency with V1 + const distanceThresholdKm = options.distanceThresholdMeters || 500 const timeThresholdMinutes = options.timeThresholdMinutes || 60 // Sort by timestamp @@ -100,7 +217,7 @@ export class RoutesLayer extends BaseLayer { // Calculate time difference in minutes const timeDiff = (curr.timestamp - prev.timestamp) / 60 - // Split if either threshold is exceeded (matching V1 logic) + // Split if any threshold is exceeded if (distance > distanceThresholdKm || timeDiff > timeThresholdMinutes) { if (currentSegment.length > 1) { segments.push(currentSegment) @@ -117,7 +234,36 @@ export class RoutesLayer extends BaseLayer { // Convert segments to LineStrings const features = segments.map(segment => { - const coordinates = segment.map(p => [p.longitude, p.latitude]) + // Unwrap coordinates to handle International Date Line (IDL) crossings + // This ensures routes draw the short way across IDL instead of wrapping around globe + const coordinates = [] + let offset = 0 // Cumulative longitude offset for unwrapping + + for (let i = 0; i < segment.length; i++) { + const point = segment[i] + let lon = point.longitude + offset + + // Check for IDL crossing between consecutive points + if (i > 0) { + const prevLon = coordinates[i - 1][0] + const lonDiff = lon - prevLon + + // If longitude jumps more than 180°, we crossed the IDL + if (lonDiff > 180) { + // Crossed from east to west (e.g., 170° to -170°) + // Subtract 360° to make it continuous (e.g., 170° to -170° becomes 170° to -170°-360° = -530°) + offset -= 360 + lon -= 360 + } else if (lonDiff < -180) { + // Crossed from west to east (e.g., -170° to 170°) + // Add 360° to make it continuous (e.g., -170° to 170° becomes -170° to 170°+360° = 530°) + offset += 360 + lon += 360 + } + } + + coordinates.push([lon, point.latitude]) + } // Calculate total distance for the segment let totalDistance = 0 diff --git a/app/javascript/maps_maplibre/services/api_client.js b/app/javascript/maps_maplibre/services/api_client.js index 1ef9e871..33b49f82 100644 --- a/app/javascript/maps_maplibre/services/api_client.js +++ b/app/javascript/maps_maplibre/services/api_client.js @@ -19,7 +19,8 @@ export class ApiClient { end_at, page: page.toString(), per_page: per_page.toString(), - slim: 'true' + slim: 'true', + order: 'asc' }) const response = await fetch(`${this.baseURL}/points?${params}`, { @@ -40,43 +41,83 @@ export class ApiClient { } /** - * Fetch all points for date range (handles pagination) - * @param {Object} options - { start_at, end_at, onProgress } + * Fetch all points for date range (handles pagination with parallel requests) + * @param {Object} options - { start_at, end_at, onProgress, maxConcurrent } * @returns {Promise} All points */ - async fetchAllPoints({ start_at, end_at, onProgress = null }) { - const allPoints = [] - let page = 1 - let totalPages = 1 - - do { - const { points, currentPage, totalPages: total } = - await this.fetchPoints({ start_at, end_at, page, per_page: 1000 }) - - allPoints.push(...points) - totalPages = total - page++ + async fetchAllPoints({ start_at, end_at, onProgress = null, maxConcurrent = 3 }) { + // First fetch to get total pages + const firstPage = await this.fetchPoints({ start_at, end_at, page: 1, per_page: 1000 }) + const totalPages = firstPage.totalPages + // If only one page, return immediately + if (totalPages === 1) { if (onProgress) { - // Avoid division by zero - if no pages, progress is 100% - const progress = totalPages > 0 ? currentPage / totalPages : 1.0 onProgress({ - loaded: allPoints.length, - currentPage, + loaded: firstPage.points.length, + currentPage: 1, + totalPages: 1, + progress: 1.0 + }) + } + return firstPage.points + } + + // Initialize results array with first page + const pageResults = [{ page: 1, points: firstPage.points }] + let completedPages = 1 + + // Create array of remaining page numbers + const remainingPages = Array.from( + { length: totalPages - 1 }, + (_, i) => i + 2 + ) + + // Process pages in batches of maxConcurrent + for (let i = 0; i < remainingPages.length; i += maxConcurrent) { + const batch = remainingPages.slice(i, i + maxConcurrent) + + // Fetch batch in parallel + const batchPromises = batch.map(page => + this.fetchPoints({ start_at, end_at, page, per_page: 1000 }) + .then(result => ({ page, points: result.points })) + ) + + const batchResults = await Promise.all(batchPromises) + pageResults.push(...batchResults) + completedPages += batchResults.length + + // Call progress callback after each batch + if (onProgress) { + const progress = totalPages > 0 ? completedPages / totalPages : 1.0 + onProgress({ + loaded: pageResults.reduce((sum, r) => sum + r.points.length, 0), + currentPage: completedPages, totalPages, progress }) } - } while (page <= totalPages) + } - return allPoints + // Sort by page number to ensure correct order + pageResults.sort((a, b) => a.page - b.page) + + // Flatten into single array + return pageResults.flatMap(r => r.points) } /** - * Fetch visits for date range + * Fetch visits for date range (paginated) + * @param {Object} options - { start_at, end_at, page, per_page } + * @returns {Promise} { visits, currentPage, totalPages } */ - async fetchVisits({ start_at, end_at }) { - const params = new URLSearchParams({ start_at, end_at }) + async fetchVisitsPage({ start_at, end_at, page = 1, per_page = 500 }) { + const params = new URLSearchParams({ + start_at, + end_at, + page: page.toString(), + per_page: per_page.toString() + }) const response = await fetch(`${this.baseURL}/visits?${params}`, { headers: this.getHeaders() @@ -86,20 +127,63 @@ export class ApiClient { throw new Error(`Failed to fetch visits: ${response.statusText}`) } - return response.json() + const visits = await response.json() + + return { + visits, + currentPage: parseInt(response.headers.get('X-Current-Page') || '1'), + totalPages: parseInt(response.headers.get('X-Total-Pages') || '1') + } } /** - * Fetch places optionally filtered by tags + * Fetch all visits for date range (handles pagination) + * @param {Object} options - { start_at, end_at, onProgress } + * @returns {Promise} All visits */ - async fetchPlaces({ tag_ids = [] } = {}) { - const params = new URLSearchParams() + async fetchVisits({ start_at, end_at, onProgress = null }) { + const allVisits = [] + let page = 1 + let totalPages = 1 + + do { + const { visits, currentPage, totalPages: total } = + await this.fetchVisitsPage({ start_at, end_at, page, per_page: 500 }) + + allVisits.push(...visits) + totalPages = total + page++ + + if (onProgress) { + const progress = totalPages > 0 ? currentPage / totalPages : 1.0 + onProgress({ + loaded: allVisits.length, + currentPage, + totalPages, + progress + }) + } + } while (page <= totalPages) + + return allVisits + } + + /** + * Fetch places (paginated) + * @param {Object} options - { tag_ids, page, per_page } + * @returns {Promise} { places, currentPage, totalPages } + */ + async fetchPlacesPage({ tag_ids = [], page = 1, per_page = 500 } = {}) { + const params = new URLSearchParams({ + page: page.toString(), + per_page: per_page.toString() + }) if (tag_ids && tag_ids.length > 0) { tag_ids.forEach(id => params.append('tag_ids[]', id)) } - const url = `${this.baseURL}/places${params.toString() ? '?' + params.toString() : ''}` + const url = `${this.baseURL}/places?${params.toString()}` const response = await fetch(url, { headers: this.getHeaders() @@ -109,7 +193,45 @@ export class ApiClient { throw new Error(`Failed to fetch places: ${response.statusText}`) } - return response.json() + const places = await response.json() + + return { + places, + currentPage: parseInt(response.headers.get('X-Current-Page') || '1'), + totalPages: parseInt(response.headers.get('X-Total-Pages') || '1') + } + } + + /** + * Fetch all places optionally filtered by tags (handles pagination) + * @param {Object} options - { tag_ids, onProgress } + * @returns {Promise} All places + */ + async fetchPlaces({ tag_ids = [], onProgress = null } = {}) { + const allPlaces = [] + let page = 1 + let totalPages = 1 + + do { + const { places, currentPage, totalPages: total } = + await this.fetchPlacesPage({ tag_ids, page, per_page: 500 }) + + allPlaces.push(...places) + totalPages = total + page++ + + if (onProgress) { + const progress = totalPages > 0 ? currentPage / totalPages : 1.0 + onProgress({ + loaded: allPlaces.length, + currentPage, + totalPages, + progress + }) + } + } while (page <= totalPages) + + return allPlaces } /** diff --git a/app/javascript/maps_maplibre/utils/settings_manager.js b/app/javascript/maps_maplibre/utils/settings_manager.js index aa12d3e8..0bf3a89a 100644 --- a/app/javascript/maps_maplibre/utils/settings_manager.js +++ b/app/javascript/maps_maplibre/utils/settings_manager.js @@ -10,7 +10,7 @@ const DEFAULT_SETTINGS = { routeOpacity: 0.6, fogOfWarRadius: 100, fogOfWarThreshold: 1, - metersBetweenRoutes: 1000, + metersBetweenRoutes: 500, minutesBetweenRoutes: 60, pointsRenderingMode: 'raw', speedColoredRoutes: false, diff --git a/app/jobs/cache/preheating_job.rb b/app/jobs/cache/preheating_job.rb index c8002fdf..90a8013f 100644 --- a/app/jobs/cache/preheating_job.rb +++ b/app/jobs/cache/preheating_job.rb @@ -28,6 +28,14 @@ class Cache::PreheatingJob < ApplicationJob user.cities_visited_uncached, expires_in: 1.day ) + + # Preheat total_distance cache + total_distance_meters = user.stats.sum(:distance) + Rails.cache.write( + "dawarich/user_#{user.id}_total_distance", + Stat.convert_distance(total_distance_meters, user.safe_settings.distance_unit), + expires_in: 1.day + ) end end end diff --git a/app/jobs/users/mailer_sending_job.rb b/app/jobs/users/mailer_sending_job.rb index 742db9eb..d8de83bf 100644 --- a/app/jobs/users/mailer_sending_job.rb +++ b/app/jobs/users/mailer_sending_job.rb @@ -6,14 +6,7 @@ class Users::MailerSendingJob < ApplicationJob def perform(user_id, email_type, **options) user = User.find(user_id) - if should_skip_email?(user, email_type) - ExceptionReporter.call( - 'Users::MailerSendingJob', - "Skipping #{email_type} email for user ID #{user_id} - #{skip_reason(user, email_type)}" - ) - - return - end + return if should_skip_email?(user, email_type) params = { user: user }.merge(options) @@ -37,15 +30,4 @@ class Users::MailerSendingJob < ApplicationJob false end end - - def skip_reason(user, email_type) - case email_type.to_s - when 'trial_expires_soon', 'trial_expired' - 'user is already subscribed' - when 'post_trial_reminder_early', 'post_trial_reminder_late' - user.active? ? 'user is subscribed' : 'user is not in trial state' - else - 'unknown reason' - end - end end diff --git a/app/models/user.rb b/app/models/user.rb index 4b35390e..4279d494 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -56,8 +56,10 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength end def total_distance - total_distance_meters = stats.sum(:distance) - Stat.convert_distance(total_distance_meters, safe_settings.distance_unit) + Rails.cache.fetch("dawarich/user_#{id}_total_distance", expires_in: 1.day) do + total_distance_meters = stats.sum(:distance) + Stat.convert_distance(total_distance_meters, safe_settings.distance_unit) + end end def total_countries diff --git a/app/services/cache/clean.rb b/app/services/cache/clean.rb index e555e6a4..af8354b7 100644 --- a/app/services/cache/clean.rb +++ b/app/services/cache/clean.rb @@ -9,6 +9,7 @@ class Cache::Clean delete_years_tracked_cache delete_points_geocoded_stats_cache delete_countries_cities_cache + delete_total_distance_cache Rails.logger.info('Cache cleaned') end @@ -40,5 +41,11 @@ class Cache::Clean Rails.cache.delete("dawarich/user_#{user.id}_cities_visited") end end + + def delete_total_distance_cache + User.find_each do |user| + Rails.cache.delete("dawarich/user_#{user.id}_total_distance") + end + end end end diff --git a/app/services/cache/invalidate_user_caches.rb b/app/services/cache/invalidate_user_caches.rb index 839efdae..fdcd3642 100644 --- a/app/services/cache/invalidate_user_caches.rb +++ b/app/services/cache/invalidate_user_caches.rb @@ -14,6 +14,7 @@ class Cache::InvalidateUserCaches invalidate_countries_visited invalidate_cities_visited invalidate_points_geocoded_stats + invalidate_total_distance end def invalidate_countries_visited @@ -28,6 +29,10 @@ class Cache::InvalidateUserCaches Rails.cache.delete("dawarich/user_#{user_id}_points_geocoded_stats") end + def invalidate_total_distance + Rails.cache.delete("dawarich/user_#{user_id}_total_distance") + end + private attr_reader :user_id diff --git a/app/services/stats/calculate_month.rb b/app/services/stats/calculate_month.rb index 0a87b803..d5cda5f2 100644 --- a/app/services/stats/calculate_month.rb +++ b/app/services/stats/calculate_month.rb @@ -50,11 +50,13 @@ class Stats::CalculateMonth def points return @points if defined?(@points) + # Select all needed columns to avoid duplicate queries + # Used for both distance calculation and toponyms extraction @points = user .points .without_raw_data .where(timestamp: start_timestamp..end_timestamp) - .select(:lonlat, :timestamp) + .select(:lonlat, :timestamp, :city, :country_name) .order(timestamp: :asc) end @@ -63,14 +65,8 @@ class Stats::CalculateMonth end def toponyms - toponym_points = - user - .points - .without_raw_data - .where(timestamp: start_timestamp..end_timestamp) - .select(:city, :country_name, :timestamp) - - CountriesAndCities.new(toponym_points).call + # Reuse already-loaded points instead of making a duplicate query + CountriesAndCities.new(points).call end def create_stats_update_failed_notification(user, error) diff --git a/app/services/visits/find_in_time.rb b/app/services/visits/find_in_time.rb index e486382a..84e381c3 100644 --- a/app/services/visits/find_in_time.rb +++ b/app/services/visits/find_in_time.rb @@ -10,7 +10,7 @@ module Visits def call Visit - .includes(:place) + .includes(:place, :area) .where(user:) .where('started_at >= ? AND ended_at <= ?', start_at, end_at) .order(started_at: :desc) diff --git a/app/services/visits/find_within_bounding_box.rb b/app/services/visits/find_within_bounding_box.rb index d5bdb74a..b73913df 100644 --- a/app/services/visits/find_within_bounding_box.rb +++ b/app/services/visits/find_within_bounding_box.rb @@ -13,7 +13,7 @@ module Visits def call Visit - .includes(:place) + .includes(:place, :area) .where(user:) .joins(:place) .where( diff --git a/app/views/map/maplibre/_settings_panel.html.erb b/app/views/map/maplibre/_settings_panel.html.erb index 143718f5..0a5cc062 100644 --- a/app/views/map/maplibre/_settings_panel.html.erb +++ b/app/views/map/maplibre/_settings_panel.html.erb @@ -620,6 +620,36 @@ + + +