Revert changes in place export/import code

This commit is contained in:
Eugene Burmakin 2025-11-22 13:28:38 +01:00
parent 50bfece971
commit 1e2c709047
11 changed files with 68 additions and 53 deletions

View file

@ -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. - 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 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. - User can create and manage tags for places.
- [ ] Tags can be added when creating or editing a place. - User can enable or disable places layers on the map to show/hide all or just some of their visited places based on tags.
- [ ] 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 define privacy zones around places with specific tags to hide map data within a certain radius. - User can define privacy zones around places with specific tags to hide map data within a certain radius.
## Fixed ## Fixed

View file

@ -22,8 +22,8 @@ module Api
if @place.save if @place.save
add_tags if tag_ids.present? 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 else
render json: { errors: @place.errors.full_messages }, status: :unprocessable_entity render json: { errors: @place.errors.full_messages }, status: :unprocessable_entity
end end
@ -70,10 +70,9 @@ module Api
end end
def tag_ids def tag_ids
# tag_ids can be in params[:place][:tag_ids] or params[:tag_ids] ids = params.dig(:place, :tag_ids)
ids = params.dig(:place, :tag_ids) || params[:tag_ids] Array(ids).compact
Array(ids).compact end
end
def add_tags def add_tags
return if tag_ids.empty? return if tag_ids.empty?

View file

@ -7,7 +7,7 @@ class DataMigrations::MigratePlacesLonlatJob < ApplicationJob
user = User.find(user_id) user = User.find(user_id)
# Find all places with nil lonlat # 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 # For each place, set the lonlat value based on longitude and latitude
places_to_update.find_each do |place| places_to_update.find_each do |place|
@ -20,7 +20,7 @@ class DataMigrations::MigratePlacesLonlatJob < ApplicationJob
end end
# Double check if there are any remaining places without lonlat # 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? return unless remaining.exists?
# Log an error for these places # Log an error for these places

View file

@ -325,7 +325,7 @@ class Users::ExportData
notifications: user.notifications.count, notifications: user.notifications.count,
points: user.points_count, points: user.points_count,
visits: user.visits.count, visits: user.visits.count,
places: user.places.count places: user.visited_places.count
} }
Rails.logger.info "Entity counts: #{counts}" Rails.logger.info "Entity counts: #{counts}"

View file

@ -76,10 +76,10 @@ class Users::ImportData::Places
logger.debug "Processing place for import: #{name} at (#{latitude}, #{longitude})" logger.debug "Processing place for import: #{name} at (#{latitude}, #{longitude})"
existing_place = Place.where( existing_place = Place.where(
user_id: user.id,
name: name, name: name,
latitude: latitude, latitude: latitude,
longitude: longitude longitude: longitude,
user_id: nil
).first ).first
if existing_place if existing_place
@ -94,7 +94,6 @@ class Users::ImportData::Places
place_attributes['lonlat'] = "POINT(#{longitude} #{latitude})" place_attributes['lonlat'] = "POINT(#{longitude} #{latitude})"
place_attributes['latitude'] = latitude place_attributes['latitude'] = latitude
place_attributes['longitude'] = longitude place_attributes['longitude'] = longitude
place_attributes['user_id'] = user.id
place_attributes.delete('user') place_attributes.delete('user')
logger.debug "Creating place with attributes: #{place_attributes.inspect}" logger.debug "Creating place with attributes: #{place_attributes.inspect}"

View file

