Merge pull request #1586 from Freika/dev

0.30.6

.app_version

@@ -1 +1 @@
-0.30.5
+0.30.6

.gitignore

@@ -65,7 +65,7 @@
 .dotnet/
 .cursorrules
 .cursormemory.md
-.serena/project.yml
+.serena/**/*
 
 /config/credentials/production.key
 /config/credentials/production.yml.enc

CHANGELOG.md

@@ -4,6 +4,25 @@ 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.30.6] - 2025-07-27
+
+## Changed
+
+- 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 accessed by a different user.
+- Fixed a bug where draw control was not being added to the map when areas layer was enabled. #1583
+
+
 # [0.30.5] - 2025-07-26
 
 ## Fixed

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 =
+    @imports = policy_scope(Import)
-      current_user
-      .imports
       .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/controllers/stats_controller.rb

@@ -57,8 +57,15 @@ class StatsController < ApplicationController
   def precompute_year_distances
     year_distances = {}
 
-    @stats.each do |year, _stats|
+    @stats.each do |year, stats|
-      year_distances[year] = Stat.year_distance(year, current_user)
+      stats_by_month = stats.index_by(&:month)
+      
+      year_distances[year] = (1..12).map do |month|
+        month_name = Date::MONTHNAMES[month]
+        distance = stats_by_month[month]&.distance || 0
+        
+        [month_name, distance]
+      end
     end
 
     year_distances

app/controllers/visits_controller.rb

@@ -11,7 +11,7 @@ class VisitsController < ApplicationController
     visits = current_user
              .visits
              .where(status:)
-             .includes(%i[suggested_places area points])
+             .includes(%i[suggested_places area points place])
              .order(started_at: order_by)
 
     @suggested_visits_count = current_user.visits.suggested.count

app/javascript/application.js

@@ -1,5 +1,6 @@
 // Configure your import map in config/importmap.rb. Read more: https://github.com/rails/importmap-rails
 
+import "@rails/ujs"
 import "@rails/actioncable"
 import "controllers"
 import "@hotwired/turbo-rails"
@@ -13,5 +14,4 @@ import "./channels"
 import "trix"
 import "@rails/actiontext"
 
-import "@rails/ujs"
 Rails.start()

app/javascript/controllers/maps_controller.js

@@ -509,6 +509,11 @@ export default class extends BaseController {
         }
       } else if (event.name === 'Tracks') {
         this.handleRouteLayerToggle('tracks');
+      } else if (event.name === 'Areas') {
+        // Show draw control when Areas layer is enabled
+        if (this.drawControl && !this.map.hasControl && !this.map._controlCorners.topleft.querySelector('.leaflet-draw')) {
+          this.map.addControl(this.drawControl);
+        }
       }
 
       // Manage pane visibility when layers are manually toggled
@@ -523,6 +528,11 @@ export default class extends BaseController {
 
         // Manage pane visibility when layers are manually toggled
         this.updatePaneVisibilityAfterLayerChange();
+      } else if (event.name === 'Areas') {
+        // Hide draw control when Areas layer is disabled
+        if (this.drawControl && this.map._controlCorners.topleft.querySelector('.leaflet-draw')) {
+          this.map.removeControl(this.drawControl);
+        }
       }
     });
   }

app/javascript/maps/areas.js

