ETQ Usager, je veux qu'on supprime mon brouillon après 3 mois d'inactivité (correction) #11145

mmagn · 2024-12-17T10:19:59Z

Un mécanisme de suppression de brouillon existait déjà, j'ai donc retiré le code ajouté en doublon récemment et modifié le comportement déjà en place pour prendre en compte les nouvelles exigences (suppression 3 mois à partir de la dernière MAJ)

corrige le calcul de la date d'expiration d'un dossier en brouillon qui se basait sur la date de création et non pas la date de dernière modification
modifie le comportement d'expiration des brouillons actuels en ne les conservant au maximum que 3 mois après leur dernière modification
suppression du code obsolète / doublon

codecov · 2024-12-17T10:26:07Z

Codecov Report

Attention: Patch coverage is 50.00000% with 19 lines in your changes missing coverage. Please review.

Project coverage is 55.93%. Comparing base (7462bc4) to head (a730b2b).

Files with missing lines	Patch %	Lines
app/services/expired/dossiers_deletion_service.rb	0.00%	13 Missing ⚠️
app/controllers/champs/carte_controller.rb	0.00%	2 Missing ⚠️
app/controllers/champs/champ_controller.rb	33.33%	2 Missing ⚠️
app/controllers/champs/rna_controller.rb	0.00%	1 Missing ⚠️
app/models/dossier.rb	90.00%	1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (7462bc4) and HEAD (a730b2b). Click for more details.

HEAD has 7 uploads less than BASE

Flag BASE (7462bc4) HEAD (a730b2b)

10 3

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #11145       +/-   ##
===========================================
- Coverage   84.41%   55.93%   -28.49%     
===========================================
  Files        1200     1198        -2     
  Lines       26380    29657     +3277     
  Branches     4965     4350      -615     
===========================================
- Hits        22269    16588     -5681     
- Misses       4111    13069     +8958

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

LeSim

classe et pas si compliqué finalement !

mfo

Le code me semble bon, par sécurité j'ai tenté de jouer la requete en prod :

avant:

Dossier.brouillon_close_to_expiration.count
=> 36075

apres:

