feat(dal): change the month add and sub for days add and sub #4071
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What change does this PR introduce?
Replaces the usage of
addMonthsandsubMonthsfromdate-fnsto calculate the data retention dates toaddDaysandsubDays. Also creates a single source of truth for the data retention values.Why was this change needed?
This is my candidate to our bug of the year.
Today the pipelines started to fail for 2 tests blocking any API deployment. Doing a
git bisectto find when we introduced the bug led to no conclusion, bug was still happening in commits from months ago!Weird. Let's debug the test. Oh, it is failing because wrong dates calculation. Let's debug more. It is just one day, so weird... we are just substracting one month... wait a minute...!
Lightbulb moment. Today it is the last day of the month. Always tricky to be in the boundary. And to work with dates. Nah, not a problem of timezones (luckily!). Daytime light savings? No, our error difference is one full day, not one hour. What's going on?
Dig, dig, dig... No... I can't believe it! It is
date-fnsbehaviour.Being at the last day of the month, if you add one month it automatically takes you to the last day of the following month. That's the behaviour of
addMonths. For substracting months, the behaviour ofsubMonthsis... to reuseaddMonthswith a negative number. D.R.Y..But there is a small problem. Well... small...
If for 28th February we add one month and then substract one month we get again the 28th February.
BUT... if for 31th August we add one month and then substract one month we get... 30th August.
Due the nature of the months it is very argueable when adding or substracting a month in which day you should end up depending on which month you are. I would favour consistency but this discussion on what's right is not easy.
Anyway, @scopsy kindly opened them a GitHub issue for discussion:
date-fns/date-fns#3506
Love, peace, and much support to the
date-fnsdevelopers, maintainers and contributors. Your work is appreciated.Other information (Screenshots)