Fix stats distances and rework stas calculating service

Gemfile

@@ -11,6 +11,7 @@ gem 'data_migrate'
 gem 'devise'
 gem 'geocoder', git: 'https://github.com/alexreisner/geocoder.git', ref: '04ee293'
 gem 'gpx'
+gem 'groupdate'
 gem 'httparty'
 gem 'importmap-rails'
 gem 'kaminari'

Gemfile.lock

@@ -142,6 +142,8 @@ GEM
     gpx (1.2.0)
       nokogiri (~> 1.7)
       rake
+    groupdate (6.5.1)
+      activesupport (>= 7)
     hashdiff (1.1.1)
     httparty (0.22.0)
       csv
@@ -433,6 +435,7 @@ DEPENDENCIES
   foreman
   geocoder!
   gpx
+  groupdate
   httparty
   importmap-rails
   kaminari

app/controllers/stats_controller.rb

@@ -13,7 +13,7 @@ class StatsController < ApplicationController
   end
 
   def update
-    StatCreatingJob.perform_later(current_user.id)
+    Stats::CalculatingJob.perform_later(current_user.id)
 
     redirect_to stats_path, notice: 'Stats are being updated', status: :see_other
   end

app/jobs/bulk_stats_calculating_job.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class BulkStatsCalculatingJob < ApplicationJob
+  queue_as :stats
+
+  def perform
+    user_ids = User.pluck(:id)
+
+    user_ids.each do |user_id|
+      Stats::CalculatingJob.perform_later(user_id)
+    end
+  end
+end

app/jobs/stat_creating_job.rb

@@ -1,11 +0,0 @@
-# frozen_string_literal: true
-
-class StatCreatingJob < ApplicationJob
-  queue_as :stats
-
-  def perform(user_ids = nil)
-    user_ids = user_ids.nil? ? User.pluck(:id) : Array(user_ids)
-
-    CreateStats.new(user_ids).call
-  end
-end

app/jobs/stats/calculating_job.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class Stats::CalculatingJob < ApplicationJob
+  queue_as :stats
+
+  def perform(user_id, start_at: nil, end_at: nil)
+    Stats::Calculate.new(user_id, start_at:, end_at:).call
+
+    create_stats_updated_notification(user_id)
+  rescue StandardError => e
+    create_stats_update_failed_notification(user_id, e)
+  end
+
+  private
+
+  def create_stats_updated_notification(user_id)
+    user = User.find(user_id)
+
+    Notifications::Create.new(
+      user:, kind: :info, title: 'Stats updated', content: 'Stats updated'
+    ).call
+  end
+
+  def create_stats_update_failed_notification(user_id, error)
+    user = User.find(user_id)
+
+    Notifications::Create.new(
+      user:,
+      kind: :error,
+      title: 'Stats update failed',
+      content: "#{error.message}, stacktrace: #{error.backtrace.join("\n")}"
+    ).call
+  end
+end

app/models/stat.rb

@@ -11,7 +11,11 @@ class Stat < ApplicationRecord
       end_of_day = day.end_of_day.to_i
 
       # We have to filter by user as well
-      points = user.tracked_points.without_raw_data.where(timestamp: beginning_of_day..end_of_day)
+      points = user
+               .tracked_points
+               .without_raw_data
+               .order(timestamp: :asc)
+               .where(timestamp: beginning_of_day..end_of_day)
 
       data = { day: index, distance: 0 }
 

app/services/imports/create.rb

@@ -35,7 +35,7 @@ class Imports::Create
   end
 
   def schedule_stats_creating(user_id)
-    StatCreatingJob.perform_later(user_id)
+    Stats::CalculatingJob.perform_later(user_id)
   end
 
   def schedule_visit_suggesting(user_id, import)

app/services/stats/calculate.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+class Stats::Calculate
+  def initialize(user_id, start_at: nil, end_at: nil)
+    @user = User.find(user_id)
+    @start_at = start_at || DateTime.new(1970, 1, 1)
+    @end_at = end_at || Time.current
+  end
+
+  def call
+    # 1. Get all points for given user and time period
+    points = points(start_timestamp, end_timestamp)
+
+    # 2. Split points by months
+    points_by_month = points.group_by_month(&:recorded_at)
+
+    # 3. Calculate stats for each month
+    points_by_month.each do |month, month_points|
+      update_month_stats(month_points, month.year, month.month)
+    end
+    # 4. Save stats
+  rescue StandardError => e
+    create_stats_update_failed_notification(user, e)
+  end
+
+  private
+
+  attr_reader :user, :start_at, :end_at
+
+  def start_timestamp = start_at.to_i
+  def end_timestamp   = end_at.to_i
+
+  def update_month_stats(month_points, year, month)
+    return if month_points.empty?
+
+    stat = current_stat(year, month)
+    distance_by_day = stat.distance_by_day
+
+    stat.daily_distance = distance_by_day
+    stat.distance = distance(distance_by_day)
+    stat.toponyms = toponyms(month_points)
+    stat.save
+  end
+
+  def points(start_at, end_at)
+    user
+      .tracked_points
+      .without_raw_data
+      .where(timestamp: start_at..end_at)
+      .order(:timestamp)
+      .select(:latitude, :longitude, :timestamp, :city, :country)
+  end
+
+  def distance(distance_by_day)
+    distance_by_day.sum { |day| day[1] }
+  end
+
+  def toponyms(points)
+    CountriesAndCities.new(points).call
+  end
+
+  def current_stat(year, month)
+    Stat.find_or_initialize_by(year:, month:, user:)
+  end
+
+  def create_stats_update_failed_notification(user, error)
+    Notifications::Create.new(
+      user:,
+      kind: :error,
+      title: 'Stats update failed',
+      content: "#{error.message}, stacktrace: #{error.backtrace.join("\n")}"
+    ).call
+  end
+end

