Merge pull request #1579 from Freika/fix/country-name-in-points

Add country_name to points and fix some bugs.

CHANGELOG.md

@@ -11,10 +11,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Put all jobs in their own queues.
 - Visits page should load faster now.
 - Reverse geocoding jobs now make less database queries.
+- Country name is now being backfilled for all points. #1562
+- Stats are now reflecting countries and cities. #1562
 
 ## Added
 - Points now support discharging and connected_not_charging battery statuses. #768
 
+## Fixed
+
+- Fixed a bug where import or notification could have been deleted by a different user.
+
 
 # [0.30.5] - 2025-07-26
 

app/controllers/imports_controller.rb

@@ -4,13 +4,15 @@ class ImportsController < ApplicationController
   include ActiveStorage::SetCurrent
 
   before_action :authenticate_user!
-  before_action :authenticate_active_user!, only: %i[new create]
   before_action :set_import, only: %i[show edit update destroy]
+  before_action :authorize_import, only: %i[show edit update destroy]
   before_action :validate_points_limit, only: %i[new create]
+
+  after_action :verify_authorized, except: %i[index]
+  after_action :verify_policy_scoped, only: %i[index]
+
   def index
-    @imports =
-      current_user
-      .imports
+    @imports = policy_scope(Import)
       .select(:id, :name, :source, :created_at, :processed, :status)
       .order(created_at: :desc)
       .page(params[:page])
@@ -22,6 +24,8 @@ class ImportsController < ApplicationController
 
   def new
     @import = Import.new
+
+    authorize @import
   end
 
   def update
@@ -31,6 +35,10 @@ class ImportsController < ApplicationController
   end
 
   def create
+    @import = Import.new
+
+    authorize @import
+
     files_params = params.dig(:import, :files)
     raw_files = Array(files_params).reject(&:blank?)
 
@@ -82,6 +90,10 @@ class ImportsController < ApplicationController
     @import = Import.find(params[:id])
   end
 
+  def authorize_import
+    authorize @import
+  end
+
   def import_params
     params.require(:import).permit(:name, :source, files: [])
   end

app/controllers/notifications_controller.rb

@@ -32,6 +32,6 @@ class NotificationsController < ApplicationController
   private
 
   def set_notification
-    @notification = Notification.find(params[:id])
+    @notification = current_user.notifications.find(params[:id])
   end
 end

app/jobs/data_migrations/backfill_country_name_job.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+class DataMigrations::BackfillCountryNameJob < ApplicationJob
+  queue_as :data_migrations
+
+  def perform(batch_size: 1000)
+    Rails.logger.info('Starting country_name backfill job')
+
+    total_count = Point.where(country_name: nil).count
+    processed_count = 0
+
+    Point.where(country_name: nil).find_in_batches(batch_size: batch_size) do |points|
+      points.each do |point|
+        country_name = country_name(point)
+        point.update_column(:country_name, country_name) if country_name.present?
+
+        processed_count += 1
+      end
+
+      Rails.logger.info("Backfilled country_name for #{processed_count}/#{total_count} points")
+    end
+
+    Rails.logger.info("Completed country_name backfill job. Processed #{processed_count} points")
+  end
+
+  private
+
+  def country_name(point)
+    point.read_attribute(:country) || point.country&.name
+  end
+end

app/models/point.rb

@@ -66,6 +66,11 @@ class Point < ApplicationRecord
     Country.containing_point(lon, lat)
   end
 
+  def country_name
+    # TODO: Remove the country column in the future.
+    read_attribute(:country_name) || self.country&.name || read_attribute(:country) || ''
+  end
+
   private
 
   # rubocop:disable Metrics/MethodLength Metrics/AbcSize
@@ -93,13 +98,6 @@ class Point < ApplicationRecord
     save! if changed?
   end
 
-  def country_name
-    # We have a country column in the database,
-    # but we also have a country_id column.
-    # TODO: rename country column to country_name
-    self.country&.name || read_attribute(:country) || ''
-  end
-
   def recalculate_track
     track.recalculate_path_and_distance!
   end

app/models/user.rb

@@ -33,24 +33,18 @@ class User < ApplicationRecord
   end
 
   def countries_visited
