Update importing process

This commit is contained in:
Eugene Burmakin 2025-07-02 20:22:40 +02:00
parent f86487f742
commit d518603719
12 changed files with 276 additions and 90 deletions

View file

@ -31,6 +31,11 @@ export default class extends BaseController {
if (pointsCell) {
pointsCell.textContent = new Intl.NumberFormat().format(data.import.points_count);
}
const statusCell = row.querySelector('[data-status-display]');
if (statusCell && data.import.status) {
statusCell.textContent = data.import.status;
}
}
}
}

View file

@ -8,7 +8,21 @@ module Imports::Broadcaster
action: 'update',
import: {
id: import.id,
points_count: index
points_count: index,
status: import.status
}
}
)
end
def broadcast_status_update
ImportsChannel.broadcast_to(
import.user,
{
action: 'status_update',
import: {
id: import.id,
status: import.status
}
}
)

View file

@ -1,6 +1,8 @@
# frozen_string_literal: true
class Imports::Create
include Imports::Broadcaster
attr_reader :user, :import
def initialize(user, import)
@ -10,6 +12,7 @@ class Imports::Create
def call
import.update!(status: :processing)
broadcast_status_update
importer(import.source).new(import, user.id).call
@ -18,10 +21,14 @@ class Imports::Create
update_import_points_count(import)
rescue StandardError => e
import.update!(status: :failed)
broadcast_status_update
create_import_failed_notification(import, user, e)
ensure
import.update!(status: :completed) if import.processing?
if import.processing?
import.update!(status: :completed)
broadcast_status_update
end
end
private

View file

@ -103,15 +103,28 @@ class Users::ImportData
import_settings(data['settings']) if data['settings']
import_areas(data['areas']) if data['areas']
import_places(data['places']) if data['places']
# Import places first to ensure they're available for visits
places_imported = import_places(data['places']) if data['places']
Rails.logger.info "Places import phase completed: #{places_imported} places imported"
import_imports(data['imports']) if data['imports']
import_exports(data['exports']) if data['exports']
import_trips(data['trips']) if data['trips']
import_stats(data['stats']) if data['stats']
import_notifications(data['notifications']) if data['notifications']
import_visits(data['visits']) if data['visits']
# Import visits after places to ensure proper place resolution
visits_imported = import_visits(data['visits']) if data['visits']
Rails.logger.info "Visits import phase completed: #{visits_imported} visits imported"
import_points(data['points']) if data['points']
# Final validation check
if data['counts']
validate_import_completeness(data['counts'])
end
Rails.logger.info "Data import completed. Stats: #{@import_stats}"
end
@ -131,6 +144,7 @@ class Users::ImportData
Rails.logger.debug "Importing #{places_data&.size || 0} places"
places_created = Users::ImportData::Places.new(user, places_data).call
@import_stats[:places_created] = places_created
places_created
end
def import_imports(imports_data)
@ -169,6 +183,7 @@ class Users::ImportData
Rails.logger.debug "Importing #{visits_data&.size || 0} visits"
visits_created = Users::ImportData::Visits.new(user, visits_data).call
@import_stats[:visits_created] = visits_created
visits_created
end
def import_points(points_data)
@ -221,4 +236,26 @@ class Users::ImportData
kind: :error
).call
end
def validate_import_completeness(expected_counts)
Rails.logger.info "Validating import completeness..."
discrepancies = []
expected_counts.each do |entity, expected_count|
actual_count = @import_stats[:"#{entity}_created"] || 0
if actual_count < expected_count
discrepancy = "#{entity}: expected #{expected_count}, got #{actual_count} (#{expected_count - actual_count} missing)"
discrepancies << discrepancy
Rails.logger.warn "Import discrepancy - #{discrepancy}"
end
end
if discrepancies.any?
Rails.logger.warn "Import completed with discrepancies: #{discrepancies.join(', ')}"
else
Rails.logger.info "Import validation successful - all entities imported correctly"
end
end
end

View file

