Skip to content

Commit

Permalink
feat: Add multiple changes to optimize reading and save email to exte…
Browse files Browse the repository at this point in the history
…nded data
  • Loading branch information
AyakorK committed Dec 31, 2024
1 parent 233f69f commit 04ca0a8
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 46 deletions.
35 changes: 27 additions & 8 deletions app/jobs/clear_duplicated_half_signup_users_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ def perform
duplicated_numbers = find_duplicated_phone_numbers
return puts("No duplicated phone numbers found") if duplicated_numbers.empty?

alerts = []

duplicated_numbers.each do |phone_info|
phone_number, phone_country = phone_info

Expand All @@ -17,8 +19,9 @@ def perform

quick_auth_users.each { |user| soft_delete_user(user, "Duplicated account") }

alert_about_duplicated_numbers(phone_number, other_users) if other_users.map(&:email).uniq.size > 1
alerts << generate_alert_message(phone_number, other_users) if other_users.map(&:email).uniq.size > 1
end
display_alerts(alerts)
end

private
Expand All @@ -34,6 +37,8 @@ def find_duplicated_phone_numbers

def soft_delete_user(user, reason)
if user.email.include?("quick_auth")
puts "\n---- Processing duplicated phone number: #{obfuscate_phone_number(user.phone_number)} ----\n"

user.update(phone_number: nil, phone_country: nil)

form = Decidim::DeleteAccountForm.from_params(delete_reason: reason)
Expand All @@ -42,6 +47,7 @@ def soft_delete_user(user, reason)
on(:ok) do
log!("User #{user.id} (#{previous_email}) has been deleted", :info)
puts("User #{user.id} (#{previous_email}) has been deleted")
save_deleted_user_email(user, previous_email)
end
on(:invalid) do
log!("Failed to delete user #{user.id} (#{user.email}): #{form.errors.full_messages}", :warn)
Expand All @@ -54,21 +60,34 @@ def soft_delete_user(user, reason)
end
end

def alert_about_duplicated_numbers(phone_number, users)
def save_deleted_user_email(user, email)
user.extended_data["deleted_user_email"] = email
user.save
end

def generate_alert_message(phone_number, users)
obfuscated_number = obfuscate_phone_number(phone_number)
emails = users.map(&:email)
emails = users.map { |user| "#{user.id} - #{user.email}" }
email_pairs = emails.each_slice(2).map { |pair| pair.join(" | ") }

message = <<~MSG
\nALERT: Duplicated Phone Number Detected!
<<~MSG
ALERT: Duplicated Phone Number Detected : #{obfuscated_number}
Phone Number: #{obfuscated_number}
Users with this number:
#{email_pairs.join("\n")}
MSG
end

log!(message, :warn)
puts(message)
def display_alerts(alerts)
return if alerts.empty?

puts "\n---- ALERTS ----"
alerts.each do |alert|
puts alert
log!(alert, :warn)
end
end

def obfuscate_phone_number(phone_number)
Expand Down
61 changes: 23 additions & 38 deletions spec/jobs/clear_duplicated_half_signup_users_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,46 +11,46 @@ module Decidim

before do
allow_any_instance_of(Decidim::DestroyAccount).to receive(:call).and_return(true)
allow(Rails.logger).to receive(:warn)
allow(Rails.logger).to receive(:info)
end

describe "#perform" do
context "when no duplicated phone numbers are found" do
it "prints that no duplicated phone numbers are found" do
allow_any_instance_of(ClearDuplicatedHalfSignupUsersJob).to receive(:find_duplicated_phone_numbers).and_return([])

expect { ClearDuplicatedHalfSignupUsersJob.perform_now }.to output(/No duplicated phone numbers found/).to_stdout
expect { described_class.perform_now }.to output(/No duplicated phone numbers found/).to_stdout
end
end

context "when duplicated phone numbers are found" do
before do
user1
user2
user3
allow_any_instance_of(ClearDuplicatedHalfSignupUsersJob).to receive(:find_duplicated_phone_numbers).and_return([%w(1234567890 US)])
end

it "soft deletes quick_auth users" do
expect_any_instance_of(ClearDuplicatedHalfSignupUsersJob).to receive(:soft_delete_user).with(user2, "Duplicated account")

ClearDuplicatedHalfSignupUsersJob.perform_now
described_class.perform_now
end

