From ed70a03844b420fbe1e3dfb0d95bcdd58e597a51 Mon Sep 17 00:00:00 2001 From: Wilfred Asomani Date: Thu, 23 Mar 2023 12:04:32 +0000 Subject: [PATCH] satellite/{console,db,analytics}: better warning handling This handles cases where a user is warned and triggers payment for their account. Previously, only a frozen account will trigger this payment, and will be unfrozen on successful payment. Now, accounts in warning state trigger payments and are removed from that state on successful payment. Issue: https://github.com/storj/storj/issues/5691 Change-Id: Icc2107f5d256657d176d8b0dd0a43a470eb01277 --- satellite/analytics/service.go | 17 +++++ satellite/console/accountfreezes.go | 50 ++++++++++--- satellite/console/accountfreezes_test.go | 34 +++++++++ .../console/consoleweb/consoleapi/payments.go | 20 ++++-- .../payments/stripecoinpayments/invoices.go | 7 +- satellite/satellitedb/accountfreezeevents.go | 12 ++++ satellite/satellitedb/dbx/satellitedb.dbx.go | 72 +++++++++++++++++++ satellite/satellitedb/dbx/user.dbx | 5 ++ 8 files changed, 197 insertions(+), 20 deletions(-) diff --git a/satellite/analytics/service.go b/satellite/analytics/service.go index 87f86b9c1..9a0d6780f 100644 --- a/satellite/analytics/service.go +++ b/satellite/analytics/service.go @@ -81,6 +81,7 @@ const ( eventProjectBandwidthLimitUpdated = "Project Bandwidth Limit Updated" eventAccountFrozen = "Account Frozen" eventAccountUnfrozen = "Account Unfrozen" + eventAccountUnwarned = "Account Unwarned" eventAccountFreezeWarning = "Account Freeze Warning" eventUnpaidLargeInvoice = "Large Invoice Unpaid" ) @@ -339,6 +340,22 @@ func (service *Service) TrackAccountUnfrozen(userID uuid.UUID, email string) { }) } +// TrackAccountUnwarned sends an account unwarned event to Segment. +func (service *Service) TrackAccountUnwarned(userID uuid.UUID, email string) { + if !service.config.Enabled { + return + } + + props := segment.NewProperties() + props.Set("email", email) + + service.enqueueMessage(segment.Track{ + UserId: userID.String(), + Event: service.satelliteName + " " + eventAccountUnwarned, + Properties: props, + }) +} + // TrackAccountFreezeWarning sends an account freeze warning event to Segment. func (service *Service) TrackAccountFreezeWarning(userID uuid.UUID, email string) { if !service.config.Enabled { diff --git a/satellite/console/accountfreezes.go b/satellite/console/accountfreezes.go index a082fda64..ac27527b9 100644 --- a/satellite/console/accountfreezes.go +++ b/satellite/console/accountfreezes.go @@ -27,9 +27,11 @@ type AccountFreezeEvents interface { // Get is a method for querying account freeze event from the database by user ID and event type. Get(ctx context.Context, userID uuid.UUID, eventType AccountFreezeEventType) (*AccountFreezeEvent, error) // GetAll is a method for querying all account freeze events from the database by user ID. - GetAll(ctx context.Context, userID uuid.UUID) (*AccountFreezeEvent, *AccountFreezeEvent, error) + GetAll(ctx context.Context, userID uuid.UUID) (freeze *AccountFreezeEvent, warning *AccountFreezeEvent, err error) // DeleteAllByUserID is a method for deleting all account freeze events from the database by user ID. DeleteAllByUserID(ctx context.Context, userID uuid.UUID) error + // DeleteByUserIDAndEvent is a method for deleting all account `eventType` events from the database by user ID. + DeleteByUserIDAndEvent(ctx context.Context, userID uuid.UUID, eventType AccountFreezeEventType) error } // AccountFreezeEvent represents an event related to account freezing. @@ -98,9 +100,18 @@ func (s *AccountFreezeService) FreezeUser(ctx context.Context, userID uuid.UUID) return ErrAccountFreeze.Wrap(err) } - event, err := s.freezeEventsDB.Get(ctx, userID, Freeze) - if errors.Is(err, sql.ErrNoRows) { - event = &AccountFreezeEvent{ + freeze, warning, err := s.freezeEventsDB.GetAll(ctx, userID) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + if warning != nil { + err = s.freezeEventsDB.DeleteByUserIDAndEvent(ctx, userID, Warning) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + } + if freeze == nil { + freeze = &AccountFreezeEvent{ UserID: userID, Type: Freeze, Limits: &AccountFreezeEventLimits{ @@ -112,8 +123,6 @@ func (s *AccountFreezeService) FreezeUser(ctx context.Context, userID uuid.UUID) Projects: make(map[uuid.UUID]UsageLimits), }, } - } else if err != nil { - return ErrAccountFreeze.Wrap(err) } userLimits := UsageLimits{ @@ -123,7 +132,7 @@ func (s *AccountFreezeService) FreezeUser(ctx context.Context, userID uuid.UUID) } // If user limits have been zeroed already, we should not override what is in the freeze table. if userLimits != (UsageLimits{}) { - event.Limits.User = userLimits + freeze.Limits.User = userLimits } projects, err := s.projectsDB.GetOwn(ctx, userID) @@ -143,11 +152,11 @@ func (s *AccountFreezeService) FreezeUser(ctx context.Context, userID uuid.UUID) } // If project limits have been zeroed already, we should not override what is in the freeze table. if projLimits != (UsageLimits{}) { - event.Limits.Projects[p.ID] = projLimits + freeze.Limits.Projects[p.ID] = projLimits } } - _, err = s.freezeEventsDB.Upsert(ctx, event) + _, err = s.freezeEventsDB.Upsert(ctx, freeze) if err != nil { return ErrAccountFreeze.Wrap(err) } @@ -228,6 +237,29 @@ func (s *AccountFreezeService) WarnUser(ctx context.Context, userID uuid.UUID) ( return nil } +// UnWarnUser reverses the warning placed on the user specified by the given ID. +func (s *AccountFreezeService) UnWarnUser(ctx context.Context, userID uuid.UUID) (err error) { + defer mon.Task()(&ctx)(&err) + + user, err := s.usersDB.Get(ctx, userID) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + + _, err = s.freezeEventsDB.Get(ctx, userID, Warning) + if errors.Is(err, sql.ErrNoRows) { + return ErrAccountFreeze.New("user is not warned") + } + + err = ErrAccountFreeze.Wrap(s.freezeEventsDB.DeleteByUserIDAndEvent(ctx, userID, Warning)) + if err != nil { + return err + } + + s.analytics.TrackAccountUnwarned(userID, user.Email) + return nil +} + // GetAll returns all events for a user. func (s *AccountFreezeService) GetAll(ctx context.Context, userID uuid.UUID) (freeze *AccountFreezeEvent, warning *AccountFreezeEvent, err error) { defer mon.Task()(&ctx)(&err) diff --git a/satellite/console/accountfreezes_test.go b/satellite/console/accountfreezes_test.go index 9628f8bac..82f9bd8d8 100644 --- a/satellite/console/accountfreezes_test.go +++ b/satellite/console/accountfreezes_test.go @@ -115,6 +115,40 @@ func TestAccountUnfreeze(t *testing.T) { }) } +func TestRemoveAccountWarning(t *testing.T) { + testplanet.Run(t, testplanet.Config{ + SatelliteCount: 1, + }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { + sat := planet.Satellites[0] + usersDB := sat.DB.Console().Users() + projectsDB := sat.DB.Console().Projects() + service := console.NewAccountFreezeService(sat.DB.Console().AccountFreezeEvents(), usersDB, projectsDB, sat.API.Analytics.Service) + + user, err := sat.AddUser(ctx, console.CreateUser{ + FullName: "Test User", + Email: "user@mail.test", + }, 2) + require.NoError(t, err) + + require.NoError(t, service.WarnUser(ctx, user.ID)) + require.NoError(t, service.UnWarnUser(ctx, user.ID)) + + freeze, warning, err := service.GetAll(ctx, user.ID) + require.NoError(t, err) + require.Nil(t, warning) + require.Nil(t, freeze) + + require.NoError(t, service.WarnUser(ctx, user.ID)) + require.NoError(t, service.FreezeUser(ctx, user.ID)) + + freeze, warning, err = service.GetAll(ctx, user.ID) + require.NoError(t, err) + require.NotNil(t, freeze) + // freezing should remove prior warning events. + require.Nil(t, warning) + }) +} + func TestAccountFreezeAlreadyFrozen(t *testing.T) { testplanet.Run(t, testplanet.Config{ SatelliteCount: 1, diff --git a/satellite/console/consoleweb/consoleapi/payments.go b/satellite/console/consoleweb/consoleapi/payments.go index 56cb80de1..a7fda2b02 100644 --- a/satellite/console/consoleweb/consoleapi/payments.go +++ b/satellite/console/consoleweb/consoleapi/payments.go @@ -136,8 +136,8 @@ func (p *Payments) ProjectsCharges(w http.ResponseWriter, r *http.Request) { } } -// triggerAttemptPaymentIfFrozen checks if the account is frozen and if frozen, will trigger attempt to pay outstanding invoices. -func (p *Payments) triggerAttemptPaymentIfFrozen(ctx context.Context) (err error) { +// 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) { defer mon.Task()(&ctx)(&err) userID, err := p.service.GetUserID(ctx) @@ -145,21 +145,27 @@ func (p *Payments) triggerAttemptPaymentIfFrozen(ctx context.Context) (err error return err } - isFrozen, err := p.accountFreezeService.IsUserFrozen(ctx, userID) + freeze, warning, err := p.accountFreezeService.GetAll(ctx, userID) if err != nil { return err } - if isFrozen { + if freeze != nil || warning != nil { err = p.service.Payments().AttemptPayOverdueInvoices(ctx) if err != nil { return err } - + } + if freeze != nil { err = p.accountFreezeService.UnfreezeUser(ctx, userID) if err != nil { return err } + } else if warning != nil { + err = p.accountFreezeService.UnWarnUser(ctx, userID) + if err != nil { + return err + } } return nil } @@ -189,7 +195,7 @@ func (p *Payments) AddCreditCard(w http.ResponseWriter, r *http.Request) { return } - err = p.triggerAttemptPaymentIfFrozen(ctx) + err = p.triggerAttemptPaymentIfFrozenOrWarned(ctx) if err != nil { p.serveJSONError(w, http.StatusInternalServerError, err) return @@ -249,7 +255,7 @@ func (p *Payments) MakeCreditCardDefault(w http.ResponseWriter, r *http.Request) return } - err = p.triggerAttemptPaymentIfFrozen(ctx) + err = p.triggerAttemptPaymentIfFrozenOrWarned(ctx) if err != nil { p.serveJSONError(w, http.StatusInternalServerError, err) return diff --git a/satellite/payments/stripecoinpayments/invoices.go b/satellite/payments/stripecoinpayments/invoices.go index 0d601dba0..0917cee9b 100644 --- a/satellite/payments/stripecoinpayments/invoices.go +++ b/satellite/payments/stripecoinpayments/invoices.go @@ -83,10 +83,9 @@ func (invoices *invoices) AttemptPayOverdueInvoices(ctx context.Context, userID } params := &stripe.InvoiceListParams{ - ListParams: stripe.ListParams{Context: ctx}, - Customer: &customerID, - Status: stripe.String(string(stripe.InvoiceStatusOpen)), - DueDateRange: &stripe.RangeQueryParams{LesserThan: time.Now().Unix()}, + ListParams: stripe.ListParams{Context: ctx}, + Customer: &customerID, + Status: stripe.String(string(stripe.InvoiceStatusOpen)), } var errGrp errs.Group diff --git a/satellite/satellitedb/accountfreezeevents.go b/satellite/satellitedb/accountfreezeevents.go index 480033fbe..2b727f655 100644 --- a/satellite/satellitedb/accountfreezeevents.go +++ b/satellite/satellitedb/accountfreezeevents.go @@ -103,6 +103,18 @@ func (events *accountFreezeEvents) DeleteAllByUserID(ctx context.Context, userID return err } +// DeleteByUserIDAndEvent is a method for deleting all account `eventType` events from the database by user ID. +func (events *accountFreezeEvents) DeleteByUserIDAndEvent(ctx context.Context, userID uuid.UUID, eventType console.AccountFreezeEventType) (err error) { + defer mon.Task()(&ctx)(&err) + + _, err = events.db.Delete_AccountFreezeEvent_By_UserId_And_Event(ctx, + dbx.AccountFreezeEvent_UserId(userID.Bytes()), + dbx.AccountFreezeEvent_Event(int(eventType)), + ) + + return err +} + // fromDBXAccountFreezeEvent converts *dbx.AccountFreezeEvent to *console.AccountFreezeEvent. func fromDBXAccountFreezeEvent(dbxEvent *dbx.AccountFreezeEvent) (_ *console.AccountFreezeEvent, err error) { if dbxEvent == nil { diff --git a/satellite/satellitedb/dbx/satellitedb.dbx.go b/satellite/satellitedb/dbx/satellitedb.dbx.go index aa4670d13..d8734f15b 100644 --- a/satellite/satellitedb/dbx/satellitedb.dbx.go +++ b/satellite/satellitedb/dbx/satellitedb.dbx.go @@ -19044,6 +19044,34 @@ func (obj *pgxImpl) Delete_AccountFreezeEvent_By_UserId(ctx context.Context, } +func (obj *pgxImpl) Delete_AccountFreezeEvent_By_UserId_And_Event(ctx context.Context, + account_freeze_event_user_id AccountFreezeEvent_UserId_Field, + account_freeze_event_event AccountFreezeEvent_Event_Field) ( + deleted bool, err error) { + defer mon.Task()(&ctx)(&err) + + var __embed_stmt = __sqlbundle_Literal("DELETE FROM account_freeze_events WHERE account_freeze_events.user_id = ? AND account_freeze_events.event = ?") + + var __values []interface{} + __values = append(__values, account_freeze_event_user_id.value(), account_freeze_event_event.value()) + + var __stmt = __sqlbundle_Render(obj.dialect, __embed_stmt) + obj.logStmt(__stmt, __values...) + + __res, err := obj.driver.ExecContext(ctx, __stmt, __values...) + if err != nil { + return false, obj.makeErr(err) + } + + __count, err := __res.RowsAffected() + if err != nil { + return false, obj.makeErr(err) + } + + return __count > 0, nil + +} + func (impl pgxImpl) isConstraintError(err error) ( constraint string, ok bool) { if e, ok := err.(*pgconn.PgError); ok { @@ -26688,6 +26716,34 @@ func (obj *pgxcockroachImpl) Delete_AccountFreezeEvent_By_UserId(ctx context.Con } +func (obj *pgxcockroachImpl) Delete_AccountFreezeEvent_By_UserId_And_Event(ctx context.Context, + account_freeze_event_user_id AccountFreezeEvent_UserId_Field, + account_freeze_event_event AccountFreezeEvent_Event_Field) ( + deleted bool, err error) { + defer mon.Task()(&ctx)(&err) + + var __embed_stmt = __sqlbundle_Literal("DELETE FROM account_freeze_events WHERE account_freeze_events.user_id = ? AND account_freeze_events.event = ?") + + var __values []interface{} + __values = append(__values, account_freeze_event_user_id.value(), account_freeze_event_event.value()) + + var __stmt = __sqlbundle_Render(obj.dialect, __embed_stmt) + obj.logStmt(__stmt, __values...) + + __res, err := obj.driver.ExecContext(ctx, __stmt, __values...) + if err != nil { + return false, obj.makeErr(err) + } + + __count, err := __res.RowsAffected() + if err != nil { + return false, obj.makeErr(err) + } + + return __count > 0, nil + +} + func (impl pgxcockroachImpl) isConstraintError(err error) ( constraint string, ok bool) { if e, ok := err.(*pgconn.PgError); ok { @@ -27881,6 +27937,17 @@ func (rx *Rx) Delete_AccountFreezeEvent_By_UserId(ctx context.Context, } +func (rx *Rx) Delete_AccountFreezeEvent_By_UserId_And_Event(ctx context.Context, + account_freeze_event_user_id AccountFreezeEvent_UserId_Field, + account_freeze_event_event AccountFreezeEvent_Event_Field) ( + deleted bool, err error) { + var tx *Tx + if tx, err = rx.getTx(ctx); err != nil { + return + } + return tx.Delete_AccountFreezeEvent_By_UserId_And_Event(ctx, account_freeze_event_user_id, account_freeze_event_event) +} + func (rx *Rx) Delete_ApiKey_By_Id(ctx context.Context, api_key_id ApiKey_Id_Field) ( deleted bool, err error) { @@ -29477,6 +29544,11 @@ type Methods interface { account_freeze_event_user_id AccountFreezeEvent_UserId_Field) ( count int64, err error) + Delete_AccountFreezeEvent_By_UserId_And_Event(ctx context.Context, + account_freeze_event_user_id AccountFreezeEvent_UserId_Field, + account_freeze_event_event AccountFreezeEvent_Event_Field) ( + deleted bool, err error) + Delete_ApiKey_By_Id(ctx context.Context, api_key_id ApiKey_Id_Field) ( deleted bool, err error) diff --git a/satellite/satellitedb/dbx/user.dbx b/satellite/satellitedb/dbx/user.dbx index a25a01855..c9e0c65af 100644 --- a/satellite/satellitedb/dbx/user.dbx +++ b/satellite/satellitedb/dbx/user.dbx @@ -225,6 +225,11 @@ update account_freeze_event ( delete account_freeze_event ( where account_freeze_event.user_id = ? ) +delete account_freeze_event ( + where account_freeze_event.user_id = ? + where account_freeze_event.event = ? +) + // user_settings table is used to persist user preferences. model user_settings ( key user_id