Skip to content
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

pfmp: fix the day count validation logic #687

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

freesteph
Copy link
Collaborator

Closes #686.

@freesteph freesteph force-pushed the fix/pfmp-day-count-validation branch from d761dcb to 4012ef6 Compare April 5, 2024 08:18
@freesteph freesteph requested a review from a team April 5, 2024 08:20
Copy link
Collaborator

@JeSuisUnCaillou JeSuisUnCaillou left a comment

Choose a reason for hiding this comment

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

Approved, avec un commentaire sur la syntaxe pas super claire utilisée pour la date de fin

Comment on lines 55 to 56
start_date: start,
end_date: start.end_of_week(:saturday)
Copy link
Collaborator

Choose a reason for hiding this comment

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

A la première lecture, j'ai eu l'impression que on allait du lundi au samedi avec cette syntaxe.

Je préfèrerait qu'on ajoute un nombre de jours explicite pour le test :

end_date: start + 4.days

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

j'entends (moi aussi ça me dérange) mais je trouve tout aussi confus que lundi + 4 jours perso 😅 mais bon ça l'est peut-être moins que le saturday

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A full week means 5 days which wasn't allowed before.
@freesteph freesteph force-pushed the fix/pfmp-day-count-validation branch from 4012ef6 to ac17c83 Compare April 5, 2024 08:42
@freesteph freesteph merged commit c6c8fa1 into main Apr 5, 2024
2 of 3 checks passed
@freesteph freesteph deleted the fix/pfmp-day-count-validation branch April 5, 2024 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

La contrainte de nombre de jours de pfmps entre les dates doit compter les deux bornes
2 participants