@ -16,7 +16,7 @@ class Users::ImportData::Places
places_data.each do |place_data|
next unless place_data.is_a?(Hash)
place = find_or_create_place(place_data)
place = find_or_create_place_for_import(place_data)
places_created += 1 if place&.respond_to?(:previously_new_record?) && place.previously_new_record?
end
@ -28,7 +28,7 @@ class Users::ImportData::Places
attr_reader :user, :places_data
def find_or_create_place(place_data)
def find_or_create_place_for_import(place_data)
name = place_data['name']
latitude = place_data['latitude']&.to_f
longitude = place_data['longitude']&.to_f
@ -38,33 +38,42 @@ class Users::ImportData::Places
return nil
end
existing_place = Place.find_by(name: name)
Rails.logger.debug "Processing place for import: #{name} at (#{latitude}, #{longitude})"
unless existing_place
existing_place = Place.where(latitude: latitude, longitude: longitude).first
end
# During import, we prioritize data integrity for the importing user
# First try exact match (name + coordinates)
existing_place = Place.where(
name: name,
latitude: latitude,
longitude: longitude
).first
if existing_place
Rails.logger.debug "Place already exists: #{name}"
Rails.logger.debug "Found exact place match: #{name} at (#{latitude}, #{longitude}) -> existing place ID #{existing_place.id}"
existing_place.define_singleton_method(:previously_new_record?) { false }
return existing_place
end
Rails.logger.debug "No exact match found for #{name} at (#{latitude}, #{longitude}). Creating new place."
# If no exact match, create a new place to ensure data integrity
# This prevents data loss during import even if similar places exist
place_attributes = place_data.except('created_at', 'updated_at', 'latitude', 'longitude')
place_attributes['lonlat'] = "POINT(#{longitude} #{latitude})"
place_attributes['latitude'] = latitude
place_attributes['longitude'] = longitude
place_attributes.delete('user')
Rails.logger.debug "Creating place with attributes: #{place_attributes.inspect}"
begin
place = Place.create!(place_attributes)
place.define_singleton_method(:previously_new_record?) { true }
Rails.logger.debug "Created place: #{place.name}"
Rails.logger.debug "Created place during import: #{place.name} (ID: #{place.id})"
place
rescue ActiveRecord::RecordInvalid => e
ExceptionReporter.call(e, 'Failed to create place')
Rails.logger.error "Failed to create place: #{place_data.inspect}, error: #{e.message}"
nil
end
end

View file

@ -135,9 +135,9 @@ class Users::ImportData::Trips
def valid_trip_data?(trip_data)
return false unless trip_data.is_a?(Hash)
validate_trip_name(trip_data)
validate_trip_started_at(trip_data)
validate_trip_ended_at(trip_data)
return false unless validate_trip_name(trip_data)
return false unless validate_trip_started_at(trip_data)
return false unless validate_trip_ended_at(trip_data)
true
rescue StandardError => e
@ -147,23 +147,29 @@ class Users::ImportData::Trips
def validate_trip_name(trip_data)
unless trip_data['name'].present?
Rails.logger.error 'Failed to create trip: Validation failed: Name can\'t be blank'
return false
if trip_data['name'].present?
true
else
Rails.logger.debug 'Trip validation failed: Name can\'t be blank'
false
end
end
def validate_trip_started_at(trip_data)
unless trip_data['started_at'].present?
Rails.logger.error 'Failed to create trip: Validation failed: Started at can\'t be blank'
return false
if trip_data['started_at'].present?
true
else
Rails.logger.debug 'Trip validation failed: Started at can\'t be blank'
false
end
end
def validate_trip_ended_at(trip_data)
unless trip_data['ended_at'].present?
Rails.logger.error 'Failed to create trip: Validation failed: Ended at can\'t be blank'
return false
if trip_data['ended_at'].present?
true
else
Rails.logger.debug 'Trip validation failed: Ended at can\'t be blank'
false
end
end
end

View file

