Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix logic for grouping consecutive points in CountriesAndCities #610

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix logic for grouping consecutive points in CountriesAndCities #610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1b767d
Fix logic for grouping consecutive points in CountriesAndCities
arne182 Jan 1, 2025
d6cbda9
Update countries_and_cities_spec.rb
arne182 Jan 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 46 additions & 25 deletions app/services/countries_and_cities.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,43 +12,64 @@ def call
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) }
.map do |country, country_points|
cities = process_country_points(country_points)
CountryData.new(country: country, cities: cities) if cities.any?
end.compact
end

private

attr_reader :points

def process_country_points(country_points)
country_points
.group_by(&:city)
.transform_values { |city_points| create_city_data_if_valid(city_points) }
.values
.compact
end
# 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

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
prev_point = sorted_points[index - 1]

build_city_data(city, points_count, timestamps, duration)
# 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

def build_city_data(city, points_count, timestamps, duration)
return nil if duration < ::MIN_MINUTES_SPENT_IN_CITY
# 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

CityData.new(
city: city,
points: points_count,
timestamp: timestamps.max,
stayed_for: duration
)
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

def calculate_duration_in_minutes(timestamps)
((timestamps.max - timestamps.min).to_i / 60)
# Process points for each country
def process_country_points(country_points)
sessions = group_points_with_consecutive_cities(country_points)
filter_sessions(sessions)
end
end
51 changes: 23 additions & 28 deletions spec/services/countries_and_cities_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,29 @@
describe '#call' do
subject(:countries_and_cities) { described_class.new(points).call }

# 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
# we have 15 points in the same city and different country within 2 hour,
# 4 points in the differnt city within 10 minutes splitting the country
# and we expect to get one country with one city which has 8 points

let(:timestamp) { DateTime.new(2021, 1, 1, 0, 0, 0) }

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: '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)
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)
]
end

Expand All @@ -37,16 +42,12 @@
expect(countries_and_cities).to eq(
[
CountriesAndCities::CountryData.new(
country: 'Germany',
country: 'Belgium',
cities: [
CountriesAndCities::CityData.new(
city: 'Berlin', points: 8, timestamp: 1_609_463_400, stayed_for: 70
city: 'Kerpen', points: 8, timestamp: 1_609_467_600, stayed_for: 70
)
]
),
CountriesAndCities::CountryData.new(
country: 'Belgium',
cities: []
)
]
)
Expand All @@ -60,21 +61,15 @@
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 countries and cities' do
expect(countries_and_cities).to eq(
[
CountriesAndCities::CountryData.new(
country: 'Germany',
cities: []
),
CountriesAndCities::CountryData.new(
country: 'Belgium',
cities: []
)
]
)
end
Expand Down