@@ -81,6 +81,19 @@ export function handleAreaCreated(areasLayer, layer, apiKey) {
   const radius = layer.getRadius();
   const center = layer.getLatLng();
 
+  // Configure the layer with the same settings as existing areas
+  layer.setStyle({
+    color: 'red',
+    fillColor: '#f03',
+    fillOpacity: 0.5,
+    weight: 2,
+    interactive: true,
+    bubblingMouseEvents: false
+  });
+  
+  // Set the pane to match existing areas
+  layer.options.pane = 'areasPane';
+
   const formHtml = `
     <div class="card w-96 bg-base-100 border border-base-300 shadow-xl">
       <div class="card-body">

app/jobs/app_version_checking_job.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AppVersionCheckingJob < ApplicationJob
-  queue_as :default
+  queue_as :app_version_checking
   sidekiq_options retry: false
 
   def perform

app/jobs/cache/cleaning_job.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Cache::CleaningJob < ApplicationJob
-  queue_as :default
+  queue_as :cache
 
   def perform
     Cache::Clean.call

app/jobs/cache/preheating_job.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Cache::PreheatingJob < ApplicationJob
-  queue_as :default
+  queue_as :cache
 
   def perform
     User.find_each do |user|

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/jobs/data_migrations/migrate_places_lonlat_job.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class DataMigrations::MigratePlacesLonlatJob < ApplicationJob
-  queue_as :default
+  queue_as :data_migrations
 
   def perform(user_id)
     user = User.find(user_id)

app/jobs/data_migrations/migrate_points_latlon_job.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class DataMigrations::MigratePointsLatlonJob < ApplicationJob
-  queue_as :default
+  queue_as :data_migrations
 
   def perform(user_id)
     user = User.find(user_id)

app/jobs/data_migrations/set_points_country_ids_job.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class DataMigrations::SetPointsCountryIdsJob < ApplicationJob
-  queue_as :default
+  queue_as :data_migrations
 
   def perform(point_id)
     point = Point.find(point_id)

app/jobs/data_migrations/set_reverse_geocoded_at_for_points_job.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class DataMigrations::SetReverseGeocodedAtForPointsJob < ApplicationJob
-  queue_as :default
+  queue_as :data_migrations
 
   def perform
     timestamp = Time.current

app/jobs/data_migrations/start_settings_points_country_ids_job.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class DataMigrations::StartSettingsPointsCountryIdsJob < ApplicationJob
-  queue_as :default
+  queue_as :data_migrations
 
   def perform
     Point.where(country_id: nil).find_each do |point|

app/models/point.rb

@@ -17,7 +17,7 @@ class Point < ApplicationRecord
     index: true
   }
 
-  enum :battery_status, { unknown: 0, unplugged: 1, charging: 2, full: 3, connected_not_charging: 4 }, suffix: true
+  enum :battery_status, { unknown: 0, unplugged: 1, charging: 2, full: 3, connected_not_charging: 4, discharging: 5 }, suffix: true
   enum :trigger, {
     unknown: 0, background_event: 1, circular_region_event: 2, beacon_event: 3,
     report_location_message_event: 4, manual_event: 5, timer_based_event: 6,
@@ -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
+    tracked_points
-  end
+      .where.not(country_name: [nil, ''])
-
+      .distinct
-  def cities_visited
+      .pluck(:country_name)
-    stats
-      .where.not(toponyms: nil)
-      .pluck(:toponyms)
-      .flatten
-      .reject { |toponym| toponym['cities'].blank? }
-      .pluck('cities')
-      .flatten
-      .pluck('city')
-      .uniq
       .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? }
+      .reject { |point| point.country_name.nil? || point.city.nil? }
-      .group_by { |point| point.read_attribute(:country) }
+      .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/places/fetch_data.rb

@@ -19,18 +19,15 @@ class ReverseGeocoding::Places::FetchData
     first_place = places.shift
     update_place(first_place)
 
-    osm_ids = places.map { |place| place.data['properties']['osm_id'].to_s }
+    osm_ids = extract_osm_ids(places)
 
     return if osm_ids.empty?
 
-    existing_places =
+    existing_places = find_existing_places(osm_ids)
-      Place.where("geodata->'properties'->>'osm_id' IN (?)", osm_ids)
-        .index_by { |p| p.geodata.dig('properties', 'osm_id').to_s }
-        .compact
 
-    places.each do |reverse_geocoded_place|
+    places_to_create, places_to_update = prepare_places_for_bulk_operations(places, existing_places)
-      fetch_and_create_place(reverse_geocoded_place, existing_places)
+
-    end
+    save_places(places_to_create, places_to_update)
   end
 
   private
@@ -42,7 +39,7 @@ class ReverseGeocoding::Places::FetchData
 
     place.update!(
       name:       place_name(data),
-      lonlat:     "POINT(#{data['geometry']['coordinates'][0]} #{data['geometry']['coordinates'][1]})",
+      lonlat:     build_point_coordinates(data['geometry']['coordinates']),
       city:       data['properties']['city'],
       country:    data['properties']['country'],
       geodata:    data,
@@ -51,33 +48,20 @@ class ReverseGeocoding::Places::FetchData
     )
   end
 
-  def fetch_and_create_place(reverse_geocoded_place, existing_places)
-    data = reverse_geocoded_place.data
-    new_place = find_place(data, existing_places)
-
-    new_place.name = place_name(data)
-    new_place.city = data['properties']['city']
-    new_place.country = data['properties']['country'] # TODO: Use country id
-    new_place.geodata = data
-    new_place.source = :photon
-    if new_place.lonlat.blank?
-      new_place.lonlat = "POINT(#{data['geometry']['coordinates'][0]} #{data['geometry']['coordinates'][1]})"
-    end
-
-    new_place.save!
-  end
 
   def find_place(place_data, existing_places)
     osm_id = place_data['properties']['osm_id'].to_s
 
     existing_place = existing_places[osm_id]
+
     return existing_place if existing_place.present?
 
-    # If not found in existing places, initialize a new one
+    coordinates = place_data['geometry']['coordinates']
+
     Place.new(
-      lonlat: "POINT(#{place_data['geometry']['coordinates'][0].to_f.round(5)} #{place_data['geometry']['coordinates'][1].to_f.round(5)})",
+      lonlat: build_point_coordinates(coordinates),
-      latitude: place_data['geometry']['coordinates'][1].to_f.round(5),
+      latitude: coordinates[1].to_f.round(5),
-      longitude: place_data['geometry']['coordinates'][0].to_f.round(5)
+      longitude: coordinates[0].to_f.round(5)
     )
   end
 
@@ -92,6 +76,75 @@ class ReverseGeocoding::Places::FetchData
     "#{name} (#{type})"
   end
 
+  def extract_osm_ids(places)
+    places.map { |place| place.data['properties']['osm_id'].to_s }
+  end
+
+  def find_existing_places(osm_ids)
+    Place.where("geodata->'properties'->>'osm_id' IN (?)", osm_ids)
+      .index_by { |p| p.geodata.dig('properties', 'osm_id').to_s }
+      .compact
+  end
+
+  def prepare_places_for_bulk_operations(places, existing_places)
+    places_to_create = []
+    places_to_update = []
+
+    places.each do |reverse_geocoded_place|
+      data = reverse_geocoded_place.data
+      new_place = find_place(data, existing_places)
+
+      populate_place_attributes(new_place, data)
+
+      if new_place.persisted?
+        places_to_update << new_place
+      else
+        places_to_create << new_place
+      end
+    end
+
+    [places_to_create, places_to_update]
+  end
+
+  def populate_place_attributes(place, data)
+    place.name = place_name(data)
+    place.city = data['properties']['city']
+    place.country = data['properties']['country']
+    place.geodata = data
+    place.source = :photon
+
+    if place.lonlat.blank?
+      place.lonlat = build_point_coordinates(data['geometry']['coordinates'])
+    end
+  end
+
+  def save_places(places_to_create, places_to_update)
+    if places_to_create.any?
+      place_attributes = places_to_create.map do |place|
+        {
+          name: place.name,
+          latitude: place.latitude,
+          longitude: place.longitude,
+          lonlat: place.lonlat,
+          city: place.city,
+          country: place.country,
+          geodata: place.geodata,
+          source: place.source,
+          created_at: Time.current,
+          updated_at: Time.current
+        }
+      end
+      Place.insert_all(place_attributes)
+    end
+
+    # Individual updates for existing places
+    places_to_update.each(&:save!) if places_to_update.any?
+  end
+
+  def build_point_coordinates(coordinates)
+    "POINT(#{coordinates[0]} #{coordinates[1]})"
+  end
+
   def reverse_geocoded_places
     data = Geocoder.search(
       [place.lat, place.lon],

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

app/views/stats/_year.html.erb

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

config/sidekiq.yml

@@ -1,6 +1,7 @@
 ---
 :concurrency: <%= ENV.fetch("BACKGROUND_PROCESSING_CONCURRENCY", 10) %>
 :queues:
+  - data_migrations
   - points
   - default
   - imports
@@ -11,3 +12,5 @@
   - reverse_geocoding
   - visit_suggesting
   - places
+  - app_version_checking
+  - cache

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/jobs/data_migrations/migrate_places_lonlat_job_spec.rb

@@ -66,8 +66,8 @@ RSpec.describe DataMigrations::MigratePlacesLonlatJob, type: :job do
   end
 
   describe 'queue' do
-    it 'uses the default queue' do
+    it 'uses the data_migrations queue' do
-      expect(described_class.queue_name).to eq('default')
+      expect(described_class.queue_name).to eq('data_migrations')
     end
   end
 end

spec/jobs/data_migrations/set_points_country_ids_job_spec.rb

@@ -21,8 +21,8 @@ RSpec.describe DataMigrations::SetPointsCountryIdsJob, type: :job do
   end
 
   describe 'queue' do
-    it 'uses the default queue' do
+    it 'uses the data_migrations queue' do
-      expect(described_class.queue_name).to eq('default')
+      expect(described_class.queue_name).to eq('data_migrations')
     end
   end
 end

spec/jobs/data_migrations/start_settings_points_country_ids_job_spec.rb

@@ -26,8 +26,8 @@ RSpec.describe DataMigrations::StartSettingsPointsCountryIdsJob, type: :job do
   end
 
   describe 'queue' do
-    it 'uses the default queue' do
+    it 'uses the data_migrations queue' do
-      expect(described_class.queue_name).to eq('default')
+      expect(described_class.queue_name).to eq('data_migrations')
     end
   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!(:point1) { create(:point, user:, country_name: 'Germany') }
-      let!(:stat2) { create(:stat, user:, toponyms: [{ 'country' => 'France' }]) }
+      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!(:point1) { create(:point, user:, city: 'Berlin') }
-      let!(:stat2) { create(:stat, user:, toponyms: [{ 'cities' => [{ 'city' => 'Paris' }] }]) }
+      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
+      let!(:point1) { create(:point, user:, city: 'Berlin') }
-        create(
+      let!(:point2) { create(:point, user:, city: 'Paris') }
-          :stat,
+      let!(:point3) { create(:point, user:, city: nil) }
-          user:,
-          toponyms: [
-            { 'cities' => [], 'country' => nil },
-            { 'cities' => [{ 'city' => 'Berlin', 'points' => 64, 'timestamp' => 1_710_446_806, 'stayed_for' => 8772 }],
-'country' => 'Germany' }
-          ]
-        )
-      end
 
       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(:user) { create(:user) }
-    let!(:stats_in_2020) { create_list(:stat, 12, year: 2020, user:) }
+    let(:stats_in_2020) { create_list(:stat, 12, year: 2020, user:) }
-    let!(:stats_in_2021) { create_list(:stat, 12, year: 2021, user:) }
+    let(:stats_in_2021) { create_list(:stat, 12, year: 2021, user:) }
-    let!(:points_in_2020) do
+    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/reverse_geocoding/places/fetch_data_spec.rb

@@ -3,6 +3,617 @@
 require 'rails_helper'
 
 RSpec.describe ReverseGeocoding::Places::FetchData do
+  subject(:service) { described_class.new(place.id) }
+
+  let(:place) { create(:place) }
+  let(:mock_geocoded_place) do
+    double(
+      data: {
+        'geometry' => {
+          'coordinates' => [13.0948638, 54.2905245]
+        },
+        'properties' => {
+          'osm_id' => 12345,
+          'name' => 'Test Place',
+          'osm_value' => 'restaurant',
+          'city' => 'Berlin',
+          'country' => 'Germany',
+          'postcode' => '10115',
+          'street' => 'Test Street',
+          'housenumber' => '1'
+        }
+      }
+    )
+  end
+
   describe '#call' do
+    context 'when reverse geocoding is enabled' do
+      before do
+        allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(true)
+        allow(Geocoder).to receive(:search).and_return([mock_geocoded_place])
+      end
+
+      it 'fetches geocoded places' do
+        service.call
+
+        expect(Geocoder).to have_received(:search).with(
+          [place.lat, place.lon],
+          limit: 10,
+          distance_sort: true,
+          radius: 1,
+          units: :km
+        )
+      end
+
+      it 'updates the original place with geocoded data' do
+        expect { service.call }.to change { place.reload.name }
+          .and change { place.reload.city }.to('Berlin')
+          .and change { place.reload.country }.to('Germany')
+      end
+
+      it 'sets reverse_geocoded_at timestamp' do
+        expect { service.call }.to change { place.reload.reverse_geocoded_at }
+          .from(nil)
+
+        expect(place.reload.reverse_geocoded_at).to be_present
+      end
+
+      it 'sets the source to photon' do
+        expect { service.call }.to change { place.reload.source }
+          .to('photon')
+      end
+
+      context 'with multiple geocoded places' do
+        let(:second_mock_place) do
+          double(
+            data: {
+              'geometry' => {
+                'coordinates' => [13.1, 54.3]
+              },
+              'properties' => {
+                'osm_id' => 67890,
+                'name' => 'Second Place',
+                'osm_value' => 'cafe',
+                'city' => 'Hamburg',
+                'country' => 'Germany'
+              }
+            }
+          )
+        end
+
+        before do
+          allow(Geocoder).to receive(:search).and_return([mock_geocoded_place, second_mock_place])
+        end
+
+        it 'creates new places for additional geocoded results' do
+          # Force place creation before counting
+          place # This triggers the let(:place) lazy loading
+          initial_count = Place.count
+          service.call
+          final_count = Place.count
+
+          expect(final_count - initial_count).to eq(1)
+        end
+
+        it 'updates the original place and creates others' do
+          service.call
+
+          created_place = Place.where.not(id: place.id).first
+          expect(created_place.name).to include('Second Place')
+          expect(created_place.city).to eq('Hamburg')
+        end
+      end
+
+      context 'with existing places in database' do
+        let!(:existing_place) { create(:place, :with_geodata) }
+
+        before do
+          # Mock geocoded place with same OSM ID as existing place
+          existing_osm_id = existing_place.geodata.dig('properties', 'osm_id')
+          mock_with_existing_osm = double(
+            data: {
+              'geometry' => { 'coordinates' => [13.0948638, 54.2905245] },
+              'properties' => {
+                'osm_id' => existing_osm_id,
+                'name' => 'Updated Name',
+                'osm_value' => 'restaurant',
+                'city' => 'Updated City',
+                'country' => 'Updated Country'
+              }
+            }
+          )
+
+          allow(Geocoder).to receive(:search).and_return([mock_geocoded_place, mock_with_existing_osm])
+        end
+
+        it 'updates existing places instead of creating duplicates' do
+          place # Force place creation
+          expect { service.call }.not_to change { Place.count }
+        end
+
+        it 'updates the existing place attributes' do
+          service.call
+
+          existing_place.reload
+          expect(existing_place.name).to include('Updated Name')
+          expect(existing_place.city).to eq('Updated City')
+        end
+      end
+
+      context 'when first geocoded place is nil' do
+        before do
+          allow(Geocoder).to receive(:search).and_return([nil, mock_geocoded_place])
+        end
+
+        it 'does not update the original place' do
+          place # Force place creation
+          expect { service.call }.not_to change { place.reload.updated_at }
+        end
+
+        it 'still processes other places' do
+          place # Force place creation
+          expect { service.call }.to change { Place.count }.by(1)
+        end
+      end
+
+      context 'when no additional places are returned' do
+        before do
+          allow(Geocoder).to receive(:search).and_return([mock_geocoded_place])
+        end
+
+        it 'only updates the original place' do
+          place # Force place creation
+          expect { service.call }.not_to change { Place.count }
+        end
+
+        it 'returns early when osm_ids is empty' do
+          # This tests the early return when osm_ids.empty?
+          service.call
+
+          expect(Geocoder).to have_received(:search).once
+        end
+      end
+    end
+
+    context 'when reverse geocoding is disabled' do
+      before do
+        allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(false)
+        allow(Rails.logger).to receive(:warn)
+      end
+
+      it 'logs a warning and returns early' do
+        service.call
+
+        expect(Rails.logger).to have_received(:warn).with('Reverse geocoding is not enabled')
+      end
+
+      it 'does not call Geocoder' do
+        allow(Geocoder).to receive(:search)
+        service.call
+
+        expect(Geocoder).not_to have_received(:search)
+      end
+
+      it 'does not update the place' do
+        expect { service.call }.not_to change { place.reload.updated_at }
+      end
+    end
+  end
+
+  describe 'private methods' do
+    before do
+      allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(true)
+    end
+
+    describe '#place_name' do
+      it 'builds place name from properties' do
+        data = {
+          'properties' => {
+            'name' => 'Test Restaurant',
+            'osm_value' => 'restaurant',
+            'postcode' => '10115',
+            'street' => 'Main Street',
+            'housenumber' => '42'
+          }
+        }
+
+        result = service.send(:place_name, data)
+        expect(result).to eq('Test Restaurant (Restaurant)')
+      end
+
+      it 'uses address when name is missing' do
+        data = {
+          'properties' => {
+            'osm_value' => 'cafe',
+            'postcode' => '10115',
+            'street' => 'Oak Street',
+            'housenumber' => '123'
+          }
+        }
+
+        result = service.send(:place_name, data)
+        expect(result).to eq('10115 Oak Street 123 (Cafe)')
+      end
+
+      it 'handles missing housenumber' do
+        data = {
+          'properties' => {
+            'name' => 'Test Place',
+            'osm_value' => 'shop',
+            'postcode' => '10115',
+            'street' => 'Pine Street'
+          }
+        }
+
+        result = service.send(:place_name, data)
+        expect(result).to eq('Test Place (Shop)')
+      end
+
+      it 'formats osm_value correctly' do
+        data = {
+          'properties' => {
+            'name' => 'Test',
+            'osm_value' => 'fast_food_restaurant'
+          }
+        }
+
+        result = service.send(:place_name, data)
+        expect(result).to eq('Test (Fast food restaurant)')
+      end
+    end
+
+    describe '#extract_osm_ids' do
+      it 'extracts OSM IDs from places' do
+        places = [
+          double(data: { 'properties' => { 'osm_id' => 123 } }),
+          double(data: { 'properties' => { 'osm_id' => 456 } })
+        ]
+
+        result = service.send(:extract_osm_ids, places)
+        expect(result).to eq(['123', '456'])
+      end
+    end
+
+    describe '#build_point_coordinates' do
+      it 'builds POINT geometry string' do
+        coordinates = [13.0948638, 54.2905245]
+        result = service.send(:build_point_coordinates, coordinates)
+        expect(result).to eq('POINT(13.0948638 54.2905245)')
+      end
+    end
+
+    describe '#find_existing_places' do
+      let!(:existing_place1) { create(:place, :with_geodata) }
+      let!(:existing_place2) do
+        create(:place, geodata: {
+          'properties' => { 'osm_id' => 999 }
+        })
+      end
+
+      it 'finds existing places by OSM IDs' do
+        osm_id1 = existing_place1.geodata.dig('properties', 'osm_id').to_s
+        osm_ids = [osm_id1, '999']
+
+        result = service.send(:find_existing_places, osm_ids)
+
+        expect(result.keys).to contain_exactly(osm_id1, '999')
+        expect(result[osm_id1]).to eq(existing_place1)
+        expect(result['999']).to eq(existing_place2)
+      end
+
+      it 'returns empty hash when no matches found' do
+        result = service.send(:find_existing_places, ['nonexistent'])
+        expect(result).to eq({})
+      end
+    end
+
+    describe '#find_place' do
+      let(:existing_places) { { '123' => create(:place) } }
+      let(:place_data) do
+        {
+          'properties' => { 'osm_id' => 123 },
+          'geometry' => { 'coordinates' => [13.1, 54.3] }
+        }
+      end
+
+      context 'when place exists' do
+        it 'returns existing place' do
+          result = service.send(:find_place, place_data, existing_places)
+          expect(result).to eq(existing_places['123'])
+        end
+      end
+
+      context 'when place does not exist' do
+        let(:place_data) do
+          {
+            'properties' => { 'osm_id' => 999 },
+            'geometry' => { 'coordinates' => [13.1, 54.3] }
+          }
+        end
+
+        it 'creates new place with coordinates' do
+          result = service.send(:find_place, place_data, existing_places)
+
+          expect(result).to be_a(Place)
+          expect(result.latitude).to eq(54.3)
+          expect(result.longitude).to eq(13.1)
+          expect(result.lonlat.to_s).to eq('POINT (13.1 54.3)')
+        end
+      end
+    end
+
+    describe '#populate_place_attributes' do
+      let(:test_place) { Place.new }
+      let(:data) do
+        {
+          'properties' => {
+            'name' => 'Test Place',
+            'osm_value' => 'restaurant',
+            'city' => 'Berlin',
+            'country' => 'Germany'
+          },
+          'geometry' => { 'coordinates' => [13.1, 54.3] }
+        }
+      end
+
+      it 'populates all place attributes' do
+        place # Ensure place exists
+        service.send(:populate_place_attributes, test_place, data)
+
+        expect(test_place.name).to include('Test Place')
+        expect(test_place.city).to eq('Berlin')
+        expect(test_place.country).to eq('Germany')
+        expect(test_place.geodata).to eq(data)
+        expect(test_place.source).to eq('photon')
+      end
+
+      it 'sets lonlat when nil' do
+        place # Ensure place exists
+        service.send(:populate_place_attributes, test_place, data)
+        expect(test_place.lonlat.to_s).to eq('POINT (13.1 54.3)')
+      end
+
+      it 'does not override existing lonlat' do
+        place # Ensure place exists
+        test_place.lonlat = 'POINT(10.0 50.0)'
+        service.send(:populate_place_attributes, test_place, data)
+        expect(test_place.lonlat.to_s).to eq('POINT (10.0 50.0)')
+      end
+    end
+
+    describe '#prepare_places_for_bulk_operations' do
+      let(:new_place_data) do
+        double(
+          data: {
+            'properties' => { 'osm_id' => 999 },
+            'geometry' => { 'coordinates' => [13.1, 54.3] }
+          }
+        )
+      end
+      let(:existing_place) { create(:place, :with_geodata) }
+      let(:existing_place_data) do
+        double(
+          data: {
+            'properties' => { 'osm_id' => existing_place.geodata.dig('properties', 'osm_id') },
+            'geometry' => { 'coordinates' => [13.2, 54.4] }
+          }
+        )
+      end
+
+      it 'separates places into create and update arrays' do
+        existing_places = { existing_place.geodata.dig('properties', 'osm_id').to_s => existing_place }
+        places = [new_place_data, existing_place_data]
+
+        places_to_create, places_to_update = service.send(:prepare_places_for_bulk_operations, places, existing_places)
+
+        expect(places_to_create.length).to eq(1)
+        expect(places_to_update.length).to eq(1)
+        expect(places_to_update.first).to eq(existing_place)
+        expect(places_to_create.first).to be_a(Place)
+        expect(places_to_create.first.persisted?).to be(false)
+      end
+    end
+
+    describe '#save_places' do
+      it 'saves new places when places_to_create is present' do
+        place # Ensure place exists
+        new_place = build(:place)
+        places_to_create = [new_place]
+        places_to_update = []
+
+        expect { service.send(:save_places, places_to_create, places_to_update) }
+          .to change { Place.count }.by(1)
+      end
+
+      it 'saves updated places when places_to_update is present' do
+        existing_place = create(:place, name: 'Old Name')
+        existing_place.name = 'New Name'
+        places_to_create = []
+        places_to_update = [existing_place]
+
+        service.send(:save_places, places_to_create, places_to_update)
+
+        expect(existing_place.reload.name).to eq('New Name')
+      end
+
+      it 'handles empty arrays gracefully' do
+        expect { service.send(:save_places, [], []) }.not_to raise_error
+      end
+    end
+  end
+
+  describe 'edge cases and error scenarios' do
+    before do
+      allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(true)
+    end
+
+    context 'when Geocoder returns empty results' do
+      before do
+        allow(Geocoder).to receive(:search).and_return([])
+      end
+
+      it 'handles empty geocoder results gracefully' do
+        expect { service.call }.not_to raise_error
+      end
+
+      it 'does not update the place' do
+        expect { service.call }.not_to change { place.reload.updated_at }
+      end
+    end
+
+    context 'when Geocoder raises an exception' do
+      before do
+        allow(Geocoder).to receive(:search).and_raise(StandardError.new('Geocoding failed'))
+      end
+
+      it 'allows the exception to bubble up' do
+        expect { service.call }.to raise_error(StandardError, 'Geocoding failed')
+      end
+    end
+
+    context 'when place data is malformed' do
+      let(:malformed_place) do
+        double(
+          data: {
+            'geometry' => {
+              'coordinates' => ['invalid', 'coordinates']
+            },
+            'properties' => {
+              'osm_id' => nil
+            }
+          }
+        )
+      end
+
+      before do
+        allow(Geocoder).to receive(:search).and_return([mock_geocoded_place, malformed_place])
+      end
+
+      it 'handles malformed data gracefully' do
+        # With bulk operations using insert_all, validation errors are bypassed
+        # Malformed data will be inserted but may cause issues at the database level
+        place # Force place creation
+        expect { service.call }.not_to raise_error
+      end
+    end
+
+    context 'when using bulk operations' do
+      let(:second_geocoded_place) do
+        double(
+          data: {
+            'geometry' => { 'coordinates' => [14.0, 55.0] },
+            'properties' => {
+              'osm_id' => 99999,
+              'name' => 'Another Place',
+              'osm_value' => 'shop'
+            }
+          }
+        )
+      end
+
+      it 'uses bulk operations for performance' do
+        place # Force place creation first
+
+        allow(Geocoder).to receive(:search).and_return([mock_geocoded_place, second_geocoded_place])
+        # With insert_all, we expect the operation to succeed even with potential validation issues
+        # since bulk operations bypass ActiveRecord validations for performance
+
+        expect { service.call }.to change { Place.count }.by(1)
+      end
+    end
+
+    context 'when database constraint violations occur' do
+      let(:duplicate_place) { create(:place, :with_geodata) }
+      let(:duplicate_data) do
+        double(
+          data: {
+            'geometry' => { 'coordinates' => [13.1, 54.3] },
+            'properties' => {
+              'osm_id' => duplicate_place.geodata.dig('properties', 'osm_id'),
+              'name' => 'Duplicate'
+            }
+          }
+        )
+      end
+
+      before do
+        allow(Geocoder).to receive(:search).and_return([mock_geocoded_place, duplicate_data])
+        # Simulate the place not being found in existing_places due to race condition
+        allow(service).to receive(:find_existing_places).and_return({})
+      end
+
+      it 'handles potential race conditions gracefully' do
+        # The service should handle cases where a place might be created
+        # between the existence check and the actual creation
+        expect { service.call }.not_to raise_error
+      end
+    end
+
+    context 'when place_id does not exist' do
+      subject(:service) { described_class.new(999999) }
+
+      it 'raises ActiveRecord::RecordNotFound' do
+        expect { service }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    context 'with missing properties in geocoded data' do
+      let(:minimal_place) do
+        double(
+          data: {
+            'geometry' => {
+              'coordinates' => [13.0, 54.0]
+            },
+            'properties' => {
+              'osm_id' => 99999
+              # Missing name, city, country, etc.
+            }
+          }
+        )
+      end
+
+      before do
+        allow(Geocoder).to receive(:search).and_return([mock_geocoded_place, minimal_place])
+      end
+
+      it 'handles missing properties gracefully' do
+        expect { service.call }.not_to raise_error
+      end
+
+      it 'creates place with available data' do
+        place # Force place creation
+        expect { service.call }.to change { Place.count }.by(1)
+
+        created_place = Place.where.not(id: place.id).first
+        expect(created_place.latitude).to eq(54.0)
+        expect(created_place.longitude).to eq(13.0)
+      end
+    end
+
+    context 'when lonlat is already present on existing place' do
+      let!(:existing_place) { create(:place, :with_geodata, lonlat: 'POINT(10.0 50.0)') }
+      let(:existing_data) do
+        double(
+          data: {
+            'geometry' => { 'coordinates' => [15.0, 55.0] },
+            'properties' => {
+              'osm_id' => existing_place.geodata.dig('properties', 'osm_id'),
+              'name' => 'Updated Name'
+            }
+          }
+        )
+      end
+
+      before do
+        allow(Geocoder).to receive(:search).and_return([mock_geocoded_place, existing_data])
+      end
+
+      it 'does not override existing lonlat' do
+        service.call
+
+        existing_place.reload
+        expect(existing_place.lonlat.to_s).to eq('POINT (10.0 50.0)')
+      end
+    end
   end
 end

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