ShiftFreeLog : refactoring, utiliser un EventListener

[raphodn]

Quoi ?

S'inspirer de TimeLogEventListenener pour créer un ShiftFreeLogEventListenener

• suppression de ShiftSubscriber
• création de ShiftFreeLogEventListener
• lorsque l'event ShiftFreedEvent est dispatché, créer le ShiftFreedLog correspondant
• quelques modifs collatérales dans ShiftInvalidatedEvent
• ne plus passer la justification (reason) via l'Entity - la passer maintenant par ShiftFreedEvent

[petitalb]

Je ne suis pas sur de bien comprendre pourquoi tu veux faire ce changement ? Je ne suis pas tres au fait du pourquoi privilégier l'une ou l'autre implémentation.

Pour moi, celle que j'avais faite a le mérite de catcher toutes les modifications. Dans celle que tu proposes, il faut bien être sur de mettre la creation d'un event partout. C'est surtout ca qui avait justifier mon choix.

[raphodn]

Je ne suis pas sur de bien comprendre pourquoi tu veux faire ce changement ? Je ne suis pas tres au fait du pourquoi privilégier l'une ou l'autre implémentation.

Pour moi, celle que j'avais faite a le mérite de catcher toutes les modifications. Dans celle que tu proposes, il faut bien être sur de mettre la creation d'un event partout. C'est surtout ca qui avait justifier mon choix.

Pourquoi on devrait catcher toutes les modifications ? Et pourquoi stocker le "reason" dans le Shift ? Cette méthode avec les Event me parait beaucoup plus propre et pérenne. Et nos table s'appelent "Log", c'est normale je trouve de les appeler quand on souhaite loger les actions qui nous importent (et pas " tout le temps")

D'ailleurs, même si on voulait catcher toutes les modifs (et donc avoir un ShiftSubcriber), je pense que ca serait plus propre de dispatcher ensuite des event en fonction de ce qu'on souhaite logger. Mais là le mapping est déjà 1-1 entre l'action du controlleur (shift_free) et l'event donc pas besoin de passer par le subscriber..

[petitalb]

Pourquoi on devrait catcher toutes les modifications ? Et pourquoi stocker le "reason" dans le Shift ? Cette méthode avec les Event me parait beaucoup plus propre et pérenne.

Faudra juste penser que lors des modifications futures (ex, suppression des créneaux lors de la fermeture de compte, gel du compte, exemption, ...) a créer l'event. Ca me parait plus error prone (ca va vite d'oublier un cas, et de ne pas s'en rendre compte vu qu'on aura des logs), l'autre solution avait le mérite de mettre le code qu'a un seul endroit, la il faut duppliquer autant fois.

[raphodn]

Faudra juste penser que lors des modifications futures (ex, suppression des créneaux lors de la fermeture de compte, gel du compte, exemption, ...) a créer l'event. Ca me parait plus error prone (ca va vite d'oublier un cas, et de ne pas s'en rendre compte vu qu'on aura des logs), l'autre solution avait le mérite de mettre le code qu'a un seul endroit, la il faut duppliquer autant fois.

Je suis d'accord, ca dépend de la stratégie, je vois plutôt le ShiftFreeLog comme une table "métier", où l'on décide de stocker seulement les event qui nous intéressent. Par exemple, si on supprime un bucket, je ne pense pas qu'on devrait décompter ça comme un créneau libéré par un membre. Ca nous "forcera" à garder la logique métier en tête et de rajouter les event où c'est nécessaire (comme les TimeLog).

Je merge, et il y aura peut-être en effet d'autres endroits où l'on aura besoin d'appeler ShiftFreedEvent 👌