From 5f452a731e55763151587d64ebc9f57243fbe85b Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 9 Nov 2023 17:46:22 +0100 Subject: [PATCH] =?UTF-8?q?amelioration(UserDeletionService):=20les=20usag?= =?UTF-8?q?ers=20ont=20uniquement=20identifi=C3=A9=20comme=20inactif=20si?= =?UTF-8?q?=20ils=20ne=20se=20sont=20pas=20connect=C3=A9=20depuis=202ans?= =?UTF-8?q?=20[et=20on=20ignore=20les=20admin,=20expert,=20instructeur,=20?= =?UTF-8?q?usager=20avec=20un=20dossier=20en=20instruction=20ou=20usager?= =?UTF-8?q?=20sans=20dossier=20dutout]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../expired/users_deletion_service.rb | 14 +++-- .../expired_users_deletion_service_spec.rb | 56 ++++++++++++------- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/app/services/expired/users_deletion_service.rb b/app/services/expired/users_deletion_service.rb index 385c510b0..ac1025b18 100644 --- a/app/services/expired/users_deletion_service.rb +++ b/app/services/expired/users_deletion_service.rb @@ -1,5 +1,8 @@ class Expired::UsersDeletionService < Expired::MailRateLimiter def process_expired + # we are working on two dataset because we apply two incompatible join on the same query + # inner join on users not having dossier.en_instruction [so we do not destroy users with dossiers.en_instruction] + # outer join on users not having dossier at all [so we destroy users without dossiers] [expiring_users_without_dossiers, expiring_users_with_dossiers].each do |expiring_segment| delete_expired_users(expiring_segment) send_inactive_close_to_expiration_notice(expiring_segment) @@ -32,21 +35,22 @@ class Expired::UsersDeletionService < Expired::MailRateLimiter users = User.arel_table dossiers = Dossier.arel_table - User.unscoped # avoid default_scope eager_loading :export, :instructeur, :administrateur - .where.missing(:expert, :instructeur, :administrateur) + expiring_users .joins( users.join(dossiers, Arel::Nodes::InnerJoin) .on(users[:id].eq(dossiers[:user_id]) .and(dossiers[:state].not_eq(Dossier.states.fetch(:en_instruction)))) .join_sources ) - .having('MAX(dossiers.created_at) < ?', Expired::INACTIVE_USER_RETATION_IN_YEAR.years.ago) - .group('users.id') end def expiring_users_without_dossiers + expiring_users.where.missing(:dossiers) + end + + def expiring_users User.unscoped - .where.missing(:expert, :instructeur, :administrateur, :dossiers) + .where.missing(:expert, :instructeur, :administrateur) .where(last_sign_in_at: ..Expired::INACTIVE_USER_RETATION_IN_YEAR.years.ago) end # rubocop:enable DS/Unscoped diff --git a/spec/services/expired/expired_users_deletion_service_spec.rb b/spec/services/expired/expired_users_deletion_service_spec.rb index 77a4f659b..e90424f95 100644 --- a/spec/services/expired/expired_users_deletion_service_spec.rb +++ b/spec/services/expired/expired_users_deletion_service_spec.rb @@ -15,11 +15,11 @@ describe Expired::UsersDeletionService do describe '#process_expired' do subject { Expired::UsersDeletionService.new.process_expired } - context 'when user has an expirable dossier' do + context 'when user is expirable and have a dossier' do let(:dossier) { create(:dossier, user:, created_at: last_signed_in_expired) } context 'when user was not notified' do - let(:user) { create(:user, inactive_close_to_expiration_notice_sent_at: before_close_to_expiration) } + let(:user) { create(:user, last_sign_in_at: last_signed_in_expired, inactive_close_to_expiration_notice_sent_at: before_close_to_expiration) } it 'update user.inactive_close_to_expiration_notice_sent_at ' do expect(UserMailer).to receive(:notify_inactive_close_to_deletion).with(user).and_return(mail_double) @@ -30,7 +30,7 @@ describe Expired::UsersDeletionService do end context 'user has been notified 1 week ago' do - let(:user) { create(:user, inactive_close_to_expiration_notice_sent_at: notified_close_to_expiration) } + let(:user) { create(:user, last_sign_in_at: last_signed_in_expired, inactive_close_to_expiration_notice_sent_at: notified_close_to_expiration) } it 'do nothing' do expect { subject }.not_to change { Dossier.count } @@ -39,7 +39,7 @@ describe Expired::UsersDeletionService do end context 'user has been notified 3 weeks ago' do - let(:user) { create(:user, inactive_close_to_expiration_notice_sent_at: due_close_to_expiration) } + let(:user) { create(:user, last_sign_in_at: last_signed_in_expired, inactive_close_to_expiration_notice_sent_at: due_close_to_expiration) } it 'destroys user and dossier' do expect { subject }.to change { Dossier.count }.by(-1) @@ -80,7 +80,7 @@ describe Expired::UsersDeletionService do end end - context 'when user is expirable' do + context 'when user is expirable but does not have a dossier' do let(:dossier) { nil } context 'when user was not notified' do @@ -128,57 +128,71 @@ describe Expired::UsersDeletionService do it { is_expected.to include(user) } end - context 'when expert last sign in at is 3 years ago' do + context 'when user is expired and has an expert' do let(:user) { create(:user, expert: create(:expert), last_sign_in_at: last_signed_in_expired) } it { is_expected.not_to include(user) } end - context 'when instructeur last sign in at is 3 years ago' do + context 'when user is expired and has an instructeur' do let(:user) { create(:user, instructeur: create(:instructeur), last_sign_in_at: last_signed_in_expired) } it { is_expected.not_to include(user) } end - context 'when admin last sign in at is 3 years ago' do + context 'when user is expired and has an admin' do let(:user) { create(:user, administrateur: create(:administrateur), last_sign_in_at: last_signed_in_expired) } it { is_expected.not_to include(user) } end + + context 'when user is expired but have a dossier' do + let(:user) { create(:user, administrateur: create(:administrateur), last_sign_in_at: last_signed_in_expired) } + let(:dossier) { create(:dossier, :brouillon, user:, created_at: last_signed_in_expired) } + it { is_expected.not_to include(user) } + end end describe '#expiring_users_with_dossiers' do - let(:user) { create(:user) } + let(:user) { create(:user, last_sign_in_at: last_signed_in_expired) } + let(:dossier) { create(:dossier, :brouillon, user:, created_at: last_signed_in_expired) } subject { Expired::UsersDeletionService.new.send(:expiring_users_with_dossiers) } - context 'when user has a dossier created 1 year ago' do - let(:dossier) { create(:dossier, user:, created_at: last_signed_in_not_expired) } + context 'when user is not expired' do + let(:user) { create(:user, last_sign_in_at: last_signed_in_not_expired) } it { is_expected.not_to include(user) } end - context 'when user has a dossier created 3 years ago' do + context 'when user is expired and has a dossier brouillon' do let(:dossier) { create(:dossier, :brouillon, user:, created_at: last_signed_in_expired) } it { is_expected.to include(user) } end - context 'when user one dossier created 3 years ago and one dossier created 1 year ago' do - let(:dossier) { create(:dossier, :brouillon, user:, created_at: last_signed_in_expired) } - it 'respects the HAVING MAX(dossier.created_at) ignores the user' do - create(:dossier, :brouillon, user:, created_at: last_signed_in_not_expired) - is_expected.not_to include(user) - end + context 'when user is expired and has a dossier en_construction' do + let(:dossier) { create(:dossier, :en_construction, user:, created_at: last_signed_in_expired) } + it { is_expected.to include(user) } end - context 'when expert last sign in at is 3 years ago' do + context 'when user is expired and has a dossier en_instruction' do + let(:dossier) { create(:dossier, :en_instruction, user:, created_at: last_signed_in_expired) } + it { is_expected.not_to include(user) } + end + + context 'when user is expired and has a dossier termine' do + let(:dossier) { create(:dossier, :accepte, user:, created_at: last_signed_in_expired) } + it { is_expected.to include(user) } + end + + context 'when user is expired and has an expert' do let(:dossier) { create(:dossier, user:, created_at: last_signed_in_expired) } let(:user) { create(:user, expert: create(:expert), last_sign_in_at: last_signed_in_expired) } it { is_expected.not_to include(user) } end - context 'when instructeur last sign in at is 3 years ago' do + context 'when user is expired and has an instructeur' do let(:dossier) { create(:dossier, user:, created_at: last_signed_in_expired) } let(:user) { create(:user, instructeur: create(:instructeur), last_sign_in_at: last_signed_in_expired) } it { is_expected.not_to include(user) } end - context 'when admin last sign in at is 3 years ago' do + context 'when user is expired and has an admin' do let(:dossier) { create(:dossier, user:, created_at: last_signed_in_expired) } let(:user) { create(:user, administrateur: create(:administrateur), last_sign_in_at: last_signed_in_expired) } it { is_expected.not_to include(user) }