-    stats.pluck(:toponyms).flatten.map { _1['country'] }.uniq.compact
-  end
-
-  def cities_visited
-    stats
-      .where.not(toponyms: nil)
-      .pluck(:toponyms)
-      .flatten
-      .reject { |toponym| toponym['cities'].blank? }
-      .pluck('cities')
-      .flatten
-      .pluck('city')
-      .uniq
+    tracked_points
+      .where.not(country_name: [nil, ''])
+      .distinct
+      .pluck(:country_name)
       .compact
   end
 
+  def cities_visited
+    tracked_points.where.not(city: [nil, '']).distinct.pluck(:city).compact
+  end
+
   def total_distance
-    # Distance is stored in meters, convert to user's preferred unit for display
     total_distance_meters = stats.sum(:distance)
     Stat.convert_distance(total_distance_meters, safe_settings.distance_unit)
   end

app/policies/import_policy.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+class ImportPolicy < ApplicationPolicy
+  # Allow users to view the imports index
+  def index?
+    user.present?
+  end
+
+  # Users can only show their own imports
+  def show?
+    user.present? && record.user == user
+  end
+
+  # Users can create new imports if they are active
+  def new?
+    create?
+  end
+
+  def create?
+    user.present? && user.active?
+  end
+
+  # Users can only edit their own imports
+  def edit?
+    update?
+  end
+
+  def update?
+    user.present? && record.user == user
+  end
+
+  # Users can only destroy their own imports
+  def destroy?
+    user.present? && record.user == user
+  end
+
+  class Scope < ApplicationPolicy::Scope
+    def resolve
+      return scope.none unless user.present?
+
+      # Users can only see their own imports
+      scope.where(user: user)
+    end
+  end
+end

app/services/countries_and_cities.rb

@@ -10,8 +10,8 @@ class CountriesAndCities
 
   def call
     points
-      .reject { |point| point.read_attribute(:country).nil? || point.city.nil? }
-      .group_by { |point| point.read_attribute(:country) }
+      .reject { |point| point.country_name.nil? || point.city.nil? }
+      .group_by { |point| point.country_name }
       .transform_values { |country_points| process_country_points(country_points) }
       .map { |country, cities| CountryData.new(country: country, cities: cities) }
   end

app/services/reverse_geocoding/points/fetch_data.rb

@@ -27,6 +27,7 @@ class ReverseGeocoding::Points::FetchData
 
     point.update!(
       city: response.city,
+      country_name: response.country,
       country_id: country_record&.id,
       geodata: response.data,
       reverse_geocoded_at: Time.current

app/services/stats/calculate_month.rb

@@ -63,7 +63,7 @@ class Stats::CalculateMonth
                       .tracked_points
                       .without_raw_data
                       .where(timestamp: start_timestamp..end_timestamp)
-                      .select(:city, :country)
+                      .select(:city, :country_name)
                       .distinct
 
     CountriesAndCities.new(toponym_points).call

db/migrate/20250728191359_add_country_name_to_points.rb

@@ -0,0 +1,10 @@
+class AddCountryNameToPoints < ActiveRecord::Migration[8.0]
+  disable_ddl_transaction!
+
+  def change
+    add_column :points, :country_name, :string
+    add_index :points, :country_name, algorithm: :concurrently
+
+    DataMigrations::BackfillCountryNameJob.perform_later
+  end
+end

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_07_23_164055) do
+ActiveRecord::Schema[8.0].define(version: 2025_07_28_191359) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_catalog.plpgsql"
   enable_extension "postgis"
@@ -186,6 +186,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_07_23_164055) do
     t.geography "lonlat", limit: {srid: 4326, type: "st_point", geographic: true}
     t.bigint "country_id"
     t.bigint "track_id"
+    t.string "country_name"
     t.index ["altitude"], name: "index_points_on_altitude"
     t.index ["battery"], name: "index_points_on_battery"
     t.index ["battery_status"], name: "index_points_on_battery_status"
@@ -193,6 +194,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_07_23_164055) do
     t.index ["connection"], name: "index_points_on_connection"
     t.index ["country"], name: "index_points_on_country"
     t.index ["country_id"], name: "index_points_on_country_id"
