From 187166e99be2628b2f4f9a6c01292aa666710d77 Mon Sep 17 00:00:00 2001
From: Guillaume MORET <90462045+AyakorK@users.noreply.github.com>
Date: Wed, 15 Jan 2025 11:29:08 +0100
Subject: [PATCH] feat: Creation of a rake task to delete duplicated half
 signup users (#651)

* feat: Creation of a rake task to delete duplicated half signup users

* feat: Add multiple changes to optimize reading and save email to extended data

* fix: Deduplicate accounts (#653)

---------

Co-authored-by: Quentin Champenois <26109239+Quentinchampenois@users.noreply.github.com>
---
 .../clear_duplicated_half_signup_users_job.rb | 127 +++++++++++++++
 app/jobs/concerns/decidim/logging.rb          |   9 +-
 app/jobs/decidim/papertrail_versions_job.rb   |   2 +-
 lib/tasks/clear_duplicated_users.rake         |  10 ++
 lib/tasks/db.rake                             |  30 ++++
 ...r_duplicated_half_signup_users_job_spec.rb | 147 ++++++++++++++++++
 6 files changed, 323 insertions(+), 2 deletions(-)
 create mode 100644 app/jobs/clear_duplicated_half_signup_users_job.rb
 create mode 100644 lib/tasks/clear_duplicated_users.rake
 create mode 100644 spec/jobs/clear_duplicated_half_signup_users_job_spec.rb

diff --git a/app/jobs/clear_duplicated_half_signup_users_job.rb b/app/jobs/clear_duplicated_half_signup_users_job.rb
new file mode 100644
index 0000000000..d954156972
--- /dev/null
+++ b/app/jobs/clear_duplicated_half_signup_users_job.rb
@@ -0,0 +1,127 @@
+# frozen_string_literal: true
+
+class ClearDuplicatedHalfSignupUsersJob < ApplicationJob
+  include Decidim::Logging
+
+  def perform
+    @dup_decidim_users_count = 0
+    @dup_half_signup_count = 0
+
+    log! "Start clearing half signup accounts..."
+    if duplicated_phone_numbers.blank?
+      log! "No duplicated phone numbers found"
+      return
+    end
+
+    log! "Found #{duplicated_phone_numbers.count} duplicated phone number to cleanup"
+    duplicated_phone_numbers.each do |phone_info|
+      phone_number, phone_country = phone_info
+      users = Decidim::User.where(phone_number: phone_number, phone_country: phone_country)
+
+      clear_data users
+    end
+
+    log! "Total distinct numbers to clear : #{duplicated_phone_numbers.size}"
+    log! "Half signup users archived : #{@dup_half_signup_count}"
+    log! "Decidim users account updated : #{@dup_decidim_users_count}"
+    log! "Total accounts modified : #{@dup_half_signup_count + @dup_decidim_users_count}"
+    log! "Terminated !"
+  end
+
+  private
+
+  def duplicated_phone_numbers
+    @duplicated_phone_numbers ||= Decidim::User
+                                  .where.not(phone_number: [nil, ""])
+                                  .where.not(phone_country: [nil, ""])
+                                  .group(:phone_number, :phone_country)
+                                  .having("count(*) > 1")
+                                  .pluck(:phone_number, :phone_country)
+  end
+
+  def clear_data(users)
+    decidim_user_dup_accounts = []
+
+    users.each do |user|
+      if user.email.include?("quick_auth")
+        @dup_half_signup_count += 1
+        soft_delete_user(user, delete_reason)
+      else
+        @dup_decidim_users_count += 1
+        decidim_user_dup_accounts << user
+      end
+    end
+
+    return if decidim_user_dup_accounts.blank?
+    # The unique user might be a user without email, if so, it should be cleared
+    return if decidim_user_dup_accounts.size <= 1 && decidim_user_dup_accounts.first.email.present?
+
+    # if there is multiple decidim user accounts, clear all phone number for these accounts
+    decidim_user_dup_accounts.each do |decidim_user|
+      clear_account_phone_number(decidim_user)
+    end
+  end
+
+  def soft_delete_user(user, reason)
+    return unless user.email&.include?("quick_auth")
+
+    email = user.email
+    phone = user.phone_number
+    user.extended_data = user.extended_data.merge({
+                                                    half_signup: {
+                                                      email: email,
+                                                      phone_number: phone,
+                                                      phone_country: user.phone_country
+                                                    }
+                                                  })
+
+    user.phone_number = nil
+    user.phone_country = nil
+
+    form = Decidim::DeleteAccountForm.from_params(delete_reason: reason)
+    Decidim::DestroyAccount.call(user, form) do
+      on(:ok) do
+        log!("User (ID/#{user.id} email/#{email} phone/#{obfuscate_phone_number(phone)}) has been deleted")
+      end
+      on(:invalid) do
+        log!("User (ID/#{user.id} email/#{email} phone/#{obfuscate_phone_number(phone)}) cannot be deleted: #{form.errors.full_messages}")
+      end
+    end
+  end
+
+  def clear_account_phone_number(user)
+    phone_number = user.phone_number
+    Decidim::User.transaction do
+      user.extended_data = user.extended_data.merge({
+                                                      half_signup: {
+                                                        phone_number: user.phone_number,
+                                                        phone_country: user.phone_country
+                                                      }
+                                                    })
+
+      user.phone_number = nil
+      user.phone_country = nil
+      user.save(validate: false)
+    end
+
+    log! "User (ID/#{user.id} phone/#{obfuscate_phone_number(phone_number)} email/#{user.email}) has been cleaned"
+  end
+
+  def obfuscate_phone_number(phone_number)
+    return "No phone number" if phone_number.blank?
+
+    visible_prefix = phone_number[0..1]
+    visible_suffix = phone_number[-2..]
+    obfuscated_middle = "*" * (phone_number.length - 4)
+
+    visible_prefix + obfuscated_middle + visible_suffix
+  end
+
+  def current_date
+    Date.current.strftime "%Y-%m-%d"
+  end
+
+  def delete_reason
+    "HalfSignup duplicated account (#{current_date})"
+  end
+end
diff --git a/app/jobs/concerns/decidim/logging.rb b/app/jobs/concerns/decidim/logging.rb
index b4563270df..77582b2ae1 100644
--- a/app/jobs/concerns/decidim/logging.rb
+++ b/app/jobs/concerns/decidim/logging.rb
@@ -5,13 +5,20 @@ module Logging
     private
 
     def log!(msg, level = :warn)
-      msg = "(#{self.class}) #{Time.current.strftime("%d-%m-%Y %H:%M")}> #{msg}"
+      msg = "(#{self.class})> #{msg}"
+
       case level
       when :info
         Rails.logger.info msg
+        stdout_logger.info msg unless Rails.env.test?
       else
         Rails.logger.warn msg
+        stdout_logger.warn msg unless Rails.env.test?
       end
     end
+
+    def stdout_logger
+      @stdout_logger ||= Logger.new($stdout)
+    end
   end
 end
diff --git a/app/jobs/decidim/papertrail_versions_job.rb b/app/jobs/decidim/papertrail_versions_job.rb
index d56427ab02..0e0748acdf 100644
--- a/app/jobs/decidim/papertrail_versions_job.rb
+++ b/app/jobs/decidim/papertrail_versions_job.rb
@@ -18,7 +18,7 @@ def perform(ret = nil)
         versions.destroy_all
       end
 
-      log! "#{total} versions have been removed"
+      log! "#{total} versions removed"
     end
 
     private
diff --git a/lib/tasks/clear_duplicated_users.rake b/lib/tasks/clear_duplicated_users.rake
new file mode 100644
index 0000000000..ba042895b7
--- /dev/null
+++ b/lib/tasks/clear_duplicated_users.rake
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+namespace :decidim do
+  desc "Clear duplicated users with the same phone_numbers in the database"
+  task clear_duplicated_users: :environment do
+    include Decidim::Logging
+
+    ClearDuplicatedHalfSignupUsersJob.perform_now
+  end
+end
diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake
index 82a8c09d8b..29aa16b9e3 100644
--- a/lib/tasks/db.rake
+++ b/lib/tasks/db.rake
@@ -49,5 +49,35 @@ namespace :decidim do
         puts "(decidim:db:versions:clean) #{Time.current.strftime("%d-%m-%Y %H:%M:%S")}> Job delayed to Sidekiq."
       end
     end
+
+    namespace :restore do
+      desc "Clear database dump to work with localhost"
+      task local: :environment do
+        puts "(decidim:db:restore:local) #{Time.current.strftime("%d-%m-%Y %H:%M:%S")}> Modifying Organization settings..."
+        organizations = Decidim::Organization.all.pluck(:id, :name, :host)
+
+        if organizations.blank?
+          puts "(decidim:db:restore:local) #{Time.current.strftime("%d-%m-%Y %H:%M:%S")}> No existing organizations..."
+          puts "(decidim:db:restore:local) #{Time.current.strftime("%d-%m-%Y %H:%M:%S")}> Terminating"
+          return
+        elsif organizations.size == 1
+          organization = Decidim::Organization.first
+        else
+          organizations.each do |org|
+            puts "#{org.id}) #{org.name} - #{org.host}"
+          end
+          puts "Select the organization ID: "
+          org_id = $stdin.gets
+          organization = Decidim::Organization.find(org_id)
+        end
+
+        organization.host = "localhost"
+        organization.smtp_settings = {}
+        organization.omniauth_settings = {}
+        organization.save(validate: false)
+
+        puts "(decidim:db:restore:local) #{Time.current.strftime("%d-%m-%Y %H:%M:%S")}> Changes done..."
+      end
+    end
   end
 end
diff --git a/spec/jobs/clear_duplicated_half_signup_users_job_spec.rb b/spec/jobs/clear_duplicated_half_signup_users_job_spec.rb
new file mode 100644
index 0000000000..886f274715
--- /dev/null
+++ b/spec/jobs/clear_duplicated_half_signup_users_job_spec.rb
@@ -0,0 +1,147 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+module Decidim
+  describe ClearDuplicatedHalfSignupUsersJob do
+    subject { described_class.perform_now }
+
+    let!(:dup_user1) { create(:user, phone_number: "1234", phone_country: "US", email: "user1@example.com") }
+    let!(:dup_user2) { create(:user, phone_number: "1234", phone_country: "US", email: "quick_auth_user@example.com") }
+    let!(:dup_user3) { create(:user, phone_number: "1234", phone_country: "US", email: "user3@example.com") }
+    let(:dup_user4) do
+      dup_user4 = build(:user, phone_number: "1234", phone_country: "US", email: "")
+      dup_user4.save(validate: false)
+      dup_user4
+    end
+    let!(:user5) { create(:user, phone_number: "6789", phone_country: "US", email: "user5@example.com") }
+    let(:delete_reason) { "HalfSignup duplicated account (#{current_date})" }
+    let(:current_date) { Date.current.strftime "%Y-%m-%d" }
+
+    before do
+      allow(Decidim::Logging).to receive(:stdout_logger).and_return(Logger.new(StringIO.new))
+    end
+
+    describe "#perform" do
+      it "soft deletes quick_auth users" do
+        expect(dup_user2.delete_reason).to be_nil
+        subject
+        dup_user1.reload
+        dup_user2.reload
+        dup_user3.reload
+        expect(dup_user1.delete_reason).to be_nil
+        expect(dup_user3.delete_reason).to be_nil
+        expect(dup_user2.delete_reason).to eq("HalfSignup duplicated account (#{current_date})")
+        expect(dup_user2.extended_data).to include("half_signup" => { "email" => "quick_auth_user@example.com", "phone_number" => "1234", "phone_country" => "US" })
+      end
+
+      it "does not soft delete non quick_auth users" do
+        expect_any_instance_of(ClearDuplicatedHalfSignupUsersJob).not_to receive(:soft_delete_user).with(dup_user1, delete_reason)
+
+        subject
+      end
+    end
+
+    describe "#clear_data" do
+      let(:object) do
+        obj = described_class.new
+        obj.instance_variable_set(:@dup_half_signup_count, 0)
+        obj.instance_variable_set(:@dup_decidim_users_count, 0)
+        obj
+      end
+
+      it "clears the phone number and country of the users" do
+        expect(dup_user1.phone_number).to eq("1234")
+        expect(dup_user1.phone_country).to eq("US")
+        object.send(:clear_data, [dup_user1, dup_user3])
+        dup_user1.reload
+
+        expect(dup_user1.phone_number).to be_nil
+        expect(dup_user1.phone_country).to be_nil
+        expect(dup_user1.extended_data).to include("half_signup" => {
+                                                     "phone_number" => "1234",
+                                                     "phone_country" => "US"
+                                                   })
+        expect(object.instance_variable_get(:@dup_half_signup_count)).to eq(0)
+        expect(object.instance_variable_get(:@dup_decidim_users_count)).to eq(2)
+      end
+
+      context "when user is not half signup and email is empty" do
+        it "does clear the phone number and country of the user" do
+          expect(dup_user4.phone_number).to eq("1234")
+          expect(dup_user4.phone_country).to eq("US")
+          object.send(:clear_data, [dup_user4])
+          dup_user4.reload
+
+          expect(dup_user4.phone_number).to be_nil
+          expect(dup_user4.phone_country).to be_nil
+          expect(dup_user4.extended_data).to include("half_signup" => {
+                                                       "phone_number" => "1234",
+                                                       "phone_country" => "US"
+                                                     })
+          expect(object.instance_variable_get(:@dup_half_signup_count)).to eq(0)
+          expect(object.instance_variable_get(:@dup_decidim_users_count)).to eq(1)
+        end
+      end
+    end
+
+    describe "#soft_delete_user" do
+      it "updates the user to nullify the phone number and country" do
+        expect(dup_user2.phone_number).to eq("1234")
+        expect(dup_user2.phone_country).to eq("US")
+        described_class.new.send(:soft_delete_user, dup_user2, delete_reason)
+        dup_user2.reload
+
+        expect(dup_user2.phone_number).to be_nil
+        expect(dup_user2.phone_country).to be_nil
+        expect(dup_user2.extended_data).to include("half_signup" => {
+                                                     "email" => "quick_auth_user@example.com",
+                                                     "phone_number" => "1234",
+                                                     "phone_country" => "US"
+                                                   })
+      end
+
+      it "calls the Decidim::DestroyAccount service to delete the half signup user" do
+        expect_any_instance_of(Decidim::DestroyAccount).to receive(:call)
+
+        described_class.new.send(:soft_delete_user, dup_user2, delete_reason)
+      end
+
+      it "does not delete a non-quick_auth account" do
+        user_with_diff_email = create(:user, phone_number: "1234", phone_country: "US", email: "user_diff@example.com")
+
+        expect_any_instance_of(Decidim::DestroyAccount).not_to receive(:call)
+
+        described_class.new.send(:soft_delete_user, user_with_diff_email, delete_reason)
+      end
+    end
+
+    describe "#obfuscate_phone_number" do
+      it "returns an obfuscated phone number with visible prefix and suffix" do
+        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 = 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 = described_class.new.send(:obfuscate_phone_number, nil)
+
+        expect(result).to eq("No phone number")
+      end
+    end
+
+    describe "#duplicated_phone_numbers" do
+      it "finds duplicated phone numbers" do
+        result = described_class.new.send(:duplicated_phone_numbers)
+
+        expect(result).to include(%w(1234 US))
+      end
+    end
+  end
+end