Skip to content

Commit

Permalink
fix: income at correct position in recommended envelopes (#899)
Browse files Browse the repository at this point in the history
  • Loading branch information
morremeyer authored Dec 24, 2023
1 parent a2bc262 commit 0b5e1e8
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 15 deletions.
5 changes: 3 additions & 2 deletions pkg/models/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (a Account) RecentEnvelopes(db *gorm.DB) ([]*uuid.UUID, error) {
latest := db.
Model(&Transaction{}).
Joins("LEFT JOIN envelopes ON envelopes.id = transactions.envelope_id AND envelopes.deleted_at IS NULL").
Select("envelopes.id as id, datetime(envelopes.created_at) as created").
Select("envelopes.id as e_id, datetime(envelopes.created_at) as created").
Where(&Transaction{
TransactionCreate: TransactionCreate{
DestinationAccountID: a.ID,
Expand All @@ -195,7 +195,8 @@ func (a Account) RecentEnvelopes(db *gorm.DB) ([]*uuid.UUID, error) {
// Group by frequency
err := db.
Table("(?)", latest).
Select("id").
// Set the nil UUID as ID if the envelope ID is NULL, since count() only counts non-null values
Select("IIF(e_id IS NOT NULL, e_id, '00000000-0000-0000-0000-000000000000') as id").
Group("id").
Order("count(id) DESC").
Order("created ASC").
Expand Down
36 changes: 23 additions & 13 deletions pkg/models/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ func (suite *TestSuiteStandard) TestAccountRecentEnvelopes() {
// Sleep for 10 milliseconds because we only save timestamps with 1 millisecond accuracy
// This is needed because the test runs so fast that all envelopes are sometimes created
// within the same millisecond, making the result non-deterministic
time.Sleep(1 * time.Second)
time.Sleep(1 * time.Millisecond)
}

// Create 15 transactions:
Expand All @@ -329,14 +329,24 @@ func (suite *TestSuiteStandard) TestAccountRecentEnvelopes() {
})
}

// Create one income transaction
_ = suite.createTestTransaction(models.TransactionCreate{
BudgetID: budget.ID,
EnvelopeID: nil,
SourceAccountID: externalAccount.ID,
DestinationAccountID: account.ID,
Amount: decimal.NewFromFloat(1337.42),
})
// Create three income transactions
//
// This is a regression test for income always showing at the last
// position in the recent envelopes (before the LIMIT) since count(id) for
// income was always 0. This is due to the envelope ID for income being NULL
// and count() not counting NULL values.
//
// Creating three income transactions puts "income" as the second most common
// envelope, verifying the fix
for i := 0; i < 3; i++ {
_ = suite.createTestTransaction(models.TransactionCreate{
BudgetID: budget.ID,
EnvelopeID: nil,
SourceAccountID: externalAccount.ID,
DestinationAccountID: account.ID,
Amount: decimal.NewFromFloat(1337.42),
})
}

ids, err := account.RecentEnvelopes(suite.db)
if err != nil {
Expand All @@ -351,9 +361,9 @@ func (suite *TestSuiteStandard) TestAccountRecentEnvelopes() {
// has been the most common one
suite.Assert().Equal(envelopeIDs[2], ids[0])

// Order for envelopes with the same frequency is undefined

// Income is the last one since it only appears once
// Income is the second one since it appears three times
var nilUUIDPointer *uuid.UUID
suite.Assert().Equal(nilUUIDPointer, ids[3])
suite.Assert().Equal(nilUUIDPointer, ids[1])

// Order for envelopes with the same frequency is undefined
}

0 comments on commit 0b5e1e8

Please sign in to comment.