Issue 680: Fix FY reports to only include specific years #847
Conversation
Codecov Report
@@ Coverage Diff @@
## master #847 +/- ##
==========================================
+ Coverage 91.32% 91.47% +0.15%
==========================================
Files 39 39
Lines 1752 1783 +31
==========================================
+ Hits 1600 1631 +31
Misses 152 152
Continue to review full report at Codecov.
|
|
@amymok Is there anything outstanding you need to finish this up? |
|
@tbaxter-18f yes, I am waiting for a conversation with Jackie about the FY, because it will alter how that is being calculated, let me ping her again. |
|
Just had a conversation with @HuixianXu, here's the definition: During the week that spans two fiscal years, with the five work days, charging will happen to the year that has more days in it. So, if there are three or more September work days, the week will belong to the year September is in, else, if there are more October work days (three or more), this week will belong to the year October is in. So I will use this to continue with the work. |
|
Fantastic. Thanks for following up, Amy. Will you be sure to note that in the code so we have it documented for posterity? Thanks! |
amymok
changed the title
|
Amy, I believe the last acceptance criteria was referring to https://github.com/18F/tock/blob/master/tock/tock/templates/hours/reporting_period_detail.html, which currently shows users who have and haven't tocked, with their name and email. The desire was to show their org, too, if I recall correctly. @abrouilette does that sound right to you? |
| first_date = datetime.date(fiscal_year - 1, 10, 1) | ||
| # converting definition of weekday for easier calculation, | ||
| # so Sun = 0, Mon = 1, ... Sat = 6 | ||
| normalized_weekday = (first_date.weekday() + 1) % 7 |
There was a problem hiding this comment.
There's no need to change anything, and I'm not even sure I'm right, but I think there may be bits in the calendar module that may make some of this easier: https://docs.python.org/2/library/calendar.html
Only mentioning it in case it's helpful in the future :-)
There was a problem hiding this comment.
I didn't know about the calendar, thanks! I think still some work using the calendar method. I will leave this as-is.
tock/hours/views.py
Outdated
| year = ReportingPeriod.get_fiscal_year_from_date(today) | ||
| context['fiscal_years'] = [] | ||
| for year in range(2017, year + 1): | ||
| year_dates = dict() |
There was a problem hiding this comment.
I probably would have written all of this to the dict in one shot in order to (hopefully) be a bit more readable...
context['fiscal_years'].append({
'year': year,
'start_date': blahblahblah,
'end_date': BobLoblaw
})
But it's fine the way you have it here too
There was a problem hiding this comment.
This is actually a really confusing piece of code now that you pointed out. I also was using year as each instance in the for loop as well as a variable for the end of range. I am rewriting it now.
|
I think this looks really great, Amy. Thanks for really thinking it through. The only thing I want to be sure about before merging it is that organization in the table. Thanks! |
|
Confirmed in Slack, it's that report html page. Thanks! |
Thanks I will take a look later today. |
|
@tbaxter-18f This is ready for review again. Thanks! |
| 'year': fy, | ||
| 'start_date': ReportingPeriod.get_fiscal_year_start_date(fy), | ||
| 'end_date': ReportingPeriod.get_fiscal_year_end_date(fy) | ||
| } for fy in range(2017, year + 1) |
There was a problem hiding this comment.
This is interesting how you're building out the list of dicts in a list comprehension. I wouldn't have thought of that, but it's very readable this way!
Description
See Issue #680
beforeparameter forget_timecardsfunction to allow getting a list of timecards before and include a specific date. This can now be used withafterparameter to get a specific date range of timecardsbeforeparameter as well as using it in conjunction with `after parameterbeforeparameters to the current reports so it is filtered with a rangeI also added a few static methods to figure out the actual fiscal year start and end date based on the discussion I have with Jackie.
Additional information
Include any of the following (as necessary):
There seems to be an inconsistency with the date in the report for FY2016. The superuser report starts with 10/1/2016, but other reports starts with 10/2/2016. 10/1/2016 is a Saturday so I am not sure if this is an exception or not for fiscal years. I changed it back to FY starts on 10/1 and ends with 9/30 unless there are exceptions, please let me know.