From 1e2c7090470ee86fe68b627272721642fd0faae9 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sat, 22 Nov 2025 13:28:38 +0100 Subject: [PATCH] Revert changes in place export/import code --- CHANGELOG.md | 4 +-- app/controllers/api/v1/places_controller.rb | 11 ++++--- .../migrate_places_lonlat_job.rb | 4 +-- app/services/users/export_data.rb | 2 +- app/services/users/import_data/places.rb | 5 ++-- app/services/visits/place_finder.rb | 29 +++++++++--------- db/seeds.rb | 15 ++++++++++ spec/services/users/export_data_spec.rb | 4 +-- .../users/export_import_integration_spec.rb | 6 ++-- .../services/users/import_data/places_spec.rb | 30 +++++++++++-------- spec/services/visits/place_finder_spec.rb | 11 ++++--- 11 files changed, 68 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91285f07..dc74b4ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,9 +23,7 @@ OIDC_REDIRECT_URI=https://your-dawarich-url.com/users/auth/openid_connect/callba - Added a commented line in the `docker-compose.yml` file to use an alternative PostGIS image for ARM architecture. - User can now create a place directly from the map and add tags and notes to it. If reverse geocoding is enabled, list of nearby places will be shown as suggestions. - User can create and manage tags for places. - - [ ] Tags can be added when creating or editing a place. - - [ ] Provide sensible list of default tags for the user to choose from. -- User can enable or disable places layers on the map to show/hide all or just some of their places based on tags. +- User can enable or disable places layers on the map to show/hide all or just some of their visited places based on tags. - User can define privacy zones around places with specific tags to hide map data within a certain radius. ## Fixed diff --git a/app/controllers/api/v1/places_controller.rb b/app/controllers/api/v1/places_controller.rb index 5675ea7f..805b40e1 100644 --- a/app/controllers/api/v1/places_controller.rb +++ b/app/controllers/api/v1/places_controller.rb @@ -22,8 +22,8 @@ module Api if @place.save add_tags if tag_ids.present? - @place.reload # Reload to get updated associations - render json: serialize_place(@place), status: :created + + render json: serialize_place(@place.reload), status: :created else render json: { errors: @place.errors.full_messages }, status: :unprocessable_entity end @@ -70,10 +70,9 @@ module Api end def tag_ids - # tag_ids can be in params[:place][:tag_ids] or params[:tag_ids] - ids = params.dig(:place, :tag_ids) || params[:tag_ids] - Array(ids).compact - end + ids = params.dig(:place, :tag_ids) + Array(ids).compact + end def add_tags return if tag_ids.empty? diff --git a/app/jobs/data_migrations/migrate_places_lonlat_job.rb b/app/jobs/data_migrations/migrate_places_lonlat_job.rb index eae51eaf..0acb89a3 100644 --- a/app/jobs/data_migrations/migrate_places_lonlat_job.rb +++ b/app/jobs/data_migrations/migrate_places_lonlat_job.rb @@ -7,7 +7,7 @@ class DataMigrations::MigratePlacesLonlatJob < ApplicationJob user = User.find(user_id) # Find all places with nil lonlat - places_to_update = user.places.where(lonlat: nil) + places_to_update = user.visited_places.where(lonlat: nil) # For each place, set the lonlat value based on longitude and latitude places_to_update.find_each do |place| @@ -20,7 +20,7 @@ class DataMigrations::MigratePlacesLonlatJob < ApplicationJob end # Double check if there are any remaining places without lonlat - remaining = user.places.where(lonlat: nil) + remaining = user.visited_places.where(lonlat: nil) return unless remaining.exists? # Log an error for these places diff --git a/app/services/users/export_data.rb b/app/services/users/export_data.rb index fa7c32b5..29caa8dd 100644 --- a/app/services/users/export_data.rb +++ b/app/services/users/export_data.rb @@ -325,7 +325,7 @@ class Users::ExportData notifications: user.notifications.count, points: user.points_count, visits: user.visits.count, - places: user.places.count + places: user.visited_places.count } Rails.logger.info "Entity counts: #{counts}" diff --git a/app/services/users/import_data/places.rb b/app/services/users/import_data/places.rb index cab8a3f6..ade22373 100644 --- a/app/services/users/import_data/places.rb +++ b/app/services/users/import_data/places.rb @@ -76,10 +76,10 @@ class Users::ImportData::Places logger.debug "Processing place for import: #{name} at (#{latitude}, #{longitude})" existing_place = Place.where( - user_id: user.id, name: name, latitude: latitude, - longitude: longitude + longitude: longitude, + user_id: nil ).first if existing_place @@ -94,7 +94,6 @@ class Users::ImportData::Places place_attributes['lonlat'] = "POINT(#{longitude} #{latitude})" place_attributes['latitude'] = latitude place_attributes['longitude'] = longitude - place_attributes['user_id'] = user.id place_attributes.delete('user') logger.debug "Creating place with attributes: #{place_attributes.inspect}" diff --git a/app/services/visits/place_finder.rb b/app/services/visits/place_finder.rb index 65a8ce24..e2f3a3ab 100644 --- a/app/services/visits/place_finder.rb +++ b/app/services/visits/place_finder.rb @@ -47,15 +47,15 @@ module Visits # Step 1: Find existing place def find_existing_place(lat, lon, name) # Try to find existing place by location first - existing_by_location = user.places.near([lat, lon], SIMILARITY_RADIUS, :m).first + existing_by_location = Place.near([lat, lon], SIMILARITY_RADIUS, :m).first return existing_by_location if existing_by_location # Then try by name if available return nil if name.blank? - user.places.where(name: name) - .near([lat, lon], SEARCH_RADIUS, :m) - .first + Place.where(name: name) + .near([lat, lon], SEARCH_RADIUS, :m) + .first end # Step 2: Collect potential places from all sources @@ -113,14 +113,14 @@ module Visits return nil if name == Place::DEFAULT_NAME # Look for existing place with this name - existing = user.places.where(name: name) - .near([point.lat, point.lon], SIMILARITY_RADIUS, :m) - .first + existing = Place.where(name: name) + .near([point.lat, point.lon], SIMILARITY_RADIUS, :m) + .first return existing if existing # Create new place - place = user.places.build( + place = Place.new( name: name, lonlat: "POINT(#{point.lon} #{point.lat})", latitude: point.lat, @@ -165,15 +165,16 @@ module Visits return nil if name == Place::DEFAULT_NAME # Look for existing place with this name - existing = user.places.where(name: name) - .near([result.latitude, result.longitude], SIMILARITY_RADIUS, :m) - .first + existing = Place.where(name: name) + .near([result.latitude, result.longitude], SIMILARITY_RADIUS, :m) + .first return existing if existing # Create new place - place = user.places.build( + place = Place.new( name: name, + lonlat: "POINT(#{result.longitude} #{result.latitude})", latitude: result.latitude, longitude: result.longitude, city: properties['city'], @@ -204,7 +205,7 @@ module Visits def create_default_place(lat, lon, suggested_name) name = suggested_name.presence || Place::DEFAULT_NAME - place = user.places.build( + place = Place.new( name: name, lonlat: "POINT(#{lon} #{lat})", latitude: lat, @@ -218,7 +219,7 @@ module Visits # Step 9: Find suggested places def find_suggested_places(lat, lon) - user.places.near([lat, lon], SEARCH_RADIUS, :m).with_distance([lat, lon], :m) + Place.near([lat, lon], SEARCH_RADIUS, :m).with_distance([lat, lon], :m) end # Helper methods diff --git a/db/seeds.rb b/db/seeds.rb index 8e96a514..a1b7342c 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -38,3 +38,18 @@ if Country.none? end end end + +if Tag.none? + puts 'Creating default tags...' + + default_tags = [ + { name: 'Home', color: '#FF5733', icon: '🏡' }, + { name: 'Work', color: '#33FF57', icon: '💼' }, + { name: 'Favorite', color: '#3357FF', icon: '⭐' }, + { name: 'Travel Plans', color: '#F1C40F', icon: '🗺️' }, + ] + + default_tags.each do |tag_attrs| + Tag.create!(tag_attrs) + end +end diff --git a/spec/services/users/export_data_spec.rb b/spec/services/users/export_data_spec.rb index 1d668ece..0953192d 100644 --- a/spec/services/users/export_data_spec.rb +++ b/spec/services/users/export_data_spec.rb @@ -47,7 +47,7 @@ RSpec.describe Users::ExportData, type: :service do allow(user).to receive(:notifications).and_return(double(count: 10)) allow(user).to receive(:points_count).and_return(15000) allow(user).to receive(:visits).and_return(double(count: 45)) - allow(user).to receive(:places).and_return(double(count: 20)) + allow(user).to receive(:visited_places).and_return(double(count: 20)) # Mock Export creation and file attachment exports_double = double('Exports', count: 3) @@ -376,7 +376,7 @@ RSpec.describe Users::ExportData, type: :service do allow(user).to receive(:notifications).and_return(double(count: 10)) allow(user).to receive(:points_count).and_return(15000) allow(user).to receive(:visits).and_return(double(count: 45)) - allow(user).to receive(:places).and_return(double(count: 20)) + allow(user).to receive(:visited_places).and_return(double(count: 20)) allow(Rails.logger).to receive(:info) end diff --git a/spec/services/users/export_import_integration_spec.rb b/spec/services/users/export_import_integration_spec.rb index ead92687..a163a29c 100644 --- a/spec/services/users/export_import_integration_spec.rb +++ b/spec/services/users/export_import_integration_spec.rb @@ -141,7 +141,7 @@ RSpec.describe 'Users Export-Import Integration', type: :service do create(:visit, user: original_user, place: nil, name: 'Unknown Location') # Calculate counts properly - places are accessed through visits - original_places_count = original_user.places.distinct.count + original_places_count = original_user.visited_places.distinct.count original_visits_count = original_user.visits.count # Export the data @@ -187,7 +187,7 @@ RSpec.describe 'Users Export-Import Integration', type: :service do "Expected #{original_visits_count} visits to be created, got #{import_stats[:visits_created]}" # Verify the imported user has access to all their data - imported_places_count = import_user.places.distinct.count + imported_places_count = import_user.visited_places.distinct.count imported_visits_count = import_user.visits.count expect(imported_places_count).to \ @@ -309,7 +309,7 @@ RSpec.describe 'Users Export-Import Integration', type: :service do notifications: user.notifications.count, points: user.points.count, visits: user.visits.count, - places: user.places.count + places: user.visited_places.count } end diff --git a/spec/services/users/import_data/places_spec.rb b/spec/services/users/import_data/places_spec.rb index 545e9d90..bcb5e7da 100644 --- a/spec/services/users/import_data/places_spec.rb +++ b/spec/services/users/import_data/places_spec.rb @@ -71,9 +71,10 @@ RSpec.describe Users::ImportData::Places, type: :service do context 'with duplicate places (same name)' do before do - # Create an existing place with same name but different coordinates for the same user - create(:place, user: user, name: 'Home', - latitude: 41.0000, longitude: -75.0000) + # Create an existing place with same name but different coordinates + create(:place, name: 'Home', + latitude: 41.0000, longitude: -75.0000, + lonlat: 'POINT(-75.0000 41.0000)') end it 'creates the place since coordinates are different' do @@ -82,7 +83,7 @@ RSpec.describe Users::ImportData::Places, type: :service do it 'creates both places with different coordinates' do service.call - home_places = user.places.where(name: 'Home') + home_places = Place.where(name: 'Home') expect(home_places.count).to eq(2) imported_home = home_places.find_by(latitude: 40.7128, longitude: -74.0060) @@ -92,9 +93,10 @@ RSpec.describe Users::ImportData::Places, type: :service do context 'with exact duplicate places (same name and coordinates)' do before do - # Create an existing place with exact same name and coordinates for the same user - create(:place, user: user, name: 'Home', - latitude: 40.7128, longitude: -74.0060) + # Create an existing place with exact same name and coordinates + create(:place, name: 'Home', + latitude: 40.7128, longitude: -74.0060, + lonlat: 'POINT(-74.0060 40.7128)') end it 'skips exact duplicate places' do @@ -116,9 +118,10 @@ RSpec.describe Users::ImportData::Places, type: :service do context 'with duplicate places (same coordinates)' do before do - # Create an existing place with same coordinates but different name for the same user - create(:place, user: user, name: 'Different Name', - latitude: 40.7128, longitude: -74.0060) + # Create an existing place with same coordinates but different name + create(:place, name: 'Different Name', + latitude: 40.7128, longitude: -74.0060, + lonlat: 'POINT(-74.0060 40.7128)') end it 'creates the place since name is different' do @@ -127,7 +130,7 @@ RSpec.describe Users::ImportData::Places, type: :service do it 'creates both places with different names' do service.call - places_at_location = user.places.where(latitude: 40.7128, longitude: -74.0060) + places_at_location = Place.where(latitude: 40.7128, longitude: -74.0060) expect(places_at_location.count).to eq(2) expect(places_at_location.pluck(:name)).to contain_exactly('Home', 'Different Name') end @@ -135,8 +138,9 @@ RSpec.describe Users::ImportData::Places, type: :service do context 'with places having same name but different coordinates' do before do - create(:place, user: user, name: 'Different Place', - latitude: 41.0000, longitude: -75.0000) + create(:place, name: 'Different Place', + latitude: 41.0000, longitude: -75.0000, + lonlat: 'POINT(-75.0000 41.0000)') end it 'creates both places since coordinates and names differ' do diff --git a/spec/services/visits/place_finder_spec.rb b/spec/services/visits/place_finder_spec.rb index 817b4f8f..3da17828 100644 --- a/spec/services/visits/place_finder_spec.rb +++ b/spec/services/visits/place_finder_spec.rb @@ -20,7 +20,7 @@ RSpec.describe Visits::PlaceFinder do end context 'when an existing place is found' do - let!(:existing_place) { create(:place, user: user, latitude: latitude, longitude: longitude) } + let!(:existing_place) { create(:place, latitude: latitude, longitude: longitude) } it 'returns the existing place as main_place' do result = subject.find_or_create_place(visit_data) @@ -38,7 +38,6 @@ RSpec.describe Visits::PlaceFinder do it 'finds an existing place by name within search radius' do similar_named_place = create(:place, - user: user, name: 'Test Place', latitude: latitude + 0.0001, longitude: longitude + 0.0001) @@ -184,9 +183,9 @@ RSpec.describe Visits::PlaceFinder do end context 'with multiple potential places' do - let!(:place1) { create(:place, user: user, name: 'Place 1', latitude: latitude, longitude: longitude) } - let!(:place2) { create(:place, user: user, name: 'Place 2', latitude: latitude + 0.0005, longitude: longitude + 0.0005) } - let!(:place3) { create(:place, user: user, name: 'Place 3', latitude: latitude + 0.001, longitude: longitude + 0.001) } + let!(:place1) { create(:place, name: 'Place 1', latitude: latitude, longitude: longitude) } + let!(:place2) { create(:place, name: 'Place 2', latitude: latitude + 0.0005, longitude: longitude + 0.0005) } + let!(:place3) { create(:place, name: 'Place 3', latitude: latitude + 0.001, longitude: longitude + 0.001) } it 'selects the closest place as main_place' do result = subject.find_or_create_place(visit_data) @@ -202,7 +201,7 @@ RSpec.describe Visits::PlaceFinder do end it 'may include places with the same name' do - dup_place = create(:place, user: user, name: 'Place 1', latitude: latitude + 0.0002, longitude: longitude + 0.0002) + dup_place = create(:place, name: 'Place 1', latitude: latitude + 0.0002, longitude: longitude + 0.0002) allow(subject).to receive(:place_name_exists?).and_return(false)