diff --git a/app/jobs/clear_duplicated_half_signup_users_job.rb b/app/jobs/clear_duplicated_half_signup_users_job.rb index a6393620bf..d954156972 100644 --- a/app/jobs/clear_duplicated_half_signup_users_job.rb +++ b/app/jobs/clear_duplicated_half_signup_users_job.rb @@ -1,93 +1,110 @@ # frozen_string_literal: true -# rubocop:disable Rails/Output class ClearDuplicatedHalfSignupUsersJob < ApplicationJob include Decidim::Logging def perform - duplicated_numbers = find_duplicated_phone_numbers - return puts("No duplicated phone numbers found") if duplicated_numbers.empty? + @dup_decidim_users_count = 0 + @dup_half_signup_count = 0 - alerts = [] + log! "Start clearing half signup accounts..." + if duplicated_phone_numbers.blank? + log! "No duplicated phone numbers found" + return + end - duplicated_numbers.each do |phone_info| + 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) - users_with_phone = Decidim::User.where(phone_number: phone_number, phone_country: phone_country) - quick_auth_users = users_with_phone.select { |user| user.email.include?("quick_auth") } - other_users = users_with_phone.reject { |user| user.email.include?("quick_auth") } - - quick_auth_users.each { |user| soft_delete_user(user, "Duplicated account") } - - alerts << generate_alert_message(phone_number, other_users) if other_users.map(&:email).uniq.size > 1 + clear_data users end - display_alerts(alerts) + + 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 find_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) + 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 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) - previous_email = user.email - Decidim::DestroyAccount.call(user, form) do - 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) - puts("Failed to delete user #{user.id} (#{user.email}): #{form.errors.full_messages}") - 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 - else - log!("Not a Quick Auth account, skipping deletion", :info) - puts("Not a Quick Auth account, skipping deletion") end - end - - 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 { |user| "#{user.id} - #{user.email}" } - email_pairs = emails.each_slice(2).map { |pair| pair.join(" | ") } - - <<~MSG - - ALERT: Duplicated Phone Number Detected : #{obfuscated_number} - Users with this number: + 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? - #{email_pairs.join("\n")} - MSG + # 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 display_alerts(alerts) - return if alerts.empty? + 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 - puts "\n---- ALERTS ----" - alerts.each do |alert| - puts alert - log!(alert, :warn) + 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) @@ -99,5 +116,12 @@ def obfuscate_phone_number(phone_number) 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 -# rubocop:enable Rails/Output 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 index dcccdfe3e8..ba042895b7 100644 --- a/lib/tasks/clear_duplicated_users.rake +++ b/lib/tasks/clear_duplicated_users.rake @@ -3,6 +3,8 @@ 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 index a57a9841a3..886f274715 100644 --- a/spec/jobs/clear_duplicated_half_signup_users_job_spec.rb +++ b/spec/jobs/clear_duplicated_half_signup_users_job_spec.rb @@ -4,89 +4,115 @@ module Decidim describe ClearDuplicatedHalfSignupUsersJob do - let!(:user1) { create(:user, phone_number: "1234567890", phone_country: "US", email: "user1@example.com") } - let!(:user2) { create(:user, phone_number: "1234567890", phone_country: "US", email: "quick_auth_user@example.com") } - let!(:user3) { create(:user, phone_number: "1234567890", phone_country: "US", email: "user3@example.com") } - let!(:user4) { create(:user, phone_number: "9876543210", phone_country: "US", email: "user4@example.com") } + 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_any_instance_of(Decidim::DestroyAccount).to receive(:call).and_return(true) - allow(Rails.logger).to receive(:warn) - allow(Rails.logger).to receive(:info) + allow(Decidim::Logging).to receive(:stdout_logger).and_return(Logger.new(StringIO.new)) 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 { described_class.perform_now }.to output(/No duplicated phone numbers found/).to_stdout - end + 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 - context "when duplicated phone numbers are found" do - before do - 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") - - 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/) - - 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(dup_user1, delete_reason) - 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") + subject + end + end - described_class.perform_now - 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 "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") + 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 - described_class.perform_now + 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 - let(:user) { create(:user, phone_number: "1234567890", phone_country: "US", email: "quick_auth_user@example.com") } - it "updates the user to nullify the phone number and country" do - expect(user).to receive(:update).with(phone_number: nil, phone_country: nil) - - described_class.new.send(:soft_delete_user, user, "Duplicated account") + 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 user" do + 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, user, "Duplicated account") + 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: "1234567890", phone_country: "US", email: "user_diff@example.com") + 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) - expect(Rails.logger).to receive(:info).with(/Not a Quick Auth account, skipping deletion/) - 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 - allow_any_instance_of(ClearDuplicatedHalfSignupUsersJob).to receive(:obfuscate_phone_number).with("1234567890").and_return("12****90") - - described_class.new.send(:generate_alert_message, "1234567890", users) + described_class.new.send(:soft_delete_user, user_with_diff_email, delete_reason) end end @@ -110,15 +136,11 @@ module Decidim end end - describe "#find_duplicated_phone_numbers" do + describe "#duplicated_phone_numbers" do it "finds duplicated phone numbers" do - user1 - user2 - user3 - - result = described_class.new.send(:find_duplicated_phone_numbers) + result = described_class.new.send(:duplicated_phone_numbers) - expect(result).to include(%w(1234567890 US)) + expect(result).to include(%w(1234 US)) end end end