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

fix week based dates #408

Merged

Conversation

joshuacronemeyer
Copy link
Collaborator

@joshuacronemeyer joshuacronemeyer commented Aug 12, 2023

While trying to get the rest of the TODO date formats to work I found a bug with the previous week based date handling. If we were given a date like Date.strptime('2006 23 0', '%Y %U %w') our code would come up with 2006/6/11. But it is actually 2006/6/4

The problem is that we were trying to use Date.commercial for everything but some weeks a given sunday based date isn't even in the same week as the monday based weeks that Date.commercial uses. See below. 6/4 is in the 22nd commercial week but 23rd non-commercial

2006 - 23rd week of the year...

Monday start    -   Sunday start
[Mon 6/5] 23 1  -  [Sun 6/4] 23 0
[Tu  6/6] 23 2  -  [Mon 6/5] 23 1
[We  6/7] 23 3  -  [Tu  6/6] 23 2
[Th  6/8] 23 4  -  [We  6/7] 23 3
[Fr  6/9] 23 5  -  [Th  6/8] 23 4
[Sa 6/10] 23 6  -  [Fr  6/9] 23 5
[Su 6/11] 23 7  -  [Sa 6/10] 23 6

I'm proposing this alternative implementation where we delegate to the unmocked strptime but pass it a fully qualified date string that is populated from our mocked time where appropriate. This is now working for all the TODO date formats. I'm not crazy about this implementation since it feels icky to have this one scenario where we are using strptime to compute the new date. Without having something like Date.non_commercial that takes the sunday based weeks I'm not sure what else to do.

@joshuacronemeyer joshuacronemeyer merged commit bad7c38 into travisjeffery:master Aug 14, 2023
11 checks passed
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Aug 14, 2023
0.9.8 includes the following PRs to fix some issues:
travisjeffery/timecop#408
travisjeffery/timecop#409

Our miq_widget_spec was failing with 0.9.7 but passing with 0.9.6 and 0.9.8.
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.

None yet

1 participant