-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[usager] Laisser un délai avant suppression des dossiers expirés #10488
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10488 +/- ##
==========================================
+ Coverage 80.23% 80.26% +0.02%
==========================================
Files 1234 1236 +2
Lines 26274 26322 +48
Branches 4718 4728 +10
==========================================
+ Hits 21082 21128 +46
- Misses 5192 5194 +2 ☔ View full report in Codecov by Sentry. |
d1742c4
to
a163d1c
Compare
@@ -251,6 +251,11 @@ def submit_brouillon | |||
|
|||
def extend_conservation | |||
dossier.extend_conservation(dossier.procedure.duree_conservation_dossiers_dans_ds.months) | |||
|
|||
if dossier.hidden_at.present? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hidden_at
est une colonne legacy et devrai être supprimé
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je crois que je vois pourquoi tu essaies de la réutiliser pour la suppression automatique, mais j'ai un peu peur qu'on ajoute encore un niveau de complexité avec maintenant trois flags... Je me demande si on ne peut pas s'en sortir avec deux. Et si non, si on ne devrait pas introduire une nouvelle machine à état sur le dossier.
c420227
to
10bef2b
Compare
10bef2b
to
983a198
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai l'impression qu'on ne peut pas se passer de l'ajout d'une colonne hidden_by_expired_at
@@ -129,32 +129,32 @@ def notify_en_construction_deletion_to_administration(dossier, to_email) | |||
mail(to: to_email, subject: @subject) | |||
end | |||
|
|||
def notify_deletion_to_administration(deleted_dossier, to_email) | |||
def notify_deletion_to_administration(hidden_dossier, to_email) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merci d'avoir renommé !
app/models/dossier.rb
Outdated
|
||
def has_expired? | ||
return false if en_instruction? | ||
expiration_notification_date < Time.zone.now && expiration_notification_date < Expired::REMAINING_WEEKS_BEFORE_EXPIRATION.weeks.ago |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai l'impression qu'on peut simplifier par
expiration_notification_date < Time.zone.now && expiration_notification_date < Expired::REMAINING_WEEKS_BEFORE_EXPIRATION.weeks.ago | |
expiration_notification_date < Expired::REMAINING_WEEKS_BEFORE_EXPIRATION.weeks.ago |
@@ -394,10 +394,13 @@ fr: | |||
expirant: Les dossiers n’expireront pas avant la période de conservation des données. | |||
archived_dossier: "Le dossier sera conservé 1 mois supplémentaire" | |||
delete_dossier: "Supprimer le dossier" | |||
deleted_by_user: "Dossier supprimé par l’usager" | |||
deleted_reason: | |||
user_request: supprimé par l'usager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
top
footer_en_construction: | ||
one: "Si vous souhaitez conserver votre dossier plus longtemps, vous pouvez <b>prolonger sa durée de conservation</b> dans l’interface." | ||
other: "Si vous souhaitez conserver vos dossiers plus longtemps, vous pouvez <b>prolonger leur durée de conservation</b> au cas par cas dans l’interface." | ||
one: Depuis la page de votre dossier vous avez la possibilité de :<br>- prolonger la durée de conservation pour un délai de 6 mois<br>- contacter l'administration qui gère votre dossier via la messagerie |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C'est pas forcément 6 mois si ? ca serait pas plutot un truc du genre :
one: Depuis la page de votre dossier vous avez la possibilité de :<br>- prolonger la durée de conservation pour un délai de 6 mois<br>- contacter l'administration qui gère votre dossier via la messagerie | |
one: Depuis la page de votre dossier vous avez la possibilité de :<br>- prolonger la durée de conservation pour un délai de %{procedure.duree_de_conservation_dans_ds} mois<br>- contacter l'administration qui gère votre dossier via la messagerie |
app/models/dossier.rb
Outdated
@@ -640,7 +644,12 @@ def expiration_notification_date | |||
|
|||
def close_to_expiration? | |||
return false if en_instruction? | |||
expiration_notification_date < Time.zone.now | |||
expiration_notification_date < Time.zone.now && expiration_notification_date > Expired::REMAINING_WEEKS_BEFORE_EXPIRATION.weeks.ago |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proposition pour conserver la comparaison <
plus facile a lire avec le plus petit a gauche
expiration_notification_date < Time.zone.now && expiration_notification_date > Expired::REMAINING_WEEKS_BEFORE_EXPIRATION.weeks.ago | |
expiration_notification_date < Time.zone.now && Expired::REMAINING_WEEKS_BEFORE_EXPIRATION.weeks.ago < expiration_notification_date |
app/models/dossier.rb
Outdated
@@ -835,17 +836,24 @@ def author_is_administration(author) | |||
author.is_a?(Instructeur) || author.is_a?(Administrateur) || author.is_a?(SuperAdmin) | |||
end | |||
|
|||
def author_is_automatic(author) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def author_is_automatic(author) | |
def automatic_author?(author) |
app/models/dossier.rb
Outdated
elsif author_is_user(author) && can_be_deleted_by_user? | ||
update(hidden_by_user_at: Time.zone.now, dossier_transfer_id: nil, hidden_by_reason: reason) | ||
log_dossier_operation(author, :supprimer, self) | ||
elsif author_is_automatic(author) && can_be_deleted_by_automatic?(reason) | ||
update(hidden_by_administration_at: Time.zone.now, hidden_by_user_at: Time.zone.now, hidden_by_reason: reason) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai l'impression qu'il faudrait rajouter une colonne hidden_by_expired_at
car la le code dit que le dossier est caché à la fois par l'administration et par l'utilisateur ce qui n'est pas vrai.
app/models/dossier.rb
Outdated
@@ -670,6 +679,10 @@ def extend_conservation(conservation_extension) | |||
brouillon_close_to_expiration_notice_sent_at: nil, | |||
en_construction_close_to_expiration_notice_sent_at: nil, | |||
termine_close_to_expiration_notice_sent_at: nil) | |||
|
|||
if hidden_by_reason == 'expired' | |||
update(hidden_by_administration_at: nil, hidden_by_user_at: nil, hidden_by_reason: nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Voir plus bas sur le hidden_by_expired_at
update(hidden_by_administration_at: nil, hidden_by_user_at: nil, hidden_by_reason: nil) | |
update(hidden_by_expired_at: nil, hidden_by_reason: nil) |
un cas ou c'est nécessaire :
- un utilisateur supprime sont dossier
hidden_by_user_at: today
- plus tard, le dossier est soft deleted car proche de l'expiration
hidden_by_user_at = hidden_by_admin_at = later
- l'administration veut finalement le conserver plus longtemps
hidden_by_user_at = hidden_by_admin_at = nil
=> bug: le dossier réapparaît coté utilisateur
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
et est-ce que pour éviter ça, mais sans créer une nouvelle colonne ce qui ne semblait pas plaire @tchak
On pourrait faire qqchose comme
elsif author_is_automatic(author) && can_be_deleted_by_automatic?(reason)
update(hidden_by_administration_at: Time.zone.now, hidden_by_user_at: hidden_by_user_at.presence || Time.zone.now, hidden_by_reason: reason)
et ici
if hidden_by_reason == 'expired'
if hidden_by_user_at != hidden_by_administration_at
update(hidden_by_administration_at: nil, hidden_by_reason: nil)
else
update(hidden_by_administration_at: nil, hidden_by_user_at: nil, hidden_by_reason: nil)
end
end
Ou c'est trop tordu ? 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alors ca me semble compliqué et vu que la logique est déjà dur à comprendre ca me fait un poil peur d'en rajouter une couche. @tchak un avis ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes je suis d'accord ! Après dans les discussions qu'on avait eu avec Paul, il y a un petit moment maintenant, il me semble qu'on avait relevé "le bug" dont tu parles et on s'etait posé la question à quel cas d'usage il repondait dans la vie réelle. Et si c'etait pas un edge case, "qui au pire" faisait réapparaître un dossier masqué… Mais je laisse répondre Paul pour pas avoir l'air de ne vouloir plus retoucher à cette PR 😅
1ea883e
to
5d67aea
Compare
65afbab
to
7da3e05
Compare
7750191
to
1d50953
Compare
1d50953
to
4f7b0c9
Compare
closes #10219
On ne supprime plus directement les dossiers après expiration mais on les met à la "corbeille" (supprimés récemments) - ils sont ainsi gardés une semaine de plus et cela permet