Eliminate some n-plus-ones.

CHANGELOG.md

@@ -4,6 +4,14 @@ 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.
+
 # [0.30.5] - 2025-07-26
 
 ## Fixed

app/controllers/stats_controller.rb

@@ -57,8 +57,15 @@ class StatsController < ApplicationController
   def precompute_year_distances
     year_distances = {}
 
-    @stats.each do |year, _stats|
-      year_distances[year] = Stat.year_distance(year, current_user)
+    @stats.each do |year, stats|
+      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/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/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/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 =
-      Place.where("geodata->'properties'->>'osm_id' IN (?)", osm_ids)
-        .index_by { |p| p.geodata.dig('properties', 'osm_id').to_s }
-        .compact
+    existing_places = find_existing_places(osm_ids)
 
-    places.each do |reverse_geocoded_place|
-      fetch_and_create_place(reverse_geocoded_place, existing_places)
-    end
+    places_to_create, places_to_update = prepare_places_for_bulk_operations(places, existing_places)
+
+    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,21 +48,6 @@ 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
@@ -74,10 +56,11 @@ class ReverseGeocoding::Places::FetchData
     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)})",
-      latitude: place_data['geometry']['coordinates'][1].to_f.round(5),
-      longitude: place_data['geometry']['coordinates'][0].to_f.round(5)
+      lonlat: build_point_coordinates(coordinates),
+      latitude: coordinates[1].to_f.round(5),
+      longitude: coordinates[0].to_f.round(5)
     )
   end
 
@@ -92,6 +75,56 @@ 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)
+    Place.create!(places_to_create) if places_to_create.any?
+    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],

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

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
-      expect(described_class.queue_name).to eq('default')
+    it 'uses the data_migrations queue' do
+      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
-      expect(described_class.queue_name).to eq('default')
+    it 'uses the data_migrations queue' do
+      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
-      expect(described_class.queue_name).to eq('default')
+    it 'uses the data_migrations queue' do
+      expect(described_class.queue_name).to eq('data_migrations')
     end
   end
 end