Skip to content
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

bug(forecast): Forecasting can miss a funding schedule depending on the start time. #1486

Closed
elliotcourant opened this issue Aug 15, 2023 · 3 comments · Fixed by #1476
Closed
Assignees
Labels
api Related to or caused by the backend Go REST API. bug Something isn't working forecast Forecast functionality for predicting balances and future expenses
Milestone

Comments

@elliotcourant
Copy link
Member

When the start time of a forecast is too close to when the next funding event would occur, the forecast might miss that funding event. This is likely due to some midnight adjustments that it's doing.

@elliotcourant elliotcourant added bug Something isn't working api Related to or caused by the backend Go REST API. forecast Forecast functionality for predicting balances and future expenses labels Aug 15, 2023
@elliotcourant elliotcourant added this to the v1.0.0 milestone Aug 15, 2023
@elliotcourant elliotcourant self-assigned this Aug 15, 2023
elliotcourant added a commit that referenced this issue Aug 15, 2023
This test proves that #1486 is a valid bug.

Related to #1486
@elliotcourant
Copy link
Member Author

t.Run("midnight goofiness", func(t *testing.T) {
// This test proves that something is wrong. The now is before the next contribution would happen, but the forecast
// code skips that contribution entirely and instead calculates as if the next contribution would be the last day
// of the month.
// This test passes at the moment, but proves that behavior is incorrect.
fundingRule := testutils.Must(t, models.NewRule, "FREQ=MONTHLY;INTERVAL=1;BYMONTHDAY=15,-1")
spendingRule := testutils.Must(t, models.NewRule, "FREQ=MONTHLY;INTERVAL=1;BYMONTHDAY=1")
timezone := testutils.Must(t, time.LoadLocation, "America/Chicago")
// Monday August 14th, 2023. Payday is Tuesday August 15th.
now := time.Date(2023, 8, 14, 19, 30, 1, 0, timezone).UTC()
// Project 1 month into the future exactly.
end := time.Date(2023, 9, 14, 19, 30, 1, 0, timezone).UTC()
log := testutils.GetLog(t)
fundingSchedules := []models.FundingSchedule{
{
Rule: fundingRule,
ExcludeWeekends: true,
NextOccurrence: time.Date(2023, 8, 15, 0, 0, 0, 0, timezone),
FundingScheduleId: 1,
DateStarted: time.Date(2022, 1, 1, 0, 0, 0, 0, timezone),
},
}
spending := []models.Spending{
{
FundingScheduleId: 1,
SpendingType: models.SpendingTypeExpense,
TargetAmount: 2000,
CurrentAmount: 0,
NextRecurrence: time.Date(2023, 9, 1, 0, 0, 0, 0, timezone),
RecurrenceRule: spendingRule,
SpendingId: 1,
},
}
forecaster := NewForecaster(log, spending, fundingSchedules)
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
result := forecaster.GetForecast(ctx, now, end, timezone)
assert.EqualValues(t, Forecast{
StartingTime: now,
EndingTime: end,
StartingBalance: 0,
EndingBalance: 0,
Events: []Event{
{
Date: time.Date(2023, 8, 31, 5, 0, 0, 0, time.UTC),
Delta: 2000,
Contribution: 2000,
Transaction: 0,
Balance: 2000,
Spending: []SpendingEvent{
{
Date: time.Date(2023, 8, 31, 5, 0, 0, 0, time.UTC),
TransactionAmount: 0,
ContributionAmount: 2000,
RollingAllocation: 2000,
Funding: []FundingEvent{
{
Date: time.Date(2023, 8, 31, 5, 0, 0, 0, time.UTC),
OriginalDate: time.Date(2023, 8, 31, 5, 0, 0, 0, time.UTC),
WeekendAvoided: false,
FundingScheduleId: 1,
},
},
SpendingId: 1,
},
},
Funding: []FundingEvent{
{
Date: time.Date(2023, 8, 31, 5, 0, 0, 0, time.UTC),
OriginalDate: time.Date(2023, 8, 31, 5, 0, 0, 0, time.UTC),
WeekendAvoided: false,
FundingScheduleId: 1,
},
},
},
{
Date: time.Date(2023, 9, 1, 5, 0, 0, 0, time.UTC),
Delta: -2000,
Contribution: 0,
Transaction: 2000,
Balance: 0,
Spending: []SpendingEvent{
{
Date: time.Date(2023, 9, 1, 5, 0, 0, 0, time.UTC),
TransactionAmount: 2000,
ContributionAmount: 0,
RollingAllocation: 0,
Funding: []FundingEvent{},
SpendingId: 1,
},
},
Funding: []FundingEvent{},
},
},
}, result, "expected forecast")
})

This test proves the bug exists

@elliotcourant
Copy link
Member Author

So the MidnightInLocal function can produce a timestamp that is in the future relative to the input, when the input is UTC and the timezone is behind UTC in its offset.

I've created a new Midnight function and marked MidnightInLocal as deprecated. The new function accounts for timezone differences between the input and timezone specified offset.

@elliotcourant
Copy link
Member Author

I've just removed the MidnightInLocal function in the fix. All tests pass with the new Midnight function which is more accurate to the desired behavior anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Related to or caused by the backend Go REST API. bug Something isn't working forecast Forecast functionality for predicting balances and future expenses
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant