amelioration(UserDeletionService): les usagers ont uniquement identifié comme inactif si ils ne se sont pas connecté depuis 2ans [et on ignore les admin, expert, instructeur, usager avec un dossier en instruction ou usager sans dossier dutout]

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
+    expiring_users
-      .where.missing(:expert, :instructeur, :administrateur)
       .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

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
+    context 'when user is not expired' do
-      let(:dossier) { create(:dossier, user:, created_at: last_signed_in_not_expired) }
+      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
+    context 'when user is expired and has a dossier en_construction' do
-      let(:dossier) { create(:dossier, :brouillon, user:, created_at: last_signed_in_expired) }
+      let(:dossier) { create(:dossier, :en_construction, user:, created_at: last_signed_in_expired) }
-      it 'respects the HAVING MAX(dossier.created_at) ignores the user' do
+      it { is_expected.to include(user) }
-        create(:dossier, :brouillon, user:, created_at: last_signed_in_not_expired)
-        is_expected.not_to include(user)
-      end
     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) }