Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions docs/bug-fixes.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,57 @@ Each bug fix entry should include:

## Bug Fixes

### 2026-05-31: Amazon charge validation incorrectly rejects orders with non-reducing non-bank entries

**Description:**
`itemize amazon -dry-run` emitted `WARN: Charge validation failed ... bank charges ($68.86) exceed expected ($61.87) by $6.99` and skipped the order. The $6.99 was a non-bank transaction entry (promotional credit / reward earned) that appears in Amazon's transaction list with no card digits but does NOT actually reduce the card charge. The validator subtracted it from the order total to compute `expectedSum`, making the real bank charge look like an overcharge.

**Test Case:**
```go
// validator/charges_test.go: TestValidateCharges_BankMatchesOrderTotalWithNonBankEntry
// bankCharges=[68.86], orderTotal=68.86, nonBankAmount=6.99
// Expected: valid (bank charge equals full order total)
// Actual before fix: invalid ("bank charges exceed expected by $6.99")
```

**Root Cause:**
`ValidateCharges` only checked `bankSum ≈ orderTotal - nonBankAmount`. When a non-bank entry doesn't reduce the card charge (e.g. rewards earned, promotional accounting), the formula underestimates what the bank should receive.

**Fix Applied:**
Added a secondary check in `ValidateCharges`: if `bankSum ≈ orderTotal` (within 2¢ tolerance), the validation passes regardless of the non-bank amount. The bank charged the full order total, which is a valid scenario.

**Verification:**
- `TestValidateCharges_BankMatchesOrderTotalWithNonBankEntry` now passes.
- All existing validator tests continue to pass.

---

### 2026-05-31: Amazon gift-card-only orders cause handler error instead of skip

**Description:**
`itemize amazon -dry-run` emitted `ERROR: Handler error order_id=112-4444156-8489869 error=failed to get bank charges: no bank charges found (order paid entirely with gift cards/points)`. Orders paid entirely with gift cards/points have no bank transaction in Monarch to match, so they should be silently skipped — not crash the handler.

**Test Case:**
```go
// handlers/amazon_test.go: TestAmazonHandler_ProcessOrder_FullyGiftCardOrder
// GetFinalCharges() returns ErrGiftCardOrder
// Expected: result.Skipped=true, no error returned
// Actual before fix: returned a fatal error, logged as ERROR
```

**Root Cause:**
`GetFinalCharges()` returned a plain `fmt.Errorf(...)` for this case instead of a sentinel error. The handler only checked `errors.Is(err, ErrPaymentPending)` and treated everything else as a fatal error.

**Fix Applied:**
Introduced `ErrGiftCardOrder` sentinel in `amazon/order.go` and updated `GetFinalCharges()` to return it. The handler now checks `errors.Is(err, ErrGiftCardOrder)` and skips the order with reason `"paid entirely with gift cards/points"`.

**Verification:**
- `TestAmazonHandler_ProcessOrder_FullyGiftCardOrder` now passes.
- `TestOrder_GetFinalCharges_OnlyGiftCard` updated to use `errors.Is(err, ErrGiftCardOrder)`.
- All existing tests continue to pass.

---

### 2026-05-24: Costco discount applied by description instead of item number

**Description:**
Expand Down
7 changes: 5 additions & 2 deletions internal/adapters/providers/amazon/order.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package amazon

