server-tools
server-tools copied to clipboard
OCA
[16.0][IMP] base_time_window: Add helper function to get next Date matching weekday
@StephaneMangin @gurneyalex Thanks for your comments.
The idea is only to get the next date matching the weekday. I structured the tests to make sure it works with whichever weekday is used and if the target is before or after the date from during a single week. Hence why it looks a bit repetitive, but IMO it's fine as long as we cover everything and we don't rely on the date when the test is executed.
The function goal is to return the "next date" for a selected weekday as for example:
- Next Monday after today
- Next Monday after YYYY-MM-DD
The include parameters allows to return the same date passed as parameter or look for the "next one", such as:
- Today is Monday and I want the function to return Today when looking for a Monday
- Today is Monday and I want the function to return Monday next week when looking for a Monday
Now if the "from date" is a Thursday, and I want the next Monday, the function will need to add 4 days to the date from to find the next Monday, so it seems to me the propositions from @StephaneMangin won't work as it's always adding 0, 1 or 7 days. But it looks to me like @gurneyalex 's proposal would work, so I'll ditch the while loop, as I guess it must be much more performant.
@thomaspaulb what's the reason for adding the "Needs fixing" here?
@gurneyalex @StephaneMangin Can you please update your review?
@grindtildeath It was based on your comment of "I'll ditch the while loop, as I guess it must be much more performant." but seeing no commits after that, so I assumed that's a WIP.
@thomaspaulb Thanks for checking. Actually there's something weird because I did amend my commit after the reviews (I'm not sure if I did before or after my last comment) but the commit still appears before all the conversation, which I agree looks like if I didn't take care of these, what I did.
This PR looks fantastic, let's merge it! Prepared branch 16.0-ocabot-merge-pr-2819-by-gurneyalex-bump-minor, awaiting test results.
Congratulations, your PR was merged at fc9f3cd2da42a7513ac4aec5806eb94b12d97d27. Thanks a lot for contributing to OCA. ❤️