diff --git a/CHANGELOG.md b/CHANGELOG.md index e8c4384f..531dcc70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). TODO: - Selecting a visit should put it above other visits on the map to make it easier to edit it. If many visits are on the same place, we should be able to click on them +- Do we need to reverse geocode places if we already got their data with address during visit suggestion? # 0.24.2 - 2025-02-24 diff --git a/app/assets/stylesheets/application.tailwind.css b/app/assets/stylesheets/application.tailwind.css index b3ac00d2..257a1910 100644 --- a/app/assets/stylesheets/application.tailwind.css +++ b/app/assets/stylesheets/application.tailwind.css @@ -75,3 +75,31 @@ .controls-shifted { right: 338px !important; } + +/* Selection Tool Styles */ +.leaflet-control-custom { + background-color: white; + border-radius: 4px; + width: 30px; + height: 30px; + display: flex; + align-items: center; + justify-content: center; + cursor: pointer; + box-shadow: 0 1px 4px rgba(0, 0, 0, 0.3); +} + +.leaflet-control-custom:hover { + background-color: #f3f4f6; +} + +#selection-tool-button.active { + background-color: #60a5fa; + color: white; +} + +/* Cancel Selection Button */ +#cancel-selection-button { + margin-bottom: 1rem; + width: 100%; +} diff --git a/app/controllers/api/v1/visits_controller.rb b/app/controllers/api/v1/visits_controller.rb index c58afe0c..4269230e 100644 --- a/app/controllers/api/v1/visits_controller.rb +++ b/app/controllers/api/v1/visits_controller.rb @@ -2,23 +2,43 @@ class Api::V1::VisitsController < ApiController def index - start_time = begin - Time.zone.parse(params[:start_at]) - rescue StandardError - Time.zone.now.beginning_of_day - end - end_time = begin - Time.zone.parse(params[:end_at]) - rescue StandardError - Time.zone.now.end_of_day - end + # If selection is true, filter by coordinates instead of time + if params[:selection] == 'true' && params[:sw_lat].present? && params[:sw_lng].present? && params[:ne_lat].present? && params[:ne_lng].present? + sw_lat = params[:sw_lat].to_f + sw_lng = params[:sw_lng].to_f + ne_lat = params[:ne_lat].to_f + ne_lng = params[:ne_lng].to_f - visits = - Visit - .includes(:place) - .where(user: current_api_user) - .where('started_at >= ? AND ended_at <= ?', start_time, end_time) - .order(started_at: :desc) + # Create the PostGIS bounding box polygon + bounding_box = "ST_MakeEnvelope(#{sw_lng}, #{sw_lat}, #{ne_lng}, #{ne_lat}, 4326)" + + visits = + Visit + .includes(:place) + .where(user: current_api_user) + .joins(:place) + .where("ST_Contains(#{bounding_box}, ST_SetSRID(places.lonlat::geometry, 4326))") + .order(started_at: :desc) + else + # Regular time-based filtering + start_time = begin + Time.zone.parse(params[:start_at]) + rescue StandardError + Time.zone.now.beginning_of_day + end + end_time = begin + Time.zone.parse(params[:end_at]) + rescue StandardError + Time.zone.now.end_of_day + end + + visits = + Visit + .includes(:place) + .where(user: current_api_user) + .where('started_at >= ? AND ended_at <= ?', start_time, end_time) + .order(started_at: :desc) + end serialized_visits = visits.map do |visit| Api::VisitSerializer.new(visit).call diff --git a/app/javascript/maps/visits.js b/app/javascript/maps/visits.js index ee4a7680..4a9389df 100644 --- a/app/javascript/maps/visits.js +++ b/app/javascript/maps/visits.js @@ -12,6 +12,9 @@ export class VisitsManager { this.confirmedVisitCircles = L.layerGroup().addTo(map); // Always visible layer for confirmed visits this.currentPopup = null; this.drawerOpen = false; + this.selectionMode = false; + this.selectionRect = null; + this.isSelectionActive = false; } /** @@ -62,6 +65,187 @@ export class VisitsManager { }); this.map.addControl(new DrawerControl({ position: 'topright' })); + + // Add the selection tool button + this.addSelectionButton(); + } + + /** + * Adds a button to enable/disable the area selection tool + */ + addSelectionButton() { + const SelectionControl = L.Control.extend({ + onAdd: (map) => { + const button = L.DomUtil.create('button', 'leaflet-bar leaflet-control leaflet-control-custom'); + button.innerHTML = ''; + button.title = 'Select Area'; + button.id = 'selection-tool-button'; + button.onclick = () => this.toggleSelectionMode(); + return button; + } + }); + + new SelectionControl({ position: 'topright' }).addTo(this.map); + } + + /** + * Toggles the area selection mode + */ + toggleSelectionMode() { + if (this.selectionMode) { + // Disable selection mode + this.selectionMode = false; + this.map.dragging.enable(); + document.getElementById('selection-tool-button').classList.remove('active'); + this.map.off('mousedown', this.onMouseDown, this); + } else { + // Enable selection mode + this.selectionMode = true; + document.getElementById('selection-tool-button').classList.add('active'); + this.map.dragging.disable(); + this.map.on('mousedown', this.onMouseDown, this); + + showFlashMessage('info', 'Selection mode enabled. Click and drag to select an area.'); + } + } + + /** + * Handles the mousedown event to start the selection + */ + onMouseDown(e) { + // Clear any existing selection + this.clearSelection(); + + // Store start point and create rectangle + this.startPoint = e.latlng; + + // Add mousemove and mouseup listeners + this.map.on('mousemove', this.onMouseMove, this); + this.map.on('mouseup', this.onMouseUp, this); + } + + /** + * Handles the mousemove event to update the selection rectangle + */ + onMouseMove(e) { + if (!this.startPoint) return; + + // If we already have a rectangle, update its bounds + if (this.selectionRect) { + const bounds = L.latLngBounds(this.startPoint, e.latlng); + this.selectionRect.setBounds(bounds); + } else { + // Create a new rectangle + this.selectionRect = L.rectangle( + L.latLngBounds(this.startPoint, e.latlng), + { color: '#3388ff', weight: 2, fillOpacity: 0.1 } + ).addTo(this.map); + } + } + + /** + * Handles the mouseup event to complete the selection + */ + onMouseUp(e) { + // Remove the mouse event listeners + this.map.off('mousemove', this.onMouseMove, this); + this.map.off('mouseup', this.onMouseUp, this); + + if (!this.selectionRect) return; + + // Finalize the selection + this.isSelectionActive = true; + + // Re-enable map dragging + this.map.dragging.enable(); + + // Disable selection mode + this.selectionMode = false; + document.getElementById('selection-tool-button').classList.remove('active'); + this.map.off('mousedown', this.onMouseDown, this); + + // Fetch visits within the selection + this.fetchVisitsInSelection(); + } + + /** + * Clears the current area selection + */ + clearSelection() { + if (this.selectionRect) { + this.map.removeLayer(this.selectionRect); + this.selectionRect = null; + } + this.isSelectionActive = false; + this.startPoint = null; + + // If the drawer is open, refresh with time-based visits + if (this.drawerOpen) { + this.fetchAndDisplayVisits(); + } + } + + /** + * Fetches visits within the selected area + */ + async fetchVisitsInSelection() { + if (!this.selectionRect) return; + + const bounds = this.selectionRect.getBounds(); + const sw = bounds.getSouthWest(); + const ne = bounds.getNorthEast(); + + try { + const response = await fetch( + `/api/v1/visits?selection=true&sw_lat=${sw.lat}&sw_lng=${sw.lng}&ne_lat=${ne.lat}&ne_lng=${ne.lng}`, + { + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${this.apiKey}`, + } + } + ); + + if (!response.ok) { + throw new Error('Network response was not ok'); + } + + const visits = await response.json(); + this.displayVisits(visits); + + // Make sure the drawer is open + if (!this.drawerOpen) { + this.toggleDrawer(); + } + + // Add cancel selection button to the drawer + this.addSelectionCancelButton(); + + } catch (error) { + console.error('Error fetching visits in selection:', error); + showFlashMessage('error', 'Failed to load visits in selected area'); + } + } + + /** + * Adds a cancel button to the drawer to clear the selection + */ + addSelectionCancelButton() { + const container = document.getElementById('visits-list'); + if (!container) return; + + // Add cancel button at the top of the drawer if it doesn't exist + if (!document.getElementById('cancel-selection-button')) { + const cancelButton = document.createElement('button'); + cancelButton.id = 'cancel-selection-button'; + cancelButton.className = 'btn btn-sm btn-warning mb-4 w-full'; + cancelButton.textContent = 'Cancel Area Selection'; + cancelButton.onclick = () => this.clearSelection(); + + // Insert at the beginning of the container + container.insertBefore(cancelButton, container.firstChild); + } } /** @@ -138,6 +322,12 @@ export class VisitsManager { */ async fetchAndDisplayVisits() { try { + // If there's an active selection, don't perform time-based fetch + if (this.isSelectionActive && this.selectionRect) { + this.fetchVisitsInSelection(); + return; + } + // Get current timeframe from URL parameters const urlParams = new URLSearchParams(window.location.search); const startAt = urlParams.get('start_at') || new Date().toISOString(); @@ -753,6 +943,7 @@ export class VisitsManager { closeButton: true, closeOnClick: true, autoClose: true, + closeOnEscapeKey: true, maxWidth: 450, // Set maximum width minWidth: 300 // Set minimum width }) diff --git a/app/jobs/bulk_visits_suggesting_job.rb b/app/jobs/bulk_visits_suggesting_job.rb new file mode 100644 index 00000000..0006239a --- /dev/null +++ b/app/jobs/bulk_visits_suggesting_job.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +class BulkVisitsSuggestingJob < ApplicationJob + queue_as :default + sidekiq_options retry: false + + # Passing timespan of more than 3 years somehow results in duplicated Places + def perform(start_at:, end_at:, user_ids: []) + users = user_ids.any? ? User.where(id: user_ids) : User.all + start_at = start_at.to_datetime + end_at = end_at.to_datetime + + time_chunks = time_chunks(start_at:, end_at:) + + users.active.find_each do |user| + next if user.tracked_points.empty? + + time_chunks.each do |time_chunk| + VisitSuggestingJob.perform_later( + user_id: user.id, start_at: time_chunk.first, end_at: time_chunk.last + ) + end + end + end + + private + + def time_chunks(start_at:, end_at:) + time_chunks = [] + + # First chunk: from start_at to end of that year + first_end = start_at.end_of_year + time_chunks << (start_at...first_end) if start_at < first_end + + # Full-year chunks + current = first_end.beginning_of_year + 1.year # Start from the next full year + while current + 1.year <= end_at.beginning_of_year + time_chunks << (current...current + 1.year) + current += 1.year + end + + # Last chunk: from start of the last year to end_at + time_chunks << (current...end_at) if current < end_at + + time_chunks + end +end diff --git a/app/jobs/visit_suggesting_job.rb b/app/jobs/visit_suggesting_job.rb index f8346d25..74bc7930 100644 --- a/app/jobs/visit_suggesting_job.rb +++ b/app/jobs/visit_suggesting_job.rb @@ -4,22 +4,14 @@ class VisitSuggestingJob < ApplicationJob queue_as :visit_suggesting sidekiq_options retry: false - def perform(user_ids: [], start_at: 1.day.ago, end_at: Time.current) - users = user_ids.any? ? User.where(id: user_ids) : User.all - start_at = start_at.to_datetime - end_at = end_at.to_datetime + # Passing timespan of more than 3 years somehow results in duplicated Places + def perform(user_id:, start_at:, end_at:) + user = User.find(user_id) - users.find_each do |user| - next unless user.active? - next if user.tracked_points.empty? + time_chunks = (start_at..end_at).step(1.day).to_a - # Split the time range into 24-hour chunks - # This prevents from places duplicates - time_chunks = (start_at..end_at).step(1.day).to_a - - time_chunks.each do |time_chunk| - Visits::Suggest.new(user, start_at: time_chunk, end_at: time_chunk + 1.day).call - end + time_chunks.each do |time_chunk| + Visits::Suggest.new(user, start_at: time_chunk, end_at: time_chunk + 1.day).call end end end diff --git a/app/models/visit.rb b/app/models/visit.rb index 9ed895f3..2ca3faf8 100644 --- a/app/models/visit.rb +++ b/app/models/visit.rb @@ -37,9 +37,9 @@ class Visit < ApplicationRecord def center if area.present? - area.to_coordinates + [area.lat, area.lon] elsif place.present? - place.to_coordinates + [place.lat, place.lon] else center_from_points end diff --git a/app/services/reverse_geocoding/places/fetch_data.rb b/app/services/reverse_geocoding/places/fetch_data.rb index 5ed94afd..c2fb29bd 100644 --- a/app/services/reverse_geocoding/places/fetch_data.rb +++ b/app/services/reverse_geocoding/places/fetch_data.rb @@ -19,7 +19,6 @@ class ReverseGeocoding::Places::FetchData first_place = reverse_geocoded_places.shift update_place(first_place) - add_suggested_place_to_a_visit reverse_geocoded_places.each { |reverse_geocoded_place| fetch_and_create_place(reverse_geocoded_place) } end @@ -52,24 +51,12 @@ class ReverseGeocoding::Places::FetchData new_place.source = :photon new_place.save! - - add_suggested_place_to_a_visit(suggested_place: new_place) end def reverse_geocoded? place.geodata.present? end - def add_suggested_place_to_a_visit(suggested_place: place) - visits = Place.near([suggested_place.latitude, suggested_place.longitude], 0.1).flat_map(&:visits) - - visits.each do |visit| - next if visit.suggested_places.include?(suggested_place) - - visit.suggested_places << suggested_place - end - end - def find_place(place_data) found_place = Place.where( "geodata->'properties'->>'osm_id' = ?", place_data['properties']['osm_id'].to_s diff --git a/app/services/visits/creator.rb b/app/services/visits/creator.rb index 4472d5ca..c7e672d6 100644 --- a/app/services/visits/creator.rb +++ b/app/services/visits/creator.rb @@ -11,31 +11,65 @@ module Visits def create_visits(visits) visits.map do |visit_data| + # Variables to store data outside the transaction + visit_instance = nil + place_data = nil + + # First transaction to create the visit ActiveRecord::Base.transaction do # Try to find matching area or place area = find_matching_area(visit_data) - place = area ? nil : PlaceFinder.new(user).find_or_create_place(visit_data) - visit = Visit.create!( + # Only find/create place if no area was found + place_data = PlaceFinder.new(user).find_or_create_place(visit_data) unless area + + main_place = place_data&.dig(:main_place) + + visit_instance = Visit.create!( user: user, area: area, - place: place, + place: main_place, started_at: Time.zone.at(visit_data[:start_time]), ended_at: Time.zone.at(visit_data[:end_time]), duration: visit_data[:duration] / 60, # Convert to minutes - name: generate_visit_name(area, place, visit_data[:suggested_name]), + name: generate_visit_name(area, main_place, visit_data[:suggested_name]), status: :suggested ) - Point.where(id: visit_data[:points].map(&:id)).update_all(visit_id: visit.id) - - visit + Point.where(id: visit_data[:points].map(&:id)).update_all(visit_id: visit_instance.id) end + + # Associate suggested places outside the main transaction + # to avoid deadlocks when multiple processes run simultaneously + if place_data&.dig(:suggested_places).present? + associate_suggested_places(visit_instance, place_data[:suggested_places]) + end + + visit_instance end end private + # Create place_visits records directly to avoid deadlocks + def associate_suggested_places(visit, suggested_places) + existing_place_ids = visit.place_visits.pluck(:place_id) + + # Only create associations that don't already exist + place_ids_to_add = suggested_places.map(&:id) - existing_place_ids + + # Skip if there's nothing to add + return if place_ids_to_add.empty? + + # Batch create place_visit records + place_visits_attrs = place_ids_to_add.map do |place_id| + { visit_id: visit.id, place_id: place_id, created_at: Time.current, updated_at: Time.current } + end + + # Use insert_all for efficient bulk insertion without callbacks + PlaceVisit.insert_all(place_visits_attrs) if place_visits_attrs.any? + end + def find_matching_area(visit_data) user.areas.find do |area| near_area?([visit_data[:center_lat], visit_data[:center_lon]], area) @@ -45,7 +79,8 @@ module Visits def near_area?(center, area) distance = Geocoder::Calculations.distance_between( center, - [area.latitude, area.longitude] + [area.latitude, area.longitude], + units: :km ) distance * 1000 <= area.radius # Convert to meters end diff --git a/app/services/visits/detector.rb b/app/services/visits/detector.rb index 2cbd3368..9ea730cb 100644 --- a/app/services/visits/detector.rb +++ b/app/services/visits/detector.rb @@ -3,10 +3,9 @@ module Visits # Detects potential visits from a collection of tracked points class Detector - MINIMUM_VISIT_DURATION = 5.minutes + MINIMUM_VISIT_DURATION = 3.minutes MAXIMUM_VISIT_GAP = 30.minutes MINIMUM_POINTS_FOR_VISIT = 3 - SIGNIFICANT_MOVEMENT_THRESHOLD = 50 # meters attr_reader :points @@ -103,7 +102,7 @@ module Visits def calculate_visit_radius(points, center) max_distance = points.map do |point| - Geocoder::Calculations.distance_between(center, [point.lat, point.lon]) + Geocoder::Calculations.distance_between(center, [point.lat, point.lon], units: :km) end.max # Convert to meters and ensure minimum radius @@ -140,7 +139,7 @@ module Visits .transform_values(&:size) most_common_name = name_counts.max_by { |_, count| count }&.first - return unless most_common_name.present? + return if most_common_name.blank? # If we have a name, try to get additional context feature = common_features.find { |f| f.dig('properties', 'name') == most_common_name } diff --git a/app/services/visits/merger.rb b/app/services/visits/merger.rb index 06604487..fd2abec1 100644 --- a/app/services/visits/merger.rb +++ b/app/services/visits/merger.rb @@ -18,7 +18,7 @@ module Visits merged = [] current_merged = visits.first - visits[1..-1].each do |visit| + visits[1..].each do |visit| if can_merge_visits?(current_merged, visit) # Merge the visits current_merged[:end_time] = visit[:end_time] @@ -70,7 +70,8 @@ module Visits max_distance = between_points.map do |point| Geocoder::Calculations.distance_between( visit_center, - [point.lat, point.lon] + [point.lat, point.lon], + units: :km ) end.max diff --git a/app/services/visits/place_finder.rb b/app/services/visits/place_finder.rb index 2914ce11..99b0a621 100644 --- a/app/services/visits/place_finder.rb +++ b/app/services/visits/place_finder.rb @@ -5,6 +5,9 @@ module Visits class PlaceFinder attr_reader :user + SEARCH_RADIUS = 500 # meters + SIMILARITY_RADIUS = 50 # meters + def initialize(user) @user = user end @@ -12,119 +15,234 @@ module Visits def find_or_create_place(visit_data) lat = visit_data[:center_lat].round(5) lon = visit_data[:center_lon].round(5) - name = visit_data[:suggested_name] - # Define the search radius in meters - search_radius = 100 # Adjust this value as needed + # First check if there's an existing place + existing_place = find_existing_place(lat, lon, visit_data[:suggested_name]) - existing_place = Place.where(name: name) - .near([lon, lat], search_radius, :m) - .first - - return existing_place if existing_place - - # Use a database transaction with a lock to prevent race conditions - Place.transaction do - # Check again within transaction to prevent race conditions - existing_place = Place.where('ST_DWithin(lonlat, ST_SetSRID(ST_MakePoint(?, ?), 4326), 50)', lon, lat) - .lock(true) - .first - - return existing_place if existing_place - - create_new_place(lat, lon, visit_data[:suggested_name]) + # If we found an exact match, return it + if existing_place + return { + main_place: existing_place, + suggested_places: find_suggested_places(lat, lon, existing_place.id) + } end + + # Get potential places from all sources + potential_places = collect_potential_places(visit_data) + + # Find or create the main place + main_place = select_or_create_main_place(potential_places, lat, lon, visit_data[:suggested_name]) + + # Get suggested places including our main place + all_suggested_places = potential_places.presence || [main_place] + + { + main_place: main_place, + suggested_places: all_suggested_places.uniq { |place| place.name } + } end private - def create_new_place(lat, lon, suggested_name) - # If no existing place is found, create a new one - place = Place.new( - lonlat: "POINT(#{lon} #{lat})", - latitude: lat, - longitude: lon - ) + # Step 1: Find existing place + def find_existing_place(lat, lon, name) + # Try to find existing place by location first + existing_by_location = Place.near([lat, lon], SIMILARITY_RADIUS, :m).first + return existing_by_location if existing_by_location - # Get reverse geocoding data - geocoded_data = Geocoder.search([lat, lon]) + # Then try by name if available + return nil unless name.present? - if geocoded_data.present? - first_result = geocoded_data.first - data = first_result.data.with_indifferent_access - properties = data['properties'] || {} + Place.where(name: name) + .near([lat, lon], SEARCH_RADIUS, :m) + .first + end - # Build a descriptive name from available components - name_components = [ - properties['name'], - properties['street'], - properties['housenumber'], - properties['postcode'], - properties['city'] - ].compact.uniq + # Step 2: Collect potential places from all sources + def collect_potential_places(visit_data) + lat = visit_data[:center_lat].round(5) + lon = visit_data[:center_lon].round(5) - place.name = name_components.any? ? name_components.join(', ') : Place::DEFAULT_NAME - place.city = properties['city'] - place.country = properties['country'] - place.geodata = data - place.source = :photon + # Get places from points' geodata + places_from_points = extract_places_from_points(visit_data[:points], lat, lon) - place.save! + # Get places from external API + places_from_api = fetch_places_from_api(lat, lon) - # Process nearby organizations outside the main transaction - process_nearby_organizations(geocoded_data.drop(1)) - else - place.name = suggested_name || Place::DEFAULT_NAME - place.source = :manual - place.save! + # Combine and deduplicate by name + combined_places = [] + + # Add API places first (usually better quality) + places_from_api.each do |api_place| + combined_places << api_place unless place_name_exists?(combined_places, api_place.name) end + # Add places from points if name doesn't already exist + places_from_points.each do |point_place| + combined_places << point_place unless place_name_exists?(combined_places, point_place.name) + end + + combined_places + end + + # Step 3: Extract places from points + def extract_places_from_points(points, center_lat, center_lon) + return [] if points.blank? + + # Filter points with geodata + points_with_geodata = points.select { |point| point.geodata.present? } + return [] if points_with_geodata.empty? + + # Process each point to create or find places + places = [] + + points_with_geodata.each do |point| + place = create_place_from_point(point) + places << place if place + end + + places.uniq { |place| place.name } + end + + # Step 4: Create place from point + def create_place_from_point(point) + return nil unless point.geodata.is_a?(Hash) + + properties = point.geodata['properties'] || {} + return nil if properties.blank? + + # Get or build a name + name = build_place_name(properties) + return nil if name == Place::DEFAULT_NAME + + # Look for existing place with this name + existing = Place.where(name: name) + .near([point.latitude, point.longitude], SIMILARITY_RADIUS, :m) + .first + + return existing if existing + + # Create new place + place = Place.new( + name: name, + lonlat: "POINT(#{point.longitude} #{point.latitude})", + latitude: point.latitude, + longitude: point.longitude, + city: properties['city'], + country: properties['country'], + geodata: point.geodata, + source: :photon + ) + + place.save! + place + rescue ActiveRecord::RecordInvalid + nil + end + + # Step 5: Fetch places from API + def fetch_places_from_api(lat, lon) + # Get broader search results from Geocoder + geocoder_results = Geocoder.search([lat, lon], radius: (SEARCH_RADIUS / 1000.0), units: :km) + return [] if geocoder_results.blank? + + places = [] + + geocoder_results.each do |result| + place = create_place_from_api_result(result) + places << place if place + end + + places + end + + # Step 6: Create place from API result + def create_place_from_api_result(result) + return nil unless result && result.data.is_a?(Hash) + + properties = result.data['properties'] || {} + return nil if properties.blank? + + # Get or build a name + name = build_place_name(properties) + return nil if name == Place::DEFAULT_NAME + + # Look for existing place with this name + existing = Place.where(name: name) + .near([result.latitude, result.longitude], SIMILARITY_RADIUS, :m) + .first + + return existing if existing + + # Create new place + place = Place.new( + name: name, + lonlat: "POINT(#{result.longitude} #{result.latitude})", + latitude: result.latitude, + longitude: result.longitude, + city: properties['city'], + country: properties['country'], + geodata: result.data, + source: :photon + ) + + place.save! + place + rescue ActiveRecord::RecordInvalid + nil + end + + # Step 7: Select or create main place + def select_or_create_main_place(potential_places, lat, lon, suggested_name) + return create_default_place(lat, lon, suggested_name) if potential_places.blank? + + # Choose the closest place as the main one + sorted_places = potential_places.sort_by do |place| + place.distance_to([lat, lon], :m) + end + + sorted_places.first + end + + # Step 8: Create default place when no other options + def create_default_place(lat, lon, suggested_name) + name = suggested_name.presence || Place::DEFAULT_NAME + + place = Place.new( + name: name, + lonlat: "POINT(#{lon} #{lat})", + latitude: lat, + longitude: lon, + source: :manual + ) + + place.save! place end - def process_nearby_organizations(geocoded_data) - # Fetch nearby organizations - nearby_organizations = fetch_nearby_organizations(geocoded_data) - - # Save each organization as a possible place - nearby_organizations.each do |org| - lon = org[:longitude] - lat = org[:latitude] - - # Check if a similar place already exists - existing = Place.where(name: org[:name]) - .where('ST_DWithin(lonlat, ST_SetSRID(ST_MakePoint(?, ?), 4326), 1)', lon, lat) - .first - - next if existing - - Place.create!( - name: org[:name], - lonlat: "POINT(#{lon} #{lat})", - latitude: lat, - longitude: lon, - city: org[:city], - country: org[:country], - geodata: org[:geodata], - source: :photon - ) - end + # Step 9: Find suggested places + def find_suggested_places(lat, lon, exclude_id = nil) + query = Place.near([lat, lon], SEARCH_RADIUS, :m).with_distance([lat, lon], :m) + query = query.where.not(id: exclude_id) if exclude_id + query.limit(5) end - def fetch_nearby_organizations(geocoded_data) - geocoded_data.map do |result| - data = result.data - properties = data['properties'] || {} + # Helper methods - { - name: properties['name'] || 'Unknown Organization', - latitude: result.latitude, - longitude: result.longitude, - city: properties['city'], - country: properties['country'], - geodata: data - } - end + def build_place_name(properties) + name_components = [ + properties['name'], + properties['street'], + properties['housenumber'], + properties['postcode'], + properties['city'] + ].compact.reject(&:empty?).uniq + + name_components.any? ? name_components.join(', ') : Place::DEFAULT_NAME + end + + def place_name_exists?(places, name) + places.any? { |place| place.name == name } end end end diff --git a/app/services/visits/smart_detect.rb b/app/services/visits/smart_detect.rb index d8773e4d..04864dee 100644 --- a/app/services/visits/smart_detect.rb +++ b/app/services/visits/smart_detect.rb @@ -3,11 +3,9 @@ module Visits # Coordinates the process of detecting and creating visits from tracked points class SmartDetect - MINIMUM_VISIT_DURATION = 5.minutes + MINIMUM_VISIT_DURATION = 3.minutes MAXIMUM_VISIT_GAP = 30.minutes MINIMUM_POINTS_FOR_VISIT = 3 - SIGNIFICANT_PLACE_VISITS = 2 # Number of visits to consider a place significant - SIGNIFICANT_MOVEMENT_THRESHOLD = 50 # meters attr_reader :user, :start_at, :end_at, :points diff --git a/app/services/visits/suggest.rb b/app/services/visits/suggest.rb index 385bb20d..292190b3 100644 --- a/app/services/visits/suggest.rb +++ b/app/services/visits/suggest.rb @@ -14,12 +14,19 @@ class Visits::Suggest def call visits = Visits::SmartDetect.new(user, start_at:, end_at:).call - create_visits_notification(user) if visits.any? + # create_visits_notification(user) if visits.any? return nil unless DawarichSettings.reverse_geocoding_enabled? visits.each(&:async_reverse_geocode) visits + rescue StandardError => e + # create a notification with stacktrace and what arguments were used + user.notifications.create!( + kind: :error, + title: 'Error suggesting visits', + content: "Error suggesting visits: #{e.message}\n#{e.backtrace.join("\n")}" + ) end private diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index f41baeda..2cc1d82e 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -10,6 +10,7 @@ + <%= stylesheet_link_tag "tailwind", "inter-font", "data-turbo-track": "reload" %> <%= stylesheet_link_tag "application", "data-turbo-track": "reload" %>