From 631ee0e64cf31d78f8d8260ccecdce15418992d7 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 26 Jun 2025 19:48:42 +0200 Subject: [PATCH] Clean up specs a bit --- Gemfile | 3 +- app/services/users/export_data.rb | 70 ++++++++++-- spec/models/country_spec.rb | 4 + spec/models/export_spec.rb | 35 ++++++ spec/models/point_spec.rb | 2 + spec/services/users/export_data/areas_spec.rb | 29 ++--- .../users/export_data/exports_spec.rb | 19 ++-- .../users/export_data/imports_spec.rb | 103 ++---------------- .../users/export_data/notifications_spec.rb | 22 ++-- .../services/users/export_data/places_spec.rb | 15 +-- .../services/users/export_data/points_spec.rb | 55 ++++------ spec/services/users/export_data/stats_spec.rb | 20 ++-- spec/services/users/export_data/trips_spec.rb | 24 ++-- .../services/users/export_data/visits_spec.rb | 43 +++----- 14 files changed, 203 insertions(+), 241 deletions(-) diff --git a/Gemfile b/Gemfile index 0f566226..8515a41c 100644 --- a/Gemfile +++ b/Gemfile @@ -34,6 +34,7 @@ gem 'rgeo-activerecord' gem 'rgeo-geojson' gem 'rswag-api' gem 'rswag-ui' +gem 'rubyzip', '~> 2.4' gem 'sentry-ruby' gem 'sentry-rails' gem 'stackprof' @@ -76,5 +77,3 @@ group :development do gem 'foreman' gem 'rubocop-rails', require: false end - -gem "rubyzip", "~> 2.4" diff --git a/app/services/users/export_data.rb b/app/services/users/export_data.rb index ad4aa43d..bf7b29da 100644 --- a/app/services/users/export_data.rb +++ b/app/services/users/export_data.rb @@ -10,7 +10,7 @@ require 'zip' # "distance_unit": "km", # "timezone": "UTC", # "immich_url": "https://immich.example.com", -# // ... other user settings +# // ... other user settings (exported via user.safe_settings.settings) # }, # "areas": [ # { @@ -18,7 +18,8 @@ require 'zip' # "latitude": "40.7128", # "longitude": "-74.0060", # "radius": 100, -# "created_at": "2024-01-01T00:00:00Z" +# "created_at": "2024-01-01T00:00:00Z", +# "updated_at": "2024-01-01T00:00:00Z" # } # ], # "imports": [ @@ -26,12 +27,14 @@ require 'zip' # "name": "2023_MARCH.json", # "source": "google_semantic_history", # "created_at": "2024-01-01T00:00:00Z", +# "updated_at": "2024-01-01T00:00:00Z", # "processed": true, -# "points_count": 1500, # "file_name": "import_1_2023_MARCH.json", # "original_filename": "2023_MARCH.json", # "file_size": 2048576, # "content_type": "application/json" +# // Note: file_error may be present if file download fails +# // Note: file_name and original_filename will be null if no file attached # } # ], # "exports": [ @@ -40,11 +43,16 @@ require 'zip' # "status": "completed", # "file_format": "json", # "file_type": "points", +# "start_at": "2024-01-01T00:00:00Z", +# "end_at": "2024-01-31T23:59:59Z", # "created_at": "2024-02-01T00:00:00Z", +# "updated_at": "2024-02-01T00:00:00Z", # "file_name": "export_1_export_2024-01-01_to_2024-01-31.json", # "original_filename": "export_2024-01-01_to_2024-01-31.json", # "file_size": 1048576, # "content_type": "application/json" +# // Note: file_error may be present if file download fails +# // Note: file_name and original_filename will be null if no file attached # } # ], # "trips": [ @@ -53,7 +61,9 @@ require 'zip' # "started_at": "2024-01-15T08:00:00Z", # "ended_at": "2024-01-18T20:00:00Z", # "distance": 1245.67, -# "created_at": "2024-01-19T00:00:00Z" +# "created_at": "2024-01-19T00:00:00Z", +# "updated_at": "2024-01-19T00:00:00Z" +# // ... other trip fields # } # ], # "stats": [ @@ -61,10 +71,12 @@ require 'zip' # "year": 2024, # "month": 1, # "distance": 456.78, +# "daily_distance": [[1, 15.2], [2, 23.5], ...], // [day, distance] pairs # "toponyms": [ # {"country": "United States", "cities": [{"city": "New York"}]} # ], -# "created_at": "2024-02-01T00:00:00Z" +# "created_at": "2024-02-01T00:00:00Z", +# "updated_at": "2024-02-01T00:00:00Z" # } # ], # "notifications": [ @@ -72,8 +84,9 @@ require 'zip' # "kind": "info", # "title": "Import completed", # "content": "Your data import has been processed successfully", -# "read": true, -# "created_at": "2024-01-01T12:00:00Z" +# "read_at": "2024-01-01T12:30:00Z", // null if unread +# "created_at": "2024-01-01T12:00:00Z", +# "updated_at": "2024-01-01T12:30:00Z" # } # ], # "points": [ @@ -84,7 +97,30 @@ require 'zip' # "altitude": 15.5, # "velocity": 25.5, # "accuracy": 5.0, +# "ping": "test-ping", +# "tracker_id": "tracker-123", +# "topic": "owntracks/user/device", +# "trigger": "manual_event", +# "bssid": "aa:bb:cc:dd:ee:ff", +# "ssid": "TestWiFi", +# "connection": "wifi", +# "vertical_accuracy": 3.0, +# "mode": 2, +# "inrids": ["region1", "region2"], +# "in_regions": ["home", "work"], +# "raw_data": {"test": "data"}, +# "city": "New York", +# "country": "United States", +# "geodata": {"address": "123 Main St"}, +# "reverse_geocoded_at": "2024-01-01T00:00:00Z", +# "course": 45.5, +# "course_accuracy": 2.5, +# "external_track_id": "ext-123", +# "lonlat": "POINT(-74.006 40.7128)", +# "longitude": -74.006, +# "latitude": 40.7128, # "created_at": "2024-01-01T00:00:00Z", +# "updated_at": "2024-01-01T00:00:00Z", # "import_reference": { # "name": "2023_MARCH.json", # "source": "google_semantic_history", @@ -105,9 +141,15 @@ require 'zip' # // Example of point without relationships (edge cases) # "timestamp": 1704070800, # "altitude": 10.0, +# "longitude": -73.9857, +# "latitude": 40.7484, +# "lonlat": "POINT(-73.9857 40.7484)", +# "created_at": "2024-01-01T00:05:00Z", +# "updated_at": "2024-01-01T00:05:00Z", # "import_reference": null, // Orphaned point # "country_info": null, // No country data # "visit_reference": null // Not part of a visit +# // ... other point fields may be null # } # ], # "visits": [ @@ -117,28 +159,38 @@ require 'zip' # "ended_at": "2024-01-01T17:00:00Z", # "duration": 32400, # "status": "suggested", +# "created_at": "2024-01-01T00:00:00Z", +# "updated_at": "2024-01-01T00:00:00Z", # "place_reference": { # "name": "Office Building", # "latitude": "40.7589", # "longitude": "-73.9851", # "source": "manual" # } +# // ... other visit fields # }, # { # // Example of visit without place # "name": "Unknown Location", # "started_at": "2024-01-02T10:00:00Z", # "ended_at": "2024-01-02T12:00:00Z", +# "duration": 7200, +# "status": "confirmed", +# "created_at": "2024-01-02T00:00:00Z", +# "updated_at": "2024-01-02T00:00:00Z", # "place_reference": null // No associated place # } # ], # "places": [ # { # "name": "Office Building", -# "lonlat": "POINT(-73.9851 40.7589)", +# "longitude": "-73.9851", +# "latitude": "40.7589", # "source": "manual", # "geodata": {"properties": {"name": "Office Building"}}, -# "created_at": "2024-01-01T00:00:00Z" +# "created_at": "2024-01-01T00:00:00Z", +# "updated_at": "2024-01-01T00:00:00Z" +# // ... other place fields # } # ] # } diff --git a/spec/models/country_spec.rb b/spec/models/country_spec.rb index 128be05d..42a85dcd 100644 --- a/spec/models/country_spec.rb +++ b/spec/models/country_spec.rb @@ -9,4 +9,8 @@ RSpec.describe Country, type: :model do it { is_expected.to validate_presence_of(:iso_a3) } it { is_expected.to validate_presence_of(:geom) } end + + describe 'associations' do + it { is_expected.to have_many(:points).dependent(:nullify) } + end end diff --git a/spec/models/export_spec.rb b/spec/models/export_spec.rb index 8bc3e3b7..8c21dd6d 100644 --- a/spec/models/export_spec.rb +++ b/spec/models/export_spec.rb @@ -10,5 +10,40 @@ RSpec.describe Export, type: :model do describe 'enums' do it { is_expected.to define_enum_for(:status).with_values(created: 0, processing: 1, completed: 2, failed: 3) } it { is_expected.to define_enum_for(:file_format).with_values(json: 0, gpx: 1, archive: 2) } + it { is_expected.to define_enum_for(:file_type).with_values(points: 0, user_data: 1) } + end + + describe 'callbacks' do + describe 'after_commit' do + context 'when the export is created' do + let(:export) { build(:export) } + + it 'enqueues the ExportJob' do + expect(ExportJob).to receive(:perform_later).with(export.id) + + export.save! + end + + context 'when the export is a user data export' do + let(:export) { build(:export, file_type: :user_data) } + + it 'does not enqueue the ExportJob' do + expect(ExportJob).not_to receive(:perform_later).with(export.id) + + export.save! + end + end + end + + context 'when the export is destroyed' do + let(:export) { create(:export) } + + it 'removes the attached file' do + expect(export.file).to receive(:purge) + + export.destroy! + end + end + end end end diff --git a/spec/models/point_spec.rb b/spec/models/point_spec.rb index d64ae806..7b5acd77 100644 --- a/spec/models/point_spec.rb +++ b/spec/models/point_spec.rb @@ -6,6 +6,8 @@ RSpec.describe Point, type: :model do describe 'associations' do it { is_expected.to belong_to(:import).optional } it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:country).optional } + it { is_expected.to belong_to(:visit).optional } end describe 'validations' do diff --git a/spec/services/users/export_data/areas_spec.rb b/spec/services/users/export_data/areas_spec.rb index 37f3eeb0..b234aa2f 100644 --- a/spec/services/users/export_data/areas_spec.rb +++ b/spec/services/users/export_data/areas_spec.rb @@ -18,24 +18,22 @@ RSpec.describe Users::ExportData::Areas, type: :service do let!(:area1) { create(:area, user: user, name: 'Home', radius: 100) } let!(:area2) { create(:area, user: user, name: 'Work', radius: 200) } + subject { service.call } + it 'returns all user areas' do - result = service.call - expect(result).to be_an(Array) - expect(result.size).to eq(2) + expect(subject).to be_an(Array) + expect(subject.size).to eq(2) end it 'excludes user_id and id fields' do - result = service.call - - result.each do |area_data| + subject.each do |area_data| expect(area_data).not_to have_key('user_id') expect(area_data).not_to have_key('id') end end it 'includes expected area attributes' do - result = service.call - area_data = result.find { |a| a['name'] == 'Home' } + area_data = subject.find { |a| a['name'] == 'Home' } expect(area_data).to include( 'name' => 'Home', @@ -51,18 +49,11 @@ RSpec.describe Users::ExportData::Areas, type: :service do let!(:user_area) { create(:area, user: user, name: 'User Area') } let!(:other_user_area) { create(:area, user: other_user, name: 'Other User Area') } - it 'only returns areas for the specified user' do - result = service.call - expect(result.size).to eq(1) - expect(result.first['name']).to eq('User Area') - end - end - end + subject { service.call } - describe 'private methods' do - describe '#user' do - it 'returns the initialized user' do - expect(service.send(:user)).to eq(user) + it 'only returns areas for the specified user' do + expect(subject.size).to eq(1) + expect(subject.first['name']).to eq('User Area') end end end diff --git a/spec/services/users/export_data/exports_spec.rb b/spec/services/users/export_data/exports_spec.rb index 53d1857b..73f45c3d 100644 --- a/spec/services/users/export_data/exports_spec.rb +++ b/spec/services/users/export_data/exports_spec.rb @@ -7,6 +7,8 @@ RSpec.describe Users::ExportData::Exports, type: :service do let(:files_directory) { Rails.root.join('tmp', 'test_export_files') } let(:service) { described_class.new(user, files_directory) } + subject { service.call } + before do FileUtils.mkdir_p(files_directory) allow(Rails.logger).to receive(:info) @@ -20,8 +22,7 @@ RSpec.describe Users::ExportData::Exports, type: :service do describe '#call' do context 'when user has no exports' do it 'returns an empty array' do - result = service.call - expect(result).to eq([]) + expect(subject).to eq([]) end end @@ -37,10 +38,10 @@ RSpec.describe Users::ExportData::Exports, type: :service do end it 'returns export data without file information' do - result = service.call - expect(result.size).to eq(1) + expect(subject.size).to eq(1) + + export_data = subject.first - export_data = result.first expect(export_data).to include( 'name' => 'Test Export', 'file_format' => 'json', @@ -71,8 +72,7 @@ RSpec.describe Users::ExportData::Exports, type: :service do end it 'returns export data with file information' do - result = service.call - export_data = result.first + export_data = subject.first expect(export_data['name']).to eq('Export with File') expect(export_data['file_name']).to eq("export_#{export_with_file.id}_export_data.json") @@ -88,9 +88,8 @@ RSpec.describe Users::ExportData::Exports, type: :service do let!(:other_user_export) { create(:export, user: other_user, name: 'Other User Export') } it 'only returns exports for the specified user' do - result = service.call - expect(result.size).to eq(1) - expect(result.first['name']).to eq('User Export') + expect(subject.size).to eq(1) + expect(subject.first['name']).to eq('User Export') end end end diff --git a/spec/services/users/export_data/imports_spec.rb b/spec/services/users/export_data/imports_spec.rb index c47d4b9d..ae86d767 100644 --- a/spec/services/users/export_data/imports_spec.rb +++ b/spec/services/users/export_data/imports_spec.rb @@ -7,6 +7,8 @@ RSpec.describe Users::ExportData::Imports, type: :service do let(:files_directory) { Pathname.new(Dir.mktmpdir('test_exports')) } let(:service) { described_class.new(user, files_directory) } + subject { service.call } + after do FileUtils.rm_rf(files_directory) if files_directory.exist? end @@ -14,8 +16,7 @@ RSpec.describe Users::ExportData::Imports, type: :service do describe '#call' do context 'when user has no imports' do it 'returns an empty array' do - result = service.call - expect(result).to eq([]) + expect(subject).to eq([]) end end @@ -24,10 +25,9 @@ RSpec.describe Users::ExportData::Imports, type: :service do let!(:import2) { create(:import, user: user, name: 'Import 2') } it 'returns import data without file information' do - result = service.call - expect(result.size).to eq(2) + expect(service.call.size).to eq(2) - first_import = result.find { |i| i['name'] == 'Import 1' } + first_import = service.call.find { |i| i['name'] == 'Import 1' } expect(first_import['file_name']).to be_nil expect(first_import['original_filename']).to be_nil expect(first_import).not_to have_key('user_id') @@ -57,8 +57,7 @@ RSpec.describe Users::ExportData::Imports, type: :service do end it 'returns import data with file information' do - result = service.call - import_data = result.first + import_data = subject.first expect(import_data['name']).to eq('Import with File') expect(import_data['file_name']).to eq("import_#{import_with_file.id}_test_file.json") @@ -68,8 +67,7 @@ RSpec.describe Users::ExportData::Imports, type: :service do end it 'downloads and saves the file to the files directory' do - result = service.call - import_data = result.first + import_data = subject.first file_path = files_directory.join(import_data['file_name']) expect(File.exist?(file_path)).to be true @@ -80,8 +78,7 @@ RSpec.describe Users::ExportData::Imports, type: :service do blob = create_blob(filename: 'test file with spaces & symbols!.json') import_with_file.file.attach(blob) - result = service.call - import_data = result.first + import_data = subject.first expect(import_data['file_name']).to match(/import_\d+_test_file_with_spaces___symbols_.json/) end @@ -99,8 +96,7 @@ RSpec.describe Users::ExportData::Imports, type: :service do end it 'handles download errors gracefully' do - result = service.call - import_data = result.find { |i| i['name'] == 'Import with error file' } + import_data = subject.find { |i| i['name'] == 'Import with error file' } expect(import_data['file_error']).to eq('Failed to download: Download failed') end @@ -128,8 +124,7 @@ RSpec.describe Users::ExportData::Imports, type: :service do end it 'returns all imports' do - result = service.call - expect(result.size).to eq(3) + expect(subject.size).to eq(3) end end @@ -139,9 +134,8 @@ RSpec.describe Users::ExportData::Imports, type: :service do let!(:other_user_import) { create(:import, user: other_user, name: 'Other User Import') } it 'only returns imports for the specified user' do - result = service.call - expect(result.size).to eq(1) - expect(result.first['name']).to eq('User Import') + expect(subject.size).to eq(1) + expect(subject.first['name']).to eq('User Import') end end @@ -165,79 +159,6 @@ RSpec.describe Users::ExportData::Imports, type: :service do end end - describe 'private methods' do - let(:import) { create(:import, user: user, name: 'Test Import') } - - describe '#process_import' do - context 'with import without file' do - it 'processes import correctly' do - result = service.send(:process_import, import) - - expect(result).to include( - 'name' => 'Test Import', - 'file_name' => nil, - 'original_filename' => nil - ) - expect(result).not_to have_key('user_id') - expect(result).not_to have_key('raw_data') - expect(result).not_to have_key('id') - end - end - - context 'with import with file' do - let(:blob) { create_blob(filename: 'test.json', content_type: 'application/json') } - - before do - import.file.attach(blob) - allow(Imports::SecureFileDownloader).to receive(:new).and_return( - double(download_with_verification: 'file content') - ) - end - - it 'processes import with file data' do - result = service.send(:process_import, import) - - expect(result['file_name']).to be_present - expect(result['original_filename']).to eq('test.json') - expect(result['content_type']).to eq('application/json') - end - end - end - - describe '#generate_sanitized_filename' do - let(:import) { create(:import, user: user, name: 'Filename test import') } - let(:blob) { create_blob(filename: 'test/file<>:"|?*\\.json') } - - before { import.file.attach(blob) } - - it 'sanitizes filename correctly' do - result = service.send(:generate_sanitized_filename, import) - - expect(result).to eq("import_#{import.id}_test-file--------.json") - end - end - - describe '#add_file_metadata_to_import' do - let(:import) { create(:import, user: user) } - let(:import_hash) { {} } - let(:filename) { 'sanitized_filename.json' } - let(:blob) { create_blob(filename: 'original.json', content_type: 'application/json') } - - before { import.file.attach(blob) } - - it 'adds correct metadata to import hash' do - service.send(:add_file_metadata_to_import, import, import_hash, filename) - - expect(import_hash).to include( - 'file_name' => 'sanitized_filename.json', - 'original_filename' => 'original.json', - 'file_size' => blob.byte_size, - 'content_type' => 'application/json' - ) - end - end - end - private def create_blob(filename: 'test.txt', content_type: 'text/plain') diff --git a/spec/services/users/export_data/notifications_spec.rb b/spec/services/users/export_data/notifications_spec.rb index c75f37a6..a26b2228 100644 --- a/spec/services/users/export_data/notifications_spec.rb +++ b/spec/services/users/export_data/notifications_spec.rb @@ -6,11 +6,12 @@ RSpec.describe Users::ExportData::Notifications, type: :service do let(:user) { create(:user) } let(:service) { described_class.new(user) } + subject { service.call } + describe '#call' do context 'when user has no notifications' do it 'returns an empty array' do - result = service.call - expect(result).to eq([]) + expect(subject).to eq([]) end end @@ -19,23 +20,19 @@ RSpec.describe Users::ExportData::Notifications, type: :service do let!(:notification2) { create(:notification, user: user, title: 'Test 2', kind: :warning) } it 'returns all user notifications' do - result = service.call - expect(result).to be_an(Array) - expect(result.size).to eq(2) + expect(subject).to be_an(Array) + expect(subject.size).to eq(2) end it 'excludes user_id and id fields' do - result = service.call - - result.each do |notification_data| + subject.each do |notification_data| expect(notification_data).not_to have_key('user_id') expect(notification_data).not_to have_key('id') end end it 'includes expected notification attributes' do - result = service.call - notification_data = result.find { |n| n['title'] == 'Test 1' } + notification_data = subject.find { |n| n['title'] == 'Test 1' } expect(notification_data).to include( 'title' => 'Test 1', @@ -52,9 +49,8 @@ RSpec.describe Users::ExportData::Notifications, type: :service do let!(:other_user_notification) { create(:notification, user: other_user, title: 'Other Notification') } it 'only returns notifications for the specified user' do - result = service.call - expect(result.size).to eq(1) - expect(result.first['title']).to eq('User Notification') + expect(subject.size).to eq(1) + expect(subject.first['title']).to eq('User Notification') end end end diff --git a/spec/services/users/export_data/places_spec.rb b/spec/services/users/export_data/places_spec.rb index fe4ecdca..a940db16 100644 --- a/spec/services/users/export_data/places_spec.rb +++ b/spec/services/users/export_data/places_spec.rb @@ -6,11 +6,12 @@ RSpec.describe Users::ExportData::Places, type: :service do let(:user) { create(:user) } let(:service) { described_class.new(user) } + subject { service.call } + describe '#call' do context 'when user has no places' do it 'returns an empty array' do - result = service.call - expect(result).to eq([]) + expect(subject).to eq([]) end end @@ -21,21 +22,17 @@ RSpec.describe Users::ExportData::Places, type: :service do let!(:visit2) { create(:visit, user: user, place: place2) } it 'returns all places' do - result = service.call - expect(result.size).to eq(2) + expect(subject.size).to eq(2) end it 'excludes id field' do - result = service.call - - result.each do |place_data| + subject.each do |place_data| expect(place_data).not_to have_key('id') end end it 'includes expected place attributes' do - result = service.call - place_data = result.find { |p| p['name'] == 'Office' } + place_data = subject.find { |p| p['name'] == 'Office' } expect(place_data).to include( 'name' => 'Office', diff --git a/spec/services/users/export_data/points_spec.rb b/spec/services/users/export_data/points_spec.rb index 1aaf4328..defc2413 100644 --- a/spec/services/users/export_data/points_spec.rb +++ b/spec/services/users/export_data/points_spec.rb @@ -6,11 +6,12 @@ RSpec.describe Users::ExportData::Points, type: :service do let(:user) { create(:user) } let(:service) { described_class.new(user) } + subject { service.call } + describe '#call' do context 'when user has no points' do it 'returns an empty array' do - result = service.call - expect(result).to eq([]) + expect(subject).to eq([]) end end @@ -66,14 +67,12 @@ RSpec.describe Users::ExportData::Points, type: :service do end it 'returns all points with correct structure' do - result = service.call - expect(result).to be_an(Array) - expect(result.size).to eq(2) + expect(subject).to be_an(Array) + expect(subject.size).to eq(2) end it 'includes all point attributes for point with relationships' do - result = service.call - point_data = result.find { |p| p['external_track_id'] == 'ext-123' } + point_data = subject.find { |p| p['external_track_id'] == 'ext-123' } expect(point_data).to include( 'battery_status' => 2, # enum value for :charging @@ -109,8 +108,7 @@ RSpec.describe Users::ExportData::Points, type: :service do end it 'includes import reference when point has import' do - result = service.call - point_data = result.find { |p| p['external_track_id'] == 'ext-123' } + point_data = subject.find { |p| p['external_track_id'] == 'ext-123' } expect(point_data['import_reference']).to eq({ 'name' => 'Test Import', @@ -120,8 +118,7 @@ RSpec.describe Users::ExportData::Points, type: :service do end it 'includes country info when point has country' do - result = service.call - point_data = result.find { |p| p['external_track_id'] == 'ext-123' } + point_data = subject.find { |p| p['external_track_id'] == 'ext-123' } # Since we're using LEFT JOIN and the country is properly associated, # this should work, but let's check if it's actually being set @@ -138,8 +135,7 @@ RSpec.describe Users::ExportData::Points, type: :service do end it 'includes visit reference when point has visit' do - result = service.call - point_data = result.find { |p| p['external_track_id'] == 'ext-123' } + point_data = subject.find { |p| p['external_track_id'] == 'ext-123' } expect(point_data['visit_reference']).to eq({ 'name' => 'Work Visit', @@ -149,8 +145,7 @@ RSpec.describe Users::ExportData::Points, type: :service do end it 'does not include relationships for points without them' do - result = service.call - point_data = result.find { |p| p['external_track_id'].nil? } + point_data = subject.find { |p| p['external_track_id'].nil? } expect(point_data['import_reference']).to be_nil expect(point_data['country_info']).to be_nil @@ -158,21 +153,19 @@ RSpec.describe Users::ExportData::Points, type: :service do end it 'correctly extracts longitude and latitude from lonlat geometry' do - result = service.call + point1 = subject.find { |p| p['external_track_id'] == 'ext-123' } - point1 = result.find { |p| p['external_track_id'] == 'ext-123' } expect(point1['longitude']).to eq(-74.006) expect(point1['latitude']).to eq(40.7128) - point2 = result.find { |p| p['external_track_id'].nil? } + point2 = subject.find { |p| p['external_track_id'].nil? } expect(point2['longitude']).to eq(-73.9857) expect(point2['latitude']).to eq(40.7484) end it 'orders points by id' do - result = service.call - expect(result.first['timestamp']).to eq(1640995200) - expect(result.last['timestamp']).to eq(1640995260) + expect(subject.first['timestamp']).to eq(1640995200) + expect(subject.last['timestamp']).to eq(1640995260) end it 'logs processing information' do @@ -187,8 +180,7 @@ RSpec.describe Users::ExportData::Points, type: :service do end it 'handles null values gracefully' do - result = service.call - point_data = result.first + point_data = subject.first expect(point_data['inrids']).to eq([]) expect(point_data['in_regions']).to eq([]) @@ -200,9 +192,10 @@ RSpec.describe Users::ExportData::Points, type: :service do let!(:user_point) { create(:point, user: user) } let!(:other_user_point) { create(:point, user: other_user) } + subject { service.call } + it 'only returns points for the specified user' do - result = service.call - expect(result.size).to eq(1) + expect(service.call.size).to eq(1) end end @@ -211,19 +204,11 @@ RSpec.describe Users::ExportData::Points, type: :service do it 'uses a single optimized query' do expect(Rails.logger).to receive(:info).with('Processing 3 points for export...') - service.call + subject end it 'avoids N+1 queries by using joins' do - expect(service.call.size).to eq(3) - end - end - end - - describe 'private methods' do - describe '#user' do - it 'returns the initialized user' do - expect(service.send(:user)).to eq(user) + expect(subject.size).to eq(3) end end end diff --git a/spec/services/users/export_data/stats_spec.rb b/spec/services/users/export_data/stats_spec.rb index a0e67e0a..2c625110 100644 --- a/spec/services/users/export_data/stats_spec.rb +++ b/spec/services/users/export_data/stats_spec.rb @@ -6,11 +6,12 @@ RSpec.describe Users::ExportData::Stats, type: :service do let(:user) { create(:user) } let(:service) { described_class.new(user) } + subject { service.call } + describe '#call' do context 'when user has no stats' do it 'returns an empty array' do - result = service.call - expect(result).to eq([]) + expect(subject).to eq([]) end end @@ -19,23 +20,19 @@ RSpec.describe Users::ExportData::Stats, type: :service do let!(:stat2) { create(:stat, user: user, year: 2024, month: 2, distance: 150) } it 'returns all user stats' do - result = service.call - expect(result).to be_an(Array) - expect(result.size).to eq(2) + expect(subject).to be_an(Array) + expect(subject.size).to eq(2) end it 'excludes user_id and id fields' do - result = service.call - - result.each do |stat_data| + subject.each do |stat_data| expect(stat_data).not_to have_key('user_id') expect(stat_data).not_to have_key('id') end end it 'includes expected stat attributes' do - result = service.call - stat_data = result.find { |s| s['month'] == 1 } + stat_data = subject.find { |s| s['month'] == 1 } expect(stat_data).to include( 'year' => 2024, @@ -53,8 +50,7 @@ RSpec.describe Users::ExportData::Stats, type: :service do let!(:other_user_stat) { create(:stat, user: other_user, year: 2024, month: 1) } it 'only returns stats for the specified user' do - result = service.call - expect(result.size).to eq(1) + expect(subject.size).to eq(1) end end end diff --git a/spec/services/users/export_data/trips_spec.rb b/spec/services/users/export_data/trips_spec.rb index 21556299..ec8bd16c 100644 --- a/spec/services/users/export_data/trips_spec.rb +++ b/spec/services/users/export_data/trips_spec.rb @@ -6,11 +6,12 @@ RSpec.describe Users::ExportData::Trips, type: :service do let(:user) { create(:user) } let(:service) { described_class.new(user) } + subject { service.call } + describe '#call' do context 'when user has no trips' do it 'returns an empty array' do - result = service.call - expect(result).to eq([]) + expect(subject).to eq([]) end end @@ -19,23 +20,19 @@ RSpec.describe Users::ExportData::Trips, type: :service do let!(:trip2) { create(:trip, user: user, name: 'Vacation', distance: 1200) } it 'returns all user trips' do - result = service.call - expect(result).to be_an(Array) - expect(result.size).to eq(2) + expect(subject).to be_an(Array) + expect(subject.size).to eq(2) end it 'excludes user_id and id fields' do - result = service.call - - result.each do |trip_data| + subject.each do |trip_data| expect(trip_data).not_to have_key('user_id') expect(trip_data).not_to have_key('id') end end it 'includes expected trip attributes' do - result = service.call - trip_data = result.find { |t| t['name'] == 'Business Trip' } + trip_data = subject.find { |t| t['name'] == 'Business Trip' } expect(trip_data).to include( 'name' => 'Business Trip', @@ -51,10 +48,11 @@ RSpec.describe Users::ExportData::Trips, type: :service do let!(:user_trip) { create(:trip, user: user, name: 'User Trip') } let!(:other_user_trip) { create(:trip, user: other_user, name: 'Other Trip') } + subject { service.call } + it 'only returns trips for the specified user' do - result = service.call - expect(result.size).to eq(1) - expect(result.first['name']).to eq('User Trip') + expect(service.call.size).to eq(1) + expect(service.call.first['name']).to eq('User Trip') end end end diff --git a/spec/services/users/export_data/visits_spec.rb b/spec/services/users/export_data/visits_spec.rb index 22c9e6c0..67bbd491 100644 --- a/spec/services/users/export_data/visits_spec.rb +++ b/spec/services/users/export_data/visits_spec.rb @@ -6,11 +6,12 @@ RSpec.describe Users::ExportData::Visits, type: :service do let(:user) { create(:user) } let(:service) { described_class.new(user) } + subject { service.call } + describe '#call' do context 'when user has no visits' do it 'returns an empty array' do - result = service.call - expect(result).to eq([]) + expect(subject).to eq([]) end end @@ -29,14 +30,12 @@ RSpec.describe Users::ExportData::Visits, type: :service do end it 'returns visits with place references' do - result = service.call - expect(result).to be_an(Array) - expect(result.size).to eq(1) + expect(subject).to be_an(Array) + expect(subject.size).to eq(1) end it 'excludes user_id, place_id, and id fields' do - result = service.call - visit_data = result.first + visit_data = subject.first expect(visit_data).not_to have_key('user_id') expect(visit_data).not_to have_key('place_id') @@ -44,8 +43,7 @@ RSpec.describe Users::ExportData::Visits, type: :service do end it 'includes visit attributes and place reference' do - result = service.call - visit_data = result.first + visit_data = subject.first expect(visit_data).to include( 'name' => 'Work Visit', @@ -64,8 +62,7 @@ RSpec.describe Users::ExportData::Visits, type: :service do end it 'includes created_at and updated_at timestamps' do - result = service.call - visit_data = result.first + visit_data = subject.first expect(visit_data).to have_key('created_at') expect(visit_data).to have_key('updated_at') @@ -86,8 +83,7 @@ RSpec.describe Users::ExportData::Visits, type: :service do end it 'returns visits with null place references' do - result = service.call - visit_data = result.first + visit_data = subject.first expect(visit_data).to include( 'name' => 'Unknown Location', @@ -104,11 +100,10 @@ RSpec.describe Users::ExportData::Visits, type: :service do let!(:visit_without_place) { create(:visit, user: user, place: nil, name: 'Random Stop') } it 'returns all visits with appropriate place references' do - result = service.call - expect(result.size).to eq(2) + expect(subject.size).to eq(2) - visit_with_place_data = result.find { |v| v['name'] == 'Workout' } - visit_without_place_data = result.find { |v| v['name'] == 'Random Stop' } + visit_with_place_data = subject.find { |v| v['name'] == 'Workout' } + visit_without_place_data = subject.find { |v| v['name'] == 'Random Stop' } expect(visit_with_place_data['place_reference']).to be_present expect(visit_without_place_data['place_reference']).to be_nil @@ -121,9 +116,8 @@ RSpec.describe Users::ExportData::Visits, type: :service do let!(:other_user_visit) { create(:visit, user: other_user, name: 'Other User Visit') } it 'only returns visits for the specified user' do - result = service.call - expect(result.size).to eq(1) - expect(result.first['name']).to eq('User Visit') + expect(subject.size).to eq(1) + expect(subject.first['name']).to eq('User Visit') end end @@ -135,15 +129,8 @@ RSpec.describe Users::ExportData::Visits, type: :service do # This test verifies that we're using .includes(:place) expect(user.visits).to receive(:includes).with(:place).and_call_original - service.call - end - end - end - describe 'private methods' do - describe '#user' do - it 'returns the initialized user' do - expect(service.send(:user)).to eq(user) + subject end end end