diff --git a/app/models/place.rb b/app/models/place.rb index 99f57756..d7cde7f6 100644 --- a/app/models/place.rb +++ b/app/models/place.rb @@ -22,10 +22,6 @@ class Place < ApplicationRecord lonlat.y end - def reverse_geocoded? - geodata.present? - end - def osm_id geodata['properties']['osm_id'] end diff --git a/app/models/visit.rb b/app/models/visit.rb index 2ca3faf8..4e452a11 100644 --- a/app/models/visit.rb +++ b/app/models/visit.rb @@ -12,10 +12,6 @@ class Visit < ApplicationRecord enum :status, { suggested: 0, confirmed: 1, declined: 2 } - def reverse_geocoded? - place.geodata.present? - end - def coordinates points.pluck(:latitude, :longitude).map { [_1[0].to_f, _1[1].to_f] } end diff --git a/app/services/reverse_geocoding/places/fetch_data.rb b/app/services/reverse_geocoding/places/fetch_data.rb index 1390c29a..733ddde7 100644 --- a/app/services/reverse_geocoding/places/fetch_data.rb +++ b/app/services/reverse_geocoding/places/fetch_data.rb @@ -57,10 +57,6 @@ class ReverseGeocoding::Places::FetchData new_place.save! end - def reverse_geocoded? - place.geodata.present? - end - def find_place(place_data) found_place = Place.where( "geodata->'properties'->>'osm_id' = ?", place_data['properties']['osm_id'].to_s diff --git a/app/services/visits/detector.rb b/app/services/visits/detector.rb index 13a2d64b..eced74ba 100644 --- a/app/services/visits/detector.rb +++ b/app/services/visits/detector.rb @@ -7,10 +7,11 @@ module Visits MAXIMUM_VISIT_GAP = 30.minutes MINIMUM_POINTS_FOR_VISIT = 2 - attr_reader :points + attr_reader :points, :place_name_suggester def initialize(points) @points = points + @place_name_suggester = PlaceNameSuggester end def detect_potential_visits @@ -111,48 +112,7 @@ module Visits end def suggest_place_name(points) - # Get points with geodata - geocoded_points = points.select { |p| p.geodata.present? && !p.geodata.empty? } - return nil if geocoded_points.empty? - - # Extract all features from points' geodata - features = geocoded_points.flat_map do |point| - next [] unless point.geodata['features'].is_a?(Array) - - point.geodata['features'] - end.compact - - return nil if features.empty? - - # Group features by type and count occurrences - feature_counts = features.group_by { |f| f.dig('properties', 'type') } - .transform_values(&:size) - - # Find the most common feature type - most_common_type = feature_counts.max_by { |_, count| count }&.first - return nil unless most_common_type - - # Get all features of the most common type - common_features = features.select { |f| f.dig('properties', 'type') == most_common_type } - - # Group these features by name and get the most common one - name_counts = common_features.group_by { |f| f.dig('properties', 'name') } - .transform_values(&:size) - most_common_name = name_counts.max_by { |_, count| count }&.first - - return if most_common_name.blank? - - # If we have a name, try to get additional context - feature = common_features.find { |f| f.dig('properties', 'name') == most_common_name } - properties = feature['properties'] - - # Build a more descriptive name if possible - [ - most_common_name, - properties['street'], - properties['city'], - properties['state'] - ].compact.uniq.join(', ') + place_name_suggester.new(points).call end end end diff --git a/app/services/visits/place_name_suggester.rb b/app/services/visits/place_name_suggester.rb new file mode 100644 index 00000000..d86e5573 --- /dev/null +++ b/app/services/visits/place_name_suggester.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Visits + # Suggests names for places based on geodata from tracked points + class PlaceNameSuggester + def initialize(points) + @points = points + end + + def call + geocoded_points = extract_geocoded_points(points) + return nil if geocoded_points.empty? + + features = extract_features(geocoded_points) + return nil if features.empty? + + most_common_type = find_most_common_feature_type(features) + return nil unless most_common_type + + most_common_name = find_most_common_name(features, most_common_type) + return nil if most_common_name.blank? + + build_descriptive_name(features, most_common_type, most_common_name) + end + + private + + attr_reader :points + + def extract_geocoded_points(points) + points.select { |p| p.geodata.present? && !p.geodata.empty? } + end + + def extract_features(geocoded_points) + geocoded_points.flat_map do |point| + next [] unless point.geodata['features'].is_a?(Array) + + point.geodata['features'] + end.compact + end + + def find_most_common_feature_type(features) + feature_counts = features.group_by { |f| f.dig('properties', 'type') } + .transform_values(&:size) + feature_counts.max_by { |_, count| count }&.first + end + + def find_most_common_name(features, feature_type) + common_features = features.select { |f| f.dig('properties', 'type') == feature_type } + name_counts = common_features.group_by { |f| f.dig('properties', 'name') } + .transform_values(&:size) + name_counts.max_by { |_, count| count }&.first + end + + def build_descriptive_name(features, feature_type, name) + feature = features.find do |f| + f.dig('properties', 'type') == feature_type && + f.dig('properties', 'name') == name + end + + properties = feature['properties'] + [ + name, + properties['street'], + properties['city'], + properties['state'] + ].compact.uniq.join(', ') + end + end +end diff --git a/app/views/stats/_reverse_geocoding_stats.html.erb b/app/views/stats/_reverse_geocoding_stats.html.erb index d7fb145e..11d3768f 100644 --- a/app/views/stats/_reverse_geocoding_stats.html.erb +++ b/app/views/stats/_reverse_geocoding_stats.html.erb @@ -1,14 +1,16 @@ -
-
- <%= number_with_delimiter @points_reverse_geocoded %> +<% if DawarichSettings.store_geodata? %> +
+
+ <%= number_with_delimiter @points_reverse_geocoded %> +
+
Reverse geocoded points
+
+ + <%= number_with_delimiter @points_reverse_geocoded_without_data %> points without data + +
-
Reverse geocoded points
-
- - <%= number_with_delimiter @points_reverse_geocoded_without_data %> points without data - -
-
+<% end %>
diff --git a/spec/services/visits/place_name_suggester_spec.rb b/spec/services/visits/place_name_suggester_spec.rb new file mode 100644 index 00000000..8b326d8a --- /dev/null +++ b/spec/services/visits/place_name_suggester_spec.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Visits::PlaceNameSuggester do + subject(:suggester) { described_class.new(points) } + + describe '#call' do + context 'when no points have geodata' do + let(:points) do + [ + double('Point', geodata: nil), + double('Point', geodata: {}) + ] + end + + it 'returns nil' do + expect(suggester.call).to be_nil + end + end + + context 'when points have geodata but no features' do + let(:points) do + [ + double('Point', geodata: { 'features' => [] }) + ] + end + + it 'returns nil' do + expect(suggester.call).to be_nil + end + end + + context 'when features exist but with different types' do + let(:points) do + [ + double('Point', geodata: { + 'features' => [ + { 'properties' => { 'type' => 'cafe', 'name' => 'Coffee Shop' } }, + { 'properties' => { 'type' => 'restaurant', 'name' => 'Pizza Place' } } + ] + }) + ] + end + + it 'returns the name of the most common type' do + # Since both types appear once, it will pick the first one alphabetically in practice + expect(suggester.call).to eq('Coffee Shop') + end + end + + context 'when features have a common type but different names' do + let(:points) do + [ + double('Point', geodata: { + 'features' => [ + { 'properties' => { 'type' => 'park', 'name' => 'Central Park' } } + ] + }), + double('Point', geodata: { + 'features' => [ + { 'properties' => { 'type' => 'park', 'name' => 'City Park' } } + ] + }), + double('Point', geodata: { + 'features' => [ + { 'properties' => { 'type' => 'park', 'name' => 'Central Park' } } + ] + }) + ] + end + + it 'returns the most common name' do + expect(suggester.call).to eq('Central Park') + end + end + + context 'when a complete place can be built' do + let(:points) do + [ + double('Point', geodata: { + 'features' => [ + { + 'properties' => { + 'type' => 'cafe', + 'name' => 'Starbucks', + 'street' => '123 Main St', + 'city' => 'San Francisco', + 'state' => 'CA' + } + } + ] + }) + ] + end + + it 'returns a descriptive name with all components' do + expect(suggester.call).to eq('Starbucks, 123 Main St, San Francisco, CA') + end + end + + context 'when only partial place details are available' do + let(:points) do + [ + double('Point', geodata: { + 'features' => [ + { + 'properties' => { + 'type' => 'cafe', + 'name' => 'Starbucks', + 'city' => 'San Francisco' + # No street or state + } + } + ] + }) + ] + end + + it 'returns a name with available components' do + expect(suggester.call).to eq('Starbucks, San Francisco') + end + end + + context 'when points have geodata with non-array features' do + let(:points) do + [ + double('Point', geodata: { 'features' => 'not an array' }) + ] + end + + it 'returns nil' do + expect(suggester.call).to be_nil + end + end + + context 'when most common name is blank' do + let(:points) do + [ + double('Point', geodata: { + 'features' => [ + { 'properties' => { 'type' => 'road', 'name' => '' } } + ] + }) + ] + end + + it 'returns nil' do + expect(suggester.call).to be_nil + end + end + end +end