it "alerts about duplicated phone numbers for non-quick_auth users" do
expect(Rails.logger).to receive(:warn).with(/ALERT: Duplicated Phone Number Detected/)

ClearDuplicatedHalfSignupUsersJob.perform_now
described_class.perform_now
end

it "does not soft delete non quick_auth users" do
expect_any_instance_of(ClearDuplicatedHalfSignupUsersJob).not_to receive(:soft_delete_user).with(user1, "Duplicated account")

ClearDuplicatedHalfSignupUsersJob.perform_now
described_class.perform_now
end

it "does not soft delete users with different phone numbers" do
expect_any_instance_of(ClearDuplicatedHalfSignupUsersJob).not_to receive(:soft_delete_user).with(user4, "Duplicated account")

ClearDuplicatedHalfSignupUsersJob.perform_now
described_class.perform_now
end
end
end
Expand All @@ -61,13 +61,13 @@ module Decidim
it "updates the user to nullify the phone number and country" do
expect(user).to receive(:update).with(phone_number: nil, phone_country: nil)

subject.send(:soft_delete_user, user, "Duplicated account")
described_class.new.send(:soft_delete_user, user, "Duplicated account")
end

it "calls the Decidim::DestroyAccount service to delete the user" do
allow_any_instance_of(Decidim::DestroyAccount).to receive(:call).and_return(true)
expect_any_instance_of(Decidim::DestroyAccount).to receive(:call)

subject.send(:soft_delete_user, user, "Duplicated account")
described_class.new.send(:soft_delete_user, user, "Duplicated account")
end

it "does not delete a non-quick_auth account" do
Expand All @@ -76,64 +76,49 @@ module Decidim
expect_any_instance_of(Decidim::DestroyAccount).not_to receive(:call)
expect(Rails.logger).to receive(:info).with(/Not a Quick Auth account, skipping deletion/)

subject.send(:soft_delete_user, user_with_diff_email, "Duplicated account")
described_class.new.send(:soft_delete_user, user_with_diff_email, "Duplicated account")
end
end

describe "#alert_about_duplicated_numbers" do
let(:users) { [user1, user2, user3] }

it "obfuscates the phone number and logs an alert message" do
obfuscated_number = "12****90"
allow(subject).to receive(:obfuscate_phone_number).with("1234567890").and_return(obfuscated_number)
expect(Rails.logger).to receive(:warn).with(/ALERT: Duplicated Phone Number Detected/)
allow_any_instance_of(ClearDuplicatedHalfSignupUsersJob).to receive(:obfuscate_phone_number).with("1234567890").and_return("12****90")

subject.send(:alert_about_duplicated_numbers, "1234567890", users)
end

it "logs the correct message with the obfuscated phone number" do
obfuscated_number = "12****90"
allow(subject).to receive(:obfuscate_phone_number).with("1234567890").and_return(obfuscated_number)

allow(Rails.logger).to receive(:warn) { |message|
expect(message).to include("Phone Number: #{obfuscated_number}")
}

subject.send(:alert_about_duplicated_numbers, "1234567890", users)
described_class.new.send(:generate_alert_message, "1234567890", users)
end
end

describe "#obfuscate_phone_number" do
it "returns an obfuscated phone number with visible prefix and suffix" do
result = subject.send(:obfuscate_phone_number, "1234567890")
result = described_class.new.send(:obfuscate_phone_number, "1234567890")

expect(result).to eq("12******90")
end

it 'returns "No phone number" if the phone number is blank' do
result = subject.send(:obfuscate_phone_number, "")
result = described_class.new.send(:obfuscate_phone_number, "")

expect(result).to eq("No phone number")
end

it 'returns "No phone number" when given nil' do
result = subject.send(:obfuscate_phone_number, nil)
result = described_class.new.send(:obfuscate_phone_number, nil)

expect(result).to eq("No phone number")
end
end

describe "#find_duplicated_phone_numbers" do
context "when there are duplicates" do
it "finds duplicated phone numbers" do
user1
user2
user3
it "finds duplicated phone numbers" do
user1
user2
user3

result = subject.send(:find_duplicated_phone_numbers)
result = described_class.new.send(:find_duplicated_phone_numbers)

expect(result).to include(%w(1234567890 US))
end
expect(result).to include(%w(1234567890 US))
end
end
end
Expand Down

0 comments on commit 04ca0a8

Please sign in to comment.