class Dossier < ApplicationRecord
   scope :interval_brouillon_close_to_expiration, -> do
    max_months = 3
    state_brouillon
      .visible_by_user
     .where("dossiers.updated_at + dossiers.conservation_extension + (LEAST(procedures.duree_conservation_dossiers_dans_ds, #{max_months}) * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION })
   end
Dossier.brouillon_close_to_expiration.count
=> 1464588

Que penses-tu de lisser ça dans le temps en capant avec un max par jour ex: avec un max à 50k, ça permet de lisser ca sur 28 jours (sinon on va tuer nos quota).

J'imagine que ca peut se passer dans app/services/expired/dossiers_deletion_service.rb, p-e en ajoutant un limit(ENV['TO_BE_DEFINED']) si la mm var d'env est presenté ?

mfo · 2025-01-16T08:42:06Z

spec/system/users/brouillon_spec.rb

    login_as(user, scope: :user)
    visit brouillon_dossier_path(user_old_dossier)

    expect(page).to have_css('.fr-callout__title', text: 'Votre dossier a expiré', visible: true)
    find('#test-user-repousser-expiration').click
    expect(page).to have_no_selector('#test-user-repousser-expiration')

-    Timecop.freeze(simple_procedure.duree_conservation_dossiers_dans_ds.month.from_now) do
+    travel_to((9.months + 1.day).from_now) do


il est étrange ce 9.months + 1.day, ça aurait été un peu plus comprehensible ac : simple_procedure.duree_conservation_dossiers_dans_ds.month.from_now ?

Ouais c'est difficilement compréhensible, en fait c'est 3 (expiration initiale) + 6 (expiration prolongée) mois. je vais rendre ça plus explicite.

mfo · 2025-01-16T08:43:37Z

app/models/dossier.rb

    state_brouillon
      .visible_by_user
-      .where("dossiers.updated_at + dossiers.conservation_extension + (LEAST(procedures.duree_conservation_dossiers_dans_ds, 3) * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION })
+      .where("dossiers.updated_at + dossiers.conservation_extension + (LEAST(procedures.duree_conservation_dossiers_dans_ds, #{max_months}) * INTERVAL '1 month') - INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: INTERVAL_BEFORE_EXPIRATION })


ce max_months ne serait t'il pas le même que Expired::MONTHS_BEFORE_BROUILLON_EXPIRATION ?

mmagn · 2025-01-16T17:55:45Z

@mfo

Que penses-tu de lisser ça dans le temps en capant avec un max par jour ex: avec un max à 50k, ça permet de lisser ca sur 28 jours (sinon on va tuer nos quota).

Oui après discussion avec Bouchra on peut même descendre à 10K dans un premier temps pour s'assurer que le support ne se fait pas déborder

mfo · 2025-01-17T09:26:41Z

app/services/expired/dossiers_deletion_service.rb

-    user_notifications = group_by_user_email(dossiers_close_to_expiration)
-
-    dossiers_close_to_expiration.in_batches.update_all(brouillon_close_to_expiration_notice_sent_at: Time.zone.now)
+    user_notifications = group_by_user_email(dossiers_close_to_expiration).take(MAX_BROUILLON_DELETION_EMAILS_TO_PROCESS_PER_DAY)


J'avais un doute le .take qui implique de charger/traiter les 1.4M de records, j'ai testé sur la prod : pas de timeout, par contre mon process c'est fait Killed, j'imagine on se prend un OOM.

Un petit limit haut dessus et je pense qu'on est bon.

…eference

…aximum

…tate

…brouillon close to expiration using `limit` instead of `take` for perf reasons

…n close to expiration windows due to limit which could skip some

…llon_close_to_expiration_notice_sent_at

…brouillon_close_to_expiration_notice_sent_at reset

…m this controller

…troller

mmagn force-pushed the fix-10951-2 branch 2 times, most recently from ee58fa4 to 7546723 Compare December 17, 2024 17:11

mmagn force-pushed the fix-10951-2 branch 2 times, most recently from b5ed9f6 to c40dd53 Compare January 9, 2025 09:13

mmagn marked this pull request as ready for review January 9, 2025 09:26

LeSim approved these changes Jan 14, 2025

View reviewed changes

mfo reviewed Jan 16, 2025

View reviewed changes

mmagn force-pushed the fix-10951-2 branch from 0cdeba5 to ce29cd9 Compare January 16, 2025 17:54

mmagn force-pushed the fix-10951-2 branch from ce29cd9 to 9e8e503 Compare January 16, 2025 18:18

mfo reviewed Jan 17, 2025

View reviewed changes

mmagn and others added 11 commits January 17, 2025 14:15

[#10951] Fix expiration date for brouillon should use updated_at as r…

08fcb8b

…eference

[#10951] A brouillon is expired when not updated during 3 months at m…

f5dae2b

…aximum

[#10951] Revert useless code

75914f8

[#10951] duree_totale_conservation_in_months depends of the dossier s…

afacd06

…tate

[#10951] fix tests

14f7ac7

[#10951] Remove test no longer relevant

6c178e7

[#10951] Remove magic number

0f73cfa

[#10951] Only process 10K emails max

a5e00da

[#10951] Fixes after review

663be4e

feat(Expired::DossiersDeletionService): limit number of user dossier …

b593619

…brouillon close to expiration using `limit` instead of `take` for perf reasons

feat(Expired::DossiersDeletionService): extend user dossiers brouillo…

22876a6

…n close to expiration windows due to limit which could skip some

mmagn force-pushed the fix-10951-2 branch 2 times, most recently from 1586d81 to 9c19adf Compare January 17, 2025 14:01

mmagn enabled auto-merge January 17, 2025 14:20

mfo approved these changes Jan 17, 2025

View reviewed changes

mmagn added 3 commits January 17, 2025 17:04

[#10951] Define touch_champs_changed method on dossier to reset broui…

973324c

…llon_close_to_expiration_notice_sent_at

[#10951] Define a test on the dossier controller update action about …

fd0b6ec

…brouillon_close_to_expiration_notice_sent_at reset

[#10951] fix tests

b3544c4

mfo added 4 commits January 17, 2025 17:04

fix(Champs::CarteController): track champ changed from this controller

6a53fa1

fix(Champs::RepetitionController): track champ repetition changed fro…

4cdfaec

…m this controller

fix(Champs::RnaController): track champ rna changed from this controller

694c63e

fix(Champs::SiretController): track champ siret changed from this con…

ffe4b19

…troller

mfo force-pushed the fix-10951-2 branch from ac72918 to ffe4b19 Compare January 17, 2025 16:04

[#10951] fix tests

a730b2b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ETQ Usager, je veux qu'on supprime mon brouillon après 3 mois d'inactivité (correction) #11145

ETQ Usager, je veux qu'on supprime mon brouillon après 3 mois d'inactivité (correction) #11145

mmagn commented Dec 17, 2024 •

edited

Loading

codecov bot commented Dec 17, 2024 •

edited

Loading

LeSim left a comment

mfo left a comment •

edited

Loading

mfo Jan 16, 2025

mmagn Jan 16, 2025

mfo Jan 16, 2025

mmagn Jan 16, 2025

mmagn commented Jan 16, 2025 •

edited

Loading

mfo Jan 17, 2025

ETQ Usager, je veux qu'on supprime mon brouillon après 3 mois d'inactivité (correction) #11145

Are you sure you want to change the base?

ETQ Usager, je veux qu'on supprime mon brouillon après 3 mois d'inactivité (correction) #11145

Conversation

mmagn commented Dec 17, 2024 • edited Loading

codecov bot commented Dec 17, 2024 • edited Loading

Codecov Report

LeSim left a comment

Choose a reason for hiding this comment

mfo left a comment • edited Loading

Choose a reason for hiding this comment

mfo Jan 16, 2025

Choose a reason for hiding this comment

mmagn Jan 16, 2025

Choose a reason for hiding this comment

mfo Jan 16, 2025

Choose a reason for hiding this comment

mmagn Jan 16, 2025

Choose a reason for hiding this comment

mmagn commented Jan 16, 2025 • edited Loading

mfo Jan 17, 2025

Choose a reason for hiding this comment

mmagn commented Dec 17, 2024 •

edited

Loading

codecov bot commented Dec 17, 2024 •

edited

Loading

mfo left a comment •

edited

Loading

mmagn commented Jan 16, 2025 •

edited

Loading