import (
"errors"
"fmt"
"log/slog"
"time"

Expand All @@ -12,6 +11,10 @@ import (
// ErrPaymentPending indicates an order has no bank charges yet because it hasn't shipped
var ErrPaymentPending = errors.New("payment pending: order has not been charged yet (awaiting shipment)")

// ErrGiftCardOrder indicates an order was paid entirely with gift cards or points —
// no bank transaction exists in Monarch to match against.
var ErrGiftCardOrder = errors.New("no bank charges found (order paid entirely with gift cards/points)")

// Order wraps a ParsedOrder to implement the providers.Order interface
type Order struct {
parsedOrder *ParsedOrder
Expand Down Expand Up @@ -137,7 +140,7 @@ func (o *Order) GetFinalCharges() ([]float64, error) {
if len(bankCharges) == 0 {
if hasNonBankPayments {
// Order was paid entirely with gift cards/points - no bank transaction to match
return nil, fmt.Errorf("no bank charges found (order paid entirely with gift cards/points)")
return nil, ErrGiftCardOrder
}
// No bank charges and no non-bank payments processed yet - still pending
return nil, ErrPaymentPending
Expand Down
3 changes: 1 addition & 2 deletions internal/adapters/providers/amazon/order_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,8 @@ func TestOrder_GetFinalCharges_OnlyGiftCard(t *testing.T) {
order := NewOrder(parsedOrder, nil)

charges, err := order.GetFinalCharges()
assert.Error(t, err, "Should return error when no bank charges found")
assert.ErrorIs(t, err, ErrGiftCardOrder, "Should return ErrGiftCardOrder when no bank charges found")
assert.Nil(t, charges)
assert.Contains(t, err.Error(), "paid entirely with gift cards/points")
}

func TestOrder_GetFinalCharges_NoTransactions_ReturnsPending(t *testing.T) {
Expand Down
14 changes: 11 additions & 3 deletions internal/application/sync/handlers/amazon.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,24 @@ func (h *AmazonHandler) ProcessOrder(
// Step 1: Get bank charges
bankCharges, err := order.GetFinalCharges()
if err != nil {
// Check if this is a pending payment (order not yet shipped/charged)
if errors.Is(err, amazonprovider.ErrPaymentPending) {
switch {
case errors.Is(err, amazonprovider.ErrPaymentPending):
h.logInfo("Order payment pending (not yet shipped)",
"order_id", order.GetID(),
"order_total", order.GetTotal())
result.Skipped = true
result.SkipReason = "payment pending"
return result, nil
case errors.Is(err, amazonprovider.ErrGiftCardOrder):
h.logInfo("Order paid entirely with gift cards/points — no bank transaction to match",
"order_id", order.GetID(),
"order_total", order.GetTotal())
result.Skipped = true
result.SkipReason = "paid entirely with gift cards/points"
return result, nil
default:
return nil, fmt.Errorf("failed to get bank charges: %w", err)
}
return nil, fmt.Errorf("failed to get bank charges: %w", err)
}

h.logDebug("Got bank charges",
Expand Down
29 changes: 29 additions & 0 deletions internal/application/sync/handlers/amazon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/eshaffer321/monarchmoney-go/pkg/monarch"
"github.com/eshaffer321/monarchmoney-sync-backend/internal/adapters/providers"
amazonprovider "github.com/eshaffer321/monarchmoney-sync-backend/internal/adapters/providers/amazon"
"github.com/eshaffer321/monarchmoney-sync-backend/internal/domain/categorizer"
"github.com/eshaffer321/monarchmoney-sync-backend/internal/domain/matcher"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -403,6 +404,34 @@ func TestAmazonHandler_ProcessOrder_WithGiftCard(t *testing.T) {
assert.InDelta(t, 100.00, result.Allocations.TotalAllocated, 0.01)
}

func TestAmazonHandler_ProcessOrder_FullyGiftCardOrder(t *testing.T) {
// Reproduces order 112-4444156-8489869:
// Order paid entirely with gift cards/points — no bank transaction exists in Monarch.
// Handler must skip (not error) so the sync continues.
order := &mockAmazonOrder{
id: "112-4444156-8489869",
date: time.Now(),
total: 50.00,
items: []providers.OrderItem{&mockItem{name: "Item", price: 50.00}},
bankChargesErr: amazonprovider.ErrGiftCardOrder,
}

handler := NewAmazonHandler(nil, nil, nil, nil, nil)

result, err := handler.ProcessOrder(
context.Background(),
order,
nil,
make(map[string]bool),
nil, nil,
false,
)

require.NoError(t, err)
assert.True(t, result.Skipped)
assert.Contains(t, result.SkipReason, "gift card")
}

func TestAllocatedItem(t *testing.T) {
item := &allocatedItem{
name: "Test Item",
Expand Down
14 changes: 14 additions & 0 deletions internal/domain/validator/charges.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,20 @@ func ValidateCharges(bankCharges []float64, orderTotal, nonBankAmount float64) *
}
}

// Also valid when bank charges match the full order total.
// Some non-bank entries (promotional credits, rewards earned) appear in the
// transaction list with no card digits but do NOT reduce the actual card charge.
diffFromTotal := roundToCents(bankSum - orderTotal)
if math.Abs(diffFromTotal) <= tolerance {
return &ChargeValidation{
Valid: true,
BankChargesSum: bankSum,
ExpectedSum: expectedSum,
Difference: diff,
Reason: "",
}
}

// Validation failed - determine reason
var reason string
if diff < 0 {
Expand Down
15 changes: 15 additions & 0 deletions internal/domain/validator/charges_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,21 @@ func TestValidateCharges_RoundingEdgeCases(t *testing.T) {
}
}

func TestValidateCharges_BankMatchesOrderTotalWithNonBankEntry(t *testing.T) {
// Reproduces order 113-6125291-9439466:
// Bank charged full order total ($68.86) but there's a $6.99 non-bank
// accounting entry (promotional credit / rewards earned) that does NOT
// actually reduce the card charge. Validation must pass.
bankCharges := []float64{68.86}
orderTotal := 68.86
nonBankAmount := 6.99

result := ValidateCharges(bankCharges, orderTotal, nonBankAmount)

assert.True(t, result.Valid, "Should pass when bank charges equal order total regardless of non-bank accounting entries")
assert.Equal(t, 68.86, result.BankChargesSum)
}

func TestValidateCharges_ReasonMessages(t *testing.T) {
t.Run("missing charge has helpful message", func(t *testing.T) {
result := ValidateCharges([]float64{50.00}, 100.00, 0.0)
Expand Down