kevinsivic/dawarich
1
0
Fork 0
mirror of https://github.com/Freika/dawarich.git synced 2026-01-10 01:01:39 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

Update digest calculation to use actual time spent in countries based on consecutive points, avoiding double-counting days when crossing borders.

Browse source
This commit is contained in:
Eugene Burmakin 2026-01-04 12:37:50 +01:00
parent 24cbabf3b7
commit a4a4e5386e
10 changed files with 230 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
app/controllers/users/digests_controller.rb
View file

@ -6,7 +6,7 @@ class Users::DigestsController < ApplicationController
before_action :authenticate_user!
before_action :authenticate_active_user!, only: [:create]
before_action :set_digest, only: [:show]
before_action :set_digest, only: %i[show destroy]
def index
@digests = current_user.digests.yearly.order(year: :desc)
@ -30,6 +30,12 @@ class Users::DigestsController < ApplicationController
end
end
def destroy
year = @digest.year
@digest.destroy!
redirect_to users_digests_path, notice: "Year-end digest for #{year} has been deleted", status: :see_other
end
private
def set_digest

13
app/models/users/digest.rb
View file

@ -132,6 +132,19 @@ class Users::Digest < ApplicationRecord
(all_time_stats['total_distance'] || 0).to_i
end
def total_tracked_minutes
top_countries_by_time.sum { |country| country['minutes'].to_i }
end
def total_tracked_days
(total_tracked_minutes / 1440.0).round(1)
end
def untracked_days
days_in_year = Date.leap?(year) ? 366 : 365
[days_in_year - total_tracked_days, 0].max.round(1)
end
def distance_km
distance.to_f / 1000
end

32
app/services/users/digests/calculate_year.rb
View file

@ -3,6 +3,8 @@
module Users
module Digests
class CalculateYear
MAX_COUNTRY_GAP_SECONDS = 60 * 60 # 60 minutes
def initialize(user_id, year)
@user = ::User.find(user_id)
@year = year.to_i
@ -50,7 +52,7 @@ module Users
next unless toponym.is_a?(Hash)
country = toponym['country']
next unless country.present?
next if country.blank?
if toponym['cities'].is_a?(Array)
toponym['cities'].each do |city|
@ -95,29 +97,32 @@ module Users
end
def calculate_country_time_spent
country_days = build_country_days_map
country_minutes = calculate_actual_country_minutes
# Convert days to minutes (days * 24 * 60) and return top 10
country_days
.transform_values { |days| days.size * 24 * 60 }
country_minutes
.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 }
def calculate_actual_country_minutes
points = fetch_year_points_with_country_ordered
country_minutes = Hash.new(0)
year_points.each do |point|
date = Time.zone.at(point.timestamp).to_date
country_days[point.country_name].add(date)
points.each_cons(2) do |point_a, point_b|
next if point_a.country_name != point_b.country_name
gap_seconds = point_b.timestamp - point_a.timestamp
next if gap_seconds > MAX_COUNTRY_GAP_SECONDS
next if gap_seconds <= 0
country_minutes[point_a.country_name] += (gap_seconds / 60)
end
country_days
country_minutes
end
def fetch_year_points_with_country
def fetch_year_points_with_country_ordered
start_of_year = Time.zone.local(year, 1, 1, 0, 0, 0)
end_of_year = start_of_year.end_of_year
@ -126,6 +131,7 @@ module Users
.where('timestamp >= ? AND timestamp <= ?', start_of_year.to_i, end_of_year.to_i)
.where.not(country_name: [nil, ''])
.select(:country_name, :timestamp)
.order(timestamp: :asc)
end
def calculate_city_time_spent

19
app/views/users/digests/show.html.erb
View file

