From 64abf2f3a85e40470d3811b4696be4a55272c053 Mon Sep 17 00:00:00 2001 From: Erick Shaffer Date: Thu, 1 Jan 2026 20:14:35 -0700 Subject: [PATCH] feat: Add pending status for Amazon orders awaiting shipment Orders that haven't shipped yet (no bank charges) are now tracked as "pending" instead of "failed". This provides clearer status reporting and ensures these orders are retried on subsequent syncs. Changes: - Add ErrPaymentPending sentinel error for orders not yet charged - Amazon handler detects pending orders and returns skip with reason - Orchestrator records pending status (not error) and logs at INFO level - UI already supports pending status with amber badge; added filter option The pending status is recorded in the database but doesn't block retries since IsProcessed() only checks for status='success'. --- internal/adapters/providers/amazon/order.go | 16 ++++++- .../adapters/providers/amazon/order_test.go | 36 ++++++++++++++- .../providers/amazon/provider_test.go | 8 ++-- internal/application/sync/handlers/amazon.go | 10 ++++ internal/application/sync/orchestrator.go | 5 +- internal/application/sync/recording.go | 46 ++++++++++++++----- web/src/app/(app)/orders/page.tsx | 1 + 7 files changed, 103 insertions(+), 19 deletions(-) diff --git a/internal/adapters/providers/amazon/order.go b/internal/adapters/providers/amazon/order.go index 6f01e63..e19542d 100644 --- a/internal/adapters/providers/amazon/order.go +++ b/internal/adapters/providers/amazon/order.go @@ -1,6 +1,7 @@ package amazon import ( + "errors" "fmt" "log/slog" "time" @@ -8,6 +9,9 @@ import ( "github.com/eshaffer321/monarchmoney-sync-backend/internal/adapters/providers" ) +// 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)") + // Order wraps a ParsedOrder to implement the providers.Order interface type Order struct { parsedOrder *ParsedOrder @@ -83,10 +87,12 @@ func (o *Order) GetRawData() interface{} { // Filters out non-bank transactions like gift cards, points, etc. func (o *Order) GetFinalCharges() ([]float64, error) { if len(o.parsedOrder.Transactions) == 0 { - return nil, fmt.Errorf("no transactions found for order") + // No transactions at all - order hasn't been charged yet (awaiting shipment) + return nil, ErrPaymentPending } var bankCharges []float64 + var hasNonBankPayments bool for _, tx := range o.parsedOrder.Transactions { // Skip refunds if tx.Type == "refund" { @@ -107,6 +113,7 @@ func (o *Order) GetFinalCharges() ([]float64, error) { // Real bank charges have Last4 populated (card ending digits) // Points, gift cards, etc. have empty Last4 if tx.Last4 == "" { + hasNonBankPayments = true if o.logger != nil { o.logger.Debug("Skipping non-bank transaction", "order_id", o.GetID(), @@ -128,7 +135,12 @@ func (o *Order) GetFinalCharges() ([]float64, error) { } if len(bankCharges) == 0 { - return nil, fmt.Errorf("no bank charges found (order may be paid with gift cards/points only)") + 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)") + } + // No bank charges and no non-bank payments processed yet - still pending + return nil, ErrPaymentPending } return bankCharges, nil diff --git a/internal/adapters/providers/amazon/order_test.go b/internal/adapters/providers/amazon/order_test.go index d62ccb7..dda79d4 100644 --- a/internal/adapters/providers/amazon/order_test.go +++ b/internal/adapters/providers/amazon/order_test.go @@ -72,7 +72,41 @@ func TestOrder_GetFinalCharges_OnlyGiftCard(t *testing.T) { charges, err := order.GetFinalCharges() assert.Error(t, err, "Should return error when no bank charges found") assert.Nil(t, charges) - assert.Contains(t, err.Error(), "no bank charges found") + assert.Contains(t, err.Error(), "paid entirely with gift cards/points") +} + +func TestOrder_GetFinalCharges_NoTransactions_ReturnsPending(t *testing.T) { + // Test order with no transactions (not yet shipped/charged) + parsedOrder := &ParsedOrder{ + ID: "test-pending-order", + Date: time.Now(), + Total: 50.00, + Transactions: []*ParsedTransaction{}, // Empty - not charged yet + } + + order := NewOrder(parsedOrder, nil) + + charges, err := order.GetFinalCharges() + assert.Error(t, err, "Should return error when no transactions") + assert.Nil(t, charges) + assert.ErrorIs(t, err, ErrPaymentPending, "Should return ErrPaymentPending for orders not yet charged") +} + +func TestOrder_GetFinalCharges_NilTransactions_ReturnsPending(t *testing.T) { + // Test order with nil transactions slice + parsedOrder := &ParsedOrder{ + ID: "test-pending-order-nil", + Date: time.Now(), + Total: 50.00, + Transactions: nil, // Nil - not charged yet + } + + order := NewOrder(parsedOrder, nil) + + charges, err := order.GetFinalCharges() + assert.Error(t, err, "Should return error when no transactions") + assert.Nil(t, charges) + assert.ErrorIs(t, err, ErrPaymentPending, "Should return ErrPaymentPending for orders not yet charged") } func TestOrder_GetFinalCharges_SkipsRefunds(t *testing.T) { diff --git a/internal/adapters/providers/amazon/provider_test.go b/internal/adapters/providers/amazon/provider_test.go index df1db06..dc8ed9a 100644 --- a/internal/adapters/providers/amazon/provider_test.go +++ b/internal/adapters/providers/amazon/provider_test.go @@ -188,7 +188,7 @@ func TestNewProvider_NilLogger(t *testing.T) { assert.NotNil(t, provider.logger) } -// TestOrder_GetFinalCharges_NoTransactions tests GetFinalCharges returns error without transactions +// TestOrder_GetFinalCharges_NoTransactions tests GetFinalCharges returns ErrPaymentPending without transactions func TestOrder_GetFinalCharges_NoTransactions(t *testing.T) { parsedOrder := &ParsedOrder{ ID: "114-0000000-0000000", @@ -201,10 +201,10 @@ func TestOrder_GetFinalCharges_NoTransactions(t *testing.T) { assert.Error(t, err) assert.Nil(t, charges) - assert.Contains(t, err.Error(), "no transactions found") + assert.ErrorIs(t, err, ErrPaymentPending, "Should return ErrPaymentPending for orders without transactions") } -// TestOrder_IsMultiDelivery_NoTransactions tests IsMultiDelivery returns error without transactions +// TestOrder_IsMultiDelivery_NoTransactions tests IsMultiDelivery returns ErrPaymentPending without transactions func TestOrder_IsMultiDelivery_NoTransactions(t *testing.T) { parsedOrder := &ParsedOrder{ ID: "114-0000000-0000000", @@ -217,7 +217,7 @@ func TestOrder_IsMultiDelivery_NoTransactions(t *testing.T) { assert.Error(t, err) assert.False(t, isMulti) - assert.Contains(t, err.Error(), "no transactions found") + assert.ErrorIs(t, err, ErrPaymentPending, "Should return ErrPaymentPending for orders without transactions") } // TestCalculateLookbackDays tests lookback days calculation diff --git a/internal/application/sync/handlers/amazon.go b/internal/application/sync/handlers/amazon.go index c2bd9c9..b7e6eca 100644 --- a/internal/application/sync/handlers/amazon.go +++ b/internal/application/sync/handlers/amazon.go @@ -3,6 +3,7 @@ package handlers import ( "context" + "errors" "fmt" "log/slog" "math" @@ -125,6 +126,15 @@ 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) { + 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 + } return nil, fmt.Errorf("failed to get bank charges: %w", err) } diff --git a/internal/application/sync/orchestrator.go b/internal/application/sync/orchestrator.go index bc9fc87..26e5762 100644 --- a/internal/application/sync/orchestrator.go +++ b/internal/application/sync/orchestrator.go @@ -19,15 +19,18 @@ func (o *Orchestrator) handleResult(order providers.Order, result *handlers.Proc return false, false, err } if result.Skipped { - o.logger.Warn("Order skipped", "order_id", order.GetID(), "reason", result.SkipReason) // Don't treat "payment pending" as an error - it's expected for new orders if result.SkipReason == "payment pending" { + o.logger.Info("Order pending (awaiting shipment/charge)", "order_id", order.GetID()) + o.recordPending(order, result.SkipReason) return false, true, nil } // Don't treat "already has splits" as an error - just skip silently if result.SkipReason == "transaction already has splits" { + o.logger.Debug("Order skipped (already has splits)", "order_id", order.GetID()) return false, true, nil } + o.logger.Warn("Order skipped", "order_id", order.GetID(), "reason", result.SkipReason) o.recordError(order, result.SkipReason) return false, false, fmt.Errorf("skipped: %s", result.SkipReason) } diff --git a/internal/application/sync/recording.go b/internal/application/sync/recording.go index 2405532..d473b7d 100644 --- a/internal/application/sync/recording.go +++ b/internal/application/sync/recording.go @@ -51,18 +51,18 @@ func convertSplits(splits []*monarch.TransactionSplit) []storage.SplitDetail { func (o *Orchestrator) recordError(order providers.Order, errorMsg string) { if o.storage != nil { record := &storage.ProcessingRecord{ - OrderID: order.GetID(), - Provider: order.GetProviderName(), - OrderDate: order.GetDate(), - OrderTotal: order.GetTotal(), + OrderID: order.GetID(), + Provider: order.GetProviderName(), + OrderDate: order.GetDate(), + OrderTotal: order.GetTotal(), OrderSubtotal: order.GetSubtotal(), - OrderTax: order.GetTax(), - OrderTip: order.GetTip(), - ItemCount: len(order.GetItems()), - ProcessedAt: time.Now(), - Status: "failed", - ErrorMessage: errorMsg, - Items: convertOrderItems(order.GetItems()), + OrderTax: order.GetTax(), + OrderTip: order.GetTip(), + ItemCount: len(order.GetItems()), + ProcessedAt: time.Now(), + Status: "failed", + ErrorMessage: errorMsg, + Items: convertOrderItems(order.GetItems()), } if err := o.storage.SaveRecord(record); err != nil { o.logger.Error("Failed to save error record", "order_id", order.GetID(), "error", err) @@ -70,6 +70,30 @@ func (o *Orchestrator) recordError(order providers.Order, errorMsg string) { } } +// recordPending records an order that is pending (not yet charged/shipped) +// This allows tracking without blocking retries on future syncs +func (o *Orchestrator) recordPending(order providers.Order, reason string) { + if o.storage != nil { + record := &storage.ProcessingRecord{ + OrderID: order.GetID(), + Provider: order.GetProviderName(), + OrderDate: order.GetDate(), + OrderTotal: order.GetTotal(), + OrderSubtotal: order.GetSubtotal(), + OrderTax: order.GetTax(), + OrderTip: order.GetTip(), + ItemCount: len(order.GetItems()), + ProcessedAt: time.Now(), + Status: "pending", + ErrorMessage: reason, + Items: convertOrderItems(order.GetItems()), + } + if err := o.storage.SaveRecord(record); err != nil { + o.logger.Error("Failed to save pending record", "order_id", order.GetID(), "error", err) + } + } +} + // recordSuccess records a successful processing to storage func (o *Orchestrator) recordSuccess(order providers.Order, transaction *monarch.Transaction, splits []*monarch.TransactionSplit, confidence float64, dryRun bool) { o.recordSuccessWithMultiDelivery(order, transaction, splits, confidence, dryRun, nil) diff --git a/web/src/app/(app)/orders/page.tsx b/web/src/app/(app)/orders/page.tsx index fd95998..584ce87 100644 --- a/web/src/app/(app)/orders/page.tsx +++ b/web/src/app/(app)/orders/page.tsx @@ -139,6 +139,7 @@ export default async function OrdersPage({ searchParams }: PageProps) { +