-
Notifications
You must be signed in to change notification settings - Fork 35
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
Affiche le nb de congés payés restants #442
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #442 +/- ##
==========================================
+ Coverage 90.35% 90.37% +0.01%
==========================================
Files 189 191 +2
Lines 2219 2254 +35
Branches 159 165 +6
==========================================
+ Hits 2005 2037 +32
- Misses 205 217 +12
+ Partials 9 0 -9 ☔ View full report in Codecov by Sentry. |
17adb76
to
94b16b3
Compare
732f1f4
to
2e5dd09
Compare
2e5dd09
to
0647c1e
Compare
src/Application/HumanResource/Leave/Query/GetLeaveRequestsOverviewQueryHandler.spec.ts
Outdated
Show resolved
Hide resolved
src/Application/HumanResource/Leave/Query/GetLeaveRequestsOverviewQueryHandler.ts
Show resolved
Hide resolved
return duration / 420; | ||
} | ||
|
||
getLeaveReferencePeriodDays(date: Date): [Date, 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.
Je me demande si c'est vraiment utile d'avoir la date en param de fonction ? En soit la période de référence est sera toujours calculée de la même façon en fonction de la date actuelle ? Je pense qu'on peut s'en affranchir sur toute la chaîne (Query, QueryHandler, etc) et calculer avec now
? Ou alors y a un truc que j'ai pas compris xD
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 pour les tests
Dans le code de DateUtilsAdapter on ne fait jamais new Date('now')
, on prend une date de l'extérieur
Car sinon les tests ne sont pas fixés... Par ex ici j'aurais comme résultat 1er juin 2024 / 31 mai 2025, mais le 1er juin 2025 la CI sera KO car le test ne passera plus
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.
D'acc, dans ce cas on pourrait peut aussi mettre une valeur par défaut pour ce param
En vrai c'est pas hyper important, je te laisse juge :)
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.
Vu qu'on fait déjà comme ça ailleurs je vais aller dans le sens de la continuité... et laisser comme ça
Même si je serais OK pour passer sur cette approche de "par défaut on prend now mais pour tester tu peux passer une date spécifique"
Merci pour la review @fuuuzz |
TODO
Aperçu