@ -47,15 +47,15 @@ module Visits
# Step 1: Find existing place # Step 1: Find existing place
def find_existing_place(lat, lon, name) def find_existing_place(lat, lon, name)
# Try to find existing place by location first # 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 return existing_by_location if existing_by_location
# Then try by name if available # Then try by name if available
return nil if name.blank? return nil if name.blank?
user.places.where(name: name) Place.where(name: name)
.near([lat, lon], SEARCH_RADIUS, :m) .near([lat, lon], SEARCH_RADIUS, :m)
.first .first
end end
# Step 2: Collect potential places from all sources # Step 2: Collect potential places from all sources
@ -113,14 +113,14 @@ module Visits
return nil if name == Place::DEFAULT_NAME return nil if name == Place::DEFAULT_NAME
# Look for existing place with this name # Look for existing place with this name
existing = user.places.where(name: name) existing = Place.where(name: name)
.near([point.lat, point.lon], SIMILARITY_RADIUS, :m) .near([point.lat, point.lon], SIMILARITY_RADIUS, :m)
.first .first
return existing if existing return existing if existing
# Create new place # Create new place
place = user.places.build( place = Place.new(
name: name, name: name,
lonlat: "POINT(#{point.lon} #{point.lat})", lonlat: "POINT(#{point.lon} #{point.lat})",
latitude: point.lat, latitude: point.lat,
@ -165,15 +165,16 @@ module Visits
return nil if name == Place::DEFAULT_NAME return nil if name == Place::DEFAULT_NAME
# Look for existing place with this name # Look for existing place with this name
existing = user.places.where(name: name) existing = Place.where(name: name)
.near([result.latitude, result.longitude], SIMILARITY_RADIUS, :m) .near([result.latitude, result.longitude], SIMILARITY_RADIUS, :m)
.first .first
return existing if existing return existing if existing
# Create new place # Create new place
place = user.places.build( place = Place.new(
name: name, name: name,
lonlat: "POINT(#{result.longitude} #{result.latitude})",
latitude: result.latitude, latitude: result.latitude,
longitude: result.longitude, longitude: result.longitude,
city: properties['city'], city: properties['city'],
@ -204,7 +205,7 @@ module Visits
def create_default_place(lat, lon, suggested_name) def create_default_place(lat, lon, suggested_name)
name = suggested_name.presence || Place::DEFAULT_NAME name = suggested_name.presence || Place::DEFAULT_NAME
place = user.places.build( place = Place.new(
name: name, name: name,
lonlat: "POINT(#{lon} #{lat})", lonlat: "POINT(#{lon} #{lat})",
latitude: lat, latitude: lat,
@ -218,7 +219,7 @@ module Visits
# Step 9: Find suggested places # Step 9: Find suggested places
def find_suggested_places(lat, lon) 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 end
# Helper methods # Helper methods

View file

@ -38,3 +38,18 @@ if Country.none?
end end
end 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

View file

@ -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(:notifications).and_return(double(count: 10))
allow(user).to receive(:points_count).and_return(15000) allow(user).to receive(:points_count).and_return(15000)
allow(user).to receive(:visits).and_return(double(count: 45)) 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 # Mock Export creation and file attachment
exports_double = double('Exports', count: 3) 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(:notifications).and_return(double(count: 10))
allow(user).to receive(:points_count).and_return(15000) allow(user).to receive(:points_count).and_return(15000)
allow(user).to receive(:visits).and_return(double(count: 45)) 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) allow(Rails.logger).to receive(:info)
end end

View file

@ -141,7 +141,7 @@ RSpec.describe 'Users Export-Import Integration', type: :service do
create(:visit, user: original_user, place: nil, name: 'Unknown Location') create(:visit, user: original_user, place: nil, name: 'Unknown Location')
# Calculate counts properly - places are accessed through visits # 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 original_visits_count = original_user.visits.count
# Export the data # 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]}" "Expected #{original_visits_count} visits to be created, got #{import_stats[:visits_created]}"
# Verify the imported user has access to all their data # 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 imported_visits_count = import_user.visits.count
expect(imported_places_count).to \ expect(imported_places_count).to \
@ -309,7 +309,7 @@ RSpec.describe 'Users Export-Import Integration', type: :service do
notifications: user.notifications.count, notifications: user.notifications.count,
points: user.points.count, points: user.points.count,
visits: user.visits.count, visits: user.visits.count,
places: user.places.count places: user.visited_places.count
} }
end end

View file

@ -71,9 +71,10 @@ RSpec.describe Users::ImportData::Places, type: :service do
context 'with duplicate places (same name)' do context 'with duplicate places (same name)' do
before do before do
# Create an existing place with same name but different coordinates for the same user # Create an existing place with same name but different coordinates
create(:place, user: user, name: 'Home', create(:place, name: 'Home',
latitude: 41.0000, longitude: -75.0000) latitude: 41.0000, longitude: -75.0000,
lonlat: 'POINT(-75.0000 41.0000)')
end end
it 'creates the place since coordinates are different' do 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 it 'creates both places with different coordinates' do
service.call service.call
home_places = user.places.where(name: 'Home') home_places = Place.where(name: 'Home')
expect(home_places.count).to eq(2) expect(home_places.count).to eq(2)
imported_home = home_places.find_by(latitude: 40.7128, longitude: -74.0060) 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 context 'with exact duplicate places (same name and coordinates)' do
before do before do
# Create an existing place with exact same name and coordinates for the same user # Create an existing place with exact same name and coordinates
create(:place, user: user, name: 'Home', create(:place, name: 'Home',
latitude: 40.7128, longitude: -74.0060) latitude: 40.7128, longitude: -74.0060,
lonlat: 'POINT(-74.0060 40.7128)')
end end
it 'skips exact duplicate places' do 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 context 'with duplicate places (same coordinates)' do
before do before do
# Create an existing place with same coordinates but different name for the same user # Create an existing place with same coordinates but different name
create(:place, user: user, name: 'Different Name', create(:place, name: 'Different Name',
latitude: 40.7128, longitude: -74.0060) latitude: 40.7128, longitude: -74.0060,
lonlat: 'POINT(-74.0060 40.7128)')
end end
it 'creates the place since name is different' do 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 it 'creates both places with different names' do
service.call 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.count).to eq(2)
expect(places_at_location.pluck(:name)).to contain_exactly('Home', 'Different Name') expect(places_at_location.pluck(:name)).to contain_exactly('Home', 'Different Name')
end end
@ -135,8 +138,9 @@ RSpec.describe Users::ImportData::Places, type: :service do
context 'with places having same name but different coordinates' do context 'with places having same name but different coordinates' do
before do before do
create(:place, user: user, name: 'Different Place', create(:place, name: 'Different Place',
latitude: 41.0000, longitude: -75.0000) latitude: 41.0000, longitude: -75.0000,
lonlat: 'POINT(-75.0000 41.0000)')
end end
it 'creates both places since coordinates and names differ' do it 'creates both places since coordinates and names differ' do

View file

@ -20,7 +20,7 @@ RSpec.describe Visits::PlaceFinder do
end end
context 'when an existing place is found' do 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 it 'returns the existing place as main_place' do
result = subject.find_or_create_place(visit_data) 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 it 'finds an existing place by name within search radius' do
similar_named_place = create(:place, similar_named_place = create(:place,
user: user,
name: 'Test Place', name: 'Test Place',
latitude: latitude + 0.0001, latitude: latitude + 0.0001,
longitude: longitude + 0.0001) longitude: longitude + 0.0001)
@ -184,9 +183,9 @@ RSpec.describe Visits::PlaceFinder do
end end
context 'with multiple potential places' do context 'with multiple potential places' do
let!(:place1) { create(:place, user: user, name: 'Place 1', latitude: latitude, longitude: longitude) } let!(:place1) { create(:place, 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!(:place2) { create(:place, 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!(:place3) { create(:place, name: 'Place 3', latitude: latitude + 0.001, longitude: longitude + 0.001) }
it 'selects the closest place as main_place' do it 'selects the closest place as main_place' do
result = subject.find_or_create_place(visit_data) result = subject.find_or_create_place(visit_data)
@ -202,7 +201,7 @@ RSpec.describe Visits::PlaceFinder do
end end
it 'may include places with the same name' do 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) allow(subject).to receive(:place_name_exists?).and_return(false)