diff --git a/pkg/database/database.go b/pkg/database/database.go index 3603837..5499bbc 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -111,6 +111,18 @@ func (c *Client) CreateTransaction(tx Transaction) (ntx Transaction, err error) // DeleteTransaction deletes a transaction func (c *Client) DeleteTransaction(id uuid.UUID) (err error) { if err = c.retryTx(func(db *gorm.DB) error { + tx, err := c.GetTransactionByID(id) + if err != nil { + return err + } + + if tx.PairKey.Valid { + // We got a paired transaction which would be out-of-sync if we + // only delete one part of it so instead of doing a delete on the + // ID of the transaction, we do a delete on the pair-key + return db.Delete(&Transaction{}, "pair_key = ?", tx.PairKey.UUID).Error + } + return db.Delete(&Transaction{}, "id = ?", id).Error }); err != nil { return fmt.Errorf("deleting transaction: %w", err) @@ -273,6 +285,8 @@ func (c *Client) TransferMoney(from, to uuid.UUID, amount float64) (err error) { return fmt.Errorf("account type mismatch: %s != %s", fromAcc.Type, toAcc.Type) } + pairKey := uuid.Must(uuid.NewRandom()) + var txs []*Transaction switch fromAcc.Type { case AccountTypeBudget, AccountTypeTracking: @@ -285,6 +299,7 @@ func (c *Client) TransferMoney(from, to uuid.UUID, amount float64) (err error) { Account: uuid.NullUUID{UUID: from, Valid: true}, Category: uuid.NullUUID{}, Cleared: false, + PairKey: uuid.NullUUID{UUID: pairKey, Valid: true}, }, { Time: time.Now().UTC(), @@ -293,6 +308,7 @@ func (c *Client) TransferMoney(from, to uuid.UUID, amount float64) (err error) { Account: uuid.NullUUID{UUID: to, Valid: true}, Category: uuid.NullUUID{}, Cleared: false, + PairKey: uuid.NullUUID{UUID: pairKey, Valid: true}, }, } @@ -306,6 +322,7 @@ func (c *Client) TransferMoney(from, to uuid.UUID, amount float64) (err error) { Account: uuid.NullUUID{}, Category: uuid.NullUUID{UUID: from, Valid: true}, Cleared: false, + PairKey: uuid.NullUUID{UUID: pairKey, Valid: true}, }, { Time: time.Now().UTC(), @@ -314,6 +331,7 @@ func (c *Client) TransferMoney(from, to uuid.UUID, amount float64) (err error) { Account: uuid.NullUUID{}, Category: uuid.NullUUID{UUID: to, Valid: true}, Cleared: false, + PairKey: uuid.NullUUID{UUID: pairKey, Valid: true}, }, } } @@ -350,6 +368,8 @@ func (c *Client) TransferMoneyWithCategory(from, to uuid.UUID, amount float64, c return fmt.Errorf("transfer contained category-type account") } + pairKey := uuid.Must(uuid.NewRandom()) + if err = c.retryTx(func(tx *gorm.DB) (err error) { fromTx := Transaction{ Time: time.Now().UTC(), @@ -358,6 +378,7 @@ func (c *Client) TransferMoneyWithCategory(from, to uuid.UUID, amount float64, c Account: uuid.NullUUID{UUID: from, Valid: true}, Category: uuid.NullUUID{}, Cleared: false, + PairKey: uuid.NullUUID{UUID: pairKey, Valid: true}, } if fromAcc.Type == AccountTypeBudget { @@ -371,6 +392,7 @@ func (c *Client) TransferMoneyWithCategory(from, to uuid.UUID, amount float64, c Account: uuid.NullUUID{UUID: to, Valid: true}, Category: uuid.NullUUID{}, Cleared: false, + PairKey: uuid.NullUUID{UUID: pairKey, Valid: true}, } if toAcc.Type == AccountTypeBudget { diff --git a/pkg/database/database_test.go b/pkg/database/database_test.go index a137ec5..75fb5bc 100644 --- a/pkg/database/database_test.go +++ b/pkg/database/database_test.go @@ -77,22 +77,48 @@ func TestAccountManagement(t *testing.T) { assert.Equal(t, "renamed", act.Name) } +func TestPairKeyRemoval(t *testing.T) { + dbc, err := New("sqlite", testDSN) + require.NoError(t, err) + + // Create two accounts to transfer from / to + tb1, err := dbc.CreateAccount("test1", AccountTypeBudget) + require.NoError(t, err) + tb2, err := dbc.CreateAccount("test2", AccountTypeBudget) + require.NoError(t, err) + + // Lets verify both of them do have zero-balance + bals, err := dbc.ListAccountBalances(false) + require.NoError(t, err) + testCheckAcctBal(t, bals, tb1.ID, 0) + testCheckAcctBal(t, bals, tb2.ID, 0) + + // Transfer some money + require.NoError(t, dbc.TransferMoney(tb1.ID, tb2.ID, 500)) + bals, err = dbc.ListAccountBalances(false) + require.NoError(t, err) + testCheckAcctBal(t, bals, tb1.ID, -500) + testCheckAcctBal(t, bals, tb2.ID, 500) + + // Now fetch the transactions on one of the accounts and delete the only one + txs, err := dbc.ListTransactionsByAccount(tb1.ID, time.Time{}, time.Now()) + require.NoError(t, err) + require.Len(t, txs, 1) // Should only be one by now + + require.NoError(t, dbc.DeleteTransaction(txs[0].ID)) + + // Check both accounts went back to zero-balance (so paired tx are gone) + bals, err = dbc.ListAccountBalances(false) + require.NoError(t, err) + testCheckAcctBal(t, bals, tb1.ID, 0) + testCheckAcctBal(t, bals, tb2.ID, 0) +} + //nolint:funlen func TestTransactions(t *testing.T) { dbc, err := New("sqlite", testDSN) require.NoError(t, err) - checkAcctBal := func(bals []AccountBalance, act uuid.UUID, bal float64) { - for _, b := range bals { - if b.ID == act { - assert.Equal(t, bal, b.Balance) - return - } - } - - t.Errorf("account %s balance not found", act) - } - // Set up some accounts for testing tb1, err := dbc.CreateAccount("test1", AccountTypeBudget) require.NoError(t, err) @@ -130,41 +156,41 @@ func TestTransactions(t *testing.T) { // Now we should have money… bals, err := dbc.ListAccountBalances(false) require.NoError(t, err) - checkAcctBal(bals, tb1.ID, 1000) - checkAcctBal(bals, tb2.ID, 0) - checkAcctBal(bals, tt.ID, 0) - checkAcctBal(bals, tc.ID, 0) - checkAcctBal(bals, UnallocatedMoney, 1000) + testCheckAcctBal(t, bals, tb1.ID, 1000) + testCheckAcctBal(t, bals, tb2.ID, 0) + testCheckAcctBal(t, bals, tt.ID, 0) + testCheckAcctBal(t, bals, tc.ID, 0) + testCheckAcctBal(t, bals, UnallocatedMoney, 1000) // Lets redistribute the money require.NoError(t, dbc.TransferMoney(UnallocatedMoney, tc.ID, 500)) bals, err = dbc.ListAccountBalances(false) require.NoError(t, err) - checkAcctBal(bals, tb1.ID, 1000) - checkAcctBal(bals, tb2.ID, 0) - checkAcctBal(bals, tt.ID, 0) - checkAcctBal(bals, tc.ID, 500) - checkAcctBal(bals, UnallocatedMoney, 500) + testCheckAcctBal(t, bals, tb1.ID, 1000) + testCheckAcctBal(t, bals, tb2.ID, 0) + testCheckAcctBal(t, bals, tt.ID, 0) + testCheckAcctBal(t, bals, tc.ID, 500) + testCheckAcctBal(t, bals, UnallocatedMoney, 500) // Now transfer some money to another budget account require.NoError(t, dbc.TransferMoney(tb1.ID, tb2.ID, 100)) bals, err = dbc.ListAccountBalances(false) require.NoError(t, err) - checkAcctBal(bals, tb1.ID, 900) - checkAcctBal(bals, tb2.ID, 100) - checkAcctBal(bals, tt.ID, 0) - checkAcctBal(bals, tc.ID, 500) - checkAcctBal(bals, UnallocatedMoney, 500) + testCheckAcctBal(t, bals, tb1.ID, 900) + testCheckAcctBal(t, bals, tb2.ID, 100) + testCheckAcctBal(t, bals, tt.ID, 0) + testCheckAcctBal(t, bals, tc.ID, 500) + testCheckAcctBal(t, bals, UnallocatedMoney, 500) // And some to a tracking account (needs category) require.NoError(t, dbc.TransferMoneyWithCategory(tb1.ID, tt.ID, 100, tc.ID)) bals, err = dbc.ListAccountBalances(false) require.NoError(t, err) - checkAcctBal(bals, tb1.ID, 800) - checkAcctBal(bals, tb2.ID, 100) - checkAcctBal(bals, tt.ID, 100) - checkAcctBal(bals, tc.ID, 400) - checkAcctBal(bals, UnallocatedMoney, 500) + testCheckAcctBal(t, bals, tb1.ID, 800) + testCheckAcctBal(t, bals, tb2.ID, 100) + testCheckAcctBal(t, bals, tt.ID, 100) + testCheckAcctBal(t, bals, tc.ID, 400) + testCheckAcctBal(t, bals, UnallocatedMoney, 500) // We might also spend money lltx, err := dbc.CreateTransaction(Transaction{ @@ -180,11 +206,11 @@ func TestTransactions(t *testing.T) { assert.False(t, lltx.Cleared) bals, err = dbc.ListAccountBalances(false) require.NoError(t, err) - checkAcctBal(bals, tb1.ID, 700) - checkAcctBal(bals, tb2.ID, 100) - checkAcctBal(bals, tt.ID, 100) - checkAcctBal(bals, tc.ID, 300) - checkAcctBal(bals, UnallocatedMoney, 500) + testCheckAcctBal(t, bals, tb1.ID, 700) + testCheckAcctBal(t, bals, tb2.ID, 100) + testCheckAcctBal(t, bals, tt.ID, 100) + testCheckAcctBal(t, bals, tc.ID, 300) + testCheckAcctBal(t, bals, UnallocatedMoney, 500) // List transactions txs, err := dbc.ListTransactionsByAccount(tb1.ID, time.Time{}, time.Now()) @@ -199,11 +225,11 @@ func TestTransactions(t *testing.T) { require.NoError(t, dbc.UpdateTransactionCategory(lltx.ID, UnallocatedMoney)) bals, err = dbc.ListAccountBalances(false) require.NoError(t, err) - checkAcctBal(bals, tb1.ID, 700) - checkAcctBal(bals, tb2.ID, 100) - checkAcctBal(bals, tt.ID, 100) - checkAcctBal(bals, tc.ID, 400) - checkAcctBal(bals, UnallocatedMoney, 400) + testCheckAcctBal(t, bals, tb1.ID, 700) + testCheckAcctBal(t, bals, tb2.ID, 100) + testCheckAcctBal(t, bals, tt.ID, 100) + testCheckAcctBal(t, bals, tc.ID, 400) + testCheckAcctBal(t, bals, UnallocatedMoney, 400) // Lets try to move it to a broken category require.Error(t, dbc.UpdateTransactionCategory(lltx.ID, tt.ID)) @@ -221,11 +247,11 @@ func TestTransactions(t *testing.T) { require.NoError(t, dbc.DeleteTransaction(lltx.ID)) bals, err = dbc.ListAccountBalances(false) require.NoError(t, err) - checkAcctBal(bals, tb1.ID, 800) - checkAcctBal(bals, tb2.ID, 100) - checkAcctBal(bals, tt.ID, 100) - checkAcctBal(bals, tc.ID, 400) - checkAcctBal(bals, UnallocatedMoney, 500) + testCheckAcctBal(t, bals, tb1.ID, 800) + testCheckAcctBal(t, bals, tb2.ID, 100) + testCheckAcctBal(t, bals, tt.ID, 100) + testCheckAcctBal(t, bals, tc.ID, 400) + testCheckAcctBal(t, bals, UnallocatedMoney, 500) // Get a deleted transaction _, err = dbc.GetTransactionByID(lltx.ID) @@ -236,3 +262,14 @@ func TestTransactions(t *testing.T) { require.NoError(t, err) assert.Len(t, txs, 3) } + +func testCheckAcctBal(t *testing.T, bals []AccountBalance, act uuid.UUID, bal float64) { + for _, b := range bals { + if b.ID == act { + assert.Equal(t, bal, b.Balance) + return + } + } + + t.Errorf("account %s balance not found", act) +} diff --git a/pkg/database/schema.go b/pkg/database/schema.go index 23dbdca..0470253 100644 --- a/pkg/database/schema.go +++ b/pkg/database/schema.go @@ -39,6 +39,8 @@ type ( Account uuid.NullUUID `gorm:"type:uuid" json:"account"` Category uuid.NullUUID `gorm:"type:uuid" json:"category"` Cleared bool `json:"cleared"` + + PairKey uuid.NullUUID `gorm:"type:uuid" json:"-"` } // BaseModel is used internally in all other models for common fields