Merge pull request #1515 from Freika/fix/existing-tracks-generation

Fixes for bulk creating job

app/jobs/tracks/bulk_creating_job.rb

@@ -1,27 +1,22 @@
 # frozen_string_literal: true
 
-# This job is being run on daily basis to create tracks for all users
-# for the past 24 hours.
+# This job is being run on daily basis to create tracks for all users.
+# For each user, it starts from the end of their last track (or from their oldest point
+# if no tracks exist) and processes points until the specified end_at time.
 #
 # 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])
+#
+# To let the job determine start times automatically (recommended):
+#   Tracks::BulkCreatingJob.perform_later(end_at: Time.current)
 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
+  def perform(start_at: nil, end_at: 1.day.ago.end_of_day, user_ids: [])
+    Tracks::BulkTrackCreator.new(start_at:, end_at:, user_ids:).call
   end
 end

app/services/tracks/bulk_track_creator.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+module Tracks
+  class BulkTrackCreator
+    def initialize(start_at: nil, end_at: 1.day.ago.end_of_day, user_ids: [])
+      @start_at = start_at&.to_datetime
+      @end_at = end_at&.to_datetime
+      @user_ids = user_ids
+    end
+
+    def call
+      users.find_each do |user|
+        next if user.tracked_points.empty?
+
+        user_start_at = start_at || start_time(user)
+
+        next unless user.tracked_points.where(timestamp: user_start_at.to_i..end_at.to_i).exists?
+
+        Tracks::CreateJob.perform_later(
+          user.id,
+          start_at: user_start_at,
+          end_at:,
+          cleaning_strategy: :daily
+        )
+      end
+    end
+
+    private
+
+    attr_reader :start_at, :end_at, :user_ids
+
+    def users
+      user_ids.any? ? User.active.where(id: user_ids) : User.active
+    end
+
+    def start_time(user)
+      latest_track = user.tracks.order(end_at: :desc).first
+
+      if latest_track
+        latest_track.end_at
+      else
+        oldest_point = user.tracked_points.order(:timestamp).first
+        oldest_point ? Time.zone.at(oldest_point.timestamp) : 1.day.ago.beginning_of_day
+      end
+    end
+  end
+end

spec/jobs/tracks/bulk_creating_job_spec.rb

@@ -4,69 +4,16 @@ 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 }
+    let(:service) { instance_double(Tracks::BulkTrackCreator) }
 
     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)
+      allow(Tracks::BulkTrackCreator).to receive(:new).with(start_at: 'foo', end_at: 'bar', user_ids: [1, 2]).and_return(service)
     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 'calls Tracks::BulkTrackCreator with the correct arguments' do
+      expect(service).to receive(:call)
 
-    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
+      described_class.new.perform(start_at: 'foo', end_at: 'bar', user_ids: [1, 2])
     end
   end
 end

spec/services/tracks/bulk_track_creator_spec.rb

