fix: Overall capacity summary is inconsistent with owner workloads #282#307
Conversation
YuliaMittova
force-pushed
the
bugfix/282-overall-capacity-summary-is-inconsistent-with-owner-workloads
branch
from
ba7a7af to
1800e65
Compare
| } | ||
|
|
||
| private CapacityPlanningResponse.OverallCapacity calculateOverallCapacity(List<Feature> features) { | ||
| final int maxCapacityPerPerson = 10; // As per simplified implementation |
There was a problem hiding this comment.
I seeee, so the capacity is just hardcoded to be 10 in the AI implementation.... Okay, we can live with this.
But re-introducing a magic constant here is a terrible code style, I extracted it into a proper constant on class level and going to force-push this PR with this change.
FAIL_TO_PASS: PlanningAnalyticsIntegrationTests
innokenty
force-pushed
the
bugfix/282-overall-capacity-summary-is-inconsistent-with-owner-workloads
branch
from
1800e65 to
cea410a
Compare
| double utilizationRate = round(totalUtilization, 1); | ||
| .mapToInt(List::size) | ||
| .sum(); | ||
| double totalUtilization = (double) totalAmountOfFeatures / (DEFAULT_CAPACITY_PER_PERSON * totalResources); |
There was a problem hiding this comment.
This bugfix introduces a small edge-case regression: when there are no owners, the calculation becomes 0 / 0, which produces NaN and then fails during rounding.
It is not the main issue, but it would be better to address it as well.
A simple way to fix this would be to add an explicit zero-owner check, for example as in the golden implementation.
There was a problem hiding this comment.
I am not sure if maybe we never end here, because there's a safeguard somewhere before, but please add a test – that's a good use case!
3 participants
PR description: Bugfix for issue #282