Store distance in meters in the database and convert to user's preferred unit on the fly.

This commit is contained in:
Eugene Burmakin 2025-07-08 18:10:10 +02:00
parent 81eb759fb8
commit f1720b859b
23 changed files with 194 additions and 148 deletions

View file

@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Don't check for new version in production. - Don't check for new version in production.
- Area popup styles are now more consistent. - Area popup styles are now more consistent.
- Notification about Photon API load is now disabled. - Notification about Photon API load is now disabled.
- All distance values are now stored in the database in meters. Conversion to user's preferred unit is done on the fly.
## Fixed ## Fixed

View file

@ -35,22 +35,16 @@ class MapController < ApplicationController
end end
def calculate_distance def calculate_distance
distance = 0 total_distance_meters = 0
@coordinates.each_cons(2) do @coordinates.each_cons(2) do
distance += Geocoder::Calculations.distance_between( distance_km = Geocoder::Calculations.distance_between(
[_1[0], _1[1]], [_2[0], _2[1]], units: current_user.safe_settings.distance_unit.to_sym [_1[0], _1[1]], [_2[0], _2[1]], units: :km
) )
total_distance_meters += distance_km * 1000 # Convert km to meters
end end
distance_in_meters = case current_user.safe_settings.distance_unit.to_s total_distance_meters.round
when 'mi'
distance * 1609.344 # miles to meters
else
distance * 1000 # km to meters
end
distance_in_meters.round
end end
def parsed_start_at def parsed_start_at

View file

@ -76,8 +76,9 @@ module ApplicationHelper
end end
def year_distance_stat(year, user) def year_distance_stat(year, user)
# In km or miles, depending on the user.safe_settings.distance_unit # Distance is now stored in meters, convert to user's preferred unit for display
Stat.year_distance(year, user).sum { _1[1] } total_distance_meters = Stat.year_distance(year, user).sum { _1[1] }
Stat.convert_distance(total_distance_meters, user.safe_settings.distance_unit)
end end
def past?(year, month) def past?(year, month)
@ -98,10 +99,13 @@ module ApplicationHelper
current_user&.theme == 'light' ? 'light' : 'dark' current_user&.theme == 'light' ? 'light' : 'dark'
end end
def sidebar_distance(distance) def sidebar_distance(distance_meters)
return unless distance return unless distance_meters
"#{distance} #{current_user.safe_settings.distance_unit}" # Convert from stored meters to user's preferred unit for display
user_unit = current_user.safe_settings.distance_unit
converted_distance = Stat.convert_distance(distance_meters, user_unit)
"#{converted_distance.round(2)} #{user_unit}"
end end
def sidebar_points(points) def sidebar_points(points)

View file

@ -9,8 +9,8 @@ module Calculateable
end end
def calculate_distance def calculate_distance
calculated_distance = calculate_distance_from_coordinates calculated_distance_meters = calculate_distance_from_coordinates
self.distance = convert_distance_for_storage(calculated_distance) self.distance = convert_distance_for_storage(calculated_distance_meters)
end end
def recalculate_path! def recalculate_path!
@ -44,16 +44,14 @@ module Calculateable
self.original_path = updated_path if respond_to?(:original_path=) self.original_path = updated_path if respond_to?(:original_path=)
end end
def user_distance_unit
user.safe_settings.distance_unit
end
def calculate_distance_from_coordinates def calculate_distance_from_coordinates
Point.total_distance(points, user_distance_unit) # Always calculate in meters for consistent storage
Point.total_distance(points, :m)
end end
def convert_distance_for_storage(calculated_distance) def convert_distance_for_storage(calculated_distance_meters)
calculated_distance.round(2) # Store as integer meters for consistency
calculated_distance_meters.round
end end
def track_model? def track_model?

View file

