-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add billable hours indicator to timecard. #955
Conversation
This new field will store the default percentage of worked hours which are expected to be billable.
Billable expectations for a user will vary with time. We'll store a value for each submitted timecard.
Projects such as "Out of Office" will not be included in calculations of utilization or hours expected to be billed. This flag will be used to exclude hours tocked to specific projects.
Codecov Report
@@ Coverage Diff @@
## master #955 +/- ##
=======================================
Coverage 91.52% 91.52%
=======================================
Files 39 39
Lines 1699 1699
=======================================
Hits 1555 1555
Misses 144 144
Continue to review full report at Codecov.
|
One additional thought: can we add some indication of the period this is for? Eventually, we'd like to show if you're on target for the current time period, for some larger time period (maybe 3 months) and for the year. I understand that may not be doable right now, but I do think it would help to say what period the graph is representing |
Great work! I like how you broke this down into simple, digestible visualization. It did take me a little bit to figure out why the billable hours circle gets filled and becomes green when you add the 8 hours of OOO, since you there is no indication of the scale for the circles. I am curious to see if other understand this more easily. There are just a couple of picky little visual design things that could be adjusted, and perhaps we can pair on those. |
@adunkman can we pick this up again? |
@igorkorenfeld would be happy to pair! Would you prefer that to happen before this PR is merged, or would you be okay to merge this first and follow-up with a tweaks PR as timing permits? |
@adunkman totally OK with having the adjustments happen in a follow up PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amy I reviewed my outstanding comments and don't see anything that should block merging. If there are any little things you'd like to clean up and have me re-review that's cool, but I'm also OK with going ahead and merging this. Thanks for picking it up!
I have a co-working session with @adunkman today. I have a few things I am looking at right now before merging this in. Will let you know once it is ready for re-review. Thanks. |
If there is more than 40 hours tracked in a week, don’t indicate that things are green — even though the billable hours may be correct.
@tbaxter-18f We have made a few updates, can you take a quick look to make sure they address your comments? |
@@ -219,7 +231,10 @@ class Project(models.Model): | |||
) | |||
project_lead = models.ForeignKey(User, null=True, on_delete=models.CASCADE) | |||
organization = models.ForeignKey(Organization, blank=True, null=True, on_delete=models.CASCADE) | |||
|
|||
exclude_from_billability = models.BooleanField( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to clarify this a little more. I think anything non-billable should be excluded. @saraheckert does that seem right to you?
If that's the case, then I'm not sure I understand the need =for the new field and check.
@amymok can you help explain it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should probably read, check if this project should be excluded from calculations of total hours
. Because this check is really to make sure we accurately calculate the number of nonbillable hours for a reporting period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the original #940, I think this explains what "excluded" was intended to mean:
The target billable hours for the week as:
(total_hours_billed - hours_billed_to_excluded_projects) * billable_expectation)
So "excluded" hours is the category to which Out of Office lives in.
@tbaxter-18f I plan to merge this PR in at the end of the day Thursday 2/20. I do not think there's any outstanding conversations we need to resolve here. We can also open new issues after this is merged as well. |
Closes #949, includes #940. Best reviewed when ignoring whitespace.
This PR implements two circular graphs to replace to Total hours section of the timecard, one showing total hours and the other showing billable hours.
Billable hours uses the implementation in #940 to determine what tock entries are billable, non-billable, or excluded from calculation (out of office).
Expand to see how I got to that design, or see the final product below.
I originally started with the design proposed in https://github.com/18F/design-lab/issues/52, but I kept coming back to how complicated the visualization ends up, due to the different data items a single bar was showing:
I started with adaptations of this using a bar graph, and a simplification of it:
It ended up being a lot of data in a very small space though, and it seemed too complicated. I circled around the idea that although the numbers were changing, the percentages are what are important — but humans are thinking in number of hours, so the percentages themselves as numbers aren’t super valuable.
Focusing in on “what does it take to get to 100%" allowed me to simplify the graphic quite a bit — and resulted in the approach below.
(I toyed around with even simpler ideas, like making a smiley face happier, but it didn’t convey enough information to be useful, despite it being charming).
As requested in https://github.com/18F/design-lab/issues/52, this does not require a visualization library — it’s simple SVG stroke manipulation. Additionally, the Billable hours header links to the associated section of the Handbook describing billable hours expectations.