+    t.index ["country_name"], name: "index_points_on_country_name"
     t.index ["external_track_id"], name: "index_points_on_external_track_id"
     t.index ["geodata"], name: "index_points_on_geodata", using: :gin
     t.index ["import_id"], name: "index_points_on_import_id"

spec/factories/imports.rb

@@ -3,7 +3,7 @@
 FactoryBot.define do
   factory :import do
     user
-    name { 'owntracks_export.json' }
+    sequence(:name) { |n| "owntracks_export_#{n}.json" }
     source { Import.sources[:owntracks] }
 
     trait :with_points do

spec/factories/points.rb

@@ -49,11 +49,13 @@ FactoryBot.define do
         end
         point.update_columns(
           country: evaluator.country,
+          country_name: evaluator.country,
           country_id: country_obj.id
         )
       elsif evaluator.country
         point.update_columns(
           country: evaluator.country.name,
+          country_name: evaluator.country.name,
           country_id: evaluator.country.id
         )
       end
@@ -101,7 +103,8 @@ FactoryBot.define do
             country.iso_a3 = iso_a3
             country.geom = "MULTIPOLYGON (((0 0, 1 0, 1 1, 0 1, 0 0)))"
           end
-          point.write_attribute(:country, country_name)        # Set the string attribute directly
+          point.write_attribute(:country, country_name)        # Set the legacy string attribute
+          point.write_attribute(:country_name, country_name)  # Set the new string attribute
           point.country_id = country_obj.id   # Set the association
         end
       end

spec/models/user_spec.rb

@@ -65,23 +65,36 @@ RSpec.describe User, type: :model do
     describe '#countries_visited' do
       subject { user.countries_visited }
 
-      let!(:stat1) { create(:stat, user:, toponyms: [{ 'country' => 'Germany' }]) }
-      let!(:stat2) { create(:stat, user:, toponyms: [{ 'country' => 'France' }]) }
+      let!(:point1) { create(:point, user:, country_name: 'Germany') }
+      let!(:point2) { create(:point, user:, country_name: 'France') }
+      let!(:point3) { create(:point, user:, country_name: nil) }
+      let!(:point4) { create(:point, user:, country_name: '') }
 
       it 'returns array of countries' do
         expect(subject).to include('Germany', 'France')
         expect(subject.count).to eq(2)
       end
+
+      it 'excludes nil and empty country names' do
+        expect(subject).not_to include(nil, '')
+      end
     end
 
     describe '#cities_visited' do
       subject { user.cities_visited }
 
-      let!(:stat1) { create(:stat, user:, toponyms: [{ 'cities' => [{ 'city' => 'Berlin' }] }]) }
-      let!(:stat2) { create(:stat, user:, toponyms: [{ 'cities' => [{ 'city' => 'Paris' }] }]) }
+      let!(:point1) { create(:point, user:, city: 'Berlin') }
+      let!(:point2) { create(:point, user:, city: 'Paris') }
+      let!(:point3) { create(:point, user:, city: nil) }
+      let!(:point4) { create(:point, user:, city: '') }
 
       it 'returns array of cities' do
-        expect(subject).to eq(%w[Berlin Paris])
+        expect(subject).to include('Berlin', 'Paris')
+        expect(subject.count).to eq(2)
+      end
+
+      it 'excludes nil and empty city names' do
+        expect(subject).not_to include(nil, '')
       end
     end
 
@@ -99,30 +112,24 @@ RSpec.describe User, type: :model do
     describe '#total_countries' do
       subject { user.total_countries }
 
-      let!(:stat) { create(:stat, user:, toponyms: [{ 'country' => 'Country' }]) }
+      let!(:point1) { create(:point, user:, country_name: 'Germany') }
+      let!(:point2) { create(:point, user:, country_name: 'France') }
+      let!(:point3) { create(:point, user:, country_name: nil) }
 
       it 'returns number of countries' do
-        expect(subject).to eq(1)
+        expect(subject).to eq(2)
       end
     end
 
     describe '#total_cities' do
       subject { user.total_cities }
 
-      let!(:stat) do
-        create(
-          :stat,
-          user:,
-          toponyms: [
-            { 'cities' => [], 'country' => nil },
-            { 'cities' => [{ 'city' => 'Berlin', 'points' => 64, 'timestamp' => 1_710_446_806, 'stayed_for' => 8772 }],
-'country' => 'Germany' }
-          ]
-        )
-      end
+      let!(:point1) { create(:point, user:, city: 'Berlin') }
+      let!(:point2) { create(:point, user:, city: 'Paris') }
+      let!(:point3) { create(:point, user:, city: nil) }
 
       it 'returns number of cities' do
