Fix a case when multiple location changes doesn't result in the right due dates #803
Conversation
…t a pause in between
| @@ -528,29 +519,38 @@ ImmutableList<DateOnly> completionDates | |||
|
|
|||
| private static IEnumerable<DateRange<Guid>> GenerateDateRanges(ImmutableList<ChildLocation> childLocations) | |||
| { | |||
| DateOnly? startDate = null; | |||
| Guid? tag = null; | |||
| (DateOnly, Guid)? entry = null; | |||
There was a problem hiding this comment.
Is it possible to use an anonymous record here?
There was a problem hiding this comment.
Try (DateOnly myDate, Guid myId)?
There was a problem hiding this comment.
Rename to previousNotPausedEntry for clarity
| @@ -528,29 +519,38 @@ ImmutableList<DateOnly> completionDates | |||
|
|
|||
| private static IEnumerable<DateRange<Guid>> GenerateDateRanges(ImmutableList<ChildLocation> childLocations) | |||
| { | |||
| DateOnly? startDate = null; | |||
| Guid? tag = null; | |||
| (DateOnly, Guid)? entry = null; | |||
There was a problem hiding this comment.
Try (DateOnly myDate, Guid myId)?
| @@ -528,29 +519,38 @@ ImmutableList<DateOnly> completionDates | |||
|
|
|||
| private static IEnumerable<DateRange<Guid>> GenerateDateRanges(ImmutableList<ChildLocation> childLocations) | |||
| { | |||
| DateOnly? startDate = null; | |||
| Guid? tag = null; | |||
| (DateOnly, Guid)? entry = null; | |||
There was a problem hiding this comment.
Rename to previousNotPausedEntry for clarity
| tag = childLocation.ChildLocationFamilyId; | ||
| yield return new DateRange<Guid>( | ||
| entry.Value.Item1, | ||
| childLocation.Date.AddDays(-1), |
There was a problem hiding this comment.
Add an inline comment to explain this:
When a child is cared for, the care duration is from the start date of childcare until the next entry's date of childcare minus one day.
What about a child changing location more than once in the same day, which can happen? This would be a negative date range.
There was a problem hiding this comment.
Step 1: Add a unit test for the multiple-changes-in-one-day scenario (start w/ parent, go to volunteer, go to another volunteer, go back to the first volunteer, then go to the parent). That will introduce some fun wrinkles. 😅
| } | ||
|
|
||
| [TestMethod] | ||
| public void CreateTimelineFilteredByFamilyIdNoPauses() |
There was a problem hiding this comment.
Need another test to test interaction with pauses, as well.
PabloDinella
force-pushed
the
fix-multiple-location-changes
branch
2 times, most recently
from
448960f to
a4640ea
Compare
PabloDinella
force-pushed
the
fix-multiple-location-changes
branch
from
a4640ea to
8582c33
Compare
When multiple location changes happens without a pause in the middle (when the child goes to the parent), the calculation ignores the subsequential locations and only calculates the due dates for the first location. This PR changes that so all location changes are taken into account.
Changes in code style were made by csharpier.