diff --git a/internal/handler/space.go b/internal/handler/space.go index 82cc08b..fa19015 100644 --- a/internal/handler/space.go +++ b/internal/handler/space.go @@ -1607,6 +1607,50 @@ func (h *spaceHandler) HandleEditTransaction(w http.ResponseWriter, r *http.Requ w.WriteHeader(http.StatusOK) } +func (h *spaceHandler) HandleDeleteTransaction(w http.ResponseWriter, r *http.Request) { + spaceID := r.PathValue("spaceID") + accountID := r.PathValue("accountID") + transactionID := r.PathValue("transactionID") + + account, err := h.accountService.GetAccount(accountID) + if err != nil || account.SpaceID != spaceID { + ui.RenderError(w, r, "Account not found", http.StatusNotFound) + return + } + + txn, err := h.transactionService.GetTransaction(transactionID) + if err != nil || txn.AccountID != accountID { + ui.RenderError(w, r, "Transaction not found", http.StatusNotFound) + return + } + + actorID := "" + if u := ctxkeys.User(r.Context()); u != nil { + actorID = u.ID + } + + if _, err := h.transactionService.DeleteTransaction(service.DeleteTransactionInput{ + TransactionID: transactionID, + ActorID: actorID, + }); err != nil { + if errors.Is(err, service.ErrTransactionPartOfTransfer) { + ui.RenderError(w, r, "Transfer transactions cannot be deleted.", http.StatusBadRequest) + return + } + slog.Error("failed to delete transaction", "error", err, "transaction_id", transactionID) + ui.RenderError(w, r, "Failed to delete transaction", http.StatusInternalServerError) + return + } + + redirectTo := routeurl.URL( + "page.app.spaces.space.accounts.account.overview", + "spaceID", spaceID, + "accountID", accountID, + ) + w.Header().Set("HX-Redirect", redirectTo) + w.WriteHeader(http.StatusOK) +} + func (h *spaceHandler) SpaceCreateTransferPage(w http.ResponseWriter, r *http.Request) { spaceID := r.PathValue("spaceID") accountID := r.PathValue("accountID") diff --git a/internal/repository/transaction.go b/internal/repository/transaction.go index d304664..fb1d63c 100644 --- a/internal/repository/transaction.go +++ b/internal/repository/transaction.go @@ -14,6 +14,7 @@ type TransactionRepository interface { CreateDepositAtomic(t *model.Transaction, newBalance decimal.Decimal) error UpdateBillAtomic(t *model.Transaction, newBalance decimal.Decimal, categoryID *string) error UpdateDepositAtomic(t *model.Transaction, newBalance decimal.Decimal) error + DeleteAtomic(transactionID, accountID string, newBalance decimal.Decimal) error TransferAtomic(withdrawal, deposit *model.Transaction, sourceNewBalance, destNewBalance decimal.Decimal) error GetByID(id string) (*model.Transaction, error) GetCategoryID(transactionID string) (*string, error) @@ -141,6 +142,23 @@ func (r *transactionRepository) UpdateDepositAtomic(t *model.Transaction, newBal }) } +// DeleteAtomic removes a standalone (non-transfer) transaction and reverses +// its effect on the account balance in a single SQL transaction. The caller is +// responsible for computing the new balance — bills credit it back, deposits +// debit it. transaction_categories is removed via ON DELETE CASCADE. +func (r *transactionRepository) DeleteAtomic(transactionID, accountID string, newBalance decimal.Decimal) error { + return WithTx(r.db, func(tx *sqlx.Tx) error { + if _, err := tx.Exec(`DELETE FROM transactions WHERE id = $1;`, transactionID); err != nil { + return err + } + updateBalance := `UPDATE accounts SET balance = $1, updated_at = $2 WHERE id = $3;` + if _, err := tx.Exec(updateBalance, newBalance, time.Now(), accountID); err != nil { + return err + } + return nil + }) +} + // TransferAtomic creates the withdrawal + deposit transaction pair, updates both // account balances, and links the two via related_transactions in a single SQL // transaction. Negative balances are allowed — overdraft enforcement is a product diff --git a/internal/routes/routes.go b/internal/routes/routes.go index bd3415e..2beaad6 100644 --- a/internal/routes/routes.go +++ b/internal/routes/routes.go @@ -124,6 +124,7 @@ func SetupRoutes(a *app.App) http.Handler { g.Get("/transactions/{transactionID}", spaceH.SpaceTransactionPage).Name("page.app.spaces.space.accounts.account.transactions.transaction") g.Get("/transactions/{transactionID}/edit", spaceH.SpaceEditTransactionPage).Name("page.app.spaces.space.accounts.account.transactions.transaction.edit") g.Post("/transactions/{transactionID}/edit", spaceH.HandleEditTransaction).Name("action.app.spaces.space.accounts.account.transactions.transaction.edit") + g.Post("/transactions/{transactionID}/delete", spaceH.HandleDeleteTransaction).Name("action.app.spaces.space.accounts.account.transactions.transaction.delete") g.Get("/transactions/{transactionID}/activity", spaceH.SpaceTransactionActivityPage).Name("page.app.spaces.space.accounts.account.transactions.transaction.activity") g.Get("/settings", spaceH.SpaceAccountSettingsPage).Name("page.app.spaces.space.accounts.account.settings") g.Post("/settings/rename", spaceH.HandleRenameAccount).Name("action.app.spaces.space.accounts.account.settings.rename") diff --git a/internal/service/transaction.go b/internal/service/transaction.go index 1bdee9a..e497efb 100644 --- a/internal/service/transaction.go +++ b/internal/service/transaction.go @@ -517,6 +517,64 @@ func (s *TransactionService) UpdateDeposit(input UpdateDepositInput) (*model.Tra return existing, nil } +type DeleteTransactionInput struct { + TransactionID string + ActorID string +} + +// DeleteTransaction removes a standalone bill or deposit. Transfers are +// rejected with ErrTransactionPartOfTransfer — they must be undone via the +// transfer flow so both halves stay consistent. Deleting a bill credits the +// account; deleting a deposit debits it. +func (s *TransactionService) DeleteTransaction(input DeleteTransactionInput) (*model.Transaction, error) { + if input.TransactionID == "" { + return nil, fmt.Errorf("transaction id is required") + } + + existing, err := s.transactionRepo.GetByID(input.TransactionID) + if err != nil { + return nil, fmt.Errorf("failed to load transaction: %w", err) + } + if related, err := s.transactionRepo.GetRelatedID(existing.ID); err != nil { + return nil, fmt.Errorf("failed to check transfer linkage: %w", err) + } else if related != nil { + return nil, ErrTransactionPartOfTransfer + } + + account, err := s.accountService.GetAccount(existing.AccountID) + if err != nil { + return nil, fmt.Errorf("failed to load account: %w", err) + } + + var newBalance decimal.Decimal + switch existing.Type { + case model.TransactionTypeWithdrawal: + newBalance = account.Balance.Add(existing.Value) + case model.TransactionTypeDeposit: + newBalance = account.Balance.Sub(existing.Value) + default: + return nil, fmt.Errorf("unsupported transaction type: %s", existing.Type) + } + + if err := s.transactionRepo.DeleteAtomic(existing.ID, existing.AccountID, newBalance); err != nil { + return nil, fmt.Errorf("failed to delete transaction: %w", err) + } + + s.auditSvc.Record(TransactionRecordOptions{ + TransactionID: existing.ID, + ActorID: input.ActorID, + Action: model.TransactionAuditActionDeleted, + Metadata: map[string]any{ + "account_id": existing.AccountID, + "transaction_type": string(existing.Type), + "title": existing.Title, + "amount": existing.Value.StringFixedBank(2), + }, + }) + + return existing, nil +} + // diffTransactionFields returns a map of field name to {old, new} for fields whose // new value differs from the existing transaction. func diffTransactionFields(existing *model.Transaction, newTitle string, newAmount decimal.Decimal, newOccurredAt time.Time, newDescription *string) map[string]any { diff --git a/internal/service/transaction_test.go b/internal/service/transaction_test.go index 14741bc..75e18e6 100644 --- a/internal/service/transaction_test.go +++ b/internal/service/transaction_test.go @@ -481,6 +481,187 @@ func TestTransactionService_Update_RejectsTransferTransactions(t *testing.T) { }) } +func TestTransactionService_DeleteTransaction_Bill_CreditsBalance(t *testing.T) { + testutil.ForEachDB(t, func(t *testing.T, dbi testutil.DBInfo) { + f := newTxnFixture(t, dbi) + + // Seed 100 then pay a 30 bill, leaving balance at 70. + _, err := f.svc.Deposit(DepositInput{ + AccountID: f.account.ID, Title: "seed", Amount: decimal.NewFromInt(100), + OccurredAt: time.Now(), ActorID: f.user.ID, + }) + require.NoError(t, err) + bill, err := f.svc.PayBill(PayBillInput{ + AccountID: f.account.ID, Title: "Cable", Amount: decimal.NewFromInt(30), + OccurredAt: time.Now(), ActorID: f.user.ID, + }) + require.NoError(t, err) + + updated, err := f.accounts.ByID(f.account.ID) + require.NoError(t, err) + require.True(t, decimal.NewFromInt(70).Equal(updated.Balance)) + + deleted, err := f.svc.DeleteTransaction(DeleteTransactionInput{ + TransactionID: bill.ID, + ActorID: f.user.ID, + }) + require.NoError(t, err) + assert.Equal(t, bill.ID, deleted.ID) + + // 70 + 30 = 100 (credit back). + updated, err = f.accounts.ByID(f.account.ID) + require.NoError(t, err) + assert.True(t, decimal.NewFromInt(100).Equal(updated.Balance)) + + // Transaction is gone. + _, err = f.svc.GetTransaction(bill.ID) + assert.Error(t, err) + + // Audit log records the deletion (created + deleted, newest first). + logs, err := f.txAudit.ListByTransaction(bill.ID, 10, 0) + require.NoError(t, err) + require.Len(t, logs, 2) + assert.Equal(t, model.TransactionAuditActionDeleted, logs[0].Action) + + var meta map[string]any + require.NoError(t, json.Unmarshal(logs[0].Metadata, &meta)) + assert.Equal(t, "withdrawal", meta["transaction_type"]) + assert.Equal(t, f.account.ID, meta["account_id"]) + assert.Equal(t, "Cable", meta["title"]) + assert.Equal(t, "30.00", meta["amount"]) + }) +} + +func TestTransactionService_DeleteTransaction_Deposit_DebitsBalance(t *testing.T) { + testutil.ForEachDB(t, func(t *testing.T, dbi testutil.DBInfo) { + f := newTxnFixture(t, dbi) + + dep, err := f.svc.Deposit(DepositInput{ + AccountID: f.account.ID, Title: "Paycheck", Amount: decimal.NewFromInt(150), + OccurredAt: time.Now(), ActorID: f.user.ID, + }) + require.NoError(t, err) + + updated, err := f.accounts.ByID(f.account.ID) + require.NoError(t, err) + require.True(t, decimal.NewFromInt(150).Equal(updated.Balance)) + + _, err = f.svc.DeleteTransaction(DeleteTransactionInput{ + TransactionID: dep.ID, + ActorID: f.user.ID, + }) + require.NoError(t, err) + + // 150 - 150 = 0. + updated, err = f.accounts.ByID(f.account.ID) + require.NoError(t, err) + assert.True(t, decimal.Zero.Equal(updated.Balance)) + + logs, err := f.txAudit.ListByTransaction(dep.ID, 10, 0) + require.NoError(t, err) + require.Len(t, logs, 2) + assert.Equal(t, model.TransactionAuditActionDeleted, logs[0].Action) + var meta map[string]any + require.NoError(t, json.Unmarshal(logs[0].Metadata, &meta)) + assert.Equal(t, "deposit", meta["transaction_type"]) + }) +} + +func TestTransactionService_DeleteTransaction_RejectsTransferHalves(t *testing.T) { + testutil.ForEachDB(t, func(t *testing.T, dbi testutil.DBInfo) { + f := newTxnFixture(t, dbi) + dest := testutil.CreateTestAccount(t, dbi.DB, f.account.SpaceID, "Savings") + + result, err := f.svc.Transfer(TransferInput{ + SourceAccountID: f.account.ID, + DestAccountID: dest.ID, + Title: "Initial", + Amount: decimal.NewFromInt(20), + OccurredAt: time.Now(), + ActorID: f.user.ID, + }) + require.NoError(t, err) + + _, err = f.svc.DeleteTransaction(DeleteTransactionInput{ + TransactionID: result.Withdrawal.ID, + ActorID: f.user.ID, + }) + require.ErrorIs(t, err, ErrTransactionPartOfTransfer) + + _, err = f.svc.DeleteTransaction(DeleteTransactionInput{ + TransactionID: result.Deposit.ID, + ActorID: f.user.ID, + }) + require.ErrorIs(t, err, ErrTransactionPartOfTransfer) + + // Both halves still exist with untouched balances and no extra audit rows. + _, err = f.svc.GetTransaction(result.Withdrawal.ID) + require.NoError(t, err) + _, err = f.svc.GetTransaction(result.Deposit.ID) + require.NoError(t, err) + + src, err := f.accounts.ByID(f.account.ID) + require.NoError(t, err) + assert.True(t, decimal.NewFromInt(-20).Equal(src.Balance)) + dst, err := f.accounts.ByID(dest.ID) + require.NoError(t, err) + assert.True(t, decimal.NewFromInt(20).Equal(dst.Balance)) + + count, err := f.txAudit.CountByTransaction(result.Withdrawal.ID) + require.NoError(t, err) + assert.Equal(t, 1, count) + }) +} + +func TestTransactionService_DeleteTransaction_RemovesCategoryLink(t *testing.T) { + testutil.ForEachDB(t, func(t *testing.T, dbi testutil.DBInfo) { + f := newTxnFixture(t, dbi) + + categories, err := f.svc.ListCategories() + require.NoError(t, err) + require.NotEmpty(t, categories, "expected at least one seeded category") + categoryID := categories[0].ID + + _, err = f.svc.Deposit(DepositInput{ + AccountID: f.account.ID, Title: "seed", Amount: decimal.NewFromInt(100), + OccurredAt: time.Now(), ActorID: f.user.ID, + }) + require.NoError(t, err) + bill, err := f.svc.PayBill(PayBillInput{ + AccountID: f.account.ID, + Title: "Groceries", + Amount: decimal.NewFromInt(40), + OccurredAt: time.Now(), + CategoryID: categoryID, + ActorID: f.user.ID, + }) + require.NoError(t, err) + + got, err := f.svc.GetTransactionCategoryID(bill.ID) + require.NoError(t, err) + require.Equal(t, categoryID, got) + + _, err = f.svc.DeleteTransaction(DeleteTransactionInput{ + TransactionID: bill.ID, + ActorID: f.user.ID, + }) + require.NoError(t, err) + + // transaction_categories cascades on the FK; the link is gone. + got, err = f.svc.GetTransactionCategoryID(bill.ID) + require.NoError(t, err) + assert.Equal(t, "", got) + }) +} + +func TestTransactionService_DeleteTransaction_RequiresID(t *testing.T) { + testutil.ForEachDB(t, func(t *testing.T, dbi testutil.DBInfo) { + f := newTxnFixture(t, dbi) + _, err := f.svc.DeleteTransaction(DeleteTransactionInput{ActorID: f.user.ID}) + assert.Error(t, err) + }) +} + func TestTransactionService_Validations(t *testing.T) { testutil.ForEachDB(t, func(t *testing.T, dbi testutil.DBInfo) { f := newTxnFixture(t, dbi) diff --git a/internal/ui/pages/space_transaction.templ b/internal/ui/pages/space_transaction.templ index d031e7b..d6be94e 100644 --- a/internal/ui/pages/space_transaction.templ +++ b/internal/ui/pages/space_transaction.templ @@ -4,6 +4,7 @@ import "git.juancwu.dev/juancwu/budgit/internal/model" import "git.juancwu.dev/juancwu/budgit/internal/routeurl" import "git.juancwu.dev/juancwu/budgit/internal/ui/components/button" import "git.juancwu.dev/juancwu/budgit/internal/ui/components/card" +import "git.juancwu.dev/juancwu/budgit/internal/ui/components/dialog" import "git.juancwu.dev/juancwu/budgit/internal/ui/components/icon" import "git.juancwu.dev/juancwu/budgit/internal/ui/layouts" import "git.juancwu.dev/juancwu/budgit/internal/ui/utils" @@ -56,14 +57,58 @@ templ SpaceTransactionPage(props SpaceTransactionPageProps) {

if props.RelatedTransaction == nil { - @button.Button(button.Props{ - Variant: button.VariantDefault, - Href: routeurl.URL("page.app.spaces.space.accounts.account.transactions.transaction.edit", "spaceID", props.SpaceID, "accountID", props.AccountID, "transactionID", props.Transaction.ID), - Class: "flex items-center gap-2", - }) { - @icon.Pencil(icon.Props{Class: "size-4"}) - Edit - } +
+ @button.Button(button.Props{ + Variant: button.VariantDefault, + Href: routeurl.URL("page.app.spaces.space.accounts.account.transactions.transaction.edit", "spaceID", props.SpaceID, "accountID", props.AccountID, "transactionID", props.Transaction.ID), + Class: "flex items-center gap-2", + }) { + @icon.Pencil(icon.Props{Class: "size-4"}) + Edit + } + @dialog.Dialog() { + @dialog.Trigger() { + @button.Button(button.Props{ + Variant: button.VariantDestructive, + Class: "flex items-center gap-2", + }) { + @icon.Trash2(icon.Props{Class: "size-4"}) + Delete + } + } + @dialog.Content() { + @dialog.Header() { + @dialog.Title() { + Delete { props.Transaction.Title }? + } + @dialog.Description() { + if props.Transaction.Type == model.TransactionTypeDeposit { + This permanently removes the deposit and debits the amount from the account balance. + } else { + This permanently removes the bill and credits the amount back to the account balance. + } + } + } + @dialog.Footer(dialog.FooterProps{Class: "mt-2"}) { + @dialog.Close() { + @button.Button(button.Props{Variant: button.VariantOutline}) { + Cancel + } + } +
+ @button.Button(button.Props{ + Type: button.TypeSubmit, + Variant: button.VariantDestructive, + Class: "flex gap-2 items-center", + }) { + @icon.Trash2(icon.Props{Class: "size-4"}) + Delete + } +
+ } + } + } +
} @card.Card() {