diff --git a/app/services/countries_and_cities.rb b/app/services/countries_and_cities.rb index 7a260256..0785107a 100644 --- a/app/services/countries_and_cities.rb +++ b/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| - cities = process_country_points(country_points) - CountryData.new(country: country, cities: cities) if cities.any? - end.compact + .transform_values { |country_points| process_country_points(country_points) } + .map { |country, cities| CountryData.new(country: country, cities: cities) } 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) - filter_sessions(sessions) + country_points + .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 diff --git a/spec/services/countries_and_cities_spec.rb b/spec/services/countries_and_cities_spec.rb index 530a534c..636823e5 100644 --- a/spec/services/countries_and_cities_spec.rb +++ b/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, - # with sufficient points to demonstrate the city grouping logic + # we have 5 points in the same city and country within 1 hour, + # 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: 'Kerpen', country: 'Belgium', timestamp: timestamp + 10.minutes), - create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 20.minutes), - create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 30.minutes), - create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 40.minutes), - create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 50.minutes), - create(:point, city: 'Kerpen', country: 'Germany', timestamp: timestamp + 60.minutes), - create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 70.minutes), - create(:point, city: 'Kerpen', country: 'Belgium', timestamp: timestamp + 80.minutes), - create(:point, city: 'Kerpen', 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) + 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: 'Berlin', country: 'Germany', timestamp: timestamp + 30.minutes), + create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 40.minutes), + create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 50.minutes), + create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 60.minutes), + create(:point, city: 'Berlin', country: 'Germany', timestamp: timestamp + 70.minutes), + create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 80.minutes), + create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 90.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 - # Germany is excluded because the consecutive points in Kerpen, Germany - # span only 30 minutes (less than MIN_MINUTES_SPENT_IN_CITY) - expect(countries_and_cities).to contain_exactly( - an_object_having_attributes( - country: 'Belgium', - cities: contain_exactly( - an_object_having_attributes( - city: 'Kerpen', - points: 11, - stayed_for: 140 - ) + expect(countries_and_cities).to eq( + [ + CountriesAndCities::CountryData.new( + country: 'Germany', + cities: [ + CountriesAndCities::CityData.new( + city: 'Berlin', points: 8, timestamp: 1_609_463_400, stayed_for: 70 + ) + ] + ), + 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: 'Berlin', country: 'Germany', timestamp: timestamp + 100.minutes), - create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 110.minutes) + create(:point, city: 'Brugges', country: 'Belgium', timestamp: timestamp + 90.minutes) ] end - it 'returns only countries with cities where the user stayed long enough' do - # Only Germany is included because Berlin points span 100 minutes - # Belgium is excluded because Brugges points are in separate visits - # spanning only 10 and 20 minutes each - expect(countries_and_cities).to contain_exactly( - an_object_having_attributes( - country: 'Germany', - cities: contain_exactly( - an_object_having_attributes( - city: 'Berlin', - points: 4, - stayed_for: 100 - ) + it 'returns countries and cities' do + expect(countries_and_cities).to eq( + [ + CountriesAndCities::CountryData.new( + country: 'Germany', + cities: [] + ), + CountriesAndCities::CountryData.new( + country: 'Belgium', + cities: [] ) - ) + ] ) end end