config/schedule.yml

@@ -1,8 +1,8 @@
 # config/schedule.yml
 
-stat_creating_job:
+bulk_stats_calculating_job:
   cron: "0 */6 * * *" # every 6 hour
-  class: "StatCreatingJob"
+  class: "BulkStatsCalculatingJob"
   queue: stats
 
 area_visits_calculation_scheduling_job:

db/schema.rb

@@ -53,9 +53,6 @@ ActiveRecord::Schema[7.2].define(version: 2024_08_22_092405) do
     t.index ["user_id"], name: "index_areas_on_user_id"
   end
 
-  create_table "data_migrations", primary_key: "version", id: :string, force: :cascade do |t|
-  end
-
   create_table "exports", force: :cascade do |t|
     t.string "name", null: false
     t.string "url"

spec/jobs/bulk_stats_calculating_job_spec.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe BulkStatsCalculatingJob, type: :job do
+  describe '#perform' do
+    it 'enqueues Stats::CalculatingJob for each user' do
+      user1 = create(:user)
+      user2 = create(:user)
+      user3 = create(:user)
+
+      expect(Stats::CalculatingJob).to receive(:perform_later).with(user1.id)
+      expect(Stats::CalculatingJob).to receive(:perform_later).with(user2.id)
+      expect(Stats::CalculatingJob).to receive(:perform_later).with(user3.id)
+
+      BulkStatsCalculatingJob.perform_now
+    end
+  end
+end

spec/jobs/import_job_spec.rb

@@ -13,8 +13,8 @@ RSpec.describe ImportJob, type: :job do
       expect { perform }.to change { Point.count }.by(9)
     end
 
-    it 'calls StatCreatingJob' do
+    it 'calls Stats::CalculatingJob' do
-      expect(StatCreatingJob).to receive(:perform_later).with(user.id)
+      expect(Stats::CalculatingJob).to receive(:perform_later).with(user.id)
 
       perform
     end

spec/jobs/stat_creating_job_spec.rb

@@ -1,20 +0,0 @@
-require 'rails_helper'
-
-RSpec.describe StatCreatingJob, type: :job do
-  describe '#perform' do
-    let(:user) { create(:user) }
-
-    subject { described_class.perform_now([user.id]) }
-
-    before do
-      allow(CreateStats).to receive(:new).and_call_original
-      allow_any_instance_of(CreateStats).to receive(:call)
-    end
-
-    it 'creates a stat' do
-      subject
-
-      expect(CreateStats).to have_received(:new).with([user.id])
-    end
-  end
-end

spec/jobs/stats/calculating_job_spec.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Stats::CalculatingJob, type: :job do
+  describe '#perform' do
+    let!(:user) { create(:user) }
+    let(:start_at) { nil }
+    let(:end_at) { nil }
+
+    subject { described_class.perform_now(user.id) }
+
+    before do
+      allow(Stats::Calculate).to receive(:new).and_call_original
+      allow_any_instance_of(Stats::Calculate).to receive(:call)
+    end
+
+    it 'calls Stats::Calculate service' do
+      subject
+
+      expect(Stats::Calculate).to have_received(:new).with(user.id, { start_at:, end_at: })
+    end
+
+    it 'created notifications' do
+      expect { subject }.to change { Notification.count }.by(1)
+    end
+  end
+end

spec/requests/stats_spec.rb

@@ -54,8 +54,8 @@ RSpec.describe '/stats', type: :request do
     describe 'POST /update' do
       let(:stat) { create(:stat, user:, year: 2024) }
 
-      it 'enqueues StatCreatingJob' do
+      it 'enqueues Stats::CalculatingJob' do
-        expect { post stats_url(stat.year) }.to have_enqueued_job(StatCreatingJob)
+        expect { post stats_url(stat.year) }.to have_enqueued_job(Stats::CalculatingJob)
       end
     end
   end

spec/services/create_stats_spec.rb

