From dc3005a441bdbb71922ecf6ac98728a80be76f3a Mon Sep 17 00:00:00 2001
From: AyakorK <guillaume.moret@yahoo.com>
Date: Tue, 31 Dec 2024 15:22:25 +0100
Subject: [PATCH] feat: Add multiple changes to optimize reading and save email
 to extended data

---
 .../clear_duplicated_half_signup_users_job.rb | 35 ++++++++---
 ...r_duplicated_half_signup_users_job_spec.rb | 61 +++++++------------
 2 files changed, 50 insertions(+), 46 deletions(-)

diff --git a/app/jobs/clear_duplicated_half_signup_users_job.rb b/app/jobs/clear_duplicated_half_signup_users_job.rb
index ce30bebeb3..a6393620bf 100644
--- a/app/jobs/clear_duplicated_half_signup_users_job.rb
+++ b/app/jobs/clear_duplicated_half_signup_users_job.rb
@@ -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
 
@@ -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
@@ -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)
@@ -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)
@@ -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)
diff --git a/spec/jobs/clear_duplicated_half_signup_users_job_spec.rb b/spec/jobs/clear_duplicated_half_signup_users_job_spec.rb
index a3fc2533e3..a57a9841a3 100644
--- a/spec/jobs/clear_duplicated_half_signup_users_job_spec.rb
+++ b/spec/jobs/clear_duplicated_half_signup_users_job_spec.rb
@@ -11,6 +11,8 @@ 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
@@ -18,39 +20,37 @@ module Decidim
         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
@@ -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
@@ -76,7 +76,7 @@ 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
 
@@ -84,56 +84,41 @@ module Decidim
       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