diff --git a/app/models/user.rb b/app/models/user.rb index 8743c132..4b35390e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -45,18 +45,13 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength def countries_visited Rails.cache.fetch("dawarich/user_#{id}_countries_visited", expires_in: 1.day) do - points - .without_raw_data - .where.not(country_name: [nil, '']) - .distinct - .pluck(:country_name) - .compact + countries_visited_uncached end end def cities_visited Rails.cache.fetch("dawarich/user_#{id}_cities_visited", expires_in: 1.day) do - points.where.not(city: [nil, '']).distinct.pluck(:city).compact + cities_visited_uncached end end @@ -139,17 +134,47 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength Time.zone.name end + # Aggregate countries from all stats' toponyms + # This is more accurate than raw point queries as it uses processed data def countries_visited_uncached - points - .without_raw_data - .where.not(country_name: [nil, '']) - .distinct - .pluck(:country_name) - .compact + countries = Set.new + + stats.find_each do |stat| + toponyms = stat.toponyms + next unless toponyms.is_a?(Array) + + toponyms.each do |toponym| + next unless toponym.is_a?(Hash) + + countries.add(toponym['country']) if toponym['country'].present? + end + end + + countries.to_a.sort end + # Aggregate cities from all stats' toponyms + # This respects MIN_MINUTES_SPENT_IN_CITY since toponyms are already filtered def cities_visited_uncached - points.where.not(city: [nil, '']).distinct.pluck(:city).compact + cities = Set.new + + stats.find_each do |stat| + toponyms = stat.toponyms + next unless toponyms.is_a?(Array) + + toponyms.each do |toponym| + next unless toponym.is_a?(Hash) + next unless toponym['cities'].is_a?(Array) + + toponym['cities'].each do |city| + next unless city.is_a?(Hash) + + cities.add(city['city']) if city['city'].present? + end + end + end + + cities.to_a.sort end def home_place_coordinates diff --git a/app/services/users/digests/calculate_year.rb b/app/services/users/digests/calculate_year.rb index b01f37cf..f97c6578 100644 --- a/app/services/users/digests/calculate_year.rb +++ b/app/services/users/digests/calculate_year.rb @@ -216,8 +216,8 @@ module Users def calculate_all_time_stats { - 'total_countries' => user.countries_visited.count, - 'total_cities' => user.cities_visited.count, + 'total_countries' => user.countries_visited_uncached.size, + 'total_cities' => user.cities_visited_uncached.size, 'total_distance' => user.stats.sum(:distance).to_s } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7eac9400..e016ce3b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -163,12 +163,16 @@ RSpec.describe User, type: :model do describe '#countries_visited' do subject { user.countries_visited } - 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: '') } + let!(:stat) do + create(:stat, user:, toponyms: [ + { 'country' => 'Germany', 'cities' => [{ 'city' => 'Berlin', 'stayed_for' => 120 }] }, + { 'country' => 'France', 'cities' => [{ 'city' => 'Paris', 'stayed_for' => 90 }] }, + { 'country' => nil, 'cities' => [] }, + { 'country' => '', 'cities' => [] } + ]) + end - it 'returns array of countries' do + it 'returns array of countries from stats toponyms' do expect(subject).to include('Germany', 'France') expect(subject.count).to eq(2) end @@ -181,12 +185,18 @@ RSpec.describe User, type: :model do describe '#cities_visited' do subject { user.cities_visited } - 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: '') } + let!(:stat) do + create(:stat, user:, toponyms: [ + { 'country' => 'Germany', 'cities' => [ + { 'city' => 'Berlin', 'stayed_for' => 120 }, + { 'city' => nil, 'stayed_for' => 60 }, + { 'city' => '', 'stayed_for' => 60 } + ] }, + { 'country' => 'France', 'cities' => [{ 'city' => 'Paris', 'stayed_for' => 90 }] } + ]) + end - it 'returns array of cities' do + it 'returns array of cities from stats toponyms' do expect(subject).to include('Berlin', 'Paris') expect(subject.count).to eq(2) end @@ -210,11 +220,15 @@ RSpec.describe User, type: :model do describe '#total_countries' do subject { user.total_countries } - 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!(:stat) do + create(:stat, user:, toponyms: [ + { 'country' => 'Germany', 'cities' => [] }, + { 'country' => 'France', 'cities' => [] }, + { 'country' => nil, 'cities' => [] } + ]) + end - it 'returns number of countries' do + it 'returns number of countries from stats toponyms' do expect(subject).to eq(2) end end @@ -222,11 +236,17 @@ RSpec.describe User, type: :model do describe '#total_cities' do subject { user.total_cities } - let!(:point1) { create(:point, user:, city: 'Berlin') } - let!(:point2) { create(:point, user:, city: 'Paris') } - let!(:point3) { create(:point, user:, city: nil) } + let!(:stat) do + create(:stat, user:, toponyms: [ + { 'country' => 'Germany', 'cities' => [ + { 'city' => 'Berlin', 'stayed_for' => 120 }, + { 'city' => 'Paris', 'stayed_for' => 90 }, + { 'city' => nil, 'stayed_for' => 60 } + ] } + ]) + end - it 'returns number of cities' do + it 'returns number of cities from stats toponyms' do expect(subject).to eq(2) end end