Jmw/week util methods #12
Conversation
1 similar comment
| end | ||
|
|
||
| def end_of_week(year, month, merch_week) | ||
| start_of_week(year, month, merch_week) + 6 |
There was a problem hiding this comment.
What's the 6 about? Can we define/name magic numbers please?
There was a problem hiding this comment.
Lol, you should see the rest of this file beyond just this small diff. One might say I was keeping consistent with the style of the code around it 😉
But, point taken. I can do better. Will address.
| def start_of_week(year, month, merch_week) | ||
| weeks = MerchCalendar.weeks_for_month(year, month) | ||
|
|
||
| week = weeks.find { |week| week.week == merch_week } |
There was a problem hiding this comment.
This is a bit smelly. Do you have any other ideas how to go about this?
There was a problem hiding this comment.
Digging through the code, MerchCalendar::MerchWeek.find(2017, 4, 1) will give the same result. So it'll clean this up here, though that code is doing something similar.
* remove magic number * use pre-existing method to locate merch week
|
After digging through the code a little bit more and discovering some new methods that I didn't know existed, I don't think this PR holds as much value as when I started out. But, I still think the convenience I've added is nice. @davidamcclain are you willing to give a 👍 ? |
I've added a few helper methods that allow me to get a start/end date for a given merch week. I need this to query AA's actualized data api for the rof.
Our UI uses merch weeks/months/quarters, and AA's API likes date ranges.