Revert cities and countries logic

app/services/countries_and_cities.rb

@@ -12,64 +12,43 @@ class CountriesAndCities
     points
       .reject { |point| point.country.nil? || point.city.nil? }
       .group_by(&:country)
-      .map do |country, country_points|
+      .transform_values { |country_points| process_country_points(country_points) }
-        cities = process_country_points(country_points)
+      .map { |country, cities| CountryData.new(country: country, cities: cities) }
-        CountryData.new(country: country, cities: cities) if cities.any?
-      end.compact
   end
 
   private
 
   attr_reader :points
 
-# Step 1: Process points to group by consecutive cities and time
-  def group_points_with_consecutive_cities(country_points)
-    sorted_points = country_points.sort_by(&:timestamp)
-
-    sessions = []
-    current_session = []
-
-    sorted_points.each_with_index do |point, index|
-      if current_session.empty?
-        current_session << point
-        next
-      end
-
-      prev_point = sorted_points[index - 1]
-
-      # Split session if city changes or time gap exceeds the threshold
-      if point.city != prev_point.city
-        sessions << current_session
-        current_session = []
-      end
-
-      current_session << point
-    end
-
-    sessions << current_session unless current_session.empty?
-    sessions
-  end
-
-  # Step 2: Filter sessions that don't meet the minimum minutes per city
-  def filter_sessions(sessions)
-    sessions.map do |session|
-      end_time = session.last.timestamp
-      duration = (end_time - session.first.timestamp) / 60 # Convert seconds to minutes
-
-      if duration >= MIN_MINUTES_SPENT_IN_CITY
-        CityData.new(
-          city: session.first.city,
-          points: session.size,
-          timestamp: end_time,
-          stayed_for: duration
-        )
-      end
-    end.compact
-  end
-
-  # Process points for each country
   def process_country_points(country_points)
-    sessions = group_points_with_consecutive_cities(country_points)
+    country_points
-    filter_sessions(sessions)
+      .group_by(&:city)
+      .transform_values { |city_points| create_city_data_if_valid(city_points) }
+      .values
+      .compact
+  end
+
+  def create_city_data_if_valid(city_points)
+    timestamps = city_points.pluck(:timestamp)
+    duration = calculate_duration_in_minutes(timestamps)
+    city = city_points.first.city
+    points_count = city_points.size
+
+    build_city_data(city, points_count, timestamps, duration)
+  end
+
+  def build_city_data(city, points_count, timestamps, duration)
+    return nil if duration < ::MIN_MINUTES_SPENT_IN_CITY
+
+    CityData.new(
+      city: city,
+      points: points_count,
+      timestamp: timestamps.max,
+      stayed_for: duration
+    )
+  end
+
+  def calculate_duration_in_minutes(timestamps)
+    ((timestamps.max - timestamps.min).to_i / 60)
   end
 end

spec/services/countries_and_cities_spec.rb

@@ -6,27 +6,24 @@ RSpec.describe CountriesAndCities do
   describe '#call' do
     subject(:countries_and_cities) { described_class.new(points).call }
 
-    # Test with a set of points in the same city (Kerpen) but different countries,
+    # we have 5 points in the same city and country within 1 hour,
-    # with sufficient points to demonstrate the city grouping logic
+    # 5 points in the differnt city within 10 minutes
+    # and we expect to get one country with one city which has 5 points
+
     let(:timestamp) { DateTime.new(2021, 1, 1, 0, 0, 0) }
 
     let(:points) do
       [
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp:),
+        create(:point, city: 'Berlin', country: 'Germany', timestamp:),
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 10.minutes),
+        create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 10.minutes),
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 20.minutes),
+        create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 20.minutes),
-        create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 30.minutes),
+        create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 30.minutes),
-        create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 40.minutes),
+        create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 40.minutes),
-        create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 50.minutes),
+        create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 50.minutes),
-        create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 60.minutes),
+        create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 60.minutes),
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 70.minutes),
+        create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 70.minutes),
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 80.minutes),
+        create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 80.minutes),
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 90.minutes),
+        create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 90.minutes)
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 100.minutes),
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 110.minutes),
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 120.minutes),
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 130.minutes),
-        create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 140.minutes)
       ]
     end
 
@@ -37,52 +34,48 @@ RSpec.describe CountriesAndCities do
 
       context 'when user stayed in the city for more than 1 hour' do
         it 'returns countries and cities' do
-          # Only Belgium has cities where the user stayed long enough
+          expect(countries_and_cities).to eq(
-          # Germany is excluded because the consecutive points in Kerpen, Germany
+            [
-          # span only 30 minutes (less than MIN_MINUTES_SPENT_IN_CITY)
+              CountriesAndCities::CountryData.new(
-          expect(countries_and_cities).to contain_exactly(
+                country: 'Germany',
-            an_object_having_attributes(
+                cities: [
-              country: 'Belgium',
+                  CountriesAndCities::CityData.new(
-              cities: contain_exactly(
+                    city: 'Berlin', points: 8, timestamp: 1_609_463_400, stayed_for: 70
-                an_object_having_attributes(
+                  )
-                  city: 'Kerpen',
+                ]
-                  points: 11,
+              ),
-                  stayed_for: 140
+              CountriesAndCities::CountryData.new(
-                )
+                country: 'Belgium',
+                cities: []
               )
-            )
+            ]
           )
         end
       end
 
-      context 'when user stayed in the city for less than 1 hour in some cities but more in others' do
+      context 'when user stayed in the city for less than 1 hour' do
         let(:points) do
           [
             create(:point, city: 'Berlin', country: 'Germany', timestamp:),
             create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 10.minutes),
             create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 20.minutes),
             create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 80.minutes),
-            create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 90.minutes),
+            create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 90.minutes)
-            create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 100.minutes),
-            create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 110.minutes)
           ]
         end
 
-        it 'returns only countries with cities where the user stayed long enough' do
+        it 'returns countries and cities' do
-          # Only Germany is included because Berlin points span 100 minutes
+          expect(countries_and_cities).to eq(
-          # Belgium is excluded because Brugges points are in separate visits
+            [
-          # spanning only 10 and 20 minutes each
+              CountriesAndCities::CountryData.new(
-          expect(countries_and_cities).to contain_exactly(
+                country: 'Germany',
-            an_object_having_attributes(
+                cities: []
-              country: 'Germany',
+              ),
-              cities: contain_exactly(
+              CountriesAndCities::CountryData.new(
-                an_object_having_attributes(
+                country: 'Belgium',
-                  city: 'Berlin',
+                cities: []
-                  points: 4,
-                  stayed_for: 100
-                )
               )
-            )
+            ]
           )
         end
       end