From 5cde596884b6d8dfba72634b640c15bd3cebd1b7 Mon Sep 17 00:00:00 2001
From: Eugene Burmakin <iamfrey@gmail.com>
Date: Wed, 11 Dec 2024 17:14:26 +0100
Subject: [PATCH] Rework countries and cities service

---
 app/jobs/stats/calculating_job.rb             |  7 ++-
 app/services/countries_and_cities.rb          | 62 ++++++++-----------
 app/views/shared/_right_sidebar.html.erb      |  3 -
 ...11170110_add_index_to_points_timestamp.rb} |  0
 4 files changed, 31 insertions(+), 41 deletions(-)
 rename db/migrate/{[timestamp]_add_index_to_points_timestamp.rb => 20241211170110_add_index_to_points_timestamp.rb} (100%)

diff --git a/app/jobs/stats/calculating_job.rb b/app/jobs/stats/calculating_job.rb
index 26f4756e..f7a5bc73 100644
--- a/app/jobs/stats/calculating_job.rb
+++ b/app/jobs/stats/calculating_job.rb
@@ -13,11 +13,14 @@ class Stats::CalculatingJob < ApplicationJob
 
   private
 
-  def create_stats_updated_notification(user_id)
+  def create_stats_updated_notification(user_id, year, month)
     user = User.find(user_id)
 
     Notifications::Create.new(
-      user:, kind: :info, title: 'Stats updated', content: 'Stats updated'
+      user:,
+      kind: :info,
+      title: "Stats updated: #{year}-#{month}",
+      content: "Stats updated for #{year}-#{month}"
     ).call
   end
 
diff --git a/app/services/countries_and_cities.rb b/app/services/countries_and_cities.rb
index 026484d1..efb36511 100644
--- a/app/services/countries_and_cities.rb
+++ b/app/services/countries_and_cities.rb
@@ -1,56 +1,46 @@
 # frozen_string_literal: true
 
 class CountriesAndCities
+  CityStats = Struct.new(:points, :last_timestamp, :stayed_for, keyword_init: true)
+  CountryData = Struct.new(:country, :cities, keyword_init: true)
+  CityData = Struct.new(:city, :points, :timestamp, :stayed_for, keyword_init: true)
+
   def initialize(points)
     @points = points
   end
 
   def call
-    grouped_records = group_points
-    mapped_with_cities = map_with_cities(grouped_records)
-    filtered_cities = filter_cities(mapped_with_cities)
-    normalize_result(filtered_cities)
+    points
+      .reject { |point| point.country.nil? || point.city.nil? }
+      .group_by(&:country)
+      .transform_values { |country_points| process_country_points(country_points) }
+      .map { |country, cities| CountryData.new(country: country, cities: cities) }
   end
 
   private
 
   attr_reader :points
 
-  def group_points
-    points.group_by(&:country)
+  def process_country_points(country_points)
+    country_points
+      .group_by(&:city)
+      .transform_values do |city_points|
+        timestamps = city_points.map(&:timestamp)
+        build_city_data(city_points.first.city, city_points.size, timestamps)
+      end
+      .values
   end
 
-  def map_with_cities(grouped_records)
-    grouped_records.transform_values do |grouped_points|
-      grouped_points
-        .pluck(:city, :timestamp) # Extract city and timestamp
-        .delete_if { _1.first.nil? } # Remove records without city
-        .group_by { |city, _| city } # Group by city
-        .transform_values do |cities|
-          {
-            points: cities.count,
-            last_timestamp: cities.map(&:last).max, # Get the maximum timestamp
-            stayed_for: ((cities.map(&:last).max - cities.map(&:last).min).to_i / 60) # Calculate the time stayed in minutes
-          }
-        end
-    end
+  def build_city_data(city, points_count, timestamps)
+    CityData.new(
+      city: city,
+      points: points_count,
+      timestamp: timestamps.max,
+      stayed_for: calculate_duration_in_minutes(timestamps)
+    )
   end
 
-  def filter_cities(mapped_with_cities)
-    # Remove cities where user stayed for less than 1 hour
-    mapped_with_cities.transform_values do |cities|
-      cities.reject { |_, data| data[:stayed_for] < MIN_MINUTES_SPENT_IN_CITY }
-    end
-  end
-
-  def normalize_result(hash)
-    hash.map do |country, cities|
-      {
-        country:,
-        cities: cities.map do |city, data|
-          { city:, points: data[:points], timestamp: data[:last_timestamp], stayed_for: data[:stayed_for] }
-        end
-      }
-    end
+  def calculate_duration_in_minutes(timestamps)
+    ((timestamps.max - timestamps.min).to_i / 60)
   end
 end
diff --git a/app/views/shared/_right_sidebar.html.erb b/app/views/shared/_right_sidebar.html.erb
index 797adaeb..a0f6b2d6 100644
--- a/app/views/shared/_right_sidebar.html.erb
+++ b/app/views/shared/_right_sidebar.html.erb
@@ -31,10 +31,7 @@
 
 <% if REVERSE_GEOCODING_ENABLED && @countries_and_cities&.any? %>
   <hr class='my-5'>
-  <h2 class='text-lg font-semibold'>Countries and cities</h2>
   <% @countries_and_cities.each do |country| %>
-    <% next if country[:cities].empty? %>
-
     <h2 class="text-lg font-semibold mt-5">
       <%= country[:country] %> (<%= country[:cities].count %> cities)
     </h2>
diff --git a/db/migrate/[timestamp]_add_index_to_points_timestamp.rb b/db/migrate/20241211170110_add_index_to_points_timestamp.rb
similarity index 100%
rename from db/migrate/[timestamp]_add_index_to_points_timestamp.rb
rename to db/migrate/20241211170110_add_index_to_points_timestamp.rb