diff --git a/app/jobs/users/destroy_job.rb b/app/jobs/users/destroy_job.rb index 3f7a2d68..45dc3302 100644 --- a/app/jobs/users/destroy_job.rb +++ b/app/jobs/users/destroy_job.rb @@ -17,6 +17,10 @@ class Users::DestroyJob < ApplicationJob Rails.logger.info "Successfully deleted user #{user_id}" rescue ActiveRecord::RecordNotFound Rails.logger.warn "User #{user_id} not found, may have already been deleted" + rescue ActiveRecord::RecordInvalid => e + # User cannot be deleted (e.g., owns a family with members) + Rails.logger.error "User deletion blocked for user_id #{user_id}: #{e.message}" + ExceptionReporter.call(e, "User deletion blocked for user_id #{user_id}") rescue StandardError => e ExceptionReporter.call(e, "User deletion failed for user_id #{user_id}") end diff --git a/app/services/users/destroy.rb b/app/services/users/destroy.rb index c2fa2932..87ab835e 100644 --- a/app/services/users/destroy.rb +++ b/app/services/users/destroy.rb @@ -11,17 +11,35 @@ class Users::Destroy user_id = user.id user_email = user.email + # Validate user can be deleted (not a family owner with other members) + # Use direct queries to avoid association cache issues with soft-deleted users + created_family = Family.find_by(creator_id: user_id) + if created_family + member_count = Family::Membership.where(family_id: created_family.id).count + if member_count > 1 + error_message = 'Cannot delete user who owns a family with other members' + Rails.logger.warn "#{error_message}: user_id=#{user_id}" + user.errors.add(:base, error_message) + raise ActiveRecord::RecordInvalid.new(user) + end + end + cancel_scheduled_jobs ActiveRecord::Base.transaction do # Delete associated records first (dependent: :destroy associations) + # IMPORTANT: Order matters due to foreign key constraints! + user.points.delete_all user.imports.delete_all user.stats.delete_all user.exports.delete_all user.notifications.delete_all - user.areas.delete_all + + # Delete visits BEFORE areas (visits has FK to areas) user.visits.delete_all + user.areas.delete_all + user.places.delete_all user.tags.delete_all user.trips.delete_all @@ -29,8 +47,19 @@ class Users::Destroy user.raw_data_archives.delete_all user.digests.delete_all user.sent_family_invitations.delete_all if user.respond_to?(:sent_family_invitations) - user.family_membership&.delete - user.created_family&.delete + + # Delete family associations (memberships before family due to FK) + # Delete ALL family memberships for this user (using direct query to avoid association cache issues) + Family::Membership.where(user_id: user.id).delete_all + + # If user created a family, delete all remaining memberships and the family + created_family = Family.find_by(creator_id: user.id) + if created_family + # Delete all remaining memberships in the created family (other users' memberships) + Family::Membership.where(family_id: created_family.id).delete_all + # Then delete the family itself + created_family.delete + end # Hard delete the user (bypasses soft-delete, skips callbacks) user.delete diff --git a/spec/jobs/users/destroy_job_spec.rb b/spec/jobs/users/destroy_job_spec.rb new file mode 100644 index 00000000..ae74d402 --- /dev/null +++ b/spec/jobs/users/destroy_job_spec.rb @@ -0,0 +1,152 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Users::DestroyJob, type: :job do + let(:user) { create(:user) } + let(:destroy_service) { instance_double(Users::Destroy, call: true) } + + before do + allow(Users::Destroy).to receive(:new).and_return(destroy_service) + end + + describe '#perform' do + context 'when user exists and is soft-deleted' do + before do + user.mark_as_deleted! + end + + it 'calls Users::Destroy service' do + expect(Users::Destroy).to receive(:new).with(user).and_return(destroy_service) + expect(destroy_service).to receive(:call) + + described_class.perform_now(user.id) + end + + it 'logs the deletion process' do + allow(Rails.logger).to receive(:info) + + described_class.perform_now(user.id) + + expect(Rails.logger).to have_received(:info).with("Starting hard deletion for user #{user.id} (#{user.email})") + expect(Rails.logger).to have_received(:info).with("Successfully deleted user #{user.id}") + end + end + + context 'when user is not soft-deleted' do + it 'does not call Users::Destroy service' do + expect(Users::Destroy).not_to receive(:new) + + described_class.perform_now(user.id) + end + + it 'returns early without processing' do + expect(destroy_service).not_to receive(:call) + + described_class.perform_now(user.id) + end + end + + context 'when user does not exist' do + it 'does not call Users::Destroy service' do + expect(Users::Destroy).not_to receive(:new) + + described_class.perform_now(999_999) + end + end + + context 'when user has already been hard deleted' do + it 'logs a warning' do + user.mark_as_deleted! + user.delete # Hard delete + + allow(Rails.logger).to receive(:warn) + + described_class.perform_now(user.id) + + # Should not raise error, just skip + expect(Rails.logger).not_to have_received(:warn) + end + end + + context 'when deletion fails' do + before do + user.mark_as_deleted! + allow(destroy_service).to receive(:call).and_raise(StandardError, 'Database error') + end + + it 'reports the exception' do + expect(ExceptionReporter).to receive(:call).with( + instance_of(StandardError), + "User deletion failed for user_id #{user.id}" + ) + + described_class.perform_now(user.id) + end + + it 'does not log success message' do + allow(Rails.logger).to receive(:info) + allow(ExceptionReporter).to receive(:call) + + described_class.perform_now(user.id) + + expect(Rails.logger).not_to have_received(:info).with("Successfully deleted user #{user.id}") + end + end + + context 'with retry configuration' do + it 'does not retry on failure' do + expect(described_class.get_sidekiq_options['retry']).to eq(false) + end + end + + context 'when user owns a family with members' do + let(:family) { create(:family, creator: user) } + let(:other_member) { create(:user) } + + before do + user.mark_as_deleted! + create(:family_membership, user: user, family: family, role: :owner) + create(:family_membership, user: other_member, family: family, role: :member) + + allow(Users::Destroy).to receive(:new).and_call_original + end + + it 'handles validation error gracefully' do + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:error) + allow(ExceptionReporter).to receive(:call) + + described_class.perform_now(user.id) + + expect(Rails.logger).to have_received(:error).with( + /User deletion blocked for user_id #{user.id}/ + ) + expect(ExceptionReporter).to have_received(:call).with( + instance_of(ActiveRecord::RecordInvalid), + "User deletion blocked for user_id #{user.id}" + ) + end + + it 'does not delete the user' do + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:error) + allow(ExceptionReporter).to receive(:call) + + described_class.perform_now(user.id) + + expect(User.deleted_accounts.find_by(id: user.id)).to be_present + end + + it 'does not log success message' do + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:error) + allow(ExceptionReporter).to receive(:call) + + described_class.perform_now(user.id) + + expect(Rails.logger).not_to have_received(:info).with("Successfully deleted user #{user.id}") + end + end + end +end diff --git a/spec/models/concerns/soft_deletable_spec.rb b/spec/models/concerns/soft_deletable_spec.rb new file mode 100644 index 00000000..aa52d77f --- /dev/null +++ b/spec/models/concerns/soft_deletable_spec.rb @@ -0,0 +1,219 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SoftDeletable do + # Use User as the test model since it includes SoftDeletable + let(:user) { create(:user) } + + describe 'scopes' do + let!(:active_user) { create(:user) } + let!(:deleted_user) { create(:user) } + + before do + deleted_user.mark_as_deleted! + end + + describe '.active_accounts' do + it 'returns only non-deleted users' do + expect(User.active_accounts).to include(active_user) + expect(User.active_accounts).not_to include(deleted_user) + end + + it 'returns all users when none are deleted' do + deleted_user.update!(deleted_at: nil) + expect(User.active_accounts.count).to eq(User.count) + end + + it 'returns empty when all users are deleted' do + active_user.mark_as_deleted! + expect(User.active_accounts).to be_empty + end + end + + describe '.deleted_accounts' do + it 'returns only deleted users' do + expect(User.deleted_accounts).to include(deleted_user) + expect(User.deleted_accounts).not_to include(active_user) + end + + it 'returns empty when no users are deleted' do + deleted_user.update!(deleted_at: nil) + expect(User.deleted_accounts).to be_empty + end + + it 'returns all users when all are deleted' do + active_user.mark_as_deleted! + expect(User.deleted_accounts.count).to eq(User.count) + end + end + end + + describe 'instance methods' do + describe '#deleted?' do + context 'when user is not deleted' do + it 'returns false' do + expect(user.deleted?).to be false + end + + it 'returns false when deleted_at is nil' do + user.deleted_at = nil + expect(user.deleted?).to be false + end + end + + context 'when user is deleted' do + before do + user.mark_as_deleted! + end + + it 'returns true' do + expect(user.deleted?).to be true + end + + it 'returns true when deleted_at is set' do + expect(user.deleted?).to be true + end + end + end + + describe '#mark_as_deleted!' do + it 'sets deleted_at timestamp' do + expect { + user.mark_as_deleted! + }.to change { user.deleted_at }.from(nil).to(be_within(1.second).of(Time.current)) + end + + it 'persists the deletion timestamp' do + user.mark_as_deleted! + expect(user.reload.deleted_at).to be_present + end + + it 'makes deleted? return true' do + user.mark_as_deleted! + expect(user.deleted?).to be true + end + + it 'can be called multiple times' do + user.mark_as_deleted! + first_deleted_at = user.deleted_at + + # Call again + user.mark_as_deleted! + second_deleted_at = user.deleted_at + + expect(second_deleted_at).to be >= first_deleted_at + end + end + + describe '#destroy' do + it 'soft deletes instead of hard deleting' do + user_id = user.id + user.destroy + + # User count doesn't change from active users perspective + expect(User.active_accounts.where(id: user_id).count).to eq(0) + # But user still exists in database + expect(User.unscoped.where(id: user_id).count).to eq(1) + end + + it 'sets deleted_at timestamp' do + expect { + user.destroy + }.to change { user.deleted_at }.from(nil).to(be_present) + end + + it 'makes the user deleted' do + user.destroy + expect(user.deleted?).to be true + end + + it 'keeps the user in the database' do + user_id = user.id + user.destroy + expect(User.unscoped.find_by(id: user_id)).to be_present + end + end + end + + describe 'Devise integration' do + describe '#active_for_authentication?' do + context 'when user is not deleted' do + it 'checks Devise conditions and deletion status' do + # Active user should be active for authentication + expect(user.deleted?).to be false + # Result depends on Devise's super implementation + end + end + + context 'when user is deleted' do + before do + user.mark_as_deleted! + end + + it 'prevents authentication for deleted users' do + # Deleted users should not be active for authentication + expect(user.deleted?).to be true + end + end + end + + describe '#inactive_message' do + context 'when user is not deleted' do + it 'does not return deleted message' do + # Active users should not have deleted message + expect(user.deleted?).to be false + end + end + + context 'when user is deleted' do + before do + user.mark_as_deleted! + end + + it 'indicates account is deleted' do + expect(user.deleted?).to be true + # The inactive_message should be :deleted + end + end + end + end + + describe 'edge cases' do + it 'handles deleted_at being set directly' do + user.deleted_at = 1.day.ago + expect(user.deleted?).to be true + end + + it 'handles deleted_at being unset after deletion' do + user.mark_as_deleted! + user.update!(deleted_at: nil) + expect(user.deleted?).to be false + end + + it 'works with User queries' do + user_id = user.id + user.mark_as_deleted! + + # Active accounts scope should not find deleted user + expect(User.active_accounts.find_by(id: user_id)).to be_nil + + # Deleted accounts scope should find deleted user + expect(User.deleted_accounts.find_by(id: user_id)).to be_present + + # Should find with unscoped + expect(User.unscoped.find_by(id: user_id)).to be_present + end + + it 'works with associations' do + point = create(:point, user: user) + user.mark_as_deleted! + + # Point should still exist + expect(Point.find_by(id: point.id)).to be_present + + # User is soft-deleted + expect(user.deleted?).to be true + end + end +end diff --git a/spec/requests/authentication_spec.rb b/spec/requests/authentication_spec.rb index b526ebd7..feee0c8b 100644 --- a/spec/requests/authentication_spec.rb +++ b/spec/requests/authentication_spec.rb @@ -160,6 +160,72 @@ RSpec.describe 'Authentication', type: :request do end end + describe 'Deleted User Authentication' do + context 'when user is soft-deleted' do + before do + user.mark_as_deleted! + end + + it 'prevents sign in for deleted users' do + post user_session_path, params: { + user: { email: user.email, password: 'password123' } + } + + expect(response).not_to be_redirect + expect(flash[:alert]).to include('deleted') + end + + it 'signs out already signed-in deleted users' do + # Sign in first (before deletion) + user.update!(deleted_at: nil) + sign_in user + + # Mark as deleted + user.mark_as_deleted! + + # Try to access a protected page + get map_path + + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq('Your account has been deleted.') + end + + it 'prevents API access for deleted users' do + get api_v1_points_url(api_key: user.api_key) + + expect(response).to have_http_status(:unauthorized) + + json_response = JSON.parse(response.body) + expect(json_response['error']).to eq('User account is not active or has been deleted') + end + end + + context 'when user is hard-deleted' do + it 'prevents sign in for non-existent users' do + user_email = user.email + user.delete + + post user_session_path, params: { + user: { email: user_email, password: 'password123' } + } + + expect(response).not_to be_redirect + end + + it 'prevents API access for hard-deleted users' do + api_key = user.api_key + user.delete + + get api_v1_points_url(api_key: api_key) + + expect(response).to have_http_status(:unauthorized) + + json_response = JSON.parse(response.body) + expect(json_response['error']).to eq('User account is not active or has been deleted') + end + end + end + describe 'Family Invitation with Authentication' do let(:family) { create(:family, creator: user) } let!(:membership) { create(:family_membership, user: user, family: family, role: :owner) } diff --git a/spec/requests/settings/users_spec.rb b/spec/requests/settings/users_spec.rb index b8bc5a38..e7f4c8da 100644 --- a/spec/requests/settings/users_spec.rb +++ b/spec/requests/settings/users_spec.rb @@ -86,6 +86,85 @@ RSpec.describe '/settings/users', type: :request do end end end + + describe 'DELETE /destroy' do + let(:user) { create(:user) } + + before { sign_in admin } + + context 'with a regular user' do + it 'soft deletes the user' do + expect { + delete settings_user_url(user) + }.not_to change(User, :count) + + expect(user.reload.deleted?).to be true + end + + it 'enqueues a background deletion job' do + expect { + delete settings_user_url(user) + }.to have_enqueued_job(Users::DestroyJob).with(user.id) + end + + it 'redirects to settings users page with notice' do + delete settings_user_url(user) + + expect(response).to redirect_to(settings_users_url) + expect(flash[:notice]).to eq('User deletion has been initiated. The account will be fully removed shortly.') + end + + it 'immediately marks user as deleted' do + delete settings_user_url(user) + + expect(user.reload.deleted_at).to be_present + end + end + + context 'when user is a family owner with members' do + let(:family) { create(:family, creator: user) } + let(:member) { create(:user) } + + before do + create(:family_membership, user: user, family: family, role: :owner) + create(:family_membership, user: member, family: family, role: :member) + end + + it 'does not delete the user' do + expect { + delete settings_user_url(user) + }.not_to change { user.reload.deleted_at } + end + + it 'redirects with error message' do + delete settings_user_url(user) + + expect(response).to have_http_status(:unprocessable_content) + expect(response).to redirect_to(settings_users_url) + expect(flash[:alert]).to eq('Cannot delete account while being owner of a family which has other members.') + end + + it 'does not enqueue deletion job' do + expect { + delete settings_user_url(user) + }.not_to have_enqueued_job(Users::DestroyJob) + end + end + + context 'concurrent deletion attempts' do + it 'handles multiple deletion requests gracefully' do + # First deletion + delete settings_user_url(user) + expect(user.reload.deleted?).to be true + + # Second deletion attempt on already-deleted user + delete settings_user_url(user) + + # Should not raise error, user still deleted + expect(user.reload.deleted?).to be true + end + end + end end end end diff --git a/spec/requests/users/registrations_spec.rb b/spec/requests/users/registrations_spec.rb index 38e5b24c..9e150ce2 100644 --- a/spec/requests/users/registrations_spec.rb +++ b/spec/requests/users/registrations_spec.rb @@ -417,6 +417,113 @@ RSpec.describe 'Users::Registrations', type: :request do end end + describe 'Account Deletion' do + let(:user) { create(:user, password: 'password123') } + + before { sign_in user } + + context 'when user deletes their own account' do + it 'soft deletes the user' do + expect { + delete user_registration_path + }.not_to change(User, :count) + + expect(user.reload.deleted?).to be true + end + + it 'enqueues a background deletion job' do + expect { + delete user_registration_path + }.to have_enqueued_job(Users::DestroyJob).with(user.id) + end + + it 'signs out the user' do + delete user_registration_path + + expect(controller.current_user).to be_nil + end + + it 'redirects with success message' do + delete user_registration_path + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('Your account has been scheduled for deletion. Goodbye!') + end + + it 'immediately marks user as deleted' do + delete user_registration_path + + expect(user.reload.deleted_at).to be_present + end + end + + context 'when user is a family owner with members' do + let(:family) { create(:family, creator: user) } + let(:member) { create(:user) } + + before do + create(:family_membership, user: user, family: family, role: :owner) + create(:family_membership, user: member, family: family, role: :member) + end + + it 'does not delete the account' do + expect { + delete user_registration_path + }.not_to change { user.reload.deleted_at } + end + + it 'redirects with error message' do + delete user_registration_path + + expect(response).to have_http_status(:unprocessable_content) + expect(response).to redirect_to(edit_user_registration_path) + expect(flash[:alert]).to eq('Cannot delete your account while you own a family with other members.') + end + + it 'does not sign out the user' do + delete user_registration_path + + expect(controller.current_user).to eq(user) + end + + it 'does not enqueue deletion job' do + expect { + delete user_registration_path + }.not_to have_enqueued_job(Users::DestroyJob) + end + end + + context 'concurrent deletion attempts' do + it 'handles multiple deletion requests gracefully' do + # First deletion + delete user_registration_path + expect(user.reload.deleted?).to be true + + # User is now signed out, try to delete again (should be unauthorized) + delete user_registration_path + + # Should redirect to sign in + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'when user can delete (family owner with no other members)' do + let(:family) { create(:family, creator: user) } + + before do + create(:family_membership, user: user, family: family, role: :owner) + end + + it 'allows deletion' do + expect { + delete user_registration_path + }.not_to change(User, :count) + + expect(user.reload.deleted?).to be true + end + end + end + describe 'UTM Parameter Tracking' do let(:utm_params) do { diff --git a/spec/services/users/destroy_spec.rb b/spec/services/users/destroy_spec.rb new file mode 100644 index 00000000..bcd56a16 --- /dev/null +++ b/spec/services/users/destroy_spec.rb @@ -0,0 +1,323 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Users::Destroy do + describe '#call' do + let(:user) { create(:user) } + let(:service) { described_class.new(user) } + + before do + user.mark_as_deleted! + end + + context 'with minimal user data' do + it 'hard deletes the user record' do + expect { service.call }.to change(User, :count).by(-1) + end + + it 'returns true on success' do + expect(service.call).to be true + end + + it 'logs the deletion' do + allow(Rails.logger).to receive(:info) + + service.call + + expect(Rails.logger).to have_received(:info).with(/User \d+ \(.+\) and all associated data deleted/) + end + end + + context 'with associated records without foreign key constraints' do + let!(:points) { create_list(:point, 5, user:) } + let!(:import) { create(:import, user:) } + let!(:stat) { create(:stat, user:, year: 2024, month: 1) } + let!(:place) { create(:place, user:) } + let!(:trip) { create(:trip, user:) } + let!(:notification) { create(:notification, user:) } + + it 'deletes all points' do + user_id = user.id + service.call + expect(Point.where(user_id: user_id).count).to eq(0) + end + + it 'deletes all imports' do + user_id = user.id + service.call + expect(Import.where(user_id: user_id).count).to eq(0) + end + + it 'deletes all stats' do + user_id = user.id + service.call + expect(Stat.where(user_id: user_id).count).to eq(0) + end + + it 'deletes all places' do + user_id = user.id + service.call + expect(Place.where(user_id: user_id).count).to eq(0) + end + + it 'deletes all trips' do + user_id = user.id + service.call + expect(Trip.where(user_id: user_id).count).to eq(0) + end + + it 'deletes all notifications' do + user_id = user.id + service.call + expect(Notification.where(user_id: user_id).count).to eq(0) + end + + it 'performs all deletions in a transaction' do + # Mock error before user deletion + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:error) + allow(ExceptionReporter).to receive(:call) + allow_any_instance_of(described_class).to receive(:cancel_scheduled_jobs) + allow(Point).to receive(:where).and_call_original + + # This will cause the transaction to fail + allow(user).to receive(:delete).and_raise(StandardError, 'Database error') + + expect { service.call }.to raise_error(StandardError) + end + end + + context 'with scheduled jobs' do + it 'attempts to cancel scheduled jobs for the user' do + allow(Rails.logger).to receive(:info) + + service.call + + expect(Rails.logger).to have_received(:info).with(/Cancelled \d+ scheduled jobs for user #{user.id}/) + end + + context 'when job cancellation fails' do + before do + allow(Sidekiq::ScheduledSet).to receive(:new).and_raise(StandardError, 'Redis error') + end + + it 'logs a warning but continues deletion' do + allow(Rails.logger).to receive(:warn) + allow(Rails.logger).to receive(:info) + allow(Rails.logger).to receive(:error) + allow(ExceptionReporter).to receive(:call) + + expect { service.call }.not_to raise_error + + expect(Rails.logger).to have_received(:warn).with(/Failed to cancel scheduled jobs/) + expect(ExceptionReporter).to have_received(:call) + end + end + end + + context 'with cache cleanup' do + before do + # Populate cache with user data + Rails.cache.write("dawarich/user_#{user.id}_countries_visited", ['US', 'CA']) + Rails.cache.write("dawarich/user_#{user.id}_cities_visited", ['NYC', 'SF']) + Rails.cache.write("dawarich/user_#{user.id}_total_distance", 1000) + Rails.cache.write("dawarich/user_#{user.id}_years_tracked", [2023, 2024]) + end + + it 'clears all user-specific cache keys' do + service.call + + expect(Rails.cache.read("dawarich/user_#{user.id}_countries_visited")).to be_nil + expect(Rails.cache.read("dawarich/user_#{user.id}_cities_visited")).to be_nil + expect(Rails.cache.read("dawarich/user_#{user.id}_total_distance")).to be_nil + expect(Rails.cache.read("dawarich/user_#{user.id}_years_tracked")).to be_nil + end + + it 'logs cache cleanup' do + allow(Rails.logger).to receive(:info) + + service.call + + expect(Rails.logger).to have_received(:info).with("Cleared cache for user #{user.id}") + end + + context 'when cache cleanup fails' do + before do + allow(Rails.cache).to receive(:delete).and_raise(StandardError, 'Cache error') + end + + it 'logs a warning but completes deletion' do + allow(Rails.logger).to receive(:warn) + + expect { service.call }.not_to raise_error + + expect(Rails.logger).to have_received(:warn).with(/Failed to clear cache/) + end + end + end + + context 'with areas and visits (foreign key constraint)' do + let!(:area) { create(:area, user:) } + let!(:visit) { create(:visit, user:, area:) } + + it 'deletes visits before areas to respect foreign key constraints' do + user_id = user.id + area_id = area.id + visit_id = visit.id + + service.call + + # Both should be deleted successfully + expect(Visit.where(id: visit_id).count).to eq(0) + expect(Area.where(id: area_id).count).to eq(0) + expect(User.unscoped.where(id: user_id).count).to eq(0) + end + end + + context 'with family associations' do + context 'when user owns a family with other members' do + let(:family) { create(:family, creator: user) } + let(:other_member) { create(:user) } + + before do + # User creates and owns a family + create(:family_membership, user: user, family: family, role: :owner) + # Another user is a member of that family + create(:family_membership, user: other_member, family: family, role: :member) + end + + it 'aborts deletion and raises error' do + expect { service.call }.to raise_error( + ActiveRecord::RecordInvalid, + /Cannot delete user who owns a family with other members/ + ) + + # User should NOT be deleted + expect(User.unscoped.where(id: user.id).count).to eq(1) + expect(user.reload.deleted?).to be true # Still soft-deleted + + # Family and memberships should still exist + expect(Family.where(id: family.id).count).to eq(1) + expect(Family::Membership.where(family_id: family.id).count).to eq(2) + end + + it 'logs the validation failure' do + allow(Rails.logger).to receive(:warn) + + expect { service.call }.to raise_error(ActiveRecord::RecordInvalid) + + expect(Rails.logger).to have_received(:warn).with( + /Cannot delete user who owns a family with other members: user_id=#{user.id}/ + ) + end + end + + context 'when user owns a family with no other members' do + let(:family) { create(:family, creator: user) } + + before do + # User creates and owns a family but is the only member + create(:family_membership, user: user, family: family, role: :owner) + end + + it 'deletes the user, membership, and family' do + user_id = user.id + family_id = family.id + + service.call + + # User should be deleted + expect(User.unscoped.where(id: user_id).count).to eq(0) + + # All family memberships should be deleted + expect(Family::Membership.where(family_id: family_id).count).to eq(0) + + # Family itself should be deleted + expect(Family.where(id: family_id).count).to eq(0) + end + end + + end + + context 'with user as family member only' do + it 'deletes member but preserves family and owner' do + # Create separate users (not using the `user` from parent context) + family_owner = create(:user) + member_user = create(:user) + member_user.mark_as_deleted! + + a_family = create(:family, creator: family_owner) + create(:family_membership, user: family_owner, family: a_family, role: :owner) + create(:family_membership, user: member_user, family: a_family, role: :member) + + member_service = described_class.new(member_user) + member_user_id = member_user.id + family_id = a_family.id + + member_service.call + + # Member user should be deleted + expect(User.unscoped.where(id: member_user_id).count).to eq(0) + + # Member's membership should be deleted + expect(Family::Membership.where(family_id: family_id, user_id: member_user_id).count).to eq(0) + + # But family should still exist (owned by family_owner) + expect(Family.where(id: family_id).count).to eq(1) + + # And owner's membership should still exist + expect(Family::Membership.where(family_id: family_id, user_id: family_owner.id).count).to eq(1) + end + end + + context 'when deletion fails' do + before do + allow(user.points).to receive(:delete_all).and_raise(StandardError, 'Database constraint violation') + end + + it 'logs the error' do + allow(Rails.logger).to receive(:error) + allow(ExceptionReporter).to receive(:call) + + expect { service.call }.to raise_error(StandardError) + + expect(Rails.logger).to have_received(:error).with(/Error during user deletion/) + end + + it 'reports the exception' do + expect(ExceptionReporter).to receive(:call).with( + instance_of(StandardError), + /User destroy service failed for user_id #{user.id}/ + ) + + expect { service.call }.to raise_error(StandardError) + end + + it 're-raises the error' do + allow(Rails.logger).to receive(:error) + allow(ExceptionReporter).to receive(:call) + + expect { service.call }.to raise_error(StandardError, 'Database constraint violation') + end + end + + context 'with large datasets' do + before do + # Create many points to simulate a real user with lots of data + create_list(:point, 100, user:) + end + + it 'successfully deletes all records' do + expect { service.call }.to change { Point.where(user_id: user.id).count }.from(100).to(0) + end + + it 'completes deletion' do + service.call + + expect(Point.where(user_id: user.id).count).to eq(0) + expect(User.unscoped.find_by(id: user.id)).to be_nil + end + end + end +end