@@ -0,0 +1,176 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Tracks::BulkTrackCreator do
+  describe '#call' 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
+
+    context 'when explicit start_at is provided' do
+      it 'schedules tracks creation jobs for active users with points in the timeframe' do
+        expect {
+          described_class.new(start_at:, end_at:).call
+        }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at:, end_at:, cleaning_strategy: :daily)
+      end
+
+      it 'does not schedule jobs for users without tracked points' do
+        expect {
+          described_class.new(start_at:, end_at:).call
+        }.not_to have_enqueued_job(Tracks::CreateJob).with(user_without_points.id, start_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(start_at:, end_at:).call
+        }.not_to have_enqueued_job(Tracks::CreateJob).with(user_with_old_points.id, start_at:, end_at:, cleaning_strategy: :daily)
+      end
+    end
+
+    context 'when specific user_ids are provided' do
+      it 'only processes the specified users' do
+        expect {
+          described_class.new(start_at:, end_at:, user_ids: [active_user.id]).call
+        }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at:, end_at:, cleaning_strategy: :daily)
+      end
+
+      it 'does not process users not in the user_ids list' do
+        expect {
+          described_class.new(start_at:, end_at:, user_ids: [active_user.id]).call
+        }.not_to have_enqueued_job(Tracks::CreateJob).with(inactive_user.id, start_at:, end_at:, cleaning_strategy: :daily)
+      end
+    end
+
+    context 'with automatic start time determination' do
+      let(:user_with_tracks) { create(:user) }
+      let(:user_without_tracks) { create(:user) }
+      let(:current_time) { Time.current }
+
+      before do
+        # Create some historical points and tracks for user_with_tracks
+        create(:point, user: user_with_tracks, timestamp: 3.days.ago.to_i)
+        create(:point, user: user_with_tracks, timestamp: 2.days.ago.to_i)
+
+        # Create a track ending 1 day ago
+        create(:track, user: user_with_tracks, end_at: 1.day.ago)
+
+        # Create newer points after the last track
+        create(:point, user: user_with_tracks, timestamp: 12.hours.ago.to_i)
+        create(:point, user: user_with_tracks, timestamp: 6.hours.ago.to_i)
+
+        # Create points for user without tracks
+        create(:point, user: user_without_tracks, timestamp: 2.days.ago.to_i)
+        create(:point, user: user_without_tracks, timestamp: 1.day.ago.to_i)
+      end
+
+      it 'starts from the end of the last track for users with existing tracks' do
+        track_end_time = user_with_tracks.tracks.order(end_at: :desc).first.end_at
+
+        expect {
+          described_class.new(end_at: current_time, user_ids: [user_with_tracks.id]).call
+        }.to have_enqueued_job(Tracks::CreateJob).with(
+          user_with_tracks.id,
+          start_at: track_end_time,
+          end_at: current_time.to_datetime,
+          cleaning_strategy: :daily
+        )
+      end
+
+      it 'starts from the oldest point for users without tracks' do
+        oldest_point_time = Time.zone.at(user_without_tracks.tracked_points.order(:timestamp).first.timestamp)
+
+        expect {
+          described_class.new(end_at: current_time, user_ids: [user_without_tracks.id]).call
+        }.to have_enqueued_job(Tracks::CreateJob).with(
+          user_without_tracks.id,
+          start_at: oldest_point_time,
+          end_at: current_time.to_datetime,
+          cleaning_strategy: :daily
+        )
+      end
+
+      it 'falls back to 1 day ago for users with no points' do
+        expect {
+          described_class.new(end_at: current_time, user_ids: [user_without_points.id]).call
+        }.not_to have_enqueued_job(Tracks::CreateJob).with(
+          user_without_points.id,
+          start_at: anything,
+          end_at: anything,
+          cleaning_strategy: :daily
+        )
+      end
+    end
+
+    context 'with default parameters' do
+      let(:user_with_recent_points) { create(:user) }
+
+      before do
+        # Create points within yesterday's timeframe
+        create(:point, user: user_with_recent_points, timestamp: 1.day.ago.beginning_of_day.to_i + 2.hours.to_i)
+        create(:point, user: user_with_recent_points, timestamp: 1.day.ago.beginning_of_day.to_i + 6.hours.to_i)
+      end
+
+      it 'uses automatic start time determination with yesterday as end_at' do
+        oldest_point_time = Time.zone.at(user_with_recent_points.tracked_points.order(:timestamp).first.timestamp)
+
+        expect {
+          described_class.new(user_ids: [user_with_recent_points.id]).call
+        }.to have_enqueued_job(Tracks::CreateJob).with(
+          user_with_recent_points.id,
+          start_at: oldest_point_time,
+          end_at: 1.day.ago.end_of_day.to_datetime,
+          cleaning_strategy: :daily
+        )
+      end
+    end
+  end
+
+  describe '#start_time' do
+    let(:user) { create(:user) }
+    let(:service) { described_class.new }
+
+    context 'when user has tracks' do
+      let!(:old_track) { create(:track, user: user, end_at: 3.days.ago) }
+      let!(:recent_track) { create(:track, user: user, end_at: 1.day.ago) }
+
+      it 'returns the end time of the most recent track' do
+        result = service.send(:start_time, user)
+        expect(result).to eq(recent_track.end_at)
+      end
+    end
+
+    context 'when user has no tracks but has points' do
+      let!(:old_point) { create(:point, user: user, timestamp: 5.days.ago.to_i) }
+      let!(:recent_point) { create(:point, user: user, timestamp: 2.days.ago.to_i) }
+
+      it 'returns the timestamp of the oldest point' do
+        result = service.send(:start_time, user)
+        expect(result).to eq(Time.zone.at(old_point.timestamp))
+      end
+    end
+
+    context 'when user has no tracks and no points' do
+      it 'returns 1 day ago beginning of day' do
+        result = service.send(:start_time, user)
+        expect(result).to eq(1.day.ago.beginning_of_day)
+      end
+    end
+  end
+end