diff --git a/docs/bug-fixes.md b/docs/bug-fixes.md index 25d7662..92d50e1 100644 --- a/docs/bug-fixes.md +++ b/docs/bug-fixes.md @@ -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:** diff --git a/internal/adapters/providers/amazon/order.go b/internal/adapters/providers/amazon/order.go index e19542d..7ae8f67 100644 --- a/internal/adapters/providers/amazon/order.go +++ b/internal/adapters/providers/amazon/order.go @@ -2,7 +2,6 @@ package amazon import ( "errors" - "fmt" "log/slog" "time" @@ -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 @@ -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 diff --git a/internal/adapters/providers/amazon/order_test.go b/internal/adapters/providers/amazon/order_test.go index dda79d4..9c89e1b 100644 --- a/internal/adapters/providers/amazon/order_test.go +++ b/internal/adapters/providers/amazon/order_test.go @@ -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) { diff --git a/internal/application/sync/handlers/amazon.go b/internal/application/sync/handlers/amazon.go index 078e88d..46ec0bd 100644 --- a/internal/application/sync/handlers/amazon.go +++ b/internal/application/sync/handlers/amazon.go @@ -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", diff --git a/internal/application/sync/handlers/amazon_test.go b/internal/application/sync/handlers/amazon_test.go index d33938b..bb8763c 100644 --- a/internal/application/sync/handlers/amazon_test.go +++ b/internal/application/sync/handlers/amazon_test.go @@ -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" @@ -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", diff --git a/internal/domain/validator/charges.go b/internal/domain/validator/charges.go index 8a0aee1..b42c1f0 100644 --- a/internal/domain/validator/charges.go +++ b/internal/domain/validator/charges.go @@ -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 { diff --git a/internal/domain/validator/charges_test.go b/internal/domain/validator/charges_test.go index 9d911cb..505f6d5 100644 --- a/internal/domain/validator/charges_test.go +++ b/internal/domain/validator/charges_test.go @@ -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)