@ -142,6 +142,19 @@
<span class="text-gray-600"><%= format_time_spent(country['minutes']) %></span>
</div>
<% end %>
<% if @digest.untracked_days > 0 %>
<div class="flex justify-between items-center p-3 bg-base-100 rounded-lg border-2 border-dashed border-gray-200">
<div class="flex items-center gap-3">
<span class="badge badge-lg badge-ghost">?</span>
<span class="text-gray-500 italic">No tracking data</span>
</div>
<span class="text-gray-500"><%= pluralize(@digest.untracked_days.round, 'day') %></span>
</div>
<p class="text-sm text-gray-500 mt-2 flex items-center justify-center gap-2">
<%= icon 'lightbulb' %> Track more in <%= @digest.year + 1 %> to see a fuller picture of your travels!
</p>
<% end %>
</div>
</div>
</div>
@ -214,6 +227,12 @@
<button class="btn btn-outline" onclick="sharing_modal.showModal()">
<%= icon 'share' %> Share
</button>
<%= button_to users_digest_path(year: @digest.year),
method: :delete,
class: 'btn btn-outline btn-error',
data: { turbo_confirm: "Are you sure you want to delete the #{@digest.year} digest? This cannot be undone." } do %>
<%= icon 'trash-2' %> Delete
<% end %>
</div>
</div>

13
app/views/users/digests_mailer/year_end_digest.html.erb
View file

@ -250,13 +250,24 @@
<div class="stat-card">
<div class="stat-label">Where You Spent the Most Time</div>
<ul class="location-list">
<% @digest.top_countries_by_time.take(3).each do |country| %>
<% @digest.top_countries_by_time.take(5).each do |country| %>
<li>
<span><%= country_flag(country['name']) %> <%= country['name'] %></span>
<span><%= format_time_spent(country['minutes']) %></span>
</li>
<% end %>
<% if @digest.untracked_days > 0 %>
<li style="border-top: 2px dashed #e2e8f0; padding-top: 12px; margin-top: 4px;">
<span style="color: #94a3b8; font-style: italic;">No tracking data</span>
<span style="color: #94a3b8;"><%= pluralize(@digest.untracked_days.round, 'day') %></span>
</li>
<% end %>
</ul>
<% if @digest.untracked_days > 0 %>
<p style="color: #64748b; font-size: 13px; margin-top: 12px;">
💡 Track more in <%= @digest.year + 1 %> to see a fuller picture of your travels!
</p>
<% end %>
</div>
<% end %>

4
config/routes.rb
View file

@ -101,8 +101,8 @@ Rails.application.routes.draw do
# User digests routes (yearly/monthly digest reports)
scope module: 'users' do
resources :digests, only: %i[index create], param: :year, as: :users_digests
get 'digests/:year', to: 'digests#show', as: :users_digest, constraints: { year: /\d{4}/ }
resources :digests, only: %i[index create show destroy], param: :year, as: :users_digests,
constraints: { year: /\d{4}/ }
end
get 'shared/digest/:uuid', to: 'shared/digests#show', as: :shared_users_digest
patch 'digests/:year/sharing',

1
db/schema.rb generated
View file

@ -522,6 +522,7 @@ ActiveRecord::Schema[8.0].define(version: 2026_01_03_114630) do
add_foreign_key "notifications", "users"
add_foreign_key "place_visits", "places"
add_foreign_key "place_visits", "visits"
add_foreign_key "points", "points_raw_data_archives", column: "raw_data_archive_id", name: "fk_rails_points_raw_data_archives", on_delete: :nullify, validate: false
add_foreign_key "points", "points_raw_data_archives", column: "raw_data_archive_id", on_delete: :nullify
add_foreign_key "points", "users"
add_foreign_key "points", "visits"

43
spec/requests/users/digests_spec.rb
View file

@ -27,6 +27,14 @@ RSpec.describe '/digests', type: :request do
expect(response.status).to eq(302)
end
end
describe 'DELETE /destroy' do
it 'redirects to the sign in page' do
delete users_digest_url(year: 2024)
expect(response).to redirect_to(new_user_session_path)
end
end
end
context 'when user is signed in' do
@ -137,5 +145,40 @@ RSpec.describe '/digests', type: :request do
end
end
end
describe 'DELETE /destroy' do
let!(:digest) { create(:users_digest, user:, year: 2024) }
it 'deletes the digest' do
expect do
delete users_digest_url(year: 2024)
end.to change(Users::Digest, :count).by(-1)
end
it 'redirects with success notice' do
delete users_digest_url(year: 2024)
expect(response).to redirect_to(users_digests_path)
expect(flash[:notice]).to eq('Year-end digest for 2024 has been deleted')
end
it 'returns not found for non-existent digest' do
delete users_digest_url(year: 2020)
expect(response).to redirect_to(users_digests_path)
expect(flash[:alert]).to eq('Digest not found')
end
it 'cannot delete another user digest' do
other_user = create(:user)
other_digest = create(:users_digest, user: other_user, year: 2023)
delete users_digest_url(year: 2023)
expect(response).to redirect_to(users_digests_path)
expect(flash[:alert]).to eq('Digest not found')
expect(other_digest.reload).to be_present
end
end
end
end