@@ -1,89 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-
-RSpec.describe CreateStats do
-  describe '#call' do
-    subject(:create_stats) { described_class.new(user_ids).call }
-
-    let(:user_ids) { [user.id] }
-    let(:user) { create(:user) }
-
-    context 'when there are no points' do
-      it 'does not create stats' do
-        expect { create_stats }.not_to(change { Stat.count })
-      end
-    end
-
-    context 'when there are points' do
-      let!(:import) { create(:import, user:) }
-      let!(:point1) { create(:point, user:, import:, latitude: 0, longitude: 0) }
-      let!(:point2) { create(:point, user:, import:, latitude: 1, longitude: 2) }
-      let!(:point3) { create(:point, user:, import:, latitude: 3, longitude: 4) }
-
-      context 'when units are kilometers' do
-        before { stub_const('DISTANCE_UNIT', :km) }
-
-        it 'creates stats' do
-          expect { create_stats }.to change { Stat.count }.by(1)
-        end
-
-        it 'calculates distance' do
-          create_stats
-
-          expect(user.stats.last.distance).to eq(563)
-        end
-
-        it 'created notifications' do
-          expect { create_stats }.to change { Notification.count }.by(1)
-        end
-
-        context 'when there is an error' do
-          before do
-            allow(Stat).to receive(:find_or_initialize_by).and_raise(StandardError)
-          end
-
-          it 'does not create stats' do
-            expect { create_stats }.not_to(change { Stat.count })
-          end
-
-          it 'created notifications' do
-            expect { create_stats }.to change { Notification.count }.by(1)
-          end
-        end
-      end
-
-      context 'when units are miles' do
-        before { stub_const('DISTANCE_UNIT', :mi) }
-
-        it 'creates stats' do
-          expect { create_stats }.to change { Stat.count }.by(1)
-        end
-
-        it 'calculates distance' do
-          create_stats
-
-          expect(user.stats.last.distance).to eq(349)
-        end
-
-        it 'created notifications' do
-          expect { create_stats }.to change { Notification.count }.by(1)
-        end
-
-        context 'when there is an error' do
-          before do
-            allow(Stat).to receive(:find_or_initialize_by).and_raise(StandardError)
-          end
-
-          it 'does not create stats' do
-            expect { create_stats }.not_to(change { Stat.count })
-          end
-
-          it 'created notifications' do
-            expect { create_stats }.to change { Notification.count }.by(1)
-          end
-        end
-      end
-    end
-  end
-end

spec/services/stats/calculate_spec.rb

@@ -0,0 +1,104 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Stats::Calculate do
+  describe '#call' do
+    subject(:calculate_stats) { described_class.new(user.id).call }
+
+    let(:user) { create(:user) }
+
+    context 'when there are no points' do
+      it 'does not create stats' do
+        expect { calculate_stats }.not_to(change { Stat.count })
+      end
+    end
+
+    context 'when there are points' do
+      let(:timestamp1) { DateTime.new(2021, 1, 1, 12).to_i }
+      let(:timestamp2) { DateTime.new(2021, 1, 1, 13).to_i }
+      let(:timestamp3) { DateTime.new(2021, 1, 1, 14).to_i }
+      let!(:import) { create(:import, user:) }
+      let!(:point1) do
+        create(:point,
+               user:,
+               import:,
+               timestamp: timestamp1,
+               latitude: 52.107902115161316,
+               longitude: 14.452712811406352)
+      end
+      let!(:point2) do
+        create(:point,
+               user:,
+               import:,
+               timestamp: timestamp2,
+               latitude: 51.9746598171507,
+               longitude: 12.291519487061901)
+      end
+      let!(:point3) do
+        create(:point,
+               user:,
+               import:,
+               timestamp: timestamp3,
+               latitude: 52.72859111523629,
+               longitude: 9.77973105800526)
+      end
+
+      context 'when units are kilometers' do
+        before { stub_const('DISTANCE_UNIT', :km) }
+
+        it 'creates stats' do
+          expect { calculate_stats }.to change { Stat.count }.by(1)
+        end
+
+        it 'calculates distance' do
+          calculate_stats
+
+          expect(user.stats.last.distance).to eq(338)
+        end
+
+        context 'when there is an error' do
+          before do
+            allow(Stat).to receive(:find_or_initialize_by).and_raise(StandardError)
+          end
+
+          it 'does not create stats' do
+            expect { calculate_stats }.not_to(change { Stat.count })
+          end
+
+          it 'creates a notification' do
+            expect { calculate_stats }.to change { Notification.count }.by(1)
+          end
+        end
+      end
+
+      context 'when units are miles' do
+        before { stub_const('DISTANCE_UNIT', :mi) }
+
+        it 'creates stats' do
+          expect { calculate_stats }.to change { Stat.count }.by(1)
+        end
+
+        it 'calculates distance' do
+          calculate_stats
+
+          expect(user.stats.last.distance).to eq(210)
+        end
+
+        context 'when there is an error' do
+          before do
+            allow(Stat).to receive(:find_or_initialize_by).and_raise(StandardError)
+          end
+
+          it 'does not create stats' do
+            expect { calculate_stats }.not_to(change { Stat.count })
+          end
+
+          it 'creates a notification' do
+            expect { calculate_stats }.to change { Notification.count }.by(1)
+          end
+        end
+      end
+    end
+  end
+end