fix(forecast): Fixed forecast timing bug.

Fixed an issue where funding events could be missed depending on the
timezone and timestamp provided for the forecasting date range. This was
caused by an incorrect conversion of the input timestamp to a midnight
timestamp. Causing that midnight timestamp to actually be in the future
relative to the input. This issue has been resolved and the faulty
midnight function has been removed. A new midnight function was added
that correctly builds the timestamps.

Resolves #1486

pkg/cmd/beta.go

@@ -86,7 +86,7 @@ var (
 
 			betaCode := fmt.Sprintf("%X-%X", random[:4], random[4:])
 
-			expires := util.MidnightInLocal(time.Now().Add(14*24*time.Hour), time.Local)
+			expires := util.Midnight(time.Now().Add(14*24*time.Hour), time.Local)
 			beta := models.Beta{
 				CodeHash:  hash.HashPassword(strings.ToLower(betaCode), betaCode),
 				ExpiresAt: expires,

pkg/controller/funding_schedules_test.go

@@ -72,10 +72,10 @@ func TestPostFundingSchedules(t *testing.T) {
 
 		timezone := testutils.MustEz(t, user.Account.GetTimezone)
 		rule := testutils.Must(t, models.NewRule, "FREQ=WEEKLY;BYDAY=FR")
-		rule.DTStart(util.MidnightInLocal(time.Now().In(timezone).Add(-30*24*time.Hour), timezone)) // Force the Rule to be in the correct TZ.
+		rule.DTStart(util.Midnight(time.Now().In(timezone).Add(-30*24*time.Hour), timezone)) // Force the Rule to be in the correct TZ.
 		nextFriday := rule.After(time.Now(), false)
 		assert.Greater(t, nextFriday, time.Now(), "next friday should be in the future relative to now")
-		nextFriday = util.MidnightInLocal(nextFriday, timezone)
+		nextFriday = util.Midnight(nextFriday, timezone)
 		response := e.POST("/api/bank_accounts/{bankAccountId}/funding_schedules").
 			WithPath("bankAccountId", bank.BankAccountId).
 			WithCookie(TestCookieName, token).

pkg/controller/helpers.go

@@ -33,5 +33,5 @@ func (c *Controller) midnightInLocal(ctx echo.Context, input time.Time) (time.Ti
 		return input, errors.Wrap(err, "failed to parse account's timezone")
 	}
 
-	return util.MidnightInLocal(input, timezone), nil
+	return util.Midnight(input, timezone), nil
 }

pkg/forecast/forecast_test.go

@@ -78,7 +78,7 @@ func TestForecasterBase_GetForecast(t *testing.T) {
 				SpendingType:   models.SpendingTypeGoal,
 				TargetAmount:   1000000,
 				CurrentAmount:  0,
-				NextRecurrence: util.MidnightInLocal(now.AddDate(1, 0, 0), timezone),
+				NextRecurrence: util.Midnight(now.AddDate(1, 0, 0), timezone),
 			}), fundingSchedules)
 			forecast := forecaster.GetForecast(context.Background(), now, now.AddDate(0, 1, 4), timezone)
 			assert.Greater(t, forecast.StartingBalance, int64(0))
@@ -128,10 +128,12 @@ func TestForecasterBase_GetForecast(t *testing.T) {
 	})
 
 	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.
+		// This test was part of a bugfix. Previously there was an issue where when we would adjust times to midnight of
+		// that day. We would sometimes return a timestamp that was actually the next day. This happened when the timezone
+		// we were working in was behind UTC, but the timestamp itself was such that UTC was the next day. This caused the
+		// forecaster to miss a funding schedule that was in just a few hours because it believed it had already happened.
+		// This test proves that the bug is resolved and if it fails again in the future, that means the timezone bug has
+		// been reintroduced somehow.
 		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")