6
spec/services/points/raw_data/verifier_spec.rb
View file

@ -61,16 +61,18 @@ RSpec.describe Points::RawData::Verifier do
end.not_to change { archive.reload.verified_at }
end
it 'detects deleted points' do
it 'still verifies successfully when points are deleted from database' do
# Force archive creation first
archive_id = archive.id
# Then delete one point from database
points.first.destroy
# Verification should still succeed - deleted points are acceptable
# (users should be able to delete their data without failing archive verification)
expect do
verifier.verify_specific_archive(archive_id)
end.not_to change { archive.reload.verified_at }
end.to change { archive.reload.verified_at }.from(nil)
end
it 'detects raw_data mismatch between archive and database' do

140
spec/services/users/digests/calculate_year_spec.rb
View file

@ -76,22 +76,28 @@ RSpec.describe Users::Digests::CalculateYear do
expect(calculate_digest.monthly_distances['3']).to eq('0') # Missing month
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
it 'calculates time spent by location using actual minutes between consecutive points' do
# Create points with specific gaps to test actual minute calculation
jan_1_10am = Time.zone.local(2024, 1, 1, 10, 0, 0).to_i
jan_1_11am = Time.zone.local(2024, 1, 1, 11, 0, 0).to_i # 60 min later
jan_1_12pm = Time.zone.local(2024, 1, 1, 12, 0, 0).to_i # 60 min later
feb_1_10am = 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')
create(:point, user: user, timestamp: jan_1_10am, country_name: 'Germany', city: 'Berlin')
create(:point, user: user, timestamp: jan_1_11am, country_name: 'Germany', city: 'Berlin')
create(:point, user: user, timestamp: jan_1_12pm, country_name: 'Germany', city: 'Munich')
create(:point, user: user, timestamp: feb_1_10am, country_name: 'France', city: 'Paris')
countries = calculate_digest.time_spent_by_location['countries']
cities = calculate_digest.time_spent_by_location['cities']
# Countries: based on unique days (2 days in Germany, 1 day in France)
# Germany: 60 min (10am->11am) + 60 min (11am->12pm) = 120 minutes
germany_country = countries.find { |c| c['name'] == 'Germany' }
expect(germany_country['minutes']).to eq(2 * 24 * 60) # 2 days = 2880 minutes
expect(germany_country['minutes']).to eq(120)
# France: only 1 point, so 0 minutes (no consecutive pair)
france_country = countries.find { |c| c['name'] == 'France' }
expect(france_country).to be_nil # No time counted for single point
# Cities: based on stayed_for from monthly stats (sum across months)
expect(cities.first['name']).to eq('Berlin')
@ -103,35 +109,37 @@ RSpec.describe Users::Digests::CalculateYear do
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
it 'calculates actual minutes from consecutive point pairs' do
# Create hourly points across multiple days in March and July
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')
# Create 3 days of hourly points in March (3 points per day = 2 gaps of 60 min each)
3.times do |day|
3.times do |hour|
timestamp = mar_start + (day * 24 * 60 * 60) + (hour * 60 * 60)
create(:point, user: user, timestamp: timestamp, country_name: 'Germany', city: 'Berlin')
end
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')
# Create 3 days of hourly points in July
3.times do |day|
3.times do |hour|
timestamp = jul_start + (day * 24 * 60 * 60) + (hour * 60 * 60)
create(:point, user: user, timestamp: timestamp, country_name: 'Germany', city: 'Munich')
end
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
{ 'city' => 'Berlin', 'stayed_for' => 14_400 }
] }
])
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
{ 'city' => 'Munich', 'stayed_for' => 14_400 }
] }
])
@ -139,13 +147,85 @@ RSpec.describe Users::Digests::CalculateYear do
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)
# Each day: 2 gaps of 60 minutes = 120 minutes
# 6 days total (3 in March + 3 in July) = 720 minutes
# But gaps between days are > 60 min threshold, so not counted
expect(germany['minutes']).to eq(6 * 2 * 60)
# 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
# Total should be much less than 365 days
total_hours = germany['minutes'] / 60.0
expect(total_hours).to eq(12) # 12 hours of tracked time
end
end
context 'when there are large gaps between points' do
it 'does not count time during gaps exceeding 60 minute threshold' do
point_1 = Time.zone.local(2024, 1, 1, 10, 0, 0).to_i
point_2 = Time.zone.local(2024, 1, 1, 12, 0, 0).to_i # 2 hours later (> 1 hour threshold)
point_3 = Time.zone.local(2024, 1, 1, 13, 0, 0).to_i # 1 hour after point_2
create(:point, user: user, timestamp: point_1, country_name: 'Germany')
create(:point, user: user, timestamp: point_2, country_name: 'Germany')
create(:point, user: user, timestamp: point_3, country_name: 'Germany')
digest = calculate_digest
germany = digest.time_spent_by_location['countries'].find { |c| c['name'] == 'Germany' }
# Only point_2 -> point_3 gap (60 min) should be counted
# point_1 -> point_2 gap (120 min) exceeds threshold
expect(germany['minutes']).to eq(60)
end
end
context 'when transitioning between countries' do
it 'does not count transition time' do
point_1 = Time.zone.local(2024, 1, 1, 10, 0, 0).to_i
point_2 = Time.zone.local(2024, 1, 1, 10, 30, 0).to_i # In Germany
point_3 = Time.zone.local(2024, 1, 1, 11, 0, 0).to_i # Now in France
point_4 = Time.zone.local(2024, 1, 1, 11, 30, 0).to_i # Still in France
create(:point, user: user, timestamp: point_1, country_name: 'Germany')
create(:point, user: user, timestamp: point_2, country_name: 'Germany')
create(:point, user: user, timestamp: point_3, country_name: 'France')
create(:point, user: user, timestamp: point_4, country_name: 'France')
digest = calculate_digest
countries = digest.time_spent_by_location['countries']
germany = countries.find { |c| c['name'] == 'Germany' }
france = countries.find { |c| c['name'] == 'France' }
expect(germany['minutes']).to eq(30) # point_1 -> point_2
expect(france['minutes']).to eq(30) # point_3 -> point_4
# Transition time (point_2 -> point_3) is NOT counted
end
end
context 'when visiting multiple countries on same day' do
it 'does not exceed the actual time in the day' do
# This tests the fix for the original bug: border crossing should not count double
jan_1_8am = Time.zone.local(2024, 1, 1, 8, 0, 0).to_i
jan_1_9am = Time.zone.local(2024, 1, 1, 9, 0, 0).to_i
jan_1_10am = Time.zone.local(2024, 1, 1, 10, 0, 0).to_i # Border crossing
jan_1_11am = Time.zone.local(2024, 1, 1, 11, 0, 0).to_i
create(:point, user: user, timestamp: jan_1_8am, country_name: 'France')
create(:point, user: user, timestamp: jan_1_9am, country_name: 'France')
create(:point, user: user, timestamp: jan_1_10am, country_name: 'Germany')
create(:point, user: user, timestamp: jan_1_11am, country_name: 'Germany')
digest = calculate_digest
countries = digest.time_spent_by_location['countries']
france = countries.find { |c| c['name'] == 'France' }
germany = countries.find { |c| c['name'] == 'Germany' }
# France: 60 min (8am->9am)
# Germany: 60 min (10am->11am)
# Total: 120 min (2 hours) - NOT 2 days (2880 min) as the bug would have caused
expect(france['minutes']).to eq(60)
expect(germany['minutes']).to eq(60)
expect(france['minutes'] + germany['minutes']).to eq(120)
end
end