@ -28,7 +28,12 @@ class Users::ImportData::Visits
visits_created += 1
Rails.logger.debug "Created visit: #{visit_record.name}"
rescue ActiveRecord::RecordInvalid => e
ExceptionReporter.call(e, 'Failed to create visit')
Rails.logger.error "Failed to create visit: #{visit_data.inspect}, error: #{e.message}"
ExceptionReporter.call(e, 'Failed to create visit during import')
next
rescue StandardError => e
Rails.logger.error "Unexpected error creating visit: #{visit_data.inspect}, error: #{e.message}"
ExceptionReporter.call(e, 'Unexpected error during visit import')
next
end
end
@ -58,29 +63,67 @@ class Users::ImportData::Visits
attributes = visit_data.except('place_reference')
if visit_data['place_reference']
place = find_referenced_place(visit_data['place_reference'])
place = find_or_create_referenced_place(visit_data['place_reference'])
attributes[:place] = place if place
end
attributes
end
def find_referenced_place(place_reference)
def find_or_create_referenced_place(place_reference)
return nil unless place_reference.is_a?(Hash)
name = place_reference['name']
latitude = place_reference['latitude'].to_f
longitude = place_reference['longitude'].to_f
latitude = place_reference['latitude']&.to_f
longitude = place_reference['longitude']&.to_f
place = Place.find_by(name: name) ||
Place.where("latitude = ? AND longitude = ?", latitude, longitude).first
return nil unless name.present? && latitude.present? && longitude.present?
Rails.logger.debug "Looking for place reference: #{name} at (#{latitude}, #{longitude})"
# First try exact match (name + coordinates)
place = Place.where(
name: name,
latitude: latitude,
longitude: longitude
).first
if place
Rails.logger.debug "Found referenced place: #{name}"
else
Rails.logger.warn "Referenced place not found: #{name} (#{latitude}, #{longitude})"
Rails.logger.debug "Found exact place match for visit: #{name} -> existing place ID #{place.id}"
return place
end
place
# Try coordinate-only match with close proximity
place = Place.where(
"latitude BETWEEN ? AND ? AND longitude BETWEEN ? AND ?",
latitude - 0.0001, latitude + 0.0001,
longitude - 0.0001, longitude + 0.0001
).first
if place
Rails.logger.debug "Found nearby place match for visit: #{name} -> #{place.name} (ID: #{place.id})"
return place
end
# If no match found, create the place to ensure visit import succeeds
# This handles cases where places weren't imported in the places phase
Rails.logger.info "Creating missing place during visit import: #{name} at (#{latitude}, #{longitude})"
begin
place = Place.create!(
name: name,
latitude: latitude,
longitude: longitude,
lonlat: "POINT(#{longitude} #{latitude})",
source: place_reference['source'] || 'manual'
)
Rails.logger.debug "Created missing place for visit: #{place.name} (ID: #{place.id})"
place
rescue ActiveRecord::RecordInvalid => e
Rails.logger.error "Failed to create missing place: #{place_reference.inspect}, error: #{e.message}"
ExceptionReporter.call(e, 'Failed to create missing place during visit import')
nil
end
end
end

View file

@ -63,7 +63,10 @@
<p class='mt-3'>Unhappy? <%= link_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?", turbo_confirm: "Are you sure?", turbo_method: :delete }, method: :delete, class: 'btn' %></p>
<div class="divider"></div>
<p class='mt-3 flex flex-col gap-2'>
<%= link_to "Export my data", export_settings_users_path, class: 'btn btn-primary' %>
<%= link_to "Export my data", export_settings_users_path, class: 'btn btn-primary', data: {
turbo_confirm: "Are you sure you want to export your data?",
turbo_method: :get
} %>
<button class='btn btn-primary' onclick="import_modal.showModal()">Import my data</button>
</p>

View file

@ -68,7 +68,7 @@
<td data-points-count>
<%= number_with_delimiter import.processed %>
</td>
<td><%= import.status %></td>
<td data-status-display><%= import.status %></td>
<td><%= human_datetime(import.created_at) %></td>
<td class="whitespace-nowrap">
<% if import.file.present? %>

