mirror of
https://github.com/Freika/dawarich.git
synced 2026-01-12 18:21:38 -05:00
Add missing tests
This commit is contained in:
parent
8465350c1b
commit
d5ed8bf943
8 changed files with 982 additions and 3 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
152
spec/jobs/users/destroy_job_spec.rb
Normal file
152
spec/jobs/users/destroy_job_spec.rb
Normal file
|
|
@ -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
|
||||
219
spec/models/concerns/soft_deletable_spec.rb
Normal file
219
spec/models/concerns/soft_deletable_spec.rb
Normal file
|
|
@ -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
|
||||
|
|
@ -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) }
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
{
|
||||
|
|
|
|||
323
spec/services/users/destroy_spec.rb
Normal file
323
spec/services/users/destroy_spec.rb
Normal file
|
|
@ -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
|
||||
Loading…
Reference in a new issue