From 13fd9da1f91178e5b9a58ed7247264ad6a941d85 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Wed, 9 Jul 2025 21:25:56 +0200 Subject: [PATCH] Add a scheduled job to create tracks for all users for the past 24 hours. --- CHANGELOG.md | 2 +- Procfile.production | 3 + app/javascript/controllers/maps_controller.js | 25 +--- app/jobs/tracks/bulk_creating_job.rb | 27 ++++ app/jobs/tracks/create_job.rb | 4 +- app/jobs/tracks/incremental_generator_job.rb | 2 +- app/services/tracks/cleaners/daily_cleaner.rb | 116 ++++++++++++++++++ .../no_op_cleaner.rb | 2 +- .../replace_cleaner.rb | 4 +- app/services/tracks/create_from_points.rb | 17 ++- app/services/tracks/generator.rb | 2 +- config/schedule.yml | 5 + ...0250704185707_create_tracks_from_points.rb | 27 +++- spec/jobs/tracks/bulk_creating_job_spec.rb | 72 +++++++++++ spec/jobs/tracks/create_job_spec.rb | 33 ++++- .../tracks/cleaners/daily_cleaner_spec.rb | 95 ++++++++++++++ .../tracks/create_from_points_spec.rb | 78 ++++++++++++ spec/services/tracks/generator_spec.rb | 6 +- spec/support/system_helpers.rb | 8 +- 19 files changed, 485 insertions(+), 43 deletions(-) create mode 100644 Procfile.production create mode 100644 app/jobs/tracks/bulk_creating_job.rb create mode 100644 app/services/tracks/cleaners/daily_cleaner.rb rename app/services/tracks/{track_cleaners => cleaners}/no_op_cleaner.rb (92%) rename app/services/tracks/{track_cleaners => cleaners}/replace_cleaner.rb (93%) create mode 100644 spec/jobs/tracks/bulk_creating_job_spec.rb create mode 100644 spec/services/tracks/cleaners/daily_cleaner_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 54d6c096..506a1463 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Added -- In the User Settings -> Background Jobs, you can now enable or disable visits suggestions. It's a background task that runs every day at midnight. Disabling it might be useful if you don't want to receive visits suggestions or if you're using the Dawarich iOS app, which has its own visits suggestions. +- In the User Settings -> Background Jobs, you can now disable visits suggestions, which is enabled by default. It's a background task that runs every day around midnight. Disabling it might be useful if you don't want to receive visits suggestions or if you're using the Dawarich iOS app, which has its own visits suggestions. - Tracks are now being calculated and stored in the database instead of being calculated on the fly in the browser. This will make the map page load faster. ## Changed diff --git a/Procfile.production b/Procfile.production new file mode 100644 index 00000000..74d29a75 --- /dev/null +++ b/Procfile.production @@ -0,0 +1,3 @@ +web: bundle exec puma -C config/puma.rb +worker: bundle exec sidekiq -C config/sidekiq.yml +prometheus_exporter: bundle exec prometheus_exporter -b ANY diff --git a/app/javascript/controllers/maps_controller.js b/app/javascript/controllers/maps_controller.js index 5c015eb7..a675c0e9 100644 --- a/app/javascript/controllers/maps_controller.js +++ b/app/javascript/controllers/maps_controller.js @@ -130,8 +130,8 @@ export default class extends BaseController { distance = distance * 0.621371; // km to miles conversion } - const unit = this.distanceUnit === 'mi' ? 'mi' : 'km'; - div.innerHTML = `${distance.toFixed(1)} ${unit} | ${pointsNumber} points`; + const unit = this.distanceUnit === 'km' ? 'km' : 'mi'; + div.innerHTML = `${distance} ${unit} | ${pointsNumber} points`; div.style.backgroundColor = 'white'; div.style.padding = '0 5px'; div.style.marginRight = '5px'; @@ -746,7 +746,7 @@ export default class extends BaseController { // Form HTML div.innerHTML = ` -
+
@@ -821,17 +821,6 @@ export default class extends BaseController { -
- -

Track Settings

- - - - -
@@ -860,14 +849,6 @@ export default class extends BaseController { editBtn.addEventListener("click", this.showGradientEditor.bind(this)); } - // Add track control event listeners - const tracksVisibleCheckbox = div.querySelector("#tracks_visible"); - if (tracksVisibleCheckbox) { - tracksVisibleCheckbox.addEventListener("change", this.toggleTracksVisibility.bind(this)); - } - - - // Add event listener to the form submission div.querySelector('#settings-form').addEventListener( 'submit', this.updateSettings.bind(this) diff --git a/app/jobs/tracks/bulk_creating_job.rb b/app/jobs/tracks/bulk_creating_job.rb new file mode 100644 index 00000000..f2bafdc8 --- /dev/null +++ b/app/jobs/tracks/bulk_creating_job.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +# This job is being run on daily basis to create tracks for all users +# for the past 24 hours. +# +# To manually run for a specific time range: +# Tracks::BulkCreatingJob.perform_later(start_at: 1.week.ago, end_at: Time.current) +# +# To run for specific users only: +# Tracks::BulkCreatingJob.perform_later(user_ids: [1, 2, 3]) +class Tracks::BulkCreatingJob < ApplicationJob + queue_as :tracks + sidekiq_options retry: false + + def perform(start_at: 1.day.ago.beginning_of_day, end_at: 1.day.ago.end_of_day, user_ids: []) + users = user_ids.any? ? User.active.where(id: user_ids) : User.active + start_at = start_at.to_datetime + end_at = end_at.to_datetime + + users.find_each do |user| + next if user.tracked_points.empty? + next unless user.tracked_points.where(timestamp: start_at.to_i..end_at.to_i).exists? + + Tracks::CreateJob.perform_later(user.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + end + end +end diff --git a/app/jobs/tracks/create_job.rb b/app/jobs/tracks/create_job.rb index 51969c87..57bc5bb4 100644 --- a/app/jobs/tracks/create_job.rb +++ b/app/jobs/tracks/create_job.rb @@ -3,9 +3,9 @@ class Tracks::CreateJob < ApplicationJob queue_as :default - def perform(user_id) + def perform(user_id, start_at: nil, end_at: nil, cleaning_strategy: :replace) user = User.find(user_id) - tracks_created = Tracks::CreateFromPoints.new(user).call + tracks_created = Tracks::CreateFromPoints.new(user, start_at:, end_at:, cleaning_strategy:).call create_success_notification(user, tracks_created) rescue StandardError => e diff --git a/app/jobs/tracks/incremental_generator_job.rb b/app/jobs/tracks/incremental_generator_job.rb index 837f6a7f..00f8a46f 100644 --- a/app/jobs/tracks/incremental_generator_job.rb +++ b/app/jobs/tracks/incremental_generator_job.rb @@ -24,7 +24,7 @@ class Tracks::IncrementalGeneratorJob < ApplicationJob user, point_loader: Tracks::PointLoaders::IncrementalLoader.new(user, day), incomplete_segment_handler: Tracks::IncompleteSegmentHandlers::BufferHandler.new(user, day, grace_period_minutes), - track_cleaner: Tracks::TrackCleaners::NoOpCleaner.new(user) + track_cleaner: Tracks::Cleaners::NoOpCleaner.new(user) ) end end diff --git a/app/services/tracks/cleaners/daily_cleaner.rb b/app/services/tracks/cleaners/daily_cleaner.rb new file mode 100644 index 00000000..6991fdfc --- /dev/null +++ b/app/services/tracks/cleaners/daily_cleaner.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +# Track cleaning strategy for daily track processing. +# +# This cleaner handles tracks that overlap with the specified time window, +# ensuring proper handling of cross-day tracks and preventing orphaned points. +# +# How it works: +# 1. Finds tracks that overlap with the time window (not just those completely contained) +# 2. For overlapping tracks, removes only points within the time window +# 3. Deletes tracks that become empty after point removal +# 4. Preserves tracks that extend beyond the time window with their remaining points +# +# Key differences from ReplaceCleaner: +# - Handles tracks that span multiple days correctly +# - Uses overlap logic instead of containment logic +# - Preserves track portions outside the processing window +# - Prevents orphaned points from cross-day tracks +# +# Used primarily for: +# - Daily track processing that handles 24-hour windows +# - Incremental processing that respects existing cross-day tracks +# - Scenarios where tracks may span the processing boundary +# +# Example usage: +# cleaner = Tracks::Cleaners::DailyCleaner.new(user, start_at: 1.day.ago.beginning_of_day, end_at: 1.day.ago.end_of_day) +# cleaner.cleanup +# +module Tracks + module Cleaners + class DailyCleaner + attr_reader :user, :start_at, :end_at + + def initialize(user, start_at: nil, end_at: nil) + @user = user + @start_at = start_at + @end_at = end_at + end + + def cleanup + return unless start_at.present? && end_at.present? + + overlapping_tracks = find_overlapping_tracks + + return if overlapping_tracks.empty? + + Rails.logger.info "Processing #{overlapping_tracks.count} overlapping tracks for user #{user.id} in time window #{start_at} to #{end_at}" + + overlapping_tracks.each do |track| + process_overlapping_track(track) + end + end + + private + + def find_overlapping_tracks + # Find tracks that overlap with our time window + # A track overlaps if: track_start < window_end AND track_end > window_start + user.tracks.where( + '(start_at < ? AND end_at > ?)', + Time.zone.at(end_at), + Time.zone.at(start_at) + ) + end + + def process_overlapping_track(track) + # Find points within our time window that belong to this track + points_in_window = track.points.where( + 'timestamp >= ? AND timestamp <= ?', + start_at.to_i, + end_at.to_i + ) + + if points_in_window.empty? + Rails.logger.debug "Track #{track.id} has no points in time window, skipping" + return + end + + # Remove these points from the track + points_in_window.update_all(track_id: nil) + + Rails.logger.debug "Removed #{points_in_window.count} points from track #{track.id}" + + # Check if the track has any remaining points + remaining_points_count = track.points.count + + if remaining_points_count == 0 + # Track is now empty, delete it + Rails.logger.debug "Track #{track.id} is now empty, deleting" + track.destroy! + elsif remaining_points_count < 2 + # Track has too few points to be valid, delete it and orphan remaining points + Rails.logger.debug "Track #{track.id} has insufficient points (#{remaining_points_count}), deleting" + track.points.update_all(track_id: nil) + track.destroy! + else + # Track still has valid points outside our window, update its boundaries + Rails.logger.debug "Track #{track.id} still has #{remaining_points_count} points, updating boundaries" + update_track_boundaries(track) + end + end + + def update_track_boundaries(track) + remaining_points = track.points.order(:timestamp) + + return if remaining_points.empty? + + # Update track start/end times based on remaining points + track.update!( + start_at: Time.zone.at(remaining_points.first.timestamp), + end_at: Time.zone.at(remaining_points.last.timestamp) + ) + end + end + end +end diff --git a/app/services/tracks/track_cleaners/no_op_cleaner.rb b/app/services/tracks/cleaners/no_op_cleaner.rb similarity index 92% rename from app/services/tracks/track_cleaners/no_op_cleaner.rb rename to app/services/tracks/cleaners/no_op_cleaner.rb index c5f76087..9d564b9d 100644 --- a/app/services/tracks/track_cleaners/no_op_cleaner.rb +++ b/app/services/tracks/cleaners/no_op_cleaner.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Tracks - module TrackCleaners + module Cleaners class NoOpCleaner def initialize(user) @user = user diff --git a/app/services/tracks/track_cleaners/replace_cleaner.rb b/app/services/tracks/cleaners/replace_cleaner.rb similarity index 93% rename from app/services/tracks/track_cleaners/replace_cleaner.rb rename to app/services/tracks/cleaners/replace_cleaner.rb index e586b49d..41eae76e 100644 --- a/app/services/tracks/track_cleaners/replace_cleaner.rb +++ b/app/services/tracks/cleaners/replace_cleaner.rb @@ -23,11 +23,11 @@ # for incremental processing where existing tracks should be preserved. # # Example usage: -# cleaner = Tracks::TrackCleaners::ReplaceCleaner.new(user, start_at: 1.week.ago, end_at: Time.current) +# cleaner = Tracks::Cleaners::ReplaceCleaner.new(user, start_at: 1.week.ago, end_at: Time.current) # cleaner.cleanup # module Tracks - module TrackCleaners + module Cleaners class ReplaceCleaner attr_reader :user, :start_at, :end_at diff --git a/app/services/tracks/create_from_points.rb b/app/services/tracks/create_from_points.rb index 2c01ea31..73c15f66 100644 --- a/app/services/tracks/create_from_points.rb +++ b/app/services/tracks/create_from_points.rb @@ -4,12 +4,13 @@ class Tracks::CreateFromPoints include Tracks::Segmentation include Tracks::TrackBuilder - attr_reader :user, :start_at, :end_at + attr_reader :user, :start_at, :end_at, :cleaning_strategy - def initialize(user, start_at: nil, end_at: nil) + def initialize(user, start_at: nil, end_at: nil, cleaning_strategy: :replace) @user = user @start_at = start_at @end_at = end_at + @cleaning_strategy = cleaning_strategy end def call @@ -46,8 +47,16 @@ class Tracks::CreateFromPoints Tracks::IncompleteSegmentHandlers::IgnoreHandler.new(user) end - def track_cleaner - @track_cleaner ||= Tracks::TrackCleaners::ReplaceCleaner.new(user, start_at: start_at, end_at: end_at) + def track_cleaner + @track_cleaner ||= + case cleaning_strategy + when :daily + Tracks::Cleaners::DailyCleaner.new(user, start_at: start_at, end_at: end_at) + when :none + Tracks::Cleaners::NoOpCleaner.new(user) + else # :replace (default) + Tracks::Cleaners::ReplaceCleaner.new(user, start_at: start_at, end_at: end_at) + end end # Legacy method for backward compatibility with tests diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index dafb3f83..9ac40ced 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -26,7 +26,7 @@ # user, # point_loader: Tracks::PointLoaders::BulkLoader.new(user), # incomplete_segment_handler: Tracks::IncompleteSegmentHandlers::IgnoreHandler.new(user), -# track_cleaner: Tracks::TrackCleaners::ReplaceCleaner.new(user) +# track_cleaner: Tracks::Cleaners::ReplaceCleaner.new(user) # ) # tracks_created = generator.call # diff --git a/config/schedule.yml b/config/schedule.yml index 7a49019f..a184df13 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -29,3 +29,8 @@ cache_preheating_job: cron: "0 0 * * *" # every day at 0:00 class: "Cache::PreheatingJob" queue: default + +tracks_bulk_creating_job: + cron: "10 0 * * *" # every day at 00:10 + class: "Tracks::BulkCreatingJob" + queue: tracks diff --git a/db/data/20250704185707_create_tracks_from_points.rb b/db/data/20250704185707_create_tracks_from_points.rb index 8c605702..aae55296 100644 --- a/db/data/20250704185707_create_tracks_from_points.rb +++ b/db/data/20250704185707_create_tracks_from_points.rb @@ -2,9 +2,34 @@ class CreateTracksFromPoints < ActiveRecord::Migration[8.0] def up + puts "Starting bulk track creation for all users..." + + total_users = User.count + processed_users = 0 + User.find_each do |user| - Tracks::CreateJob.perform_later(user.id) + points_count = user.tracked_points.count + + if points_count > 0 + puts "Enqueuing track creation for user #{user.id} (#{points_count} points)" + + # Use explicit parameters for bulk historical processing: + # - No time limits (start_at: nil, end_at: nil) = process ALL historical data + # - Replace strategy = clean slate, removes any existing tracks first + Tracks::CreateJob.perform_later( + user.id, + start_at: nil, + end_at: nil, + cleaning_strategy: :replace + ) + + processed_users += 1 + else + puts "Skipping user #{user.id} (no tracked points)" + end end + + puts "Enqueued track creation jobs for #{processed_users}/#{total_users} users" end def down diff --git a/spec/jobs/tracks/bulk_creating_job_spec.rb b/spec/jobs/tracks/bulk_creating_job_spec.rb new file mode 100644 index 00000000..b40f5d43 --- /dev/null +++ b/spec/jobs/tracks/bulk_creating_job_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Tracks::BulkCreatingJob, type: :job do + describe '#perform' do + let!(:active_user) { create(:user) } + let!(:inactive_user) { create(:user, :inactive) } + let!(:user_without_points) { create(:user) } + + let(:start_at) { 1.day.ago.beginning_of_day } + let(:end_at) { 1.day.ago.end_of_day } + + before do + # Create points for active user in the target timeframe + create(:point, user: active_user, timestamp: start_at.to_i + 1.hour.to_i) + create(:point, user: active_user, timestamp: start_at.to_i + 2.hours.to_i) + + # Create points for inactive user in the target timeframe + create(:point, user: inactive_user, timestamp: start_at.to_i + 1.hour.to_i) + end + + it 'schedules tracks creation jobs for active users with points in the timeframe' do + expect { + described_class.new.perform(start_at: start_at, end_at: end_at) + }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + end + + it 'does not schedule jobs for users without tracked points' do + expect { + described_class.new.perform(start_at: start_at, end_at: end_at) + }.not_to have_enqueued_job(Tracks::CreateJob).with(user_without_points.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + end + + it 'does not schedule jobs for users without points in the specified timeframe' do + # Create a user with points outside the timeframe + user_with_old_points = create(:user) + create(:point, user: user_with_old_points, timestamp: 2.days.ago.to_i) + + expect { + described_class.new.perform(start_at: start_at, end_at: end_at) + }.not_to have_enqueued_job(Tracks::CreateJob).with(user_with_old_points.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + end + + context 'when specific user_ids are provided' do + it 'only processes the specified users' do + expect { + described_class.new.perform(start_at: start_at, end_at: end_at, user_ids: [active_user.id]) + }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + end + + it 'does not process users not in the user_ids list' do + expect { + described_class.new.perform(start_at: start_at, end_at: end_at, user_ids: [active_user.id]) + }.not_to have_enqueued_job(Tracks::CreateJob).with(inactive_user.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + end + end + + context 'with default parameters' do + it 'uses yesterday as the default timeframe' do + expect { + described_class.new.perform + }.to have_enqueued_job(Tracks::CreateJob).with( + active_user.id, + start_at: 1.day.ago.beginning_of_day.to_datetime, + end_at: 1.day.ago.end_of_day.to_datetime, + cleaning_strategy: :daily + ) + end + end + end +end diff --git a/spec/jobs/tracks/create_job_spec.rb b/spec/jobs/tracks/create_job_spec.rb index cf88c8a2..2cbba7de 100644 --- a/spec/jobs/tracks/create_job_spec.rb +++ b/spec/jobs/tracks/create_job_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Tracks::CreateJob, type: :job do let(:notification_service) { instance_double(Notifications::Create) } before do - allow(Tracks::CreateFromPoints).to receive(:new).with(user).and_return(service_instance) + allow(Tracks::CreateFromPoints).to receive(:new).with(user, start_at: nil, end_at: nil, cleaning_strategy: :replace).and_return(service_instance) allow(service_instance).to receive(:call).and_return(3) allow(Notifications::Create).to receive(:new).and_return(notification_service) allow(notification_service).to receive(:call) @@ -19,7 +19,7 @@ RSpec.describe Tracks::CreateJob, type: :job do it 'calls the service and creates a notification' do described_class.new.perform(user.id) - expect(Tracks::CreateFromPoints).to have_received(:new).with(user) + expect(Tracks::CreateFromPoints).to have_received(:new).with(user, start_at: nil, end_at: nil, cleaning_strategy: :replace) expect(service_instance).to have_received(:call) expect(Notifications::Create).to have_received(:new).with( user: user, @@ -30,13 +30,40 @@ RSpec.describe Tracks::CreateJob, type: :job do expect(notification_service).to have_received(:call) end + context 'with custom parameters' do + let(:start_at) { 1.day.ago.beginning_of_day.to_i } + let(:end_at) { 1.day.ago.end_of_day.to_i } + let(:cleaning_strategy) { :daily } + + before do + allow(Tracks::CreateFromPoints).to receive(:new).with(user, start_at: start_at, end_at: end_at, cleaning_strategy: cleaning_strategy).and_return(service_instance) + allow(service_instance).to receive(:call).and_return(2) + allow(Notifications::Create).to receive(:new).and_return(notification_service) + allow(notification_service).to receive(:call) + end + + it 'passes custom parameters to the service' do + described_class.new.perform(user.id, start_at: start_at, end_at: end_at, cleaning_strategy: cleaning_strategy) + + expect(Tracks::CreateFromPoints).to have_received(:new).with(user, start_at: start_at, end_at: end_at, cleaning_strategy: cleaning_strategy) + expect(service_instance).to have_received(:call) + expect(Notifications::Create).to have_received(:new).with( + user: user, + kind: :info, + title: 'Tracks Generated', + content: 'Created 2 tracks from your location data. Check your tracks section to view them.' + ) + expect(notification_service).to have_received(:call) + end + end + context 'when service raises an error' do let(:error_message) { 'Something went wrong' } let(:service_instance) { instance_double(Tracks::CreateFromPoints) } let(:notification_service) { instance_double(Notifications::Create) } before do - allow(Tracks::CreateFromPoints).to receive(:new).with(user).and_return(service_instance) + allow(Tracks::CreateFromPoints).to receive(:new).with(user, start_at: nil, end_at: nil, cleaning_strategy: :replace).and_return(service_instance) allow(service_instance).to receive(:call).and_raise(StandardError, error_message) allow(Notifications::Create).to receive(:new).and_return(notification_service) allow(notification_service).to receive(:call) diff --git a/spec/services/tracks/cleaners/daily_cleaner_spec.rb b/spec/services/tracks/cleaners/daily_cleaner_spec.rb new file mode 100644 index 00000000..06e64bf4 --- /dev/null +++ b/spec/services/tracks/cleaners/daily_cleaner_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Tracks::Cleaners::DailyCleaner do + let(:user) { create(:user) } + let(:start_at) { 1.day.ago.beginning_of_day } + let(:end_at) { 1.day.ago.end_of_day } + let(:cleaner) { described_class.new(user, start_at: start_at.to_i, end_at: end_at.to_i) } + + describe '#cleanup' do + context 'when there are no overlapping tracks' do + before do + # Create a track that ends before our window + track = create(:track, user: user, start_at: 2.days.ago, end_at: 2.days.ago + 1.hour) + create(:point, user: user, track: track, timestamp: 2.days.ago.to_i) + end + + it 'does not remove any tracks' do + expect { cleaner.cleanup }.not_to change { user.tracks.count } + end + end + + context 'when a track is completely within the time window' do + let!(:track) { create(:track, user: user, start_at: start_at + 1.hour, end_at: end_at - 1.hour) } + let!(:point1) { create(:point, user: user, track: track, timestamp: (start_at + 1.hour).to_i) } + let!(:point2) { create(:point, user: user, track: track, timestamp: (start_at + 2.hours).to_i) } + + it 'removes all points from the track and deletes it' do + expect { cleaner.cleanup }.to change { user.tracks.count }.by(-1) + expect(point1.reload.track_id).to be_nil + expect(point2.reload.track_id).to be_nil + end + end + + context 'when a track spans across the time window' do + let!(:track) { create(:track, user: user, start_at: start_at - 1.hour, end_at: end_at + 1.hour) } + let!(:point_before) { create(:point, user: user, track: track, timestamp: (start_at - 30.minutes).to_i) } + let!(:point_during1) { create(:point, user: user, track: track, timestamp: (start_at + 1.hour).to_i) } + let!(:point_during2) { create(:point, user: user, track: track, timestamp: (start_at + 2.hours).to_i) } + let!(:point_after) { create(:point, user: user, track: track, timestamp: (end_at + 30.minutes).to_i) } + + it 'removes only points within the window and updates track boundaries' do + expect { cleaner.cleanup }.not_to change { user.tracks.count } + + # Points outside window should remain attached + expect(point_before.reload.track_id).to eq(track.id) + expect(point_after.reload.track_id).to eq(track.id) + + # Points inside window should be detached + expect(point_during1.reload.track_id).to be_nil + expect(point_during2.reload.track_id).to be_nil + + # Track boundaries should be updated + track.reload + expect(track.start_at).to be_within(1.second).of(Time.zone.at(point_before.timestamp)) + expect(track.end_at).to be_within(1.second).of(Time.zone.at(point_after.timestamp)) + end + end + + context 'when a track overlaps but has insufficient remaining points' do + let!(:track) { create(:track, user: user, start_at: start_at - 1.hour, end_at: end_at + 1.hour) } + let!(:point_before) { create(:point, user: user, track: track, timestamp: (start_at - 30.minutes).to_i) } + let!(:point_during) { create(:point, user: user, track: track, timestamp: (start_at + 1.hour).to_i) } + + it 'removes the track entirely and orphans remaining points' do + expect { cleaner.cleanup }.to change { user.tracks.count }.by(-1) + + expect(point_before.reload.track_id).to be_nil + expect(point_during.reload.track_id).to be_nil + end + end + + context 'when track has no points in the time window' do + let!(:track) { create(:track, user: user, start_at: start_at - 2.hours, end_at: end_at + 2.hours) } + let!(:point_before) { create(:point, user: user, track: track, timestamp: (start_at - 30.minutes).to_i) } + let!(:point_after) { create(:point, user: user, track: track, timestamp: (end_at + 30.minutes).to_i) } + + it 'does not modify the track' do + expect { cleaner.cleanup }.not_to change { user.tracks.count } + expect(track.reload.start_at).to be_within(1.second).of(track.start_at) + expect(track.reload.end_at).to be_within(1.second).of(track.end_at) + end + end + + context 'without start_at and end_at' do + let(:cleaner) { described_class.new(user) } + + it 'does not perform any cleanup' do + create(:track, user: user) + expect { cleaner.cleanup }.not_to change { user.tracks.count } + end + end + end +end diff --git a/spec/services/tracks/create_from_points_spec.rb b/spec/services/tracks/create_from_points_spec.rb index 00307ffe..df64439d 100644 --- a/spec/services/tracks/create_from_points_spec.rb +++ b/spec/services/tracks/create_from_points_spec.rb @@ -13,6 +13,10 @@ RSpec.describe Tracks::CreateFromPoints do expect(service.time_threshold_minutes).to eq(user.safe_settings.minutes_between_routes.to_i) end + it 'defaults to replace cleaning strategy' do + expect(service.cleaning_strategy).to eq(:replace) + end + context 'with custom user settings' do before do user.update!(settings: user.settings.merge({ @@ -27,6 +31,28 @@ RSpec.describe Tracks::CreateFromPoints do expect(service.time_threshold_minutes).to eq(60) end end + + context 'with custom cleaning strategy' do + it 'accepts daily cleaning strategy' do + service = described_class.new(user, cleaning_strategy: :daily) + expect(service.cleaning_strategy).to eq(:daily) + end + + it 'accepts none cleaning strategy' do + service = described_class.new(user, cleaning_strategy: :none) + expect(service.cleaning_strategy).to eq(:none) + end + + it 'accepts custom date range with cleaning strategy' do + start_time = 1.day.ago.beginning_of_day.to_i + end_time = 1.day.ago.end_of_day.to_i + service = described_class.new(user, start_at: start_time, end_at: end_time, cleaning_strategy: :daily) + + expect(service.start_at).to eq(start_time) + expect(service.end_at).to eq(end_time) + expect(service.cleaning_strategy).to eq(:daily) + end + end end describe '#call' do @@ -154,6 +180,58 @@ RSpec.describe Tracks::CreateFromPoints do expect { service.call }.to change(Track, :count).by(0) # -1 + 1 expect(Track.exists?(existing_track.id)).to be false end + + context 'with none cleaning strategy' do + let(:service) { described_class.new(user, cleaning_strategy: :none) } + + it 'preserves existing tracks and creates new ones' do + expect { service.call }.to change(Track, :count).by(1) # +1, existing preserved + expect(Track.exists?(existing_track.id)).to be true + end + end + end + + context 'with different cleaning strategies' do + let!(:points) do + [ + create(:point, user: user, timestamp: 1.hour.ago.to_i, + lonlat: 'POINT(-74.0060 40.7128)'), + create(:point, user: user, timestamp: 50.minutes.ago.to_i, + lonlat: 'POINT(-74.0070 40.7130)') + ] + end + + it 'works with replace strategy (default)' do + service = described_class.new(user, cleaning_strategy: :replace) + expect { service.call }.to change(Track, :count).by(1) + end + + it 'works with daily strategy' do + # Create points within the daily range we're testing + start_time = 1.day.ago.beginning_of_day.to_i + end_time = 1.day.ago.end_of_day.to_i + + # Create test points within the daily range + create(:point, user: user, timestamp: start_time + 1.hour.to_i, + lonlat: 'POINT(-74.0060 40.7128)') + create(:point, user: user, timestamp: start_time + 2.hours.to_i, + lonlat: 'POINT(-74.0070 40.7130)') + + # Create an existing track that overlaps with our time window + existing_track = create(:track, user: user, + start_at: Time.zone.at(start_time - 1.hour), + end_at: Time.zone.at(start_time + 30.minutes)) + + service = described_class.new(user, start_at: start_time, end_at: end_time, cleaning_strategy: :daily) + + # Daily cleaning should handle existing tracks properly and create new ones + expect { service.call }.to change(Track, :count).by(0) # existing cleaned and new created + end + + it 'works with none strategy' do + service = described_class.new(user, cleaning_strategy: :none) + expect { service.call }.to change(Track, :count).by(1) + end end context 'with mixed elevation data' do diff --git a/spec/services/tracks/generator_spec.rb b/spec/services/tracks/generator_spec.rb index 2463c1bf..851508f8 100644 --- a/spec/services/tracks/generator_spec.rb +++ b/spec/services/tracks/generator_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Tracks::Generator do let(:user) { create(:user) } let(:point_loader) { double('PointLoader') } let(:incomplete_segment_handler) { double('IncompleteSegmentHandler') } - let(:track_cleaner) { double('TrackCleaner') } + let(:track_cleaner) { double('Cleaner') } let(:generator) do described_class.new( @@ -200,7 +200,7 @@ RSpec.describe Tracks::Generator do context 'with bulk processing strategies' do let(:bulk_loader) { Tracks::PointLoaders::BulkLoader.new(user) } let(:ignore_handler) { Tracks::IncompleteSegmentHandlers::IgnoreHandler.new(user) } - let(:replace_cleaner) { Tracks::TrackCleaners::ReplaceCleaner.new(user) } + let(:replace_cleaner) { Tracks::Cleaners::ReplaceCleaner.new(user) } let(:bulk_generator) do described_class.new( @@ -231,7 +231,7 @@ RSpec.describe Tracks::Generator do context 'with incremental processing strategies' do let(:incremental_loader) { Tracks::PointLoaders::IncrementalLoader.new(user) } let(:buffer_handler) { Tracks::IncompleteSegmentHandlers::BufferHandler.new(user, Date.current, 5) } - let(:noop_cleaner) { Tracks::TrackCleaners::NoOpCleaner.new(user) } + let(:noop_cleaner) { Tracks::Cleaners::NoOpCleaner.new(user) } let(:incremental_generator) do described_class.new( diff --git a/spec/support/system_helpers.rb b/spec/support/system_helpers.rb index 2c7cf3ff..9418e8b6 100644 --- a/spec/support/system_helpers.rb +++ b/spec/support/system_helpers.rb @@ -1,8 +1,11 @@ # frozen_string_literal: true module SystemHelpers + include Rails.application.routes.url_helpers + def sign_in_user(user, password = 'password123') - visit new_user_session_path + visit '/users/sign_in' + expect(page).to have_field('Email', wait: 10) fill_in 'Email', with: user.email fill_in 'Password', with: password click_button 'Log in' @@ -10,11 +13,12 @@ module SystemHelpers def sign_in_and_visit_map(user, password = 'password123') sign_in_user(user, password) - expect(page).to have_current_path(map_path) + expect(page).to have_current_path('/map') expect(page).to have_css('.leaflet-container', wait: 10) end end RSpec.configure do |config| config.include SystemHelpers, type: :system + config.include Rails.application.routes.url_helpers, type: :system end