From 88ae7c43c62b74fed664fab2b9f527f7fe5a682d Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 4 Jan 2026 17:12:39 +0100 Subject: [PATCH] Adjust calculate_duration_in_minutes to only count continuous presence within cities, excluding long gaps. --- app/controllers/users/digests_controller.rb | 2 +- app/services/countries_and_cities.rb | 13 +++++- app/services/users/export_data/points.rb | 16 +++---- spec/services/countries_and_cities_spec.rb | 52 +++++++++++++++++++++ spec/services/stats/calculate_month_spec.rb | 15 +++++- 5 files changed, 86 insertions(+), 12 deletions(-) diff --git a/app/controllers/users/digests_controller.rb b/app/controllers/users/digests_controller.rb index abbaa883..f289fbc6 100644 --- a/app/controllers/users/digests_controller.rb +++ b/app/controllers/users/digests_controller.rb @@ -48,7 +48,7 @@ class Users::DigestsController < ApplicationController tracked_years = current_user.stats.select(:year).distinct.pluck(:year) existing_digests = current_user.digests.yearly.pluck(:year) - (tracked_years - existing_digests).sort.reverse + (tracked_years - existing_digests - [Time.current.year]).sort.reverse end def valid_year?(year) diff --git a/app/services/countries_and_cities.rb b/app/services/countries_and_cities.rb index 3d3ff2f4..b08d20f3 100644 --- a/app/services/countries_and_cities.rb +++ b/app/services/countries_and_cities.rb @@ -49,6 +49,17 @@ class CountriesAndCities end def calculate_duration_in_minutes(timestamps) - ((timestamps.max - timestamps.min).to_i / 60) + return 0 if timestamps.size < 2 + + sorted = timestamps.sort + total_minutes = 0 + gap_threshold_seconds = ::MIN_MINUTES_SPENT_IN_CITY * 60 + + sorted.each_cons(2) do |prev_ts, curr_ts| + interval_seconds = curr_ts - prev_ts + total_minutes += (interval_seconds / 60) if interval_seconds < gap_threshold_seconds + end + + total_minutes end end diff --git a/app/services/users/export_data/points.rb b/app/services/users/export_data/points.rb index cf224afa..62a20b5e 100644 --- a/app/services/users/export_data/points.rb +++ b/app/services/users/export_data/points.rb @@ -35,7 +35,7 @@ class Users::ExportData::Points output_file.write('[') - user.points.find_in_batches(batch_size: BATCH_SIZE).with_index do |batch, batch_index| + user.points.find_in_batches(batch_size: BATCH_SIZE).with_index do |batch, _batch_index| batch_sql = build_batch_query(batch.map(&:id)) result = ActiveRecord::Base.connection.exec_query(batch_sql, 'Points Export Batch') @@ -188,13 +188,13 @@ class Users::ExportData::Points } end - if row['visit_name'] - point_hash['visit_reference'] = { - 'name' => row['visit_name'], - 'started_at' => row['visit_started_at'], - 'ended_at' => row['visit_ended_at'] - } - end + return unless row['visit_name'] + + point_hash['visit_reference'] = { + 'name' => row['visit_name'], + 'started_at' => row['visit_started_at'], + 'ended_at' => row['visit_ended_at'] + } end def log_progress(processed, total) diff --git a/spec/services/countries_and_cities_spec.rb b/spec/services/countries_and_cities_spec.rb index 636823e5..5e58d67f 100644 --- a/spec/services/countries_and_cities_spec.rb +++ b/spec/services/countries_and_cities_spec.rb @@ -79,6 +79,58 @@ RSpec.describe CountriesAndCities do ) end end + + context 'when points have a gap larger than threshold (passing through)' do + let(:points) do + [ + # User in Berlin at 9:00, leaves, returns at 11:00 + create(:point, city: 'Berlin', country: 'Germany', timestamp:), + create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 15.minutes), + # 105-minute gap here (user left the city) + create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 120.minutes), + create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 130.minutes) + ] + end + + it 'only counts time between consecutive points within threshold' do + # Old logic would count 130 minutes (span from first to last) + # New logic counts: 15 min (0->15) + 10 min (120->130) = 25 minutes + # Since 25 < 60, Berlin should be filtered out + expect(countries_and_cities).to eq( + [ + CountriesAndCities::CountryData.new( + country: 'Germany', + cities: [] + ) + ] + ) + end + end + + context 'when points span a long time but have continuous presence' do + let(:points) do + # Points every 30 minutes for 2.5 hours = continuous presence + (0..5).map do |i| + create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + (i * 30).minutes) + end + end + + it 'counts the full duration when all intervals are within threshold' do + # 5 intervals of 30 minutes each = 150 minutes total + expect(countries_and_cities).to eq( + [ + CountriesAndCities::CountryData.new( + country: 'Germany', + cities: [ + CountriesAndCities::CityData.new( + city: 'Berlin', points: 6, timestamp: (timestamp + 150.minutes).to_i, stayed_for: 150 + ) + ] + ) + ] + ) + end + end end end end diff --git a/spec/services/stats/calculate_month_spec.rb b/spec/services/stats/calculate_month_spec.rb index dbb1928a..12ea5f9f 100644 --- a/spec/services/stats/calculate_month_spec.rb +++ b/spec/services/stats/calculate_month_spec.rb @@ -155,10 +155,14 @@ RSpec.describe Stats::CalculateMonth do context 'when user visited multiple cities with mixed durations' do let!(:mixed_points) do [ - # Berlin: 70 minutes (should be included) + # Berlin: 70 minutes with continuous presence (should be included) + # Points every 35 minutes: 0, 35, 70 = 70 min total create(:point, user:, import:, timestamp: timestamp_base, city: 'Berlin', country_name: 'Germany', lonlat: 'POINT(13.404954 52.520008)'), + create(:point, user:, import:, timestamp: timestamp_base + 35.minutes, + city: 'Berlin', country_name: 'Germany', + lonlat: 'POINT(13.404954 52.520008)'), create(:point, user:, import:, timestamp: timestamp_base + 70.minutes, city: 'Berlin', country_name: 'Germany', lonlat: 'POINT(13.404954 52.520008)'), @@ -171,10 +175,17 @@ RSpec.describe Stats::CalculateMonth do city: 'Prague', country_name: 'Czech Republic', lonlat: 'POINT(14.4378 50.0755)'), - # Vienna: 90 minutes (should be included) + # Vienna: 90 minutes with continuous presence (should be included) + # Points every 30 minutes: 150, 180, 210, 240 = 90 min total create(:point, user:, import:, timestamp: timestamp_base + 150.minutes, city: 'Vienna', country_name: 'Austria', lonlat: 'POINT(16.3738 48.2082)'), + create(:point, user:, import:, timestamp: timestamp_base + 180.minutes, + city: 'Vienna', country_name: 'Austria', + lonlat: 'POINT(16.3738 48.2082)'), + create(:point, user:, import:, timestamp: timestamp_base + 210.minutes, + city: 'Vienna', country_name: 'Austria', + lonlat: 'POINT(16.3738 48.2082)'), create(:point, user:, import:, timestamp: timestamp_base + 240.minutes, city: 'Vienna', country_name: 'Austria', lonlat: 'POINT(16.3738 48.2082)')