-
Notifications
You must be signed in to change notification settings - Fork 42
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
TimeLog : TimeLogService, cleanup, refactoring #705
Conversation
$log->setTime($shift->getDuration()); | ||
$log->setShift($shift); | ||
$log->setType(TimeLog::TYPE_SHIFT); | ||
$log->setCreatedAt($shift->getStart()); |
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.
Pourquoi mettre le createdAt à la date du créneau ? Ca veut dire que tu peux créer des timelog dans le passé. Je ne suis pas sur que c'est ce qu'on veut. Ca va être très compliqué à gérer si il faut des créneaux sont validés dans des cycles antérieurs.
Pour moi, il faut mettre le timelog à la date où il est vraiment créé.
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 n'ai pas modifié le code existant, c'est une PR de refactoring
cf TimeLogEventListener.php#L161 & InitTimeLogCommand.php & FixTimeLogCommand.php#L51
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 corrigé
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.
ok faut qu'on en discute avant de merger, ca me semble casser la logique actuelle dans le cas d'épiceries qui n'utilisent pas use_card_reader_to_validate_shifts
:
- ShiftController > bookShiftAction > new ShiftBookedEvent()
- TimeLogEventListener > onShiftBooked
- createShiftLog
avec tes modifs ca va mettre le createdAt
à la date du booking et non à la date du shift
si je book un shift pour le cycle suivant, le TimeLog le comptabilisera au cycle actuel 🤔
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.
du coup la date devrait pouvoir être définie (et now()
si vide
edit : on en a discuté, je revert pour gérer les 2 cas
$log->setType(TimeLog::TYPE_CYCLE_END); | ||
if ($date) { | ||
$log->setCreatedAt($date); | ||
} |
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.
La encore ce n'est pas à nous de mettre la date, il faut laisser symfony mettre la date courante.
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.
idem réponse
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 corrigé
7f4d320
to
7806982
Compare
13a6184
to
b73e1e2
Compare
* Clarify shift free action * Create TimeLogService. Refactor createShiftLog & initCycleBeginningLog * Fix date & description init * Fix PHPStan errors
J'ai commencé à regarder le fonctionnement de
TimeLog
(lié à #704)TimeLogService
pour y bouger des fonctions réutilisables (pour l'instant seulementinitShiftLog
&initCycleBeginningLog
)ShiftController.freeShiftAction
gérer le cas d'unShiftInvalidatedEvent
en plus deShiftFreedEvent
Shift.php
: utilisesetWasCarriedOut()