-        expect(subject).to eq(1)
+        expect(subject).to eq(2)
       end
     end
 

spec/policies/import_policy_spec.rb

@@ -0,0 +1,159 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe ImportPolicy, type: :policy do
+  let(:user) { create(:user) }
+  let(:other_user) { create(:user) }
+  let(:import) { create(:import, user: user) }
+  let(:other_import) { create(:import, user: other_user) }
+
+  describe 'index?' do
+    it 'allows authenticated users' do
+      policy = ImportPolicy.new(user, Import)
+
+      expect(policy).to permit(:index)
+    end
+
+    it 'denies unauthenticated users' do
+      policy = ImportPolicy.new(nil, Import)
+
+      expect(policy).not_to permit(:index)
+    end
+  end
+
+  describe 'show?' do
+    it 'allows users to view their own imports' do
+      policy = ImportPolicy.new(user, import)
+
+      expect(policy).to permit(:show)
+    end
+
+    it 'denies users from viewing other users imports' do
+      policy = ImportPolicy.new(user, other_import)
+
+      expect(policy).not_to permit(:show)
+    end
+
+    it 'denies unauthenticated users' do
+      policy = ImportPolicy.new(nil, import)
+
+      expect(policy).not_to permit(:show)
+    end
+  end
+
+  describe 'new?' do
+    context 'when user is active' do
+      before { allow(user).to receive(:active?).and_return(true) }
+
+      it 'allows active users to access new imports form' do
+        policy = ImportPolicy.new(user, Import.new)
+
+        expect(policy).to permit(:new)
+      end
+    end
+
+    context 'when user is not active' do
+      before { allow(user).to receive(:active?).and_return(false) }
+
+      it 'denies inactive users from accessing new imports form' do
+        policy = ImportPolicy.new(user, Import.new)
+
+        expect(policy).not_to permit(:new)
+      end
+    end
+
+    it 'denies unauthenticated users' do
+      policy = ImportPolicy.new(nil, Import.new)
+
+      expect(policy).not_to permit(:new)
+    end
+  end
+
+  describe 'create?' do
+    context 'when user is active' do
+      before { allow(user).to receive(:active?).and_return(true) }
+
+      it 'allows active users to create imports' do
+        policy = ImportPolicy.new(user, Import.new)
+
+        expect(policy).to permit(:create)
+      end
+    end
+
+    context 'when user is not active' do
+      before { allow(user).to receive(:active?).and_return(false) }
+
+      it 'denies inactive users from creating imports' do
+        policy = ImportPolicy.new(user, Import.new)
+
+        expect(policy).not_to permit(:create)
+      end
+    end
+
+    it 'denies unauthenticated users' do
+      policy = ImportPolicy.new(nil, Import.new)
+
+      expect(policy).not_to permit(:create)
+    end
+  end
+
+  describe 'update?' do
+    it 'allows users to update their own imports' do
+      policy = ImportPolicy.new(user, import)
+
+      expect(policy).to permit(:update)
+    end
+
+    it 'denies users from updating other users imports' do
+      policy = ImportPolicy.new(user, other_import)
+
+      expect(policy).not_to permit(:update)
+    end
+
+    it 'denies unauthenticated users' do
+      policy = ImportPolicy.new(nil, import)
+
+      expect(policy).not_to permit(:update)
+    end
+  end
+
+  describe 'destroy?' do
+    it 'allows users to destroy their own imports' do
+      policy = ImportPolicy.new(user, import)
+
+      expect(policy).to permit(:destroy)
+    end
+
+    it 'denies users from destroying other users imports' do
+      policy = ImportPolicy.new(user, other_import)
+
+      expect(policy).not_to permit(:destroy)
+    end
+
+    it 'denies unauthenticated users' do
+      policy = ImportPolicy.new(nil, import)
+
+      expect(policy).not_to permit(:destroy)
+    end
+  end
+
+  describe 'Scope' do
+    let!(:user_import1) { create(:import, user: user) }
+    let!(:user_import2) { create(:import, user: user) }
+    let!(:other_user_import) { create(:import, user: other_user) }
+
+    it 'returns only the users imports' do
+      scope = ImportPolicy::Scope.new(user, Import).resolve
+
+      expect(scope).to contain_exactly(user_import1, user_import2)
+      expect(scope).not_to include(other_user_import)
+    end
+
+    it 'returns no imports for unauthenticated users' do
+      scope = ImportPolicy::Scope.new(nil, Import).resolve
+
+      expect(scope).to be_empty
+    end
+  end
+end

