From 17837979e21e8fd65131d30639a7d679b004b215 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 8 Jan 2026 20:29:28 +0100 Subject: [PATCH 1/2] Move user deletions to background job --- app/controllers/api_controller.rb | 15 ++++- app/controllers/application_controller.rb | 8 +++ app/controllers/settings/users_controller.rb | 14 +++-- .../users/registrations_controller.rb | 19 ++++++ app/jobs/users/destroy_job.rb | 28 +++++++++ app/models/concerns/user_family.rb | 8 +++ app/models/user.rb | 24 ++++++++ app/services/users/destroy.rb | 61 +++++++++++++++++++ config/locales/devise.en.yml | 4 +- .../20260108192905_add_deleted_at_to_users.rb | 8 +++ db/schema.rb | 4 +- 11 files changed, 185 insertions(+), 8 deletions(-) create mode 100644 app/jobs/users/destroy_job.rb create mode 100644 app/services/users/destroy.rb create mode 100644 db/migrate/20260108192905_add_deleted_at_to_users.rb diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 74848252..01bd1b61 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -27,13 +27,24 @@ class ApiController < ApplicationController end def authenticate_active_api_user! - render json: { error: 'User is not active' }, status: :unauthorized unless current_api_user&.active_until&.future? + if current_api_user.nil? + render json: { error: 'User account is not active or has been deleted' }, status: :unauthorized + return false + end + + unless current_api_user.active_until&.future? + render json: { error: 'User subscription is not active' }, status: :unauthorized + return false + end true end def current_api_user - @current_api_user ||= User.find_by(api_key:) + @current_api_user ||= begin + user = User.active_accounts.find_by(api_key:) + user if user&.active_for_authentication? + end end def api_key diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bdf00702..26ba29e8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -5,6 +5,7 @@ class ApplicationController < ActionController::Base rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized + before_action :sign_out_deleted_users before_action :unread_notifications, :set_self_hosted_status, :store_client_header protected @@ -72,6 +73,13 @@ class ApplicationController < ActionController::Base private + def sign_out_deleted_users + if current_user&.deleted? + sign_out current_user + redirect_to root_path, alert: 'Your account has been deleted.' + end + end + def set_self_hosted_status @self_hosted = DawarichSettings.self_hosted? end diff --git a/app/controllers/settings/users_controller.rb b/app/controllers/settings/users_controller.rb index d60c5bf9..c05d1487 100644 --- a/app/controllers/settings/users_controller.rb +++ b/app/controllers/settings/users_controller.rb @@ -40,10 +40,16 @@ class Settings::UsersController < ApplicationController def destroy @user = User.find(params[:id]) - if @user.destroy - redirect_to settings_url, notice: 'User was successfully deleted.' - else - redirect_to settings_url, notice: 'User could not be deleted.', status: :unprocessable_content + begin + @user.mark_as_deleted! + Users::DestroyJob.perform_later(@user.id) + + redirect_to settings_users_url, + notice: 'User deletion has been initiated. The account will be fully removed shortly.' + rescue ActiveRecord::RecordInvalid + redirect_to settings_users_url, + alert: 'Cannot delete account while being a family owner with other members.', + status: :unprocessable_content end end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index b29b31b4..c307bc60 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -26,6 +26,25 @@ class Users::RegistrationsController < Devise::RegistrationsController end end + def destroy + begin + resource.mark_as_deleted! + + # Sign out immediately + Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name) + + # Enqueue background job + Users::DestroyJob.perform_later(resource.id) + + set_flash_message! :notice, :destroyed + yield resource if block_given? + respond_with_navigational(resource) { redirect_to after_sign_out_path_for(resource_name) } + rescue ActiveRecord::RecordInvalid + set_flash_message! :alert, :cannot_delete + redirect_to edit_user_registration_path, status: :unprocessable_content + end + end + protected def after_sign_up_path_for(resource) diff --git a/app/jobs/users/destroy_job.rb b/app/jobs/users/destroy_job.rb new file mode 100644 index 00000000..8b161dd4 --- /dev/null +++ b/app/jobs/users/destroy_job.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class Users::DestroyJob < ApplicationJob + queue_as :default + + sidekiq_options retry: false # No retry for destructive operations + + def perform(user_id) + user = User.deleted_accounts.find_by(id: user_id) + + unless user + Rails.logger.warn "User #{user_id} not found or not marked for deletion, skipping" + return + end + + Rails.logger.info "Starting hard deletion for user #{user.id} (#{user.email})" + + Users::Destroy.new(user).call + + 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 StandardError => e + Rails.logger.error "Failed to delete user #{user_id}: #{e.message}" + ExceptionReporter.call(e, "User deletion failed for user_id #{user_id}") + # Don't raise - leave user in deleted state for manual cleanup + end +end diff --git a/app/models/concerns/user_family.rb b/app/models/concerns/user_family.rb index 53119792..1d5201ed 100644 --- a/app/models/concerns/user_family.rb +++ b/app/models/concerns/user_family.rb @@ -10,6 +10,7 @@ module UserFamily has_many :sent_family_invitations, class_name: 'Family::Invitation', foreign_key: 'invited_by_id', inverse_of: :invited_by, dependent: :destroy + validate :cannot_delete_with_family_members, if: :deleted_at_changed? before_destroy :check_family_ownership end @@ -107,6 +108,13 @@ module UserFamily private + def cannot_delete_with_family_members + return unless deleted_at.present? && deleted_at_changed? + return if can_delete_account? + + errors.add(:base, 'Cannot delete account while being a family owner with other members') + end + def check_family_ownership return if can_delete_account? diff --git a/app/models/user.rb b/app/models/user.rb index 4279d494..ec87b297 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -36,9 +36,33 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength attribute :points_count, :integer, default: 0 scope :active_or_trial, -> { where(status: %i[active trial]) } + scope :active_accounts, -> { where(deleted_at: nil) } + scope :deleted_accounts, -> { where.not(deleted_at: nil) } enum :status, { inactive: 0, active: 1, trial: 2 } + # Soft-delete methods + def deleted? + deleted_at.present? + end + + def mark_as_deleted! + update!(deleted_at: Time.current) + end + + def destroy + mark_as_deleted! + end + + # Devise authentication overrides + def active_for_authentication? + super && !deleted? + end + + def inactive_message + deleted? ? :deleted : super + end + def safe_settings Users::SafeSettings.new(settings) end diff --git a/app/services/users/destroy.rb b/app/services/users/destroy.rb new file mode 100644 index 00000000..12e9589e --- /dev/null +++ b/app/services/users/destroy.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +class Users::Destroy + attr_reader :user + + def initialize(user) + @user = user + end + + def call + user_id = user.id + user_email = user.email + + cancel_scheduled_jobs + + # Hard delete with transaction - all associations cascade via dependent: :destroy + ActiveRecord::Base.transaction do + user.destroy! + end + + Rails.logger.info "User #{user_id} (#{user_email}) and all associated data deleted" + + cleanup_user_cache(user_id) + + true + rescue StandardError => e + Rails.logger.error "Error during user deletion: #{e.message}" + ExceptionReporter.call(e, "User destroy service failed for user_id #{user_id}") + raise + end + + private + + def cancel_scheduled_jobs + scheduled_set = Sidekiq::ScheduledSet.new + + jobs_cancelled = scheduled_set.select { |job| + job.klass == 'Users::MailerSendingJob' && job.args.first == user.id + }.map(&:delete).count + + Rails.logger.info "Cancelled #{jobs_cancelled} scheduled jobs for user #{user.id}" + rescue StandardError => e + Rails.logger.warn "Failed to cancel scheduled jobs for user #{user.id}: #{e.message}" + ExceptionReporter.call(e, "Failed to cancel scheduled jobs during user deletion") + end + + def cleanup_user_cache(user_id) + cache_keys = [ + "dawarich/user_#{user_id}_countries_visited", + "dawarich/user_#{user_id}_cities_visited", + "dawarich/user_#{user_id}_total_distance", + "dawarich/user_#{user_id}_years_tracked" + ] + + cache_keys.each { |key| Rails.cache.delete(key) } + + Rails.logger.info "Cleared cache for user #{user_id}" + rescue StandardError => e + Rails.logger.warn "Failed to clear cache for user #{user_id}: #{e.message}" + end +end diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index 260e1c4b..0415002d 100644 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -16,6 +16,7 @@ en: timeout: "Your session expired. Please sign in again to continue." unauthenticated: "You need to sign in or sign up before continuing." unconfirmed: "You have to confirm your email address before continuing." + deleted: "Your account has been deleted." mailer: confirmation_instructions: subject: "Confirmation instructions" @@ -37,7 +38,8 @@ en: updated: "Your password has been changed successfully. You are now signed in." updated_not_active: "Your password has been changed successfully." registrations: - destroyed: "Bye! Your account has been successfully cancelled. We hope to see you again soon." + destroyed: "Your account has been scheduled for deletion. Goodbye!" + cannot_delete: "Cannot delete your account while you are a family owner with other members." signed_up: "Welcome! You have signed up successfully." signed_up_but_inactive: "You have signed up successfully. However, we could not sign you in because your account is not yet activated." signed_up_but_locked: "You have signed up successfully. However, we could not sign you in because your account is locked." diff --git a/db/migrate/20260108192905_add_deleted_at_to_users.rb b/db/migrate/20260108192905_add_deleted_at_to_users.rb new file mode 100644 index 00000000..a21412aa --- /dev/null +++ b/db/migrate/20260108192905_add_deleted_at_to_users.rb @@ -0,0 +1,8 @@ +class AddDeletedAtToUsers < ActiveRecord::Migration[8.0] + disable_ddl_transaction! + + def change + add_column :users, :deleted_at, :datetime + add_index :users, :deleted_at, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index d7baaeb4..cd7c8d08 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_01_03_114630) do +ActiveRecord::Schema[8.0].define(version: 2026_01_08_112905) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "postgis" @@ -484,7 +484,9 @@ ActiveRecord::Schema[8.0].define(version: 2026_01_03_114630) do t.string "utm_campaign" t.string "utm_term" t.string "utm_content" + t.datetime "deleted_at" t.index ["api_key"], name: "index_users_on_api_key" + t.index ["deleted_at"], name: "index_users_on_deleted_at" t.index ["email"], name: "index_users_on_email", unique: true t.index ["provider", "uid"], name: "index_users_on_provider_and_uid", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true From 8465350c1b484fc9ffb10aac736d19e9e65cf286 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 8 Jan 2026 21:12:47 +0100 Subject: [PATCH 2/2] Rework some parts --- CHANGELOG.md | 1 + app/controllers/api_controller.rb | 4 ++- app/controllers/application_controller.rb | 8 ++--- app/controllers/settings/users_controller.rb | 2 +- .../users/registrations_controller.rb | 2 -- app/jobs/users/destroy_job.rb | 9 ++---- app/jobs/users/trial_webhook_job.rb | 3 ++ app/models/concerns/soft_deletable.rb | 31 +++++++++++++++++++ app/models/user.rb | 25 +-------------- app/services/users/destroy.rb | 22 +++++++++++-- config/locales/devise.en.yml | 2 +- .../20260108192905_add_deleted_at_to_users.rb | 4 +-- db/schema.rb | 2 +- spec/jobs/users/trial_webhook_job_spec.rb | 8 ++--- spec/models/user_family_spec.rb | 10 +++--- 15 files changed, 80 insertions(+), 53 deletions(-) create mode 100644 app/models/concerns/soft_deletable.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 906dd1b8..22398f99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Map V2 points loading is significantly sped up. - Points size on Map V2 was reduced to prevent overlapping. +- User deletion now being done in the background to prevent request timeouts for users with large amount of data. # [0.37.2] - 2026-01-04 diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 01bd1b61..0073c192 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -29,11 +29,13 @@ class ApiController < ApplicationController def authenticate_active_api_user! if current_api_user.nil? render json: { error: 'User account is not active or has been deleted' }, status: :unauthorized + return false end - unless current_api_user.active_until&.future? + if current_api_user.active_until&.past? render json: { error: 'User subscription is not active' }, status: :unauthorized + return false end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 26ba29e8..3060d9e0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -74,10 +74,10 @@ class ApplicationController < ActionController::Base private def sign_out_deleted_users - if current_user&.deleted? - sign_out current_user - redirect_to root_path, alert: 'Your account has been deleted.' - end + return unless current_user&.deleted? + + sign_out current_user + redirect_to root_path, alert: 'Your account has been deleted.' end def set_self_hosted_status diff --git a/app/controllers/settings/users_controller.rb b/app/controllers/settings/users_controller.rb index c05d1487..54377cb8 100644 --- a/app/controllers/settings/users_controller.rb +++ b/app/controllers/settings/users_controller.rb @@ -48,7 +48,7 @@ class Settings::UsersController < ApplicationController notice: 'User deletion has been initiated. The account will be fully removed shortly.' rescue ActiveRecord::RecordInvalid redirect_to settings_users_url, - alert: 'Cannot delete account while being a family owner with other members.', + alert: 'Cannot delete account while being owner of a family which has other members.', status: :unprocessable_content end end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index c307bc60..3442bd7f 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -30,10 +30,8 @@ class Users::RegistrationsController < Devise::RegistrationsController begin resource.mark_as_deleted! - # Sign out immediately Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name) - # Enqueue background job Users::DestroyJob.perform_later(resource.id) set_flash_message! :notice, :destroyed diff --git a/app/jobs/users/destroy_job.rb b/app/jobs/users/destroy_job.rb index 8b161dd4..3f7a2d68 100644 --- a/app/jobs/users/destroy_job.rb +++ b/app/jobs/users/destroy_job.rb @@ -3,15 +3,12 @@ class Users::DestroyJob < ApplicationJob queue_as :default - sidekiq_options retry: false # No retry for destructive operations + sidekiq_options retry: false def perform(user_id) user = User.deleted_accounts.find_by(id: user_id) - unless user - Rails.logger.warn "User #{user_id} not found or not marked for deletion, skipping" - return - end + return unless user Rails.logger.info "Starting hard deletion for user #{user.id} (#{user.email})" @@ -21,8 +18,6 @@ class Users::DestroyJob < ApplicationJob rescue ActiveRecord::RecordNotFound Rails.logger.warn "User #{user_id} not found, may have already been deleted" rescue StandardError => e - Rails.logger.error "Failed to delete user #{user_id}: #{e.message}" ExceptionReporter.call(e, "User deletion failed for user_id #{user_id}") - # Don't raise - leave user in deleted state for manual cleanup end end diff --git a/app/jobs/users/trial_webhook_job.rb b/app/jobs/users/trial_webhook_job.rb index 512dd075..48ed5660 100644 --- a/app/jobs/users/trial_webhook_job.rb +++ b/app/jobs/users/trial_webhook_job.rb @@ -6,6 +6,9 @@ class Users::TrialWebhookJob < ApplicationJob def perform(user_id) user = User.find(user_id) + # Skip webhook for soft-deleted users + return if user.deleted? + payload = { user_id: user.id, email: user.email, diff --git a/app/models/concerns/soft_deletable.rb b/app/models/concerns/soft_deletable.rb new file mode 100644 index 00000000..f8bd582c --- /dev/null +++ b/app/models/concerns/soft_deletable.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module SoftDeletable + extend ActiveSupport::Concern + + included do + scope :active_accounts, -> { where(deleted_at: nil) } + scope :deleted_accounts, -> { where.not(deleted_at: nil) } + end + + def deleted? + deleted_at.present? + end + + def mark_as_deleted! + update!(deleted_at: Time.current) + end + + def destroy + mark_as_deleted! + end + + # Devise authentication overrides + def active_for_authentication? + super && !deleted? + end + + def inactive_message + deleted? ? :deleted : super + end +end diff --git a/app/models/user.rb b/app/models/user.rb index ec87b297..46c5003b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,6 +3,7 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength include UserFamily include Omniauthable + include SoftDeletable devise :database_authenticatable, :registerable, :recoverable, :rememberable, :validatable, :trackable, @@ -36,33 +37,9 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength attribute :points_count, :integer, default: 0 scope :active_or_trial, -> { where(status: %i[active trial]) } - scope :active_accounts, -> { where(deleted_at: nil) } - scope :deleted_accounts, -> { where.not(deleted_at: nil) } enum :status, { inactive: 0, active: 1, trial: 2 } - # Soft-delete methods - def deleted? - deleted_at.present? - end - - def mark_as_deleted! - update!(deleted_at: Time.current) - end - - def destroy - mark_as_deleted! - end - - # Devise authentication overrides - def active_for_authentication? - super && !deleted? - end - - def inactive_message - deleted? ? :deleted : super - end - def safe_settings Users::SafeSettings.new(settings) end diff --git a/app/services/users/destroy.rb b/app/services/users/destroy.rb index 12e9589e..c2fa2932 100644 --- a/app/services/users/destroy.rb +++ b/app/services/users/destroy.rb @@ -13,9 +13,27 @@ class Users::Destroy cancel_scheduled_jobs - # Hard delete with transaction - all associations cascade via dependent: :destroy ActiveRecord::Base.transaction do - user.destroy! + # Delete associated records first (dependent: :destroy associations) + user.points.delete_all + user.imports.delete_all + user.stats.delete_all + user.exports.delete_all + user.notifications.delete_all + user.areas.delete_all + user.visits.delete_all + user.places.delete_all + user.tags.delete_all + user.trips.delete_all + user.tracks.delete_all + 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 + + # Hard delete the user (bypasses soft-delete, skips callbacks) + user.delete end Rails.logger.info "User #{user_id} (#{user_email}) and all associated data deleted" diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index 0415002d..43ed54fa 100644 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -39,7 +39,7 @@ en: updated_not_active: "Your password has been changed successfully." registrations: destroyed: "Your account has been scheduled for deletion. Goodbye!" - cannot_delete: "Cannot delete your account while you are a family owner with other members." + cannot_delete: "Cannot delete your account while you own a family with other members." signed_up: "Welcome! You have signed up successfully." signed_up_but_inactive: "You have signed up successfully. However, we could not sign you in because your account is not yet activated." signed_up_but_locked: "You have signed up successfully. However, we could not sign you in because your account is locked." diff --git a/db/migrate/20260108192905_add_deleted_at_to_users.rb b/db/migrate/20260108192905_add_deleted_at_to_users.rb index a21412aa..7426d3f0 100644 --- a/db/migrate/20260108192905_add_deleted_at_to_users.rb +++ b/db/migrate/20260108192905_add_deleted_at_to_users.rb @@ -2,7 +2,7 @@ class AddDeletedAtToUsers < ActiveRecord::Migration[8.0] disable_ddl_transaction! def change - add_column :users, :deleted_at, :datetime - add_index :users, :deleted_at, algorithm: :concurrently + add_column :users, :deleted_at, :datetime unless column_exists?(:users, :deleted_at) + add_index :users, :deleted_at, algorithm: :concurrently unless index_exists?(:users, :deleted_at) end end diff --git a/db/schema.rb b/db/schema.rb index cd7c8d08..a87e3017 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_01_08_112905) do +ActiveRecord::Schema[8.0].define(version: 2026_01_08_192905) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "postgis" diff --git a/spec/jobs/users/trial_webhook_job_spec.rb b/spec/jobs/users/trial_webhook_job_spec.rb index 94a9e581..384c6093 100644 --- a/spec/jobs/users/trial_webhook_job_spec.rb +++ b/spec/jobs/users/trial_webhook_job_spec.rb @@ -44,12 +44,12 @@ RSpec.describe Users::TrialWebhookJob, type: :job do end context 'when user is deleted' do - it 'raises ActiveRecord::RecordNotFound' do + it 'skips the webhook for soft-deleted users' do user.destroy - expect { - described_class.perform_now(user.id) - }.to raise_error(ActiveRecord::RecordNotFound) + expect(HTTParty).not_to receive(:post) + + described_class.perform_now(user.id) end end end diff --git a/spec/models/user_family_spec.rb b/spec/models/user_family_spec.rb index 0a0d2879..f2473c54 100644 --- a/spec/models/user_family_spec.rb +++ b/spec/models/user_family_spec.rb @@ -118,8 +118,9 @@ RSpec.describe User, 'family methods', type: :model do create(:family_invitation, family: family, invited_by: user) end - it 'destroys associated invitations when user is destroyed' do - expect { user.destroy }.to change(Family::Invitation, :count).by(-1) + it 'soft-deletes user but keeps invitations' do + expect { user.destroy }.not_to change(Family::Invitation, :count) + expect(user.deleted?).to be true end end @@ -128,8 +129,9 @@ RSpec.describe User, 'family methods', type: :model do create(:family_membership, user: user, family: family) end - it 'destroys associated membership when user is destroyed' do - expect { user.destroy }.to change(Family::Membership, :count).by(-1) + it 'soft-deletes user but keeps membership' do + expect { user.destroy }.not_to change(Family::Membership, :count) + expect(user.deleted?).to be true end end end