Reopen for sandbox #662
Reopen for sandbox #662
Conversation
|
(oui je sais breakman, je change ça) |
brakeman* |
Tu veux que je regarde ou tu préfères que j'attende ? Là j'ai en ligne de mire la feature donc je ne poserai pas de questions. |
En fait j'hésite encore à pousser cette PR pour la gestion du statut "mise à jour" lorsqu'on continue en prod une sandbox réouverte validée. EDIT : tu peux y aller |
JeSuisUnCaillou
force-pushed
the
reopen_for_sandbox
branch
from
ee803a8 to
e423b34
Compare
JeSuisUnCaillou
requested review from
skelz0r,
jbfeldis,
Samuelfaure and
Isalafont
There was a problem hiding this comment.
J'ai fait quelques commentaires mais en fait je pense que niveau conception c'est pas bon. Il faut à mon sens:
- Se baser sur les stages (les previous) pour savoir si on peut revenir en arrière et non les classes qui sont trop bancales imo
- Faire un
reopen_previous, parce qu'on ne réouvre pas l'habilitation actuelle mais une autre. Par ailleurs, il faut créer un event lié (car ce n'est pas la même chose à mon sens)
L'ensemble des micros méthodes qui manipulent des trucs bizarres dans les modèles montrent que c'est pas assez bien fait et trop border imo.
| @@ -5,5 +5,5 @@ class ReopenAuthorization < ApplicationOrganizer | |||
| context.authorization_request = context.authorization.request | |||
| end | |||
|
|
|||
| organize ExecuteAuthorizationRequestTransitionWithCallbacks | |||
| organize TransitionAuthorizationRequestToPreviousStage, ExecuteAuthorizationRequestTransitionWithCallbacks | |||
There was a problem hiding this comment.
| organize TransitionAuthorizationRequestToPreviousStage, ExecuteAuthorizationRequestTransitionWithCallbacks | |
| organize TransitionAuthorizationRequestToPreviousStage, | |
| ExecuteAuthorizationRequestTransitionWithCallbacks |
c'est plus lisible et consistant avec le reste
| @@ -95,6 +95,14 @@ def latest_authorization_of_class(authorization_request_class) | |||
| authorizations.where(authorization_request_class: authorization_request_class).order(created_at: :desc).limit(1).first | |||
| end | |||
|
|
|||
| def available_classes_for_reopen | |||
There was a problem hiding this comment.
C'est techniquement faux pour plein de raisons, mais la principale: on peut complètement avoir une habilitation existante qui n'est lié à aucun flow, parce que typiquement.. on a ajouté une nouvelle étape après.
| @@ -0,0 +1,20 @@ | |||
| class TransitionAuthorizationRequestToPreviousStage < ApplicationInteractor | |||
There was a problem hiding this comment.
Tu bypasses complètement la SM ici, c'est dangereux.
|
Le point 2. implique donc un nouvel organizer, et donc un nouveau controller ( |
Il me semblait qu'on avait justement discuté de ça et qu'on était tombés sur le fait que les previous sont pas pratiques pour la réouvertur et qu'il fallait plutôt se baser sur les classes des habilitations existantes. Par ce que en fait, on veut réouvrir une habilitation existante en soi. cf https://mattermost.incubateur.net/betagouv/pl/7asj617ykidm5fbbj18if34zbw |
|
Hum.. maybe. C'est peut-être le fait de tordre l'organizer qui fait que ça fait messy. Et en effet tu wrappes une partie de la logique de la classe |
Ca marche !
Il manque juste un truc, c'est d'afficher un badge de mise à jour lors de la transition "sandbox mise à jour validée" à "production".
@skelz0r t'es autorisé à regarder même si c'est pas tout à fait fini cette fois :D
Closes https://linear.app/pole-api/issue/API2-334/reouvrir-une-demande-en-production-en-sandbox-ou-en-prod