spec/requests/api/v1/stats_spec.rb

@@ -3,22 +3,38 @@
 require 'rails_helper'
 
 RSpec.describe 'Api::V1::Stats', type: :request do
-  let(:user) { create(:user) }
-
   describe 'GET /index' do
-    let!(:user) { create(:user) }
-    let!(:stats_in_2020) { create_list(:stat, 12, year: 2020, user:) }
-    let!(:stats_in_2021) { create_list(:stat, 12, year: 2021, user:) }
-    let!(:points_in_2020) do
+    let(:user) { create(:user) }
+    let(:stats_in_2020) { create_list(:stat, 12, year: 2020, user:) }
+    let(:stats_in_2021) { create_list(:stat, 12, year: 2021, user:) }
+    let(:points_in_2020) do
       (1..85).map do |i|
-        create(:point, :with_geodata, :reverse_geocoded, timestamp: Time.zone.local(2020, 1, 1).to_i + i.hours, user:)
+        create(:point, :with_geodata,
+               timestamp: Time.zone.local(2020, 1, 1).to_i + i.hours,
+               user:,
+               country_name: 'Test Country',
+               city: 'Test City',
+               reverse_geocoded_at: Time.current)
       end
     end
-    let!(:points_in_2021) do
+    let(:points_in_2021) do
       (1..95).map do |i|
-        create(:point, :with_geodata, :reverse_geocoded, timestamp: Time.zone.local(2021, 1, 1).to_i + i.hours, user:)
+        create(:point, :with_geodata,
+               timestamp: Time.zone.local(2021, 1, 1).to_i + i.hours,
+               user:,
+               country_name: 'Test Country',
+               city: 'Test City',
+               reverse_geocoded_at: Time.current)
       end
     end
+
+    before do
+      stats_in_2020
+      stats_in_2021
+      points_in_2020
+      points_in_2021
+    end
+
     let(:expected_json) do
       {
         totalDistanceKm: (stats_in_2020.map(&:distance).sum + stats_in_2021.map(&:distance).sum) / 1000,
@@ -84,3 +100,4 @@ RSpec.describe 'Api::V1::Stats', type: :request do
     end
   end
 end
+

spec/requests/imports_spec.rb

@@ -31,6 +31,84 @@ RSpec.describe 'Imports', type: :request do
           expect(response.body).to include(import.name)
         end
       end
+
+      context 'when other users have imports' do
+        let!(:other_user) { create(:user) }
+        let!(:other_import) { create(:import, user: other_user) }
+        let!(:user_import) { create(:import, user: user) }
+
+        it 'only displays current users imports' do
+          get imports_path
+
+          expect(response.body).to include(user_import.name)
+          expect(response.body).not_to include(other_import.name)
+        end
+      end
+    end
+  end
+
+  describe 'GET /imports/:id' do
+    let(:user) { create(:user) }
+    let(:other_user) { create(:user) }
+    let(:import) { create(:import, user: user) }
+    let(:other_import) { create(:import, user: other_user) }
+
+    context 'when user is logged in' do
+      before { sign_in user }
+
+      it 'allows viewing own import' do
+        get import_path(import)
+        expect(response).to have_http_status(200)
+      end
+
+      it 'prevents viewing other users import' do
+        expect {
+          get import_path(other_import)
+        }.to raise_error(Pundit::NotAuthorizedError)
+      end
+    end
+
+    context 'when user is not logged in' do
+      it 'redirects to login' do
+        get import_path(import)
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+  end
+
+  describe 'GET /imports/new' do
+    let(:user) { create(:user) }
+
+    context 'when user is active' do
+      before do
+        allow(user).to receive(:active?).and_return(true)
+        sign_in user
+      end
+
+      it 'allows access to new import form' do
+        get new_import_path
+        expect(response).to have_http_status(200)
+      end
+    end
+
+    context 'when user is inactive' do
+      before do
+        allow(user).to receive(:active?).and_return(false)
+        sign_in user
+      end
+
+      it 'prevents access to new import form' do
+        expect {
+          get new_import_path
+        }.to raise_error(Pundit::NotAuthorizedError)
+      end
+    end
+
+    context 'when user is not logged in' do
+      it 'redirects to login' do
+        get new_import_path
+        expect(response).to redirect_to(new_user_session_path)
+      end
     end
   end
 
@@ -97,24 +175,17 @@ RSpec.describe 'Imports', type: :request do
         let(:signed_id2) { generate_signed_id_for_blob(blob2) }
 
         it 'deletes any created imports' do
-          # The first blob should be found correctly
           allow(ActiveStorage::Blob).to receive(:find_signed).with(signed_id1).and_return(blob1)
 
-          # The second blob find will raise an error
           allow(ActiveStorage::Blob).to receive(:find_signed).with(signed_id2).and_raise(StandardError, 'Test error')
 
-          # Allow ExceptionReporter to be called without actually calling it
           allow(ExceptionReporter).to receive(:call)
 
-          # The request should not ultimately create any imports
           expect do
             post imports_path, params: { import: { source: 'owntracks', files: [signed_id1, signed_id2] } }
           end.not_to change(Import, :count)
 
-          # Check that we were redirected with an error message
           expect(response).to have_http_status(422)
-          # Just check that we have an alert message, not its exact content
-          # since error handling might transform the message
           expect(flash[:alert]).not_to be_nil
         end
       end
@@ -183,7 +254,6 @@ RSpec.describe 'Imports', type: :request do
     end
   end
 
-  # Helper methods for creating ActiveStorage blobs and signed IDs in tests
   def create_blob_for_file(file)
     ActiveStorage::Blob.create_and_upload!(
       io: file.open,

spec/serializers/point_serializer_spec.rb

@@ -34,7 +34,8 @@ RSpec.describe PointSerializer do
         'course' => point.course,
         'course_accuracy' => point.course_accuracy,
         'external_track_id' => point.external_track_id,
-        'track_id' => point.track_id
+        'track_id' => point.track_id,
+        'country_name' => point.read_attribute(:country_name)
       }
     end
 

spec/serializers/stats_serializer_spec.rb

@@ -30,12 +30,22 @@ RSpec.describe StatsSerializer do
       let!(:stats_in_2021) { create_list(:stat, 12, year: 2021, user:) }
       let!(:points_in_2020) do
         (1..85).map do |i|
-          create(:point, :with_geodata, :reverse_geocoded, timestamp: Time.zone.local(2020, 1, 1).to_i + i.hours, user:)
+          create(:point, :with_geodata,
+                 timestamp: Time.zone.local(2020, 1, 1).to_i + i.hours,
+                 user:,
+                 country_name: 'Test Country',
+                 city: 'Test City',
+                 reverse_geocoded_at: Time.current)
         end
       end
       let!(:points_in_2021) do
         (1..95).map do |i|
-          create(:point, :with_geodata, :reverse_geocoded, timestamp: Time.zone.local(2021, 1, 1).to_i + i.hours, user:)
+          create(:point, :with_geodata,
+                 timestamp: Time.zone.local(2021, 1, 1).to_i + i.hours,
+                 user:,
+                 country_name: 'Test Country',
+                 city: 'Test City',
+                 reverse_geocoded_at: Time.current)
         end
       end
       let(:expected_json) do

spec/services/tracks/track_builder_spec.rb

@@ -49,7 +49,7 @@ RSpec.describe Tracks::TrackBuilder do
         expect(track.start_at).to be_within(1.second).of(Time.zone.at(points.first.timestamp))
         expect(track.end_at).to be_within(1.second).of(Time.zone.at(points.last.timestamp))
         expect(track.distance).to eq(1500)
-        expect(track.duration).to eq(90.minutes.to_i)
+        expect(track.duration).to be_within(3.seconds).of(90.minutes.to_i)
         expect(track.avg_speed).to be > 0
         expect(track.original_path).to be_present
       end