Skip to content

Conversation

@tonial
Copy link
Contributor

@tonial tonial commented Jun 18, 2024

🤔 Pourquoi ?

https://itou.sentry.io/issues/4496383184/events/1b7ec012828045fea86fac383d8520b3/?project=6164438&query=is%3Aunresolved+issue.priority%3A%5Bhigh%2C+medium%5D&referrer=previous-event&statsPeriod=14d&stream_index=14

L'idée de la PR est d'éviter une erreur 500 en attendant qu'on refonde un peu plus les demandes de prolongation

A faire : ajouter les tests quand validé par le métier

💻 Captures d'écran

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?

🏝️ Comment tester

Les instructions pour reproduire le problème, les profils de test, le parcours spécifique à utiliser, etc. Si vous disposez d'une recette jetable, mettre l'URL pour tester dans cette partie.

@tonial tonial added the modifié Modifié dans le changelog. label Jun 18, 2024
@tonial tonial requested review from rsebille and xavfernandez June 18, 2024 08:35
@tonial tonial self-assigned this Jun 18, 2024
<form method="post" action="{% url "approvals:prolongation_request_grant" prolongation_request.pk %}">
{% csrf_token %}
<button class="btn btn-primary btn-block btn-ico justify-content-center">
<button class="btn btn-primary btn-block btn-ico justify-content-center"{% if block_validation %} disabled{% endif %}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ça me semble mieux de bloquer les deux boutons et de rediriger vers le support pour mise en cohérence :

  • Si une erreur de ce type arrive il y a 99,99% de chance que ça soit dû à une modification dans l'admin donc ça me semble préférable de corriger l'incohérence plutôt que de faire comme si de rien n'était car ça va certainement nous revenir dessus à un autre moment.
  • Refuser une demande de prolongation impose des choses aux prescripteurs, là c'est notre faute donc pas de raison que ça soit lui ainsi que l'employeur (qui devrais refaire une demande) qui soit pénalisé.
  • Si on peux éviter d'avoir de la donnée jetable dans un système d'"audit" c'est mieux.

Autres avantages que je vois de faire le message ici plutôt qu'en toast :

  • Ça nous permet d'être éventuellement moins succinct car plus de place que dans un toast
  • Moins de code car plus trop besoin du toast alors que c'est jamais sensé arriver, au pire ça casse mais si l'utilisateur accède à l'URL c'est soit qu'il l'a forgée soit qu'on a raté un (autre) truc donc on veux sans doute être alerté plutôt que faire comme si c'était une erreur attendue

Comment on lines 467 to 469
data["block_validation"] = self.prolongation_request.start_at < max(
self.prolongation_request.approval.prolongation_set.values_list("end_at", flat=True)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je réfléchis à voix haute mais je me demande si faire ceci ne suffirais pas, c'est pas tout à fait pareil mais je me dit que ça pourrais aussi choper les erreurs liées aux suspensions :

Suggested change
data["block_validation"] = self.prolongation_request.start_at < max(
self.prolongation_request.approval.prolongation_set.values_list("end_at", flat=True)
)
data["block_validation"] = self.prolongation_request.start_at != self.prolongation_request.approval.end_at

@rsebille
Copy link
Contributor

Attention l'erreur sentry en question celle-ci : LES-EMPLOIS-PROD-12C

@tonial tonial force-pushed the alaurent/prolongatio_request_db_error branch from 76faec0 to 8e0aa62 Compare June 18, 2024 11:31
@tonial
Copy link
Contributor Author

tonial commented Jun 18, 2024

Suite aux discussion sur slack, on part plutôt sur juste mettre le toast en attendant que les demandes de prolongations soient reprises (https://www.notion.so/plateforme-inclusion/2-3-Refonte-suspensions-Nouvelles-r-gles-de-suppression-des-suspensions-6e31e80959c8442c92d9a176be2d255e?pvs=4)

@tonial tonial requested review from rsebille and xavfernandez June 18, 2024 11:35
@tonial tonial force-pushed the alaurent/prolongatio_request_db_error branch from 8e0aa62 to 81f8ecf Compare June 18, 2024 12:00
@tonial tonial force-pushed the alaurent/prolongatio_request_db_error branch from 81f8ecf to d2600d9 Compare June 20, 2024 07:36
@tonial tonial enabled auto-merge June 20, 2024 07:36
@tonial tonial added this pull request to the merge queue Jun 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 20, 2024
@tonial tonial added this pull request to the merge queue Jun 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 20, 2024
@tonial tonial added this pull request to the merge queue Jun 20, 2024
@tonial tonial removed this pull request from the merge queue due to a manual request Jun 20, 2024
@tonial tonial added this pull request to the merge queue Jun 20, 2024
Merged via the queue into master with commit 5c83dca Jun 20, 2024
@tonial tonial deleted the alaurent/prolongatio_request_db_error branch June 20, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

modifié Modifié dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants