From 0a8115b14964285be1a8f914982c4b41e895fd29 Mon Sep 17 00:00:00 2001 From: Wilfred Asomani Date: Wed, 12 Jul 2023 17:32:49 +0000 Subject: [PATCH] satellite/{console,payments}: fix handling for autofreeze flow This change adds an extra step to the auto freeze chore to attempt payment before freezing/warning a user. It also attempts payment after modifying user's cards whether the user is frozen/warned or not. Issue: https://github.com/storj/storj-private/issues/341 Change-Id: Ia9c0c5a2d37837bca5153fe720fef61f1385cb15 --- .../console/consoleweb/consoleapi/payments.go | 17 +++-- satellite/payments/accountfreeze/chore.go | 24 +++++++ .../payments/accountfreeze/chore_test.go | 66 ++++++++++++++++++- satellite/payments/stripe/stripemock.go | 26 +++++++- 4 files changed, 120 insertions(+), 13 deletions(-) diff --git a/satellite/console/consoleweb/consoleapi/payments.go b/satellite/console/consoleweb/consoleapi/payments.go index 81462d1e2..25d2b92b4 100644 --- a/satellite/console/consoleweb/consoleapi/payments.go +++ b/satellite/console/consoleweb/consoleapi/payments.go @@ -155,8 +155,8 @@ func (p *Payments) ProjectsCharges(w http.ResponseWriter, r *http.Request) { } } -// triggerAttemptPaymentIfFrozenOrWarned checks if the account is frozen and if frozen, will trigger attempt to pay outstanding invoices. -func (p *Payments) triggerAttemptPaymentIfFrozenOrWarned(ctx context.Context) (err error) { +// triggerAttemptPayment attempts payment and unfreezes/unwarn user if needed. +func (p *Payments) triggerAttemptPayment(ctx context.Context) (err error) { defer mon.Task()(&ctx)(&err) userID, err := p.service.GetUserID(ctx) @@ -169,12 +169,11 @@ func (p *Payments) triggerAttemptPaymentIfFrozenOrWarned(ctx context.Context) (e return err } - if freeze != nil || warning != nil { - err = p.service.Payments().AttemptPayOverdueInvoices(ctx) - if err != nil { - return err - } + err = p.service.Payments().AttemptPayOverdueInvoices(ctx) + if err != nil { + return err } + if freeze != nil { err = p.accountFreezeService.UnfreezeUser(ctx, userID) if err != nil { @@ -214,7 +213,7 @@ func (p *Payments) AddCreditCard(w http.ResponseWriter, r *http.Request) { return } - err = p.triggerAttemptPaymentIfFrozenOrWarned(ctx) + err = p.triggerAttemptPayment(ctx) if err != nil { p.serveJSONError(ctx, w, http.StatusInternalServerError, err) return @@ -274,7 +273,7 @@ func (p *Payments) MakeCreditCardDefault(w http.ResponseWriter, r *http.Request) return } - err = p.triggerAttemptPaymentIfFrozenOrWarned(ctx) + err = p.triggerAttemptPayment(ctx) if err != nil { p.serveJSONError(ctx, w, http.StatusInternalServerError, err) return diff --git a/satellite/payments/accountfreeze/chore.go b/satellite/payments/accountfreeze/chore.go index 0cf91a833..2e1b880e9 100644 --- a/satellite/payments/accountfreeze/chore.go +++ b/satellite/payments/accountfreeze/chore.go @@ -170,6 +170,30 @@ func (chore *Chore) Run(ctx context.Context) (err error) { errorLog("Could not get freeze status", err) continue } + + // try to pay the invoice before freezing/warning. + err = chore.payments.Invoices().AttemptPayOverdueInvoices(ctx, userID) + if err == nil { + debugLog("Ignoring invoice; Payment attempt successful") + + if warning != nil { + err = chore.freezeService.UnWarnUser(ctx, userID) + if err != nil { + errorLog("Could not remove warning event", err) + } + } + if freeze != nil { + err = chore.freezeService.UnfreezeUser(ctx, userID) + if err != nil { + errorLog("Could not remove freeze event", err) + } + } + + continue + } else { + errorLog("Could not attempt payment", err) + } + if freeze != nil { debugLog("Ignoring invoice; account already frozen") continue diff --git a/satellite/payments/accountfreeze/chore_test.go b/satellite/payments/accountfreeze/chore_test.go index 27cb34bb5..8a544e930 100644 --- a/satellite/payments/accountfreeze/chore_test.go +++ b/satellite/payments/accountfreeze/chore_test.go @@ -113,7 +113,7 @@ func TestAutoFreezeChore(t *testing.T) { require.Nil(t, warning) }) - t.Run("Freeze event for failed invoice", func(t *testing.T) { + t.Run("Freeze event for failed invoice (failed later payment attempt)", func(t *testing.T) { // AnalyticsMock tests that events are sent once. service.TestChangeFreezeTracker(newFreezeTrackerMock(t)) // reset chore clock @@ -171,6 +171,68 @@ func TestAutoFreezeChore(t *testing.T) { freeze, _, err = service.GetAll(ctx, user.ID) require.NoError(t, err) require.NotNil(t, freeze) + + // Pay invoice so it doesn't show up in the next test. + inv, err = stripeClient.Invoices().Pay(inv.ID, &stripe.InvoicePayParams{ + Params: stripe.Params{Context: ctx}, + PaymentMethod: stripe.String(stripe1.MockInvoicesPaySuccess), + }) + require.NoError(t, err) + require.Equal(t, stripe.InvoiceStatusPaid, inv.Status) + + // unfreeze user so they're not frozen in the next test. + err = service.UnfreezeUser(ctx, user.ID) + require.NoError(t, err) + }) + + t.Run("No freeze event for failed invoice (successful later payment attempt)", func(t *testing.T) { + // AnalyticsMock tests that events are sent once. + service.TestChangeFreezeTracker(newFreezeTrackerMock(t)) + // reset chore clock + chore.TestSetNow(time.Now) + + item, err := stripeClient.InvoiceItems().New(&stripe.InvoiceItemParams{ + Params: stripe.Params{Context: ctx}, + Amount: &amount, + Currency: &curr, + Customer: &cus1, + }) + require.NoError(t, err) + + items := make([]*stripe.InvoiceUpcomingInvoiceItemParams, 0, 1) + items = append(items, &stripe.InvoiceUpcomingInvoiceItemParams{ + InvoiceItem: &item.ID, + Amount: &amount, + Currency: &curr, + }) + inv, err := stripeClient.Invoices().New(&stripe.InvoiceParams{ + Params: stripe.Params{Context: ctx}, + Customer: &cus1, + InvoiceItems: items, + DefaultPaymentMethod: stripe.String(stripe1.MockInvoicesPaySuccess), + }) + require.NoError(t, err) + + inv, err = stripeClient.Invoices().FinalizeInvoice(inv.ID, nil) + require.NoError(t, err) + require.Equal(t, stripe.InvoiceStatusOpen, inv.Status) + + failed, err := invoicesDB.ListFailed(ctx) + require.NoError(t, err) + require.Equal(t, 1, len(failed)) + require.Equal(t, inv.ID, failed[0].ID) + + chore.Loop.TriggerWait() + + // Payment should have succeeded in the chore. + failed, err = invoicesDB.ListFailed(ctx) + require.NoError(t, err) + require.Equal(t, 0, len(failed)) + + freeze, warning, err := service.GetAll(ctx, user.ID) + require.NoError(t, err) + require.Nil(t, warning) + require.Nil(t, freeze) }) t.Run("Storjscan exceptions", func(t *testing.T) { @@ -237,7 +299,7 @@ func TestAutoFreezeChore(t *testing.T) { failed, err := invoicesDB.ListFailed(ctx) require.NoError(t, err) - require.Equal(t, 2, len(failed)) + require.Equal(t, 1, len(failed)) invFound := false for _, failedInv := range failed { if failedInv.ID == inv.ID { diff --git a/satellite/payments/stripe/stripemock.go b/satellite/payments/stripe/stripemock.go index 21b43e62c..186b3c682 100644 --- a/satellite/payments/stripe/stripemock.go +++ b/satellite/payments/stripe/stripemock.go @@ -576,6 +576,10 @@ func (m *mockInvoices) New(params *stripe.InvoiceParams) (*stripe.Invoice, error }, AmountDue: amountDue, AmountRemaining: amountDue, + Total: amountDue, + } + if params.DefaultPaymentMethod != nil { + invoice.DefaultPaymentMethod = &stripe.PaymentMethod{ID: *params.DefaultPaymentMethod} } m.invoices[*params.Customer] = append(m.invoices[*params.Customer], invoice) @@ -608,6 +612,15 @@ func (m *mockInvoices) List(listParams *stripe.InvoiceListParams) *invoice.Iter } } } + } else if listParams.Customer != nil && listParams.Status != nil { + // filter by status and customer + for _, invoices := range m.invoices { + for _, inv := range invoices { + if inv.Status == stripe.InvoiceStatus(*listParams.Status) && inv.Customer.ID == *listParams.Customer { + ret = append(ret, inv) + } + } + } } else if listParams.Customer == nil { for _, invoices := range m.invoices { for _, invoice := range invoices { @@ -640,10 +653,9 @@ func (m *mockInvoices) Update(id string, params *stripe.InvoiceParams) (invoice // FinalizeInvoice forwards the invoice's status from draft to open. func (m *mockInvoices) FinalizeInvoice(id string, params *stripe.InvoiceFinalizeParams) (*stripe.Invoice, error) { for _, invoices := range m.invoices { - for i, invoice := range invoices { + for _, invoice := range invoices { if invoice.ID == id && invoice.Status == stripe.InvoiceStatusDraft { invoice.Status = stripe.InvoiceStatusOpen - m.invoices[invoice.Customer.ID][i].Status = stripe.InvoiceStatusOpen return invoice, nil } } @@ -665,6 +677,16 @@ func (m *mockInvoices) Pay(id string, params *stripe.InvoicePayParams) (*stripe. invoice.AmountRemaining = 0 return invoice, nil } + } else if invoice.DefaultPaymentMethod != nil { + if invoice.DefaultPaymentMethod.ID == MockInvoicesPaySuccess { + invoice.Status = stripe.InvoiceStatusPaid + invoice.AmountRemaining = 0 + return invoice, nil + } + if invoice.DefaultPaymentMethod.ID == MockInvoicesNewFailure { + invoice.Status = stripe.InvoiceStatusOpen + return invoice, &stripe.Error{} + } } else if invoice.AmountRemaining == 0 || (params.PaidOutOfBand != nil && *params.PaidOutOfBand) { invoice.Status = stripe.InvoiceStatusPaid invoice.AmountRemaining = 0