diff --git a/satellite/console/consoleweb/consoleapi/payments.go b/satellite/console/consoleweb/consoleapi/payments.go index a7fda2b02..c73e1856e 100644 --- a/satellite/console/consoleweb/consoleapi/payments.go +++ b/satellite/console/consoleweb/consoleapi/payments.go @@ -499,12 +499,6 @@ func (p *Payments) PurchasePackage(w http.ResponseWriter, r *http.Request) { return } - _, err = p.service.Payments().ApplyCoupon(ctx, pkg.CouponID) - if err != nil { - p.serveJSONError(w, http.StatusInternalServerError, err) - return - } - card, err := p.service.Payments().AddCreditCard(ctx, token) if err != nil { switch { @@ -516,8 +510,22 @@ func (p *Payments) PurchasePackage(w http.ResponseWriter, r *http.Request) { return } - err = p.service.Payments().Purchase(ctx, pkg.Price, fmt.Sprintf("%s package plan", string(u.UserAgent)), card.ID) + description := fmt.Sprintf("%s package plan", string(u.UserAgent)) + err = p.service.Payments().UpdatePackage(ctx, description, time.Now()) if err != nil { + if !console.ErrAlreadyHasPackage.Has(err) { + p.serveJSONError(w, http.StatusInternalServerError, err) + return + } + } + + err = p.service.Payments().Purchase(ctx, pkg.Price, description, card.ID) + if err != nil { + p.serveJSONError(w, http.StatusInternalServerError, err) + return + } + + if err = p.service.Payments().ApplyCredit(ctx, pkg.Credit, description); err != nil { p.serveJSONError(w, http.StatusInternalServerError, err) return } diff --git a/satellite/console/consoleweb/consoleapi/payments_test.go b/satellite/console/consoleweb/consoleapi/payments_test.go index 4d6b1ee3f..54eeda3a5 100644 --- a/satellite/console/consoleweb/consoleapi/payments_test.go +++ b/satellite/console/consoleweb/consoleapi/payments_test.go @@ -25,7 +25,6 @@ import ( func Test_PurchasePackage(t *testing.T) { partner := "partner1" - partner2 := "partner2" testplanet.Run(t, testplanet.Config{ SatelliteCount: 1, StorageNodeCount: 0, UplinkCount: 1, @@ -35,8 +34,7 @@ func Test_PurchasePackage(t *testing.T) { config.Console.RateLimit.Burst = 10 config.Payments.StripeCoinPayments.StripeFreeTierCouponID = stripecoinpayments.MockCouponID1 config.Payments.PackagePlans.Packages = map[string]payments.PackagePlan{ - partner: {CouponID: stripecoinpayments.MockCouponID2, Price: 1000}, - partner2: {CouponID: "invalidCouponID", Price: 1000}, + partner: {Credit: 2000, Price: 1000}, } }, }, @@ -53,12 +51,6 @@ func Test_PurchasePackage(t *testing.T) { "No matching package plan for partner", validCardToken, "unknownPartner", http.StatusNotFound, }, - { - // partner2's coupon ID configured above in Reconfigure does not exist in underlying - // stipe mock coupons list. - "Coupon doesn't exist", validCardToken, partner2, - http.StatusInternalServerError, - }, { "Add credit card fails", stripecoinpayments.TestPaymentMethodsNewFailure, partner, http.StatusInternalServerError, @@ -71,6 +63,10 @@ func Test_PurchasePackage(t *testing.T) { "Success", validCardToken, partner, http.StatusOK, }, + { + "Subsequent request succeeds", validCardToken, partner, + http.StatusOK, + }, } for i, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -88,7 +84,7 @@ func Test_PurchasePackage(t *testing.T) { tokenInfo, err := sat.API.Console.Service.Token(ctx, console.AuthUser{Email: user.Email, Password: user.FullName}) require.NoError(t, err) - req, err := http.NewRequestWithContext(userCtx, "POST", "http://"+planet.Satellites[0].API.Console.Listener.Addr().String()+"/api/v0/payments/purchase-package", strings.NewReader(tt.cardToken)) + req, err := http.NewRequestWithContext(userCtx, "POST", "http://"+sat.API.Console.Listener.Addr().String()+"/api/v0/payments/purchase-package", strings.NewReader(tt.cardToken)) require.NoError(t, err) expire := time.Now().AddDate(0, 0, 1) @@ -119,7 +115,7 @@ func Test_PackageAvailable(t *testing.T) { Reconfigure: testplanet.Reconfigure{ Satellite: func(log *zap.Logger, index int, config *satellite.Config) { config.Payments.PackagePlans.Packages = map[string]payments.PackagePlan{ - pkgPartner: {CouponID: stripecoinpayments.MockCouponID1, Price: 1000}, + pkgPartner: {Credit: 2000, Price: 1000}, } }, }, diff --git a/satellite/console/service.go b/satellite/console/service.go index b639ca6b3..f3b1bb137 100644 --- a/satellite/console/service.go +++ b/satellite/console/service.go @@ -3109,6 +3109,8 @@ func (payment Payments) WalletPayments(ctx context.Context) (_ WalletPayments, e } // Purchase makes a purchase of `price` amount with description of `desc` and payment method with id of `paymentMethodID`. +// If a paid invoice with the same description exists, then we assume this is a retried request and don't create and pay +// another invoice. func (payment Payments) Purchase(ctx context.Context, price int64, desc string, paymentMethodID string) (err error) { defer mon.Task()(&ctx)(&err) @@ -3127,8 +3129,12 @@ func (payment Payments) Purchase(ctx context.Context, price int64, desc string, // check for any previously created unpaid invoice with the same description. // If draft, delete it and create new and pay. If open, pay it and don't create new. + // If paid, skip. for _, inv := range invoices { if inv.Description == desc { + if inv.Status == payments.InvoiceStatusPaid { + return nil + } if inv.Status == payments.InvoiceStatusDraft { _, err := payment.service.accounts.Invoices().Delete(ctx, inv.ID) if err != nil { diff --git a/satellite/console/service_test.go b/satellite/console/service_test.go index d758096be..066a95eb2 100644 --- a/satellite/console/service_test.go +++ b/satellite/console/service_test.go @@ -1656,7 +1656,7 @@ func TestPaymentsPurchasePreexistingInvoice(t *testing.T) { userCtx, err := sat.UserContext(ctx, user.ID) require.NoError(t, err) - testDesc := "testDescription" + draftInvDesc := "testDraftDescription" testPaymentMethod := "testPaymentMethod" invs, err := sat.API.Payments.StripeService.Accounts().Invoices().List(ctx, user.ID) @@ -1664,7 +1664,7 @@ func TestPaymentsPurchasePreexistingInvoice(t *testing.T) { require.Len(t, invs, 0) // test purchase with draft invoice - inv, err := sat.API.Payments.StripeService.Accounts().Invoices().Create(ctx, user.ID, 1000, testDesc) + inv, err := sat.API.Payments.StripeService.Accounts().Invoices().Create(ctx, user.ID, 1000, draftInvDesc) require.NoError(t, err) require.Equal(t, payments.InvoiceStatusDraft, inv.Status) @@ -1675,7 +1675,7 @@ func TestPaymentsPurchasePreexistingInvoice(t *testing.T) { require.Len(t, invs, 1) require.Equal(t, draftInv, invs[0].ID) - require.NoError(t, p.Purchase(userCtx, 1000, testDesc, testPaymentMethod)) + require.NoError(t, p.Purchase(userCtx, 1000, draftInvDesc, testPaymentMethod)) invs, err = sat.API.Payments.StripeService.Accounts().Invoices().List(ctx, user.ID) require.NoError(t, err) @@ -1684,7 +1684,8 @@ func TestPaymentsPurchasePreexistingInvoice(t *testing.T) { require.Equal(t, payments.InvoiceStatusPaid, invs[0].Status) // test purchase with open invoice - inv, err = sat.API.Payments.StripeService.Accounts().Invoices().Create(ctx, user.ID, 1000, testDesc) + openInvDesc := "testOpenDescription" + inv, err = sat.API.Payments.StripeService.Accounts().Invoices().Create(ctx, user.ID, 1000, openInvDesc) require.NoError(t, err) openInv := inv.ID @@ -1705,7 +1706,7 @@ func TestPaymentsPurchasePreexistingInvoice(t *testing.T) { } require.True(t, foundInv) - require.NoError(t, p.Purchase(userCtx, 1000, testDesc, testPaymentMethod)) + require.NoError(t, p.Purchase(userCtx, 1000, openInvDesc, testPaymentMethod)) invs, err = sat.API.Payments.StripeService.Accounts().Invoices().List(ctx, user.ID) require.NoError(t, err) @@ -1718,6 +1719,13 @@ func TestPaymentsPurchasePreexistingInvoice(t *testing.T) { } } require.True(t, foundInv) + + // purchase with paid invoice skips creating and or paying invoice + require.NoError(t, p.Purchase(userCtx, 1000, openInvDesc, testPaymentMethod)) + + invs, err = sat.API.Payments.StripeService.Accounts().Invoices().List(ctx, user.ID) + require.NoError(t, err) + require.Len(t, invs, 2) }) } diff --git a/satellite/payments/balance.go b/satellite/payments/balance.go index ca38d15b1..eabd38d3a 100644 --- a/satellite/payments/balance.go +++ b/satellite/payments/balance.go @@ -40,3 +40,10 @@ type BalanceTransaction struct { Amount int64 Description string } + +// PackagePlan is an amount to charge a user one time in exchange for credit of greater value. +// Price and Credit are in cents USD. +type PackagePlan struct { + Price int64 + Credit int64 +} diff --git a/satellite/payments/coupons.go b/satellite/payments/coupons.go index bb343630c..5fa0f4e87 100644 --- a/satellite/payments/coupons.go +++ b/satellite/payments/coupons.go @@ -68,10 +68,3 @@ const ( // SignupCoupon represents a valid promo code coupon. SignupCoupon = "signupCoupon" ) - -// PackagePlan is an amount to charge a user one time in exchange for a coupon of greater value. -// Price is in cents USD. -type PackagePlan struct { - CouponID string - Price int64 -} diff --git a/satellite/payments/paymentsconfig/config.go b/satellite/payments/paymentsconfig/config.go index 97e369b76..3357da666 100644 --- a/satellite/payments/paymentsconfig/config.go +++ b/satellite/payments/paymentsconfig/config.go @@ -37,7 +37,7 @@ type Config struct { NodeAuditBandwidthPrice int64 `help:"price node receive for storing TB of audit in cents" default:"1000"` NodeDiskSpacePrice int64 `help:"price node receive for storing disk space in cents/TB" default:"150"` UsagePriceOverrides ProjectUsagePriceOverrides `help:"semicolon-separated usage price overrides in the format partner:storage,egress,segment"` - PackagePlans PackagePlans `help:"semicolon-separated partner package plans in the format partner:couponID,price. Price is in cents USD."` + PackagePlans PackagePlans `help:"semicolon-separated partner package plans in the format partner:price,credit. Price and credit are in cents USD."` } // ProjectUsagePrice holds the configuration for the satellite's project usage price model. @@ -172,7 +172,7 @@ func (p *PackagePlans) String() string { var s strings.Builder left := len(p.Packages) for partner, pkg := range p.Packages { - s.WriteString(fmt.Sprintf("%s:%s,%d", partner, pkg.CouponID, pkg.Price)) + s.WriteString(fmt.Sprintf("%s:%d,%d", partner, pkg.Price, pkg.Credit)) left-- if left > 0 { s.WriteRune(';') @@ -191,7 +191,7 @@ func (p *PackagePlans) Set(s string) error { info := strings.Split(packagePlansStr, ":") if len(info) != 2 { - return Error.New("Invalid package plan (expected format partner:couponID,price got %s)", packagePlansStr) + return Error.New("Invalid package plan (expected format partner:price,credit got %s)", packagePlansStr) } partner := strings.TrimSpace(info[0]) @@ -202,21 +202,26 @@ func (p *PackagePlans) Set(s string) error { packageStr := info[1] pkg := strings.Split(packageStr, ",") if len(pkg) != 2 || pkg[0] == "" { - return Error.New("Invalid package (expected format couponID,price got %s)", packageStr) + return Error.New("Invalid package (expected format price,credit got %s)", packageStr) } if _, err := decimal.NewFromString(pkg[1]); err != nil { return Error.New("Invalid price (%s)", err) } - cents, err := strconv.Atoi(pkg[1]) + priceCents, err := strconv.Atoi(pkg[0]) + if err != nil { + return Error.Wrap(err) + } + + creditCents, err := strconv.Atoi(pkg[1]) if err != nil { return Error.Wrap(err) } packages[info[0]] = payments.PackagePlan{ - CouponID: pkg[0], - Price: int64(cents), + Price: int64(priceCents), + Credit: int64(creditCents), } } p.Packages = packages diff --git a/satellite/payments/paymentsconfig/config_test.go b/satellite/payments/paymentsconfig/config_test.go index 99b141ea3..7b9c31bbd 100644 --- a/satellite/payments/paymentsconfig/config_test.go +++ b/satellite/payments/paymentsconfig/config_test.go @@ -120,43 +120,43 @@ func TestPackagePlans(t *testing.T) { }, { testID: "missing partner", - configValue: ":abc123,100", - }, { - testID: "empty coupon ID", - configValue: "partner:,1", + configValue: ":100,100", }, { testID: "empty price", - configValue: "partner:abc123,", + configValue: "partner:,100", + }, { + testID: "empty credit", + configValue: "partner:100,", }, { testID: "too few values", - configValue: "partner:abc123", + configValue: "partner:100", }, { testID: "too many values", - configValue: "partner:abc123,100,200", + configValue: "partner:100,100,200", }, { testID: "single package plan", - configValue: "partner1:abc123,100", + configValue: "partner1:100,200", expectedPackagePlans: packages{ "partner1": payments.PackagePlan{ - CouponID: "abc123", - Price: 100, + Price: 100, + Credit: 200, }, }, }, { testID: "multiple package plans", - configValue: "partner1:abc123,100;partner2:321bca,200", + configValue: "partner1:100,200;partner2:200,300", expectedPackagePlans: packages{ "partner1": payments.PackagePlan{ - CouponID: "abc123", - Price: 100, + Price: 100, + Credit: 200, }, "partner2": payments.PackagePlan{ - CouponID: "321bca", - Price: 200, + Price: 200, + Credit: 300, }, }, }, @@ -188,9 +188,9 @@ func TestPackagePlans(t *testing.T) { func TestPackagePlansGet(t *testing.T) { partner := "partnerName1" - coupon := "abc123" + credit := int64(200) price := int64(100) - configStr := fmt.Sprintf("%s:%s,%d", partner, coupon, price) + configStr := fmt.Sprintf("%s:%d,%d", partner, price, credit) packagePlans := paymentsconfig.PackagePlans{} require.NoError(t, packagePlans.Set(configStr)) @@ -231,7 +231,7 @@ func TestPackagePlansGet(t *testing.T) { p, err := packagePlans.Get(c.userAgent) if c.shouldPass { require.NoError(t, err) - require.Equal(t, coupon, p.CouponID) + require.Equal(t, credit, p.Credit) require.Equal(t, price, p.Price) } else { require.Error(t, err) diff --git a/satellite/payments/stripecoinpayments/coupons.go b/satellite/payments/stripecoinpayments/coupons.go index a090c1939..800f20fc1 100644 --- a/satellite/payments/stripecoinpayments/coupons.go +++ b/satellite/payments/stripecoinpayments/coupons.go @@ -66,24 +66,6 @@ func (coupons *coupons) ApplyCoupon(ctx context.Context, userID uuid.UUID, coupo func (coupons *coupons) ApplyCouponCode(ctx context.Context, userID uuid.UUID, couponCode string) (_ *payments.Coupon, err error) { defer mon.Task()(&ctx, userID, couponCode)(&err) - user, err := coupons.service.usersDB.Get(ctx, userID) - if err != nil { - return nil, Error.Wrap(err) - } - - if user.UserAgent != nil { - partner := string(user.UserAgent) - if plan, ok := coupons.service.packagePlans[partner]; ok { - coupon, err := coupons.GetByUserID(ctx, userID) - if err != nil { - return nil, err - } - if coupon != nil && coupon.ID == plan.CouponID { - return nil, payments.ErrCouponConflict.New("coupon for partner '%s' should not be replaced", partner) - } - } - } - promoCodeIter := coupons.service.stripeClient.PromoCodes().List(&stripe.PromotionCodeListParams{ ListParams: stripe.ListParams{Context: ctx}, Code: stripe.String(couponCode), diff --git a/satellite/payments/stripecoinpayments/coupons_test.go b/satellite/payments/stripecoinpayments/coupons_test.go index 691a054c9..a4b4d9763 100644 --- a/satellite/payments/stripecoinpayments/coupons_test.go +++ b/satellite/payments/stripecoinpayments/coupons_test.go @@ -13,64 +13,9 @@ import ( "storj.io/common/testrand" "storj.io/storj/private/testplanet" "storj.io/storj/satellite" - "storj.io/storj/satellite/console" - "storj.io/storj/satellite/payments" "storj.io/storj/satellite/payments/stripecoinpayments" ) -func TestCouponConflict(t *testing.T) { - const ( - partnerName = "partner" - partnerCode = "promo1" - standardCode = "promo2" - ) - testplanet.Run(t, testplanet.Config{ - SatelliteCount: 1, - Reconfigure: testplanet.Reconfigure{ - Satellite: func(log *zap.Logger, index int, config *satellite.Config) { - config.Payments.PackagePlans.Packages = map[string]payments.PackagePlan{ - partnerName: {CouponID: "c1"}, - } - }, - }, - }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { - sat := planet.Satellites[0] - coupons := sat.Core.Payments.Accounts.Coupons() - - t.Run("standard user can replace partner coupon", func(t *testing.T) { - user, err := sat.AddUser(ctx, console.CreateUser{ - FullName: "Test User", - Email: "user@mail.test", - }, 2) - require.NoError(t, err) - - _, err = coupons.ApplyCouponCode(ctx, user.ID, partnerCode) - require.NoError(t, err) - _, err = coupons.ApplyCouponCode(ctx, user.ID, standardCode) - require.NoError(t, err) - }) - - partneredUser, err := sat.AddUser(ctx, console.CreateUser{ - FullName: "Test User", - Email: "user2@mail.test", - UserAgent: []byte(partnerName), - }, 2) - require.NoError(t, err) - - t.Run("partnered user can replace standard coupon", func(t *testing.T) { - _, err = coupons.ApplyCouponCode(ctx, partneredUser.ID, standardCode) - require.NoError(t, err) - _, err = coupons.ApplyCouponCode(ctx, partneredUser.ID, partnerCode) - require.NoError(t, err) - }) - - t.Run("partnered user cannot replace partner coupon", func(t *testing.T) { - _, err = coupons.ApplyCouponCode(ctx, partneredUser.ID, standardCode) - require.True(t, payments.ErrCouponConflict.Has(err)) - }) - }) -} - func TestCoupons(t *testing.T) { testplanet.Run(t, testplanet.Config{ SatelliteCount: 1, StorageNodeCount: 0, UplinkCount: 1, diff --git a/scripts/testdata/satellite-config.yaml.lock b/scripts/testdata/satellite-config.yaml.lock index 1fda14f2e..d91a89586 100755 --- a/scripts/testdata/satellite-config.yaml.lock +++ b/scripts/testdata/satellite-config.yaml.lock @@ -826,7 +826,7 @@ identity.key-path: /root/.local/share/storj/identity/satellite/identity.key # price node receive for storing TB of repair in cents # payments.node-repair-bandwidth-price: 1000 -# semicolon-separated partner package plans in the format partner:couponID,price. Price is in cents USD. +# semicolon-separated partner package plans in the format partner:price,credit. Price and credit are in cents USD. # payments.package-plans: "" # payments provider to use