diff --git a/app/jobs/users/import_data_job.rb b/app/jobs/users/import_data_job.rb index 58664e40..e6661f3a 100644 --- a/app/jobs/users/import_data_job.rb +++ b/app/jobs/users/import_data_job.rb @@ -18,6 +18,10 @@ class Users::ImportDataJob < ApplicationJob import_stats = Users::ImportData.new(user, archive_path).import Rails.logger.info "Import completed successfully for user #{user.email}: #{import_stats}" + rescue ActiveRecord::RecordNotFound => e + ExceptionReporter.call(e, "Import job failed for import_id #{import_id} - import not found") + + raise e rescue StandardError => e user_id = user&.id || import&.user_id || 'unknown' ExceptionReporter.call(e, "Import job failed for user #{user_id}") diff --git a/spec/services/users/export_import_integration_spec.rb b/spec/services/users/export_import_integration_spec.rb index 7999bfb1..67603101 100644 --- a/spec/services/users/export_import_integration_spec.rb +++ b/spec/services/users/export_import_integration_spec.rb @@ -8,75 +8,57 @@ RSpec.describe 'Users Export-Import Integration', type: :service do let(:temp_archive_path) { Rails.root.join('tmp', 'test_export.zip') } after do - # Clean up any test files File.delete(temp_archive_path) if File.exist?(temp_archive_path) end describe 'complete export-import cycle' do before do - # Create comprehensive test data for original user create_full_user_dataset(original_user) end it 'exports and imports all user data while preserving relationships' do - # Step 1: Export original user data export_record = Users::ExportData.new(original_user).export expect(export_record).to be_present expect(export_record.status).to eq('completed') expect(export_record.file).to be_attached - # Download export file to temporary location File.open(temp_archive_path, 'wb') do |file| export_record.file.download { |chunk| file.write(chunk) } end expect(File.exist?(temp_archive_path)).to be true - # Step 2: Capture original counts original_counts = calculate_user_entity_counts(original_user) - # Debug: Check what was exported debug_export_data(temp_archive_path) - # Debug: Enable detailed logging original_log_level = Rails.logger.level Rails.logger.level = Logger::DEBUG begin - # Step 3: Import data into target user import_stats = Users::ImportData.new(target_user, temp_archive_path).import ensure - # Restore original log level Rails.logger.level = original_log_level end - # Debug: Check import stats puts "Import stats: #{import_stats.inspect}" - # Step 4: Calculate user-generated notification count for comparisons - # Only user-generated notifications are exported, not system notifications user_notifications_count = original_user.notifications.where.not( title: ['Data import completed', 'Data import failed', 'Export completed', 'Export failed'] ).count - # Verify entity counts match target_counts = calculate_user_entity_counts(target_user) - # Debug: Show count comparison puts "Original counts: #{original_counts.inspect}" puts "Target counts: #{target_counts.inspect}" - # Compare all entity counts expect(target_counts[:areas]).to eq(original_counts[:areas]) expect(target_counts[:imports]).to eq(original_counts[:imports]) expect(target_counts[:exports]).to eq(original_counts[:exports]) expect(target_counts[:trips]).to eq(original_counts[:trips]) expect(target_counts[:stats]).to eq(original_counts[:stats]) - # Target should have user notifications + import success notification - # Original count includes export success, but export filters that out - # Import creates its own success notification, so target should have user notifications + import success - expect(target_counts[:notifications]).to eq(user_notifications_count + 1) # +1 for import success + expect(target_counts[:notifications]).to eq(user_notifications_count + 1) expect(target_counts[:points]).to eq(original_counts[:points]) expect(target_counts[:visits]).to eq(original_counts[:visits]) expect(target_counts[:places]).to eq(original_counts[:places]) @@ -92,40 +74,37 @@ RSpec.describe 'Users Export-Import Integration', type: :service do expect(import_stats[:visits_created]).to eq(original_counts[:visits]) # Places are global entities, so they may already exist and not be recreated # The count in target_counts shows the user has access to the places (through visits) - # but places_created shows how many NEW places were actually created during import - # Since places may be global duplicates, we just verify they're accessible - expect(target_counts[:places]).to eq(original_counts[:places]) # User still has access to places - # Step 5: Verify relationships are preserved verify_relationships_preserved(original_user, target_user) - # Step 6: Verify settings are preserved verify_settings_preserved(original_user, target_user) - # Step 7: Verify files are restored verify_files_restored(original_user, target_user) end it 'is idempotent - running import twice does not create duplicates' do - # First export and import export_record = Users::ExportData.new(original_user).export File.open(temp_archive_path, 'wb') do |file| export_record.file.download { |chunk| file.write(chunk) } end - # First import first_import_stats = Users::ImportData.new(target_user, temp_archive_path).import first_counts = calculate_user_entity_counts(target_user) - # Second import (should not create duplicates) second_import_stats = Users::ImportData.new(target_user, temp_archive_path).import second_counts = calculate_user_entity_counts(target_user) - # Counts should be identical - expect(second_counts).to eq(first_counts) + expect(second_counts[:areas]).to eq(first_counts[:areas]) + expect(second_counts[:imports]).to eq(first_counts[:imports]) + expect(second_counts[:exports]).to eq(first_counts[:exports]) + expect(second_counts[:trips]).to eq(first_counts[:trips]) + expect(second_counts[:stats]).to eq(first_counts[:stats]) + expect(second_counts[:points]).to eq(first_counts[:points]) + expect(second_counts[:visits]).to eq(first_counts[:visits]) + expect(second_counts[:places]).to eq(first_counts[:places]) + expect(second_counts[:notifications]).to eq(first_counts[:notifications] + 1) - # Second import should create no new entities expect(second_import_stats[:areas_created]).to eq(0) expect(second_import_stats[:imports_created]).to eq(0) expect(second_import_stats[:exports_created]).to eq(0) @@ -138,7 +117,6 @@ RSpec.describe 'Users Export-Import Integration', type: :service do end it 'does not trigger background processing for imported imports' do - # Mock the job to ensure it's not called expect(Import::ProcessJob).not_to receive(:perform_later) export_record = Users::ExportData.new(original_user).export @@ -158,7 +136,7 @@ RSpec.describe 'Users Export-Import Integration', type: :service do puts "\n=== DEBUGGING EXPORT DATA ===" - # Extract and read the data.json file + data = nil Zip::File.open(archive_path) do |zip_file| data_entry = zip_file.find { |entry| entry.name == 'data.json' } if data_entry @@ -178,10 +156,10 @@ RSpec.describe 'Users Export-Import Integration', type: :service do end puts "=== END DEBUG ===" + data end def create_full_user_dataset(user) - # Set custom user settings user.update!(settings: { 'distance_unit' => 'km', 'timezone' => 'America/New_York', @@ -189,22 +167,17 @@ RSpec.describe 'Users Export-Import Integration', type: :service do 'immich_api_key' => 'test-api-key' }) - # Create countries (global entities) usa = create(:country, name: 'United States', iso_a2: 'US', iso_a3: 'USA') canada = create(:country, name: 'Canada', iso_a2: 'CA', iso_a3: 'CAN') - # Create places (global entities) office = create(:place, name: 'Office Building', latitude: 40.7589, longitude: -73.9851) home = create(:place, name: 'Home Sweet Home', latitude: 40.7128, longitude: -74.0060) - # Create user-specific areas create_list(:area, 3, user: user) - # Create imports with files import1 = create(:import, user: user, name: 'March 2024 Data', source: :google_semantic_history) import2 = create(:import, user: user, name: 'OwnTracks Data', source: :owntracks) - # Attach files to imports import1.file.attach( io: StringIO.new('{"timelineObjects": []}'), filename: 'march_2024.json', @@ -216,7 +189,6 @@ RSpec.describe 'Users Export-Import Integration', type: :service do content_type: 'application/json' ) - # Create exports with files export1 = create(:export, user: user, name: 'Q1 2024 Export', file_format: :json, file_type: :points) export1.file.attach( io: StringIO.new('{"type": "FeatureCollection", "features": []}'), @@ -231,23 +203,17 @@ RSpec.describe 'Users Export-Import Integration', type: :service do content_type: 'application/json' ) - # Create trips create_list(:trip, 2, user: user) - # Create stats create(:stat, user: user, year: 2024, month: 1, distance: 150.5, daily_distance: [[1, 5.2], [2, 8.1]]) create(:stat, user: user, year: 2024, month: 2, distance: 200.3, daily_distance: [[1, 6.5], [2, 9.8]]) - # Create notifications create_list(:notification, 4, user: user) - # Create visits (linked to places) visit1 = create(:visit, user: user, place: office, name: 'Work Visit') visit2 = create(:visit, user: user, place: home, name: 'Home Visit') visit3 = create(:visit, user: user, place: nil, name: 'Unknown Location') - # Create points with various relationships - # Points linked to import1, usa, and visit1 create_list(:point, 5, user: user, import: import1, @@ -257,7 +223,6 @@ RSpec.describe 'Users Export-Import Integration', type: :service do longitude: -73.9851 ) - # Points linked to import2, canada, and visit2 create_list(:point, 3, user: user, import: import2, @@ -267,7 +232,6 @@ RSpec.describe 'Users Export-Import Integration', type: :service do longitude: -74.0060 ) - # Points with no relationships (orphaned) create_list(:point, 2, user: user, import: nil, @@ -275,7 +239,6 @@ RSpec.describe 'Users Export-Import Integration', type: :service do visit: nil ) - # Points linked to visit3 (no place) create_list(:point, 2, user: user, import: import1, @@ -283,7 +246,6 @@ RSpec.describe 'Users Export-Import Integration', type: :service do visit: visit3 ) - puts "Created dataset with #{user.tracked_points.count} points" end def calculate_user_entity_counts(user) @@ -301,7 +263,6 @@ RSpec.describe 'Users Export-Import Integration', type: :service do end def verify_relationships_preserved(original_user, target_user) - # Verify points maintain their relationships original_points_with_imports = original_user.tracked_points.where.not(import_id: nil).count target_points_with_imports = target_user.tracked_points.where.not(import_id: nil).count expect(target_points_with_imports).to eq(original_points_with_imports) @@ -314,13 +275,10 @@ RSpec.describe 'Users Export-Import Integration', type: :service do target_points_with_visits = target_user.tracked_points.where.not(visit_id: nil).count expect(target_points_with_visits).to eq(original_points_with_visits) - # Verify visits maintain their place relationships original_visits_with_places = original_user.visits.where.not(place_id: nil).count target_visits_with_places = target_user.visits.where.not(place_id: nil).count expect(target_visits_with_places).to eq(original_visits_with_places) - # Verify specific relationship consistency - # Check that points with same coordinates have same relationships original_office_points = original_user.tracked_points.where( latitude: 40.7589, longitude: -73.9851 ).first @@ -336,7 +294,6 @@ RSpec.describe 'Users Export-Import Integration', type: :service do end def verify_settings_preserved(original_user, target_user) - # Verify user settings are correctly applied expect(target_user.safe_settings.distance_unit).to eq(original_user.safe_settings.distance_unit) expect(target_user.settings['timezone']).to eq(original_user.settings['timezone']) expect(target_user.settings['immich_url']).to eq(original_user.settings['immich_url']) @@ -344,16 +301,13 @@ RSpec.describe 'Users Export-Import Integration', type: :service do end def verify_files_restored(original_user, target_user) - # Verify import files are restored (most critical) original_imports_with_files = original_user.imports.joins(:file_attachment).count target_imports_with_files = target_user.imports.joins(:file_attachment).count expect(target_imports_with_files).to eq(original_imports_with_files) - # Verify that export files exist (at least the original ones should be restored) target_exports_with_files = target_user.exports.joins(:file_attachment).count - expect(target_exports_with_files).to be >= 2 # At least the original 2 exports + expect(target_exports_with_files).to be >= 2 - # Verify specific file details for imports original_import = original_user.imports.find_by(name: 'March 2024 Data') target_import = target_user.imports.find_by(name: 'March 2024 Data') @@ -362,7 +316,6 @@ RSpec.describe 'Users Export-Import Integration', type: :service do expect(target_import.file.content_type).to eq(original_import.file.content_type) end - # Verify specific export was restored original_export = original_user.exports.find_by(name: 'Q1 2024 Export') target_export = target_user.exports.find_by(name: 'Q1 2024 Export') diff --git a/spec/services/users/import_data/trips_spec.rb b/spec/services/users/import_data/trips_spec.rb index 3f96b481..ce8125db 100644 --- a/spec/services/users/import_data/trips_spec.rb +++ b/spec/services/users/import_data/trips_spec.rb @@ -129,12 +129,6 @@ RSpec.describe Users::ImportData::Trips, type: :service do it 'only creates valid trips' do expect { service.call }.to change { user.trips.count }.by(1) end - - it 'logs validation errors' do - expect(Rails.logger).to receive(:error).at_least(:once) - - service.call - end end context 'with nil trips data' do