diff --git a/.app_version b/.app_version index 0f1a7dfc..8570a3ae 100644 --- a/.app_version +++ b/.app_version @@ -1 +1 @@ -0.37.0 +0.37.2 diff --git a/CHANGELOG.md b/CHANGELOG.md index fe29bbb3..b0aff878 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,22 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +# [0.37.2] - 2026-01-03 + +## Fixed + +- Months are now correctly ordered (Jan-Dec) in the year-end digest chart instead of being sorted alphabetically. +- Time spent in a country and city is now calculated correctly for the year-end digest email. #2104 +- Updated Trix to fix a XSS vulnerability. #2102 +- Map v2 UI no longer blocks when Immich/Photoprism integration has a bad URL or is unreachable. Added 10-second timeout to photo API requests and improved error handling to prevent UI freezing during initial load. #2085 + +# [0.37.1] - 2025-12-30 + +## Fixed + +- The db migration preventing the app from starting. +- Raw data archive verifier now allows having points deleted from the db after archiving. + # [0.37.0] - 2025-12-30 ## Added diff --git a/Gemfile.lock b/Gemfile.lock index 7e4e9766..f17cc615 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -129,11 +129,11 @@ GEM rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) - chartkick (5.2.0) + chartkick (5.2.1) chunky_png (1.4.0) coderay (1.1.3) concurrent-ruby (1.3.6) - connection_pool (2.5.5) + connection_pool (3.0.2) crack (1.0.1) bigdecimal rexml @@ -215,7 +215,7 @@ GEM csv mini_mime (>= 1.0.0) multi_xml (>= 0.5.2) - i18n (1.14.7) + i18n (1.14.8) concurrent-ruby (~> 1.0) importmap-rails (2.2.2) actionpack (>= 6.0.0) @@ -227,7 +227,7 @@ GEM rdoc (>= 4.0.0) reline (>= 0.4.2) jmespath (1.6.2) - json (2.15.0) + json (2.18.0) json-jwt (1.17.0) activesupport (>= 4.2) aes_key_wrap @@ -273,7 +273,8 @@ GEM method_source (1.1.0) mini_mime (1.1.5) mini_portile2 (2.8.9) - minitest (5.26.2) + minitest (6.0.1) + prism (~> 1.5) msgpack (1.7.3) multi_json (1.15.0) multi_xml (0.8.0) @@ -356,7 +357,7 @@ GEM json yaml parallel (1.27.0) - parser (3.3.9.0) + parser (3.3.10.0) ast (~> 2.4.1) racc patience_diff (1.2.0) @@ -369,7 +370,7 @@ GEM pp (0.6.3) prettyprint prettyprint (0.2.0) - prism (1.5.1) + prism (1.7.0) prometheus_exporter (2.2.0) webrick pry (0.15.2) @@ -512,7 +513,7 @@ GEM rswag-ui (2.17.0) actionpack (>= 5.2, < 8.2) railties (>= 5.2, < 8.2) - rubocop (1.81.1) + rubocop (1.82.1) json (~> 2.3) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.1.0) @@ -520,20 +521,20 @@ GEM parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 2.9.3, < 3.0) - rubocop-ast (>= 1.47.1, < 2.0) + rubocop-ast (>= 1.48.0, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 4.0) - rubocop-ast (1.47.1) + rubocop-ast (1.49.0) parser (>= 3.3.7.2) - prism (~> 1.4) - rubocop-rails (2.33.4) + prism (~> 1.7) + rubocop-rails (2.34.2) activesupport (>= 4.2.0) lint_roller (~> 1.1) rack (>= 1.1) rubocop (>= 1.75.0, < 2.0) rubocop-ast (>= 1.44.0, < 2.0) ruby-progressbar (1.13.0) - rubyzip (3.2.0) + rubyzip (3.2.2) securerandom (0.4.1) selenium-webdriver (4.35.0) base64 (~> 0.2) @@ -613,7 +614,7 @@ GEM unicode (0.4.4.5) unicode-display_width (3.2.0) unicode-emoji (~> 4.1) - unicode-emoji (4.1.0) + unicode-emoji (4.2.0) uri (1.1.1) useragent (0.16.11) validate_url (1.0.15) diff --git a/app/javascript/controllers/maps/maplibre/data_loader.js b/app/javascript/controllers/maps/maplibre/data_loader.js index 165702e8..f4e266fb 100644 --- a/app/javascript/controllers/maps/maplibre/data_loader.js +++ b/app/javascript/controllers/maps/maplibre/data_loader.js @@ -56,22 +56,36 @@ export class DataLoader { } data.visitsGeoJSON = this.visitsToGeoJSON(data.visits) - // Fetch photos - try { - console.log('[Photos] Fetching photos from:', startDate, 'to', endDate) - data.photos = await this.api.fetchPhotos({ - start_at: startDate, - end_at: endDate - }) - console.log('[Photos] Fetched photos:', data.photos.length, 'photos') - console.log('[Photos] Sample photo:', data.photos[0]) - } catch (error) { - console.error('[Photos] Failed to fetch photos:', error) + // Fetch photos - only if photos layer is enabled and integration is configured + // Skip API call if photos are disabled to avoid blocking on failed integrations + if (this.settings.photosEnabled) { + try { + console.log('[Photos] Fetching photos from:', startDate, 'to', endDate) + // Use Promise.race to enforce a client-side timeout + const photosPromise = this.api.fetchPhotos({ + start_at: startDate, + end_at: endDate + }) + const timeoutPromise = new Promise((_, reject) => + setTimeout(() => reject(new Error('Photo fetch timeout')), 15000) // 15 second timeout + ) + + data.photos = await Promise.race([photosPromise, timeoutPromise]) + console.log('[Photos] Fetched photos:', data.photos.length, 'photos') + console.log('[Photos] Sample photo:', data.photos[0]) + } catch (error) { + console.warn('[Photos] Failed to fetch photos (non-blocking):', error.message) + data.photos = [] + } + } else { + console.log('[Photos] Photos layer disabled, skipping fetch') data.photos = [] } data.photosGeoJSON = this.photosToGeoJSON(data.photos) console.log('[Photos] Converted to GeoJSON:', data.photosGeoJSON.features.length, 'features') - console.log('[Photos] Sample feature:', data.photosGeoJSON.features[0]) + if (data.photosGeoJSON.features.length > 0) { + console.log('[Photos] Sample feature:', data.photosGeoJSON.features[0]) + } // Fetch areas try { diff --git a/app/services/immich/request_photos.rb b/app/services/immich/request_photos.rb index 0dfcbcd5..7018bbe3 100644 --- a/app/services/immich/request_photos.rb +++ b/app/services/immich/request_photos.rb @@ -31,7 +31,10 @@ class Immich::RequestPhotos while page <= max_pages response = JSON.parse( HTTParty.post( - immich_api_base_url, headers: headers, body: request_body(page) + immich_api_base_url, + headers: headers, + body: request_body(page), + timeout: 10 ).body ) Rails.logger.debug('==== IMMICH RESPONSE ====') @@ -46,6 +49,9 @@ class Immich::RequestPhotos end data.flatten + rescue HTTParty::Error, Net::OpenTimeout, Net::ReadTimeout => e + Rails.logger.error("Immich photo fetch failed: #{e.message}") + [] end def headers diff --git a/app/services/photoprism/request_photos.rb b/app/services/photoprism/request_photos.rb index 0f7fd93b..44005811 100644 --- a/app/services/photoprism/request_photos.rb +++ b/app/services/photoprism/request_photos.rb @@ -43,13 +43,17 @@ class Photoprism::RequestPhotos end data.flatten + rescue HTTParty::Error, Net::OpenTimeout, Net::ReadTimeout => e + Rails.logger.error("Photoprism photo fetch failed: #{e.message}") + [] end def fetch_page(offset) response = HTTParty.get( photoprism_api_base_url, headers: headers, - query: request_params(offset) + query: request_params(offset), + timeout: 10 ) if response.code != 200 diff --git a/app/services/points/raw_data/verifier.rb b/app/services/points/raw_data/verifier.rb index de42229f..2da7dfc2 100644 --- a/app/services/points/raw_data/verifier.rb +++ b/app/services/points/raw_data/verifier.rb @@ -110,18 +110,24 @@ module Points return { success: false, error: 'Point IDs checksum mismatch' } end - # 8. Verify all points still exist in database + # 8. Check which points still exist in database (informational only) existing_count = Point.where(id: point_ids).count if existing_count != point_ids.count - return { - success: false, - error: "Missing points in database: expected #{point_ids.count}, found #{existing_count}" - } + Rails.logger.info( + "Archive #{archive.id}: #{point_ids.count - existing_count} points no longer in database " \ + "(#{existing_count}/#{point_ids.count} remaining). This is OK if user deleted their data." + ) end - # 9. Verify archived raw_data matches current database raw_data - verification_result = verify_raw_data_matches(archived_data) - return verification_result unless verification_result[:success] + # 9. Verify archived raw_data matches current database raw_data (only for existing points) + if existing_count.positive? + verification_result = verify_raw_data_matches(archived_data) + return verification_result unless verification_result[:success] + else + Rails.logger.info( + "Archive #{archive.id}: Skipping raw_data verification - no points remain in database" + ) + end { success: true } end @@ -149,11 +155,18 @@ module Points point_ids_to_check = archived_data.keys.sample(100) end - mismatches = [] - found_points = 0 + # Filter to only check points that still exist in the database + existing_point_ids = Point.where(id: point_ids_to_check).pluck(:id) + + if existing_point_ids.empty? + # No points remain to verify, but that's OK + Rails.logger.info("No points remaining to verify raw_data matches") + return { success: true } + end - Point.where(id: point_ids_to_check).find_each do |point| - found_points += 1 + mismatches = [] + + Point.where(id: existing_point_ids).find_each do |point| archived_raw_data = archived_data[point.id] current_raw_data = point.raw_data @@ -167,14 +180,6 @@ module Points end end - # Check if we found all the points we were looking for - if found_points != point_ids_to_check.size - return { - success: false, - error: "Missing points during data verification: expected #{point_ids_to_check.size}, found #{found_points}" - } - end - if mismatches.any? return { success: false, diff --git a/app/services/users/digests/calculate_year.rb b/app/services/users/digests/calculate_year.rb index faea7d50..857effc9 100644 --- a/app/services/users/digests/calculate_year.rb +++ b/app/services/users/digests/calculate_year.rb @@ -88,35 +88,86 @@ module Users end def calculate_time_spent - country_time = Hash.new(0) + { + 'countries' => calculate_country_time_spent, + 'cities' => calculate_city_time_spent + } + end + + def calculate_country_time_spent + country_days = build_country_days_map + + # Convert days to minutes (days * 24 * 60) and return top 10 + country_days + .transform_values { |days| days.size * 24 * 60 } + .sort_by { |_, minutes| -minutes } + .first(10) + .map { |name, minutes| { 'name' => name, 'minutes' => minutes } } + end + + def build_country_days_map + year_points = fetch_year_points_with_country + country_days = Hash.new { |h, k| h[k] = Set.new } + + year_points.each do |point| + date = Time.zone.at(point.timestamp).to_date + country_days[point.country_name].add(date) + end + + country_days + end + + def fetch_year_points_with_country + start_of_year = Time.zone.local(year, 1, 1, 0, 0, 0) + end_of_year = start_of_year.end_of_year + + user.points + .without_raw_data + .where('timestamp >= ? AND timestamp <= ?', start_of_year.to_i, end_of_year.to_i) + .where.not(country_name: [nil, '']) + .select(:country_name, :timestamp) + end + + def calculate_city_time_spent + city_time = aggregate_city_time_from_monthly_stats + + city_time + .sort_by { |_, minutes| -minutes } + .first(10) + .map { |name, minutes| { 'name' => name, 'minutes' => minutes } } + end + + def aggregate_city_time_from_monthly_stats city_time = Hash.new(0) monthly_stats.each do |stat| - toponyms = stat.toponyms - next unless toponyms.is_a?(Array) - - toponyms.each do |toponym| - next unless toponym.is_a?(Hash) - - country = toponym['country'] - next unless toponym['cities'].is_a?(Array) - - toponym['cities'].each do |city| - next unless city.is_a?(Hash) - - stayed_for = city['stayed_for'].to_i - city_name = city['city'] - - country_time[country] += stayed_for if country.present? - city_time[city_name] += stayed_for if city_name.present? - end - end + process_stat_toponyms(stat, city_time) end - { - 'countries' => country_time.sort_by { |_, v| -v }.first(10).map { |name, minutes| { 'name' => name, 'minutes' => minutes } }, - 'cities' => city_time.sort_by { |_, v| -v }.first(10).map { |name, minutes| { 'name' => name, 'minutes' => minutes } } - } + city_time + end + + def process_stat_toponyms(stat, city_time) + toponyms = stat.toponyms + return unless toponyms.is_a?(Array) + + toponyms.each do |toponym| + process_toponym_cities(toponym, city_time) + end + end + + def process_toponym_cities(toponym, city_time) + return unless toponym.is_a?(Hash) + return unless toponym['cities'].is_a?(Array) + + toponym['cities'].each do |city| + next unless city.is_a?(Hash) + + stayed_for = city['stayed_for'].to_i + city_name = city['city'] + + city_time[city_name] += stayed_for if city_name.present? + end end def calculate_first_time_visits diff --git a/app/views/users/digests/public_year.html.erb b/app/views/users/digests/public_year.html.erb index ec07863b..4f56bbd9 100644 --- a/app/views/users/digests/public_year.html.erb +++ b/app/views/users/digests/public_year.html.erb @@ -79,7 +79,7 @@
<%= column_chart( - @digest.monthly_distances.sort.map { |month, distance_meters| + @digest.monthly_distances.sort_by { |month, _| month.to_i }.map { |month, distance_meters| [Date::ABBR_MONTHNAMES[month.to_i], Users::Digest.convert_distance(distance_meters.to_i, @distance_unit).round] }, height: '200px', diff --git a/app/views/users/digests/show.html.erb b/app/views/users/digests/show.html.erb index 3b69657b..e9ef7ed5 100644 --- a/app/views/users/digests/show.html.erb +++ b/app/views/users/digests/show.html.erb @@ -101,7 +101,7 @@
<%= column_chart( - @digest.monthly_distances.sort.map { |month, distance_meters| + @digest.monthly_distances.sort_by { |month, _| month.to_i }.map { |month, distance_meters| [Date::ABBR_MONTHNAMES[month.to_i], Users::Digest.convert_distance(distance_meters.to_i, @distance_unit).round] }, height: '250px', diff --git a/db/migrate/20250513164521_add_visited_countries_to_trips.rb b/db/migrate/20250513164521_add_visited_countries_to_trips.rb index 27797428..d40359d5 100644 --- a/db/migrate/20250513164521_add_visited_countries_to_trips.rb +++ b/db/migrate/20250513164521_add_visited_countries_to_trips.rb @@ -2,10 +2,8 @@ class AddVisitedCountriesToTrips < ActiveRecord::Migration[8.0] def change - # safety_assured do - execute <<-SQL + execute <<-SQL ALTER TABLE trips ADD COLUMN visited_countries JSONB DEFAULT '{}'::jsonb NOT NULL; - SQL - # end + SQL end end diff --git a/db/migrate/20250918215512_add_h3_hex_ids_to_stats.rb b/db/migrate/20250918215512_add_h3_hex_ids_to_stats.rb index fbb19073..18d56394 100644 --- a/db/migrate/20250918215512_add_h3_hex_ids_to_stats.rb +++ b/db/migrate/20250918215512_add_h3_hex_ids_to_stats.rb @@ -5,10 +5,8 @@ class AddH3HexIdsToStats < ActiveRecord::Migration[8.0] def change add_column :stats, :h3_hex_ids, :jsonb, default: {}, if_not_exists: true - # safety_assured do - add_index :stats, :h3_hex_ids, using: :gin, - where: "(h3_hex_ids IS NOT NULL AND h3_hex_ids != '{}'::jsonb)", - algorithm: :concurrently, if_not_exists: true - # end + add_index :stats, :h3_hex_ids, using: :gin, + where: "(h3_hex_ids IS NOT NULL AND h3_hex_ids != '{}'::jsonb)", + algorithm: :concurrently, if_not_exists: true end end diff --git a/db/migrate/20251227223614_change_digests_distance_to_bigint.rb b/db/migrate/20251227223614_change_digests_distance_to_bigint.rb index 9467fa39..a504614e 100644 --- a/db/migrate/20251227223614_change_digests_distance_to_bigint.rb +++ b/db/migrate/20251227223614_change_digests_distance_to_bigint.rb @@ -4,10 +4,10 @@ class ChangeDigestsDistanceToBigint < ActiveRecord::Migration[8.0] disable_ddl_transaction! def up - safety_assured { change_column :digests, :distance, :bigint, null: false, default: 0 } + change_column :digests, :distance, :bigint, null: false, default: 0 end def down - safety_assured { change_column :digests, :distance, :integer, null: false, default: 0 } + change_column :digests, :distance, :integer, null: false, default: 0 end end diff --git a/db/migrate/20251228163703_install_rails_pulse_tables.rb b/db/migrate/20251228163703_install_rails_pulse_tables.rb index 02548e36..ec63bdb6 100644 --- a/db/migrate/20251228163703_install_rails_pulse_tables.rb +++ b/db/migrate/20251228163703_install_rails_pulse_tables.rb @@ -3,21 +3,19 @@ class InstallRailsPulseTables < ActiveRecord::Migration[8.0] def change # Load and execute the Rails Pulse schema directly # This ensures the migration is always in sync with the schema file - schema_file = File.join(::Rails.root.to_s, "db/rails_pulse_schema.rb") + schema_file = Rails.root.join('db/rails_pulse_schema.rb').to_s - if File.exist?(schema_file) - say "Loading Rails Pulse schema from db/rails_pulse_schema.rb" + raise 'Rails Pulse schema file not found at db/rails_pulse_schema.rb' unless File.exist?(schema_file) - # Load the schema file to define RailsPulse::Schema - load schema_file + say 'Loading Rails Pulse schema from db/rails_pulse_schema.rb' - # Execute the schema in the context of this migration - RailsPulse::Schema.call(connection) + # Load the schema file to define RailsPulse::Schema + load schema_file - say "Rails Pulse tables created successfully" - say "The schema file db/rails_pulse_schema.rb remains as your single source of truth" - else - raise "Rails Pulse schema file not found at db/rails_pulse_schema.rb" - end + # Execute the schema in the context of this migration + RailsPulse::Schema.call(connection) + + say 'Rails Pulse tables created successfully' + say 'The schema file db/rails_pulse_schema.rb remains as your single source of truth' end -end \ No newline at end of file +end diff --git a/db/migrate/20260103114630_add_indexes_to_points_for_stats_query.rb b/db/migrate/20260103114630_add_indexes_to_points_for_stats_query.rb new file mode 100644 index 00000000..f91edb61 --- /dev/null +++ b/db/migrate/20260103114630_add_indexes_to_points_for_stats_query.rb @@ -0,0 +1,21 @@ +class AddIndexesToPointsForStatsQuery < ActiveRecord::Migration[8.0] + disable_ddl_transaction! + + def change + # Index for counting reverse geocoded points + # This speeds up: COUNT(reverse_geocoded_at) + add_index :points, [:user_id, :reverse_geocoded_at], + where: "reverse_geocoded_at IS NOT NULL", + algorithm: :concurrently, + if_not_exists: true, + name: 'index_points_on_user_id_and_reverse_geocoded_at' + + # Index for finding points with empty geodata + # This speeds up: COUNT(CASE WHEN geodata = '{}'::jsonb THEN 1 END) + add_index :points, [:user_id, :geodata], + where: "geodata = '{}'::jsonb", + algorithm: :concurrently, + if_not_exists: true, + name: 'index_points_on_user_id_and_empty_geodata' + end +end diff --git a/db/schema.rb b/db/schema.rb index a5ddaf50..b472e90b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_12_28_163703) do +ActiveRecord::Schema[8.0].define(version: 2026_01_03_114630) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "postgis" @@ -260,6 +260,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_12_28_163703) do t.index ["track_id"], name: "index_points_on_track_id" t.index ["user_id", "city"], name: "idx_points_user_city" t.index ["user_id", "country_name"], name: "idx_points_user_country_name" + t.index ["user_id", "geodata"], name: "index_points_on_user_id_and_empty_geodata", where: "(geodata = '{}'::jsonb)" t.index ["user_id", "reverse_geocoded_at"], name: "index_points_on_user_id_and_reverse_geocoded_at", where: "(reverse_geocoded_at IS NOT NULL)" t.index ["user_id", "timestamp", "track_id"], name: "idx_points_track_generation" t.index ["user_id", "timestamp"], name: "idx_points_user_visit_null_timestamp", where: "(visit_id IS NULL)" diff --git a/package-lock.json b/package-lock.json index dccd4527..82e72055 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "leaflet": "^1.9.4", "maplibre-gl": "^5.13.0", "postcss": "^8.4.49", - "trix": "^2.1.15" + "trix": "^2.1.16" }, "devDependencies": { "@playwright/test": "^1.56.1", @@ -575,12 +575,14 @@ "license": "ISC" }, "node_modules/trix": { - "version": "2.1.15", - "resolved": "https://registry.npmjs.org/trix/-/trix-2.1.15.tgz", - "integrity": "sha512-LoaXWczdTUV8+3Box92B9b1iaDVbxD14dYemZRxi3PwY+AuDm97BUJV2aHLBUFPuDABhxp0wzcbf0CxHCVmXiw==", - "license": "MIT", + "version": "2.1.16", + "resolved": "https://registry.npmjs.org/trix/-/trix-2.1.16.tgz", + "integrity": "sha512-XtZgWI+oBvLzX7CWnkIf+ZWC+chL+YG/TkY43iMTV0Zl+CJjn18B1GJUCEWJ8qgfpcyMBuysnNAfPWiv2sV14A==", "dependencies": { "dompurify": "^3.2.5" + }, + "engines": { + "node": ">= 18" } }, "node_modules/undici-types": { @@ -986,9 +988,9 @@ "integrity": "sha512-gRa9gwYU3ECmQYv3lslts5hxuIa90veaEcxDYuu3QGOIAEM2mOZkVHp48ANJuu1CURtRdHKUBY5Lm1tHV+sD4g==" }, "trix": { - "version": "2.1.15", - "resolved": "https://registry.npmjs.org/trix/-/trix-2.1.15.tgz", - "integrity": "sha512-LoaXWczdTUV8+3Box92B9b1iaDVbxD14dYemZRxi3PwY+AuDm97BUJV2aHLBUFPuDABhxp0wzcbf0CxHCVmXiw==", + "version": "2.1.16", + "resolved": "https://registry.npmjs.org/trix/-/trix-2.1.16.tgz", + "integrity": "sha512-XtZgWI+oBvLzX7CWnkIf+ZWC+chL+YG/TkY43iMTV0Zl+CJjn18B1GJUCEWJ8qgfpcyMBuysnNAfPWiv2sV14A==", "requires": { "dompurify": "^3.2.5" } diff --git a/package.json b/package.json index 9946febf..cadd2963 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "leaflet": "^1.9.4", "maplibre-gl": "^5.13.0", "postcss": "^8.4.49", - "trix": "^2.1.15" + "trix": "^2.1.16" }, "engines": { "node": "18.17.1", diff --git a/spec/services/users/digests/calculate_year_spec.rb b/spec/services/users/digests/calculate_year_spec.rb index fe01120f..cef3ff98 100644 --- a/spec/services/users/digests/calculate_year_spec.rb +++ b/spec/services/users/digests/calculate_year_spec.rb @@ -77,18 +77,78 @@ RSpec.describe Users::Digests::CalculateYear do end it 'calculates time spent by location' do + # Create points to enable country time calculation based on unique days + jan_1 = Time.zone.local(2024, 1, 1, 10, 0, 0).to_i + jan_2 = Time.zone.local(2024, 1, 2, 10, 0, 0).to_i + feb_1 = Time.zone.local(2024, 2, 1, 10, 0, 0).to_i + + create(:point, user: user, timestamp: jan_1, country_name: 'Germany', city: 'Berlin') + create(:point, user: user, timestamp: jan_2, country_name: 'Germany', city: 'Munich') + create(:point, user: user, timestamp: feb_1, country_name: 'France', city: 'Paris') + countries = calculate_digest.time_spent_by_location['countries'] cities = calculate_digest.time_spent_by_location['cities'] - expect(countries.first['name']).to eq('Germany') - expect(countries.first['minutes']).to eq(720) # 480 + 240 + # Countries: based on unique days (2 days in Germany, 1 day in France) + germany_country = countries.find { |c| c['name'] == 'Germany' } + expect(germany_country['minutes']).to eq(2 * 24 * 60) # 2 days = 2880 minutes + + # Cities: based on stayed_for from monthly stats (sum across months) expect(cities.first['name']).to eq('Berlin') + expect(cities.first['minutes']).to eq(480) end it 'calculates all time stats' do expect(calculate_digest.all_time_stats['total_distance']).to eq('125000') end + context 'when user visits same country across multiple months' do + it 'does not double-count days' do + # Create a user who was in Germany for 10 days in March and 10 days in July + # If we summed the stayed_for values from cities, we might get inflated numbers + # The fix counts unique days to prevent exceeding 365 days per year + mar_start = Time.zone.local(2024, 3, 1, 10, 0, 0).to_i + jul_start = Time.zone.local(2024, 7, 1, 10, 0, 0).to_i + + # Create 10 days of points in March + 10.times do |i| + timestamp = mar_start + (i * 24 * 60 * 60) + create(:point, user: user, timestamp: timestamp, country_name: 'Germany', city: 'Berlin') + end + + # Create 10 days of points in July + 10.times do |i| + timestamp = jul_start + (i * 24 * 60 * 60) + create(:point, user: user, timestamp: timestamp, country_name: 'Germany', city: 'Munich') + end + + # Create the monthly stats (simulating what would be created by the stats calculation) + create(:stat, user: user, year: 2024, month: 3, distance: 10_000, toponyms: [ + { 'country' => 'Germany', 'cities' => [ + { 'city' => 'Berlin', 'stayed_for' => 14_400 } # 10 days in minutes + ] } + ]) + + create(:stat, user: user, year: 2024, month: 7, distance: 15_000, toponyms: [ + { 'country' => 'Germany', 'cities' => [ + { 'city' => 'Munich', 'stayed_for' => 14_400 } # 10 days in minutes + ] } + ]) + + digest = calculate_digest + countries = digest.time_spent_by_location['countries'] + germany = countries.find { |c| c['name'] == 'Germany' } + + # Should be 20 days total (10 unique days in Mar + 10 unique days in Jul) + expected_minutes = 20 * 24 * 60 # 28,800 minutes + expect(germany['minutes']).to eq(expected_minutes) + + # Verify this is less than 365 days (the bug would cause inflated numbers) + total_days = germany['minutes'] / (24 * 60) + expect(total_days).to be <= 365 + end + end + context 'when digest already exists' do let!(:existing_digest) do create(:users_digest, user: user, year: 2024, period_type: :yearly, distance: 10_000)