@@ -172,17 +174,49 @@ func TestForecasterBase_GetForecast(t *testing.T) {
 			StartingBalance: 0,
 			EndingBalance:   0,
 			Events: []Event{
+				{
+					Date:         time.Date(2023, 8, 15, 5, 0, 0, 0, time.UTC),
+					Delta:        1000,
+					Contribution: 1000,
+					Transaction:  0,
+					Balance:      1000,
+					Spending: []SpendingEvent{
+						{
+							Date:               time.Date(2023, 8, 15, 5, 0, 0, 0, time.UTC),
+							TransactionAmount:  0,
+							ContributionAmount: 1000,
+							RollingAllocation:  1000,
+							Funding: []FundingEvent{
+								{
+									Date:              time.Date(2023, 8, 15, 5, 0, 0, 0, time.UTC),
+									OriginalDate:      time.Date(2023, 8, 15, 5, 0, 0, 0, time.UTC),
+									WeekendAvoided:    false,
+									FundingScheduleId: 1,
+								},
+							},
+							SpendingId: 1,
+						},
+					},
+					Funding: []FundingEvent{
+						{
+							Date:              time.Date(2023, 8, 15, 5, 0, 0, 0, time.UTC),
+							OriginalDate:      time.Date(2023, 8, 15, 5, 0, 0, 0, time.UTC),
+							WeekendAvoided:    false,
+							FundingScheduleId: 1,
+						},
+					},
+				},
 				{
 					Date:         time.Date(2023, 8, 31, 5, 0, 0, 0, time.UTC),
-					Delta:        2000,
-					Contribution: 2000,
+					Delta:        1000,
+					Contribution: 1000,
 					Transaction:  0,
 					Balance:      2000,
 					Spending: []SpendingEvent{
 						{
 							Date:               time.Date(2023, 8, 31, 5, 0, 0, 0, time.UTC),
 							TransactionAmount:  0,
-							ContributionAmount: 2000,
+							ContributionAmount: 1000,
 							RollingAllocation:  2000,
 							Funding: []FundingEvent{
 								{

pkg/forecast/funding.go

@@ -42,7 +42,7 @@ func NewFundingScheduleFundingInstructions(log *logrus.Entry, fundingSchedule mo
 }
 
 func (f *fundingScheduleBase) GetNextFundingEventAfter(ctx context.Context, input time.Time, timezone *time.Location) FundingEvent {
-	input = util.MidnightInLocal(input, timezone)
+	input = util.Midnight(input, timezone)
 	rule := f.fundingSchedule.Rule.RRule
 	var nextContributionDate time.Time
 	if f.fundingSchedule.NextOccurrence.IsZero() {
@@ -54,7 +54,7 @@ func (f *fundingScheduleBase) GetNextFundingEventAfter(ctx context.Context, inpu
 			corrected := dateStarted.In(timezone)
 			rule.DTStart(corrected)
 		}
-		nextContributionDate = util.MidnightInLocal(rule.Before(input, false), timezone)
+		nextContributionDate = util.Midnight(rule.Before(input, false), timezone)
 	} else {
 		// If we have the date started defined on the funding schedule. Then use that so we can see the past and the future.
 		if f.fundingSchedule.DateStarted.IsZero() {
@@ -64,7 +64,7 @@ func (f *fundingScheduleBase) GetNextFundingEventAfter(ctx context.Context, inpu
 			corrected := dateStarted.In(timezone)
 			rule.DTStart(corrected)
 		}
-		nextContributionDate = util.MidnightInLocal(f.fundingSchedule.NextOccurrence, timezone)
+		nextContributionDate = util.Midnight(f.fundingSchedule.NextOccurrence, timezone)
 	}
 	if input.Before(nextContributionDate) {
 		// If now is before the already established next occurrence, then just return that.
@@ -124,7 +124,7 @@ AfterLoop:
 			}
 		}
 
-		nextContributionDate = util.MidnightInLocal(nextContributionDate, timezone)
+		nextContributionDate = util.Midnight(nextContributionDate, timezone)
 	}
 
 	return FundingEvent{
@@ -182,7 +182,7 @@ func (f *fundingScheduleBase) GetFundingEventsBetween(ctx context.Context, start
 	// We also need to truncate the hours on the start time. To make sure that we are operating relative to
 	// midnight.
 	if f.fundingSchedule.DateStarted.IsZero() {
-		dtStart := util.MidnightInLocal(start, timezone)
+		dtStart := util.Midnight(start, timezone)
 		rule.DTStart(dtStart)
 	} else {
 		dateStarted := f.fundingSchedule.DateStarted

pkg/forecast/spending.go

@@ -163,7 +163,7 @@ func (s *spendingInstructionBase) GetRecurrencesBetween(ctx context.Context, sta
 	case models.SpendingTypeExpense:
 		rule := s.spending.RecurrenceRule.RRule
 		if s.spending.DateStarted.IsZero() {
-			dtMidnight := util.MidnightInLocal(start, timezone)
+			dtMidnight := util.Midnight(start, timezone)
 			rule.DTStart(dtMidnight)
 		} else {
 			dateStarted := s.spending.DateStarted
@@ -175,7 +175,7 @@ func (s *spendingInstructionBase) GetRecurrencesBetween(ctx context.Context, sta
 		// because this function is **INTENDED** to be called with the start being now or the next funding event, and
 		// end being the next funding event immediately after that. We can't control what happens after the later
 		// funding event, so we need to know how much will be spent before then, so we know how much to allocate.
-		items := rule.Between(start, end.Add(-1 * time.Second), true)
+		items := rule.Between(start, end.Add(-1*time.Second), true)
 		return items
 	case models.SpendingTypeGoal:
 		if s.spending.NextRecurrence.After(start) && s.spending.NextRecurrence.Before(end) {
@@ -193,15 +193,15 @@ func (s *spendingInstructionBase) getNextSpendingEventAfter(ctx context.Context,
 		return nil
 	}
 
-	input = util.MidnightInLocal(input, timezone)
+	input = util.Midnight(input, timezone)
 
 	var rule *rrule.RRule
 	if s.spending.RecurrenceRule != nil {
 		// This is terrible and I hate it :tada:
 		rule = &(*s.spending.RecurrenceRule).RRule
 	}
 
-	nextRecurrence := util.MidnightInLocal(s.spending.NextRecurrence, timezone)
+	nextRecurrence := util.Midnight(s.spending.NextRecurrence, timezone)
 	switch s.spending.SpendingType {
 	case models.SpendingTypeOverflow:
 		return nil

pkg/internal/fixtures/funding_schedules.go

@@ -24,19 +24,19 @@ func GivenIHaveAFundingSchedule(t *testing.T, bankAccount *models.BankAccount, r
 	rule := testutils.Must(t, models.NewRule, ruleString)
 	tz := testutils.MustEz(t, bankAccount.Account.GetTimezone)
 	rule.DTStart(time.Now().In(tz).Add(-30 * 24 * time.Hour))
-	nextOccurrence := util.MidnightInLocal(rule.Before(time.Now(), false), tz)
+	nextOccurrence := util.Midnight(rule.Before(time.Now(), false), tz)
 
 	fundingSchedule := models.FundingSchedule{
-		AccountId:         bankAccount.AccountId,
-		Account:           bankAccount.Account,
-		BankAccountId:     bankAccount.BankAccountId,
-		BankAccount:       bankAccount,
-		Name:              gofakeit.Generate("Payday {uuid}"),
-		Description:       gofakeit.Generate("{sentence:5}"),
-		Rule:              rule,
-		ExcludeWeekends:   excludeWeekends,
-		LastOccurrence:    nil,
-		NextOccurrence:    nextOccurrence,
+		AccountId:       bankAccount.AccountId,
+		Account:         bankAccount.Account,
+		BankAccountId:   bankAccount.BankAccountId,
+		BankAccount:     bankAccount,
+		Name:            gofakeit.Generate("Payday {uuid}"),
+		Description:     gofakeit.Generate("{sentence:5}"),
+		Rule:            rule,
+		ExcludeWeekends: excludeWeekends,
+		LastOccurrence:  nil,
+		NextOccurrence:  nextOccurrence,
 	}
 
 	require.NoError(t, repo.CreateFundingSchedule(context.Background(), &fundingSchedule), "must be able to create funding schedule")

pkg/internal/fixtures/transactions.go

@@ -36,7 +36,7 @@ func GivenIHaveNTransactions(t *testing.T, bankAccount models.BankAccount, n int
 	transactions := make([]models.Transaction, n)
 
 	for i := 0; i < n; i++ {
-		date := util.MidnightInLocal(time.Now(), timezone)
+		date := util.Midnight(time.Now(), timezone)
 
 		prefix := gofakeit.RandomString([]string{
 			fmt.Sprintf("DEBIT FOR CHECKCARD XXXXXX%s %s", gofakeit.Generate("####"), date.Format("01/02/06")),
@@ -60,7 +60,7 @@ func GivenIHaveNTransactions(t *testing.T, bankAccount models.BankAccount, n int
 			SpendingAmount:            nil,
 			Categories:                nil,
 			OriginalCategories:        nil,
-			Date:                      util.MidnightInLocal(time.Now(), timezone),
+			Date:                      util.Midnight(time.Now(), timezone),
 			AuthorizedDate:            nil,
 			Name:                      name,
 			CustomName:                nil,

pkg/internal/fixtures/transactions_test.go

@@ -91,7 +91,7 @@ func TestCountPendingTransactions(t *testing.T) {
 		timezone, err := bankAccount.Account.GetTimezone()
 		require.NoError(t, err, "must be able to get the timezone from the account")
 
-		date := util.MidnightInLocal(time.Now(), timezone)
+		date := util.Midnight(time.Now(), timezone)
 
 		prefix := gofakeit.RandomString([]string{
 			fmt.Sprintf("DEBIT FOR CHECKCARD XXXXXX%s %s", gofakeit.Generate("####"), date.Format("01/02/06")),
@@ -109,7 +109,7 @@ func TestCountPendingTransactions(t *testing.T) {
 			BankAccount:          &bankAccount,
 			PlaidTransactionId:   gofakeit.UUID(),
 			Amount:               int64(gofakeit.Number(100, 10000)),
-			Date:                 util.MidnightInLocal(time.Now(), timezone),
+			Date:                 util.Midnight(time.Now(), timezone),
 			Name:                 name,
 			OriginalName:         name,
 			MerchantName:         company,

pkg/models/funding_schedule.go

@@ -36,7 +36,7 @@ func (f *FundingSchedule) GetNumberOfContributionsBetween(start, end time.Time,
 	// Make sure that the rule is using the timezone of the dates provided. This is an easy way to force that.
 	// We also need to truncate the hours on the start time. To make sure that we are operating relative to
 	// midnight.
-	dtStart := util.MidnightInLocal(start, timezone)
+	dtStart := util.Midnight(start, timezone)
 	rule.DTStart(dtStart)
 	items := rule.Between(start, end, true)
 	return int64(len(items))
@@ -56,11 +56,11 @@ func (f *FundingSchedule) GetNextContributionDateAfter(now time.Time, timezone *
 	now = now.In(timezone)
 	var nextContributionDate time.Time
 	if !f.NextOccurrence.IsZero() {
-		nextContributionDate = util.MidnightInLocal(f.NextOccurrence, timezone)
+		nextContributionDate = util.Midnight(f.NextOccurrence, timezone)
 	} else {
 		// Hack to determine the previous contribution date before we figure out the next one.
 		f.Rule.RRule.DTStart(now.AddDate(-1, 0, 0))
-		nextContributionDate = util.MidnightInLocal(f.Rule.Before(now, false), timezone)
+		nextContributionDate = util.Midnight(f.Rule.Before(now, false), timezone)
 	}
 	if now.Before(nextContributionDate) {
 		// If now is before the already established next occurrence, then just return that.
@@ -98,7 +98,7 @@ func (f *FundingSchedule) GetNextContributionDateAfter(now time.Time, timezone *
 			}
 		}
 
-		nextContributionDate = util.MidnightInLocal(nextContributionDate, timezone)
+		nextContributionDate = util.Midnight(nextContributionDate, timezone)
 	}
 
 	return nextContributionDate

pkg/models/spending.go

@@ -75,7 +75,7 @@ func (e *Spending) GetRecurrencesBefore(now, before time.Time, timezone *time.Lo
 	switch e.SpendingType {
 	case SpendingTypeExpense:
 		if e.DateStarted.IsZero() {
-			dtMidnight := util.MidnightInLocal(now, timezone)
+			dtMidnight := util.Midnight(now, timezone)
 			e.RecurrenceRule.DTStart(dtMidnight)
 		} else {
 			dateStarted := e.DateStarted
@@ -159,7 +159,7 @@ func CalculateNextContribution(
 	now = now.In(timezone)
 
 	fundingFirst, fundingSecond := fundingSchedule.GetNextTwoContributionDatesAfter(now, timezone)
-	nextRecurrence := util.MidnightInLocal(spending.NextRecurrence, timezone)
+	nextRecurrence := util.Midnight(spending.NextRecurrence, timezone)
 	if spending.RecurrenceRule != nil {
 		// Same thing as the contribution rule, make sure that we are incrementing with the existing dates as the base
 		// rather than the current timestamp (which is what RRule defaults to).

pkg/models/spending_test.go

@@ -59,9 +59,9 @@ func GiveMeAFundingSchedule(nextContributionDate time.Time, rule *Rule) *Funding
 
 func TestSpending_CalculateNextContribution(t *testing.T) {
 	t.Run("next funding in the past updated", func(t *testing.T) {
-		today := util.MidnightInLocal(time.Now(), time.UTC)
-		dayAfterTomorrow := util.MidnightInLocal(today.Add(48*time.Hour), time.UTC)
-		dayAfterDayAfterTomorrow := util.MidnightInLocal(time.Now().Add(72*time.Hour), time.UTC)
+		today := util.Midnight(time.Now(), time.UTC)
+		dayAfterTomorrow := util.Midnight(today.Add(48*time.Hour), time.UTC)
+		dayAfterDayAfterTomorrow := util.Midnight(time.Now().Add(72*time.Hour), time.UTC)
 		assert.True(t, dayAfterDayAfterTomorrow.After(today), "dayAfterDayAfterTomorrow timestamp must come after today's")
 		rule, err := NewRule("FREQ=WEEKLY;INTERVAL=2;BYDAY=FR") // Every other friday
 		assert.NoError(t, err, "must be able to parse the rrule")
@@ -86,8 +86,8 @@ func TestSpending_CalculateNextContribution(t *testing.T) {
 
 	// This might eventually become obsolete, but it covers a bug scenario I discovered while working on institutions.
 	t.Run("next funding in the past is behind", func(t *testing.T) {
-		today := util.MidnightInLocal(time.Now(), time.UTC)
-		dayAfterTomorrow := util.MidnightInLocal(time.Now().Add(48*time.Hour), time.UTC)
+		today := util.Midnight(time.Now(), time.UTC)
+		dayAfterTomorrow := util.Midnight(time.Now().Add(48*time.Hour), time.UTC)
 		assert.True(t, dayAfterTomorrow.After(today), "dayAfterTomorrow timestamp must come after today's")
 		rule, err := NewRule("FREQ=WEEKLY;INTERVAL=2;BYDAY=FR") // Every other friday
 		assert.NoError(t, err, "must be able to parse the rrule")

pkg/util/midnight.go

@@ -6,32 +6,36 @@ import (
 	"github.com/pkg/errors"
 )
 
-// MidnightInLocal will take the provided timestamp and return the midnight of that timestamp in the provided timezone.
-// This can sometimes result in returning the day prior when the function is evaluated at such a time that the current
-// input time is far enough ahead that the timezone is still the previous day.
-func MidnightInLocal(input time.Time, timezone *time.Location) time.Time {
+// Midnight will take the provider timestamp and a desired timezone. It does not assume that the provided timestamp is
+// in the desired timezone, or that it is in UTC. It calculates the difference in timezone offsets and adjusts the
+// timestamp accordingly. Then truncates the timestamp to produce a midnight or "start of day" timestamp which it
+// returns.
+// As a result, this function should **NEVER** return a timestamp that comes after the provided input. The output may be
+// a later date, but will still be before the provided input once timezone offsets have been accounted for.
+func Midnight(input time.Time, timezone *time.Location) time.Time {
 	if input.IsZero() {
 		panic("cannot calculate the midnight in local of an empty time")
 	}
-	clone := time.Date(
-		input.Year(),
-		input.Month(),
-		input.Day(),
-		input.Hour(),
-		input.Minute(),
-		input.Second(),
-		input.Nanosecond(),
-		time.UTC,
-	)
-	// We need to do this because we need to know the offset for a given timezone at the provided input's timestamp. This
-	// way we can adjust the input to be in that timezone then truncate the time.
-	_, offset := clone.Zone()
-	inputTwo := input.Add(time.Second * time.Duration(offset))
+	// Get the timezone offsets from the input and from the timezone specified.
+	_, inputZoneOffset := input.Zone()
+	_, tzOffset := input.In(timezone).Zone()
+
+	// Calculate the difference betwen them.
+	// TODO: This might need to be math.Abs instead. If the timezone is _ahead_ of the input location I think there would
+	// be a bug here. Not quite sure yet.
+	delta := inputZoneOffset - tzOffset
 
+	// Subtract the delta from the input time, this accounts for the timezone difference potential between the input
+	// timezone and the provided timezone.
+	// The input time should be treated as the absolute time of the moment. But might be in ANY timezone. It could already
+	// be in the specified timezone, or it could be in UTC or some other timezone for example. But we must make sure that
+	// our adjustment takes that into account. So this delta will do just that.
+	adjusted := input.Add(time.Duration(-delta) * time.Second)
+	// Create a timestamp in the specified timezone that is midnight.
 	midnight := time.Date(
-		inputTwo.Year(),  // Year
-		inputTwo.Month(), // Month
-		inputTwo.Day(),   // Day
+		adjusted.Year(),  // Year
+		adjusted.Month(), // Month
+		adjusted.Day(),   // Day
 		0,                // Hours
 		0,                // Minutes
 		0,                // Seconds