@ -0,0 +1,75 @@
# frozen_string_literal: true
# Module for converting distances from stored meters to user's preferred unit at runtime.
#
# All distances are stored in meters in the database for consistency. This module provides
# methods to convert those stored meter values to the user's preferred unit (km, mi, etc.)
# for display purposes.
#
# This approach ensures:
# - Consistent data storage regardless of user preferences
# - No data corruption when users change distance units
# - Easy conversion for display without affecting stored data
#
# Usage:
# class Track < ApplicationRecord
# include DistanceConvertible
# end
#
# track.distance # => 5000 (meters stored in DB)
# track.distance_in_unit('km') # => 5.0 (converted to km)
# track.distance_in_unit('mi') # => 3.11 (converted to miles)
# track.formatted_distance('km') # => "5.0 km"
#
module DistanceConvertible
extend ActiveSupport::Concern
def distance_in_unit(unit)
return 0.0 unless distance.present?
unit_sym = unit.to_sym
conversion_factor = ::DISTANCE_UNITS[unit_sym]
unless conversion_factor
raise ArgumentError, "Invalid unit '#{unit}'. Supported units: #{::DISTANCE_UNITS.keys.join(', ')}"
end
# Distance is stored in meters, convert to target unit
distance.to_f / conversion_factor
end
def formatted_distance(unit, precision: 2)
converted_distance = distance_in_unit(unit)
"#{converted_distance.round(precision)} #{unit}"
end
def distance_for_user(user)
user_unit = user.safe_settings.distance_unit
distance_in_unit(user_unit)
end
def formatted_distance_for_user(user, precision: 2)
user_unit = user.safe_settings.distance_unit
formatted_distance(user_unit, precision: precision)
end
module ClassMethods
def convert_distance(distance_meters, unit)
return 0.0 unless distance_meters.present?
unit_sym = unit.to_sym
conversion_factor = ::DISTANCE_UNITS[unit_sym]
unless conversion_factor
raise ArgumentError, "Invalid unit '#{unit}'. Supported units: #{::DISTANCE_UNITS.keys.join(', ')}"
end
distance_meters.to_f / conversion_factor
end
def format_distance(distance_meters, unit, precision: 2)
converted = convert_distance(distance_meters, unit)
"#{converted.round(precision)} #{unit}"
end
end
end

View file

@ -117,7 +117,7 @@ class Point < ApplicationRecord
def trigger_incremental_track_generation def trigger_incremental_track_generation
point_date = Time.zone.at(timestamp).to_date point_date = Time.zone.at(timestamp).to_date
return unless point_date >= 1.day.ago.to_date return if point_date < 1.day.ago.to_date
Tracks::IncrementalGeneratorJob.perform_later(user_id, point_date.to_s, 5) Tracks::IncrementalGeneratorJob.perform_later(user_id, point_date.to_s, 5)
end end

View file

@ -1,6 +1,8 @@
# frozen_string_literal: true # frozen_string_literal: true
class Stat < ApplicationRecord class Stat < ApplicationRecord
include DistanceConvertible
validates :year, :month, presence: true validates :year, :month, presence: true
belongs_to :user belongs_to :user
@ -37,8 +39,9 @@ class Stat < ApplicationRecord
def calculate_daily_distances(monthly_points) def calculate_daily_distances(monthly_points)
timespan.to_a.map.with_index(1) do |day, index| timespan.to_a.map.with_index(1) do |day, index|
daily_points = filter_points_for_day(monthly_points, day) daily_points = filter_points_for_day(monthly_points, day)
distance = Point.total_distance(daily_points, user.safe_settings.distance_unit) # Calculate distance in meters for consistent storage
[index, distance.round] distance_meters = Point.total_distance(daily_points, :m)
[index, distance_meters.round]
end end
end end

View file

@ -2,6 +2,7 @@
class Track < ApplicationRecord class Track < ApplicationRecord
include Calculateable include Calculateable
include DistanceConvertible
belongs_to :user belongs_to :user
has_many :points, dependent: :nullify has_many :points, dependent: :nullify

View file

@ -2,6 +2,7 @@
class Trip < ApplicationRecord class Trip < ApplicationRecord
include Calculateable include Calculateable
include DistanceConvertible
has_rich_text :notes has_rich_text :notes

View file

@ -50,8 +50,9 @@ class User < ApplicationRecord
end end
def total_distance def total_distance
# In km or miles, depending on user.safe_settings.distance_unit # Distance is stored in meters, convert to user's preferred unit for display
stats.sum(:distance) total_distance_meters = stats.sum(:distance)
Stat.convert_distance(total_distance_meters, safe_settings.distance_unit)
end end
def total_countries def total_countries

View file

@ -9,7 +9,7 @@ class StatsSerializer
def call def call
{ {
totalDistanceKm: total_distance, totalDistanceKm: total_distance_km,
totalPointsTracked: user.tracked_points.count, totalPointsTracked: user.tracked_points.count,
totalReverseGeocodedPoints: reverse_geocoded_points, totalReverseGeocodedPoints: reverse_geocoded_points,
totalCountriesVisited: user.countries_visited.count, totalCountriesVisited: user.countries_visited.count,
@ -20,8 +20,10 @@ class StatsSerializer
private private
def total_distance def total_distance_km
user.stats.sum(:distance) # Convert from stored meters to kilometers
total_distance_meters = user.stats.sum(:distance)
(total_distance_meters / 1000.0).round(2)
end end
def reverse_geocoded_points def reverse_geocoded_points
@ -32,7 +34,7 @@ class StatsSerializer
user.stats.group_by(&:year).sort.reverse.map do |year, stats| user.stats.group_by(&:year).sort.reverse.map do |year, stats|
{ {
year:, year:,
totalDistanceKm: stats.sum(&:distance), totalDistanceKm: stats_distance_km(stats),
totalCountriesVisited: user.countries_visited.count, totalCountriesVisited: user.countries_visited.count,
totalCitiesVisited: user.cities_visited.count, totalCitiesVisited: user.cities_visited.count,
monthlyDistanceKm: monthly_distance(year, stats) monthlyDistanceKm: monthly_distance(year, stats)
@ -40,15 +42,23 @@ class StatsSerializer
end end
end end
def stats_distance_km(stats)
# Convert from stored meters to kilometers
total_meters = stats.sum(&:distance)
(total_meters / 1000.0).round(2)
end
def monthly_distance(year, stats) def monthly_distance(year, stats)
months = {} months = {}
(1..12).each { |month| months[Date::MONTHNAMES[month]&.downcase] = distance(month, year, stats) } (1..12).each { |month| months[Date::MONTHNAMES[month]&.downcase] = distance_km(month, year, stats) }
months months
end end
def distance(month, year, stats) def distance_km(month, year, stats)
stats.find { _1.month == month && _1.year == year }&.distance.to_i # Convert from stored meters to kilometers
distance_meters = stats.find { _1.month == month && _1.year == year }&.distance.to_i
(distance_meters / 1000.0).round(2)
end end
end end

View file

@ -15,14 +15,14 @@
# 6. Associates all points with the created track # 6. Associates all points with the created track
# #
# Statistics calculated: # Statistics calculated:
# - Distance: In user's preferred unit (km/miles) with 2 decimal precision # - Distance: Always stored in meters as integers for consistency
# - Duration: Total time in seconds between first and last point # - Duration: Total time in seconds between first and last point
# - Average speed: In km/h regardless of user's distance unit preference # - Average speed: In km/h regardless of user's distance unit preference
# - Elevation gain/loss: Cumulative ascent and descent in meters # - Elevation gain/loss: Cumulative ascent and descent in meters
# - Elevation max/min: Highest and lowest altitudes in the track # - Elevation max/min: Highest and lowest altitudes in the track
# #
# The module respects user preferences for distance units and handles missing # Distance is converted to user's preferred unit only at display time, not storage time.
# elevation data gracefully by providing default values. # This ensures consistency when users change their distance unit preferences.
# #
# Used by: # Used by:
# - Tracks::Generator for creating tracks during generation # - Tracks::Generator for creating tracks during generation
@ -85,27 +85,20 @@ module Tracks::TrackBuilder
end end
def calculate_track_distance(points) def calculate_track_distance(points)
distance_in_user_unit = Point.total_distance(points, user.safe_settings.distance_unit || 'km') # Always calculate and store distance in meters for consistency
distance_in_user_unit.round(2) distance_in_meters = Point.total_distance(points, :m)
distance_in_meters.round
end end
def calculate_duration(points) def calculate_duration(points)
points.last.timestamp - points.first.timestamp points.last.timestamp - points.first.timestamp
end end
def calculate_average_speed(distance_in_user_unit, duration_seconds) def calculate_average_speed(distance_in_meters, duration_seconds)
return 0.0 if duration_seconds <= 0 || distance_in_user_unit <= 0 return 0.0 if duration_seconds <= 0 || distance_in_meters <= 0
# Convert distance to meters for speed calculation
distance_meters = case user.safe_settings.distance_unit
when 'mi'
distance_in_user_unit * 1609.344 # miles to meters
else
distance_in_user_unit * 1000 # km to meters
end
# Speed in meters per second, then convert to km/h for storage # Speed in meters per second, then convert to km/h for storage
speed_mps = distance_meters.to_f / duration_seconds speed_mps = distance_in_meters.to_f / duration_seconds
(speed_mps * 3.6).round(2) # m/s to km/h (speed_mps * 3.6).round(2) # m/s to km/h
end end

View file

@ -1,31 +1,28 @@
<div id="<%= dom_id stat %>" class="card w-full bg-base-200 shadow-xl"> <div class="border border-gray-500 rounded-md border-opacity-30 bg-gray-100 dark:bg-gray-800 p-3">
<div class="card-body"> <div class="flex justify-between">
<div class="flex justify-between items-center"> <h4 class="stat-title text-left"><%= Date::MONTHNAMES[stat.month] %> <%= stat.year %></h4>
<h2 class="card-title">
<%= link_to map_url(timespan(stat.month, stat.year)), class: "underline hover:no-underline text-#{header_colors.sample}" do %>
<%= Date::MONTHNAMES[stat.month] %>
<% end %>
</h2>
<div class="gap-2"> <div class="flex items-center space-x-2">
<span class='text-xs text-gray-500'>Last update <%= human_date(stat.updated_at) %></span> <%= link_to "Details", points_path(year: stat.year, month: stat.month),
<%= link_to '🔄', update_year_month_stats_path(stat.year, stat.month), data: { turbo_method: :put }, class: 'text-sm text-gray-500 hover:underline' %> class: "link link-primary" %>
</div>
</div> </div>
<p><%= number_with_delimiter stat.distance %><%= current_user.safe_settings.distance_unit %></p>
<% if DawarichSettings.reverse_geocoding_enabled? %>
<div class="card-actions justify-end">
<%= countries_and_cities_stat_for_month(stat) %>
</div>
<% end %>
<% if stat.daily_distance %>
<%= column_chart(
stat.daily_distance,
height: '100px',
suffix: " #{current_user.safe_settings.distance_unit}",
xtitle: 'Days',
ytitle: 'Distance'
) %>
<% end %>
</div> </div>
<div class="flex">
<div class="stat-value">
<p><%= number_with_delimiter stat.distance_in_unit(current_user.safe_settings.distance_unit).round %><%= current_user.safe_settings.distance_unit %></p>
</div>
</div>
<div class="stat-desc">
<%= countries_and_cities_stat_for_month(stat) %>
</div>
<canvas id="distance-chart-<%= stat.id %>"
data-daily-distance="<%= stat.daily_distance %>"
data-distance-type="monthly"
data-title="<%= Date::MONTHNAMES[stat.month] %> <%= stat.year %>"
data-y-axis-title="Distance"
suffix: " <%= current_user.safe_settings.distance_unit %>",
data-user-settings="<%= current_user.safe_settings.default_settings.to_json %>"></canvas>
</div> </div>

View file

@ -82,7 +82,9 @@
</div> </div>
<% end %> <% end %>
<%= column_chart( <%= column_chart(
Stat.year_distance(year, current_user), Stat.year_distance(year, current_user).map { |month_name, distance_meters|
[month_name, Stat.convert_distance(distance_meters, current_user.safe_settings.distance_unit).round(2)]
},
height: '200px', height: '200px',
suffix: " #{current_user.safe_settings.distance_unit}", suffix: " #{current_user.safe_settings.distance_unit}",
xtitle: 'Days', xtitle: 'Days',

View file

@ -2,7 +2,7 @@
<div class="card bg-base-200 shadow-lg"> <div class="card bg-base-200 shadow-lg">
<div class="card-body p-4"> <div class="card-body p-4">
<div class="stat-title text-xs">Distance</div> <div class="stat-title text-xs">Distance</div>
<div class="stat-value text-lg"><%= trip.distance %> <%= distance_unit %></div> <div class="stat-value text-lg"><%= trip.distance_for_user(current_user).round %> <%= distance_unit %></div>
</div> </div>
</div> </div>
<div class="card bg-base-200 shadow-lg"> <div class="card bg-base-200 shadow-lg">

View file

@ -1,5 +1,5 @@
<% if trip.distance.present? %> <% if trip.distance.present? %>
<span class="text-md"><%= trip.distance %> <%= distance_unit %></span> <span class="text-md"><%= trip.distance_for_user(current_user).round %> <%= distance_unit %></span>
<% else %> <% else %>
<span class="text-md">Calculating...</span> <span class="text-md">Calculating...</span>
<span class="loading loading-dots loading-sm"></span> <span class="loading loading-dots loading-sm"></span>

View file

@ -5,7 +5,7 @@
<span class="hover:underline"><%= trip.name %></span> <span class="hover:underline"><%= trip.name %></span>
</h2> </h2>
<p class="text-sm text-gray-600 text-center"> <p class="text-sm text-gray-600 text-center">
<%= "#{human_date(trip.started_at)} #{human_date(trip.ended_at)}, #{trip.distance} #{current_user.safe_settings.distance_unit}" %> <%= "#{human_date(trip.started_at)} #{human_date(trip.ended_at)}, #{trip.distance_for_user(current_user).round} #{current_user.safe_settings.distance_unit}" %>
</p> </p>
<div style="width: 100%; aspect-ratio: 1/1;" <div style="width: 100%; aspect-ratio: 1/1;"

View file

@ -133,11 +133,13 @@ RSpec.describe Point, type: :model do
end end
describe '#trigger_incremental_track_generation' do describe '#trigger_incremental_track_generation' do
let(:point) { create(:point, track: track) } let(:point) do
create(:point, track: track, import_id: nil, timestamp: 1.hour.ago.to_i, reverse_geocoded_at: 1.hour.ago)
end
let(:track) { create(:track) } let(:track) { create(:track) }
it 'enqueues Tracks::IncrementalGeneratorJob' do it 'enqueues Tracks::IncrementalGeneratorJob' do
expect { point.trigger_incremental_track_generation }.to have_enqueued_job(Tracks::IncrementalGeneratorJob) expect { point.send(:trigger_incremental_track_generation) }.to have_enqueued_job(Tracks::IncrementalGeneratorJob).with(point.user_id, point.recorded_at.to_date.to_s, 5)
end end
end end
end end

View file

@ -29,7 +29,7 @@ RSpec.describe Stat, type: :model do
create(:point, user:, lonlat: 'POINT(2 2)', timestamp: DateTime.new(year, 1, 1, 2)) create(:point, user:, lonlat: 'POINT(2 2)', timestamp: DateTime.new(year, 1, 1, 2))
end end
before { expected_distance[0][1] = 157 } before { expected_distance[0][1] = 156_876 }
it 'returns distance by day' do it 'returns distance by day' do
expect(subject).to eq(expected_distance) expect(subject).to eq(expected_distance)

View file

@ -146,13 +146,12 @@ RSpec.describe Track, type: :model do
expect(track.distance).to be_a(Numeric) expect(track.distance).to be_a(Numeric)
end end
it 'stores distance in user preferred unit for Track model' do it 'stores distance in meters consistently' do
allow(user).to receive(:safe_settings).and_return(double(distance_unit: 'km')) allow(Point).to receive(:total_distance).and_return(1500) # 1500 meters
allow(Point).to receive(:total_distance).and_return(1.5) # 1.5 km
track.calculate_distance track.calculate_distance
expect(track.distance).to eq(1.5) # Should be 1.5 km with 2 decimal places precision expect(track.distance).to eq(1500) # Should be stored as meters regardless of user unit preference
end end
end end

View file

@ -88,11 +88,11 @@ RSpec.describe User, type: :model do
describe '#total_distance' do describe '#total_distance' do
subject { user.total_distance } subject { user.total_distance }
let!(:stat1) { create(:stat, user:, distance: 10) } let!(:stat1) { create(:stat, user:, distance: 10_000) }
let!(:stat2) { create(:stat, user:, distance: 20) } let!(:stat2) { create(:stat, user:, distance: 20_000) }
it 'returns sum of distances' do it 'returns sum of distances' do
expect(subject).to eq(30) expect(subject).to eq(30) # 30 km
end end
end end

View file

@ -53,15 +53,17 @@ RSpec.describe Stats::CalculateMonth do
lonlat: 'POINT(9.77973105800526 52.72859111523629)') lonlat: 'POINT(9.77973105800526 52.72859111523629)')
end end
context 'when units are kilometers' do context 'when calculating distance' do
it 'creates stats' do it 'creates stats' do
expect { calculate_stats }.to change { Stat.count }.by(1) expect { calculate_stats }.to change { Stat.count }.by(1)
end end
it 'calculates distance' do it 'calculates distance in meters consistently' do
calculate_stats calculate_stats
expect(user.stats.last.distance).to eq(340) # Distance should be calculated in meters regardless of user unit preference
# The actual distance between the test points is approximately 340 km = 340,000 meters
expect(user.stats.last.distance).to be_within(1000).of(340_000)
end end
context 'when there is an error' do context 'when there is an error' do
@ -79,33 +81,16 @@ RSpec.describe Stats::CalculateMonth do
end end
end end
context 'when units are miles' do context 'when user prefers miles' do
before do before do
user.update(settings: { maps: { distance_unit: 'mi' } }) user.update(settings: { maps: { distance_unit: 'mi' } })
end end
it 'creates stats' do it 'still stores distance in meters (same as km users)' do
expect { calculate_stats }.to change { Stat.count }.by(1)
end
it 'calculates distance' do
calculate_stats calculate_stats
expect(user.stats.last.distance).to eq(211) # Distance stored should be the same regardless of user preference (meters)
end expect(user.stats.last.distance).to be_within(1000).of(340_000)
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 end

View file

@ -132,40 +132,20 @@ RSpec.describe Tracks::TrackBuilder do
] ]
end end
context 'with km unit' do before do
before do # Mock Point.total_distance to return distance in meters
allow_any_instance_of(Users::SafeSettings).to receive(:distance_unit).and_return('km') allow(Point).to receive(:total_distance).and_return(1500) # 1500 meters
allow(Point).to receive(:total_distance).and_return(1.5) # 1.5 km
end
it 'stores distance in km' do
result = builder.calculate_track_distance(points)
expect(result).to eq(1.5) # 1.5 km with 2 decimal places precision
end
end end
context 'with miles unit' do it 'stores distance in meters regardless of user unit preference' do
before do result = builder.calculate_track_distance(points)
allow_any_instance_of(Users::SafeSettings).to receive(:distance_unit).and_return('miles') expect(result).to eq(1500) # Always stored as meters
allow(Point).to receive(:total_distance).and_return(1.0) # 1 mile
end
it 'stores distance in miles' do
result = builder.calculate_track_distance(points)
expect(result).to eq(1) # 1 mile
end
end end
context 'with nil distance unit (defaults to km)' do it 'rounds distance to nearest meter' do
before do allow(Point).to receive(:total_distance).and_return(1500.7)
allow_any_instance_of(Users::SafeSettings).to receive(:distance_unit).and_return(nil) result = builder.calculate_track_distance(points)
allow(Point).to receive(:total_distance).and_return(2.0) expect(result).to eq(1501) # Rounded to nearest meter
end
it 'defaults to km and stores distance in km' do
result = builder.calculate_track_distance(points)
expect(result).to eq(2.0) # 2.0 km with 2 decimal places precision
end
end end
end end