16
db/schema.rb generated
View file

@ -77,9 +77,6 @@ ActiveRecord::Schema[8.0].define(version: 2025_06_27_184017) do
t.index ["name"], name: "index_countries_on_name"
end
create_table "data_migrations", primary_key: "version", id: :string, force: :cascade do |t|
end
create_table "exports", force: :cascade do |t|
t.string "name", null: false
t.string "url"
@ -232,18 +229,6 @@ ActiveRecord::Schema[8.0].define(version: 2025_06_27_184017) do
t.index ["user_id"], name: "index_trips_on_user_id"
end
create_table "user_data_imports", force: :cascade do |t|
t.bigint "user_id", null: false
t.string "status", default: "pending", null: false
t.string "archive_file_name"
t.text "error_message"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["status"], name: "index_user_data_imports_on_status"
t.index ["user_id", "created_at"], name: "index_user_data_imports_on_user_id_and_created_at"
t.index ["user_id"], name: "index_user_data_imports_on_user_id"
end
create_table "users", force: :cascade do |t|
t.string "email", default: "", null: false
t.string "encrypted_password", default: "", null: false
@ -296,7 +281,6 @@ ActiveRecord::Schema[8.0].define(version: 2025_06_27_184017) do
add_foreign_key "points", "visits"
add_foreign_key "stats", "users"
add_foreign_key "trips", "users"
add_foreign_key "user_data_imports", "users"
add_foreign_key "visits", "areas"
add_foreign_key "visits", "places"
add_foreign_key "visits", "users"

View file

@ -31,8 +31,6 @@ RSpec.describe 'Users Export-Import Integration', type: :service do
original_counts = calculate_user_entity_counts(original_user)
debug_export_data(temp_archive_path)
original_log_level = Rails.logger.level
Rails.logger.level = Logger::DEBUG
@ -129,36 +127,94 @@ RSpec.describe 'Users Export-Import Integration', type: :service do
end
end
private
describe 'places and visits import integrity' do
it 'imports all places and visits without losses due to global deduplication' do
# Create a user with specific places and visits
original_user = create(:user, email: 'original@example.com')
def debug_export_data(archive_path)
require 'zip'
# Create places with different characteristics
home_place = create(:place, name: 'Home', latitude: 40.7128, longitude: -74.0060)
office_place = create(:place, name: 'Office', latitude: 40.7589, longitude: -73.9851)
gym_place = create(:place, name: 'Gym', latitude: 40.7505, longitude: -73.9934)
puts "\n=== DEBUGGING EXPORT DATA ==="
# Create visits associated with those places
create(:visit, user: original_user, place: home_place, name: 'Home Visit')
create(:visit, user: original_user, place: office_place, name: 'Work Visit')
create(:visit, user: original_user, place: gym_place, name: 'Workout')
data = nil
Zip::File.open(archive_path) do |zip_file|
data_entry = zip_file.find { |entry| entry.name == 'data.json' }
if data_entry
json_content = data_entry.get_input_stream.read
data = JSON.parse(json_content)
# Create a visit without a place
create(:visit, user: original_user, place: nil, name: 'Unknown Location')
puts "Export counts: #{data['counts'].inspect}"
puts "Points in export: #{data['points']&.size || 0}"
puts "Places in export: #{data['places']&.size || 0}"
puts "First point sample: #{data['points']&.first&.slice('timestamp', 'longitude', 'latitude', 'import_reference', 'country_info', 'visit_reference')}"
puts "First place sample: #{data['places']&.first&.slice('name', 'latitude', 'longitude', 'source')}"
puts "Imports in export: #{data['imports']&.size || 0}"
puts "Countries referenced: #{data['points']&.map { |p| p['country_info']&.dig('name') }&.compact&.uniq || []}"
else
puts "No data.json found in export!"
end
# Calculate counts properly - places are accessed through visits
original_places_count = original_user.places.distinct.count
original_visits_count = original_user.visits.count
# Export the data
export_service = Users::ExportData.new(original_user)
export_record = export_service.export
# Download and save to a temporary file for processing
archive_content = export_record.file.download
temp_export_file = Tempfile.new(['test_export', '.zip'])
temp_export_file.binmode
temp_export_file.write(archive_content)
temp_export_file.close
# SIMULATE FRESH DATABASE: Remove the original places to simulate database migration
# This simulates the scenario where we're importing into a different database
place_ids_to_remove = [home_place.id, office_place.id, gym_place.id]
Place.where(id: place_ids_to_remove).destroy_all
# Create another user on a "different database" scenario
import_user = create(:user, email: 'import@example.com')
# Create some existing global places that might conflict
# These should NOT prevent import of the user's places
create(:place, name: 'Home', latitude: 40.8000, longitude: -74.1000) # Different coordinates
create(:place, name: 'Coffee Shop', latitude: 40.7589, longitude: -73.9851) # Same coordinates, different name
# Simulate import into "new database"
temp_import_file = Tempfile.new(['test_import', '.zip'])
temp_import_file.binmode
temp_import_file.write(archive_content)
temp_import_file.close
# Import the data
import_service = Users::ImportData.new(import_user, temp_import_file.path)
import_stats = import_service.import
# Verify all entities were imported correctly
expect(import_stats[:places_created]).to eq(original_places_count),
"Expected #{original_places_count} places to be created, got #{import_stats[:places_created]}"
expect(import_stats[:visits_created]).to eq(original_visits_count),
"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_visits_count = import_user.visits.count
expect(imported_places_count).to eq(original_places_count),
"Expected user to have access to #{original_places_count} places, got #{imported_places_count}"
expect(imported_visits_count).to eq(original_visits_count),
"Expected user to have #{original_visits_count} visits, got #{imported_visits_count}"
# Verify specific visits have their place associations
imported_visits = import_user.visits.includes(:place)
visits_with_places = imported_visits.where.not(place: nil)
expect(visits_with_places.count).to eq(3) # Home, Office, Gym
# Verify place names are preserved
place_names = visits_with_places.map { |v| v.place.name }.sort
expect(place_names).to eq(['Gym', 'Home', 'Office'])
# Cleanup
temp_export_file.unlink
temp_import_file.unlink
end
puts "=== END DEBUG ==="
data
end
private
def create_full_user_dataset(user)
user.update!(settings: {
'distance_unit' => 'km',

View file

@ -71,19 +71,41 @@ RSpec.describe Users::ImportData::Places, type: :service do
context 'with duplicate places (same name)' do
before do
# Create an existing place with same name
# 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
expect { service.call }.to change { Place.count }.by(2)
end
it 'creates both places with different coordinates' do
service.call
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)
expect(imported_home).to be_present
end
end
context 'with exact duplicate places (same name and coordinates)' do
before do
# 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 duplicate places' do
it 'skips exact duplicate places' do
expect { service.call }.to change { Place.count }.by(1)
end
it 'logs when skipping duplicates' do
it 'logs when finding exact duplicates' do
allow(Rails.logger).to receive(:debug) # Allow any debug logs
expect(Rails.logger).to receive(:debug).with("Place already exists: Home")
expect(Rails.logger).to receive(:debug).with(/Found exact place match: Home at \(40\.7128, -74\.006\) -> existing place ID \d+/)
service.call
end
@ -102,15 +124,15 @@ RSpec.describe Users::ImportData::Places, type: :service do
lonlat: 'POINT(-74.0060 40.7128)')
end
it 'skips duplicate places by coordinates' do
expect { service.call }.to change { Place.count }.by(1)
it 'creates the place since name is different' do
expect { service.call }.to change { Place.count }.by(2)
end
it 'logs when skipping duplicates' do
allow(Rails.logger).to receive(:debug) # Allow any debug logs
expect(Rails.logger).to receive(:debug).with("Place already exists: Home")
it 'creates both places with different names' do
service.call
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
end