From 5c629796d34130d8e4fe3f96a569c46f7a129d29 Mon Sep 17 00:00:00 2001 From: PavelBrm Date: Tue, 19 Nov 2024 20:38:32 +1300 Subject: [PATCH 1/3] feat: use customer id when present whilst creating a premium order --- services/skus/model/model.go | 1 + services/skus/service.go | 17 +++++- services/skus/service_nonint_test.go | 85 ++++++++++++++++++++++++++-- 3 files changed, 96 insertions(+), 7 deletions(-) diff --git a/services/skus/model/model.go b/services/skus/model/model.go index 5edc68108..bdb6d1642 100644 --- a/services/skus/model/model.go +++ b/services/skus/model/model.go @@ -413,6 +413,7 @@ type OrderItemRequest struct { // CreateOrderRequestNew includes information needed to create an order. type CreateOrderRequestNew struct { Email string `json:"email" validate:"required,email"` + CustomerID string `json:"customer_id"` // Optional. Currency string `json:"currency" validate:"required,iso4217"` StripeMetadata *OrderStripeMetadata `json:"stripe_metadata"` RadomMetadata *OrderRadomMetadata `json:"radom_metadata"` diff --git a/services/skus/service.go b/services/skus/service.go index 89a4f2a16..360311839 100644 --- a/services/skus/service.go +++ b/services/skus/service.go @@ -2021,6 +2021,7 @@ func (s *Service) createStripeSession(ctx context.Context, req *model.CreateOrde sreq := createStripeSessionRequest{ orderID: oid, email: req.Email, + customerID: req.CustomerID, successURL: surl, cancelURL: curl, trialDays: order.GetTrialDays(), @@ -2572,6 +2573,7 @@ func (s *Service) recreateStripeSession(ctx context.Context, dbi sqlx.ExecerCont req := createStripeSessionRequest{ orderID: ord.ID.String(), email: email, + customerID: oldSess.Customer.ID, successURL: oldSess.SuccessURL, cancelURL: oldSess.CancelURL, trialDays: ord.GetTrialDays(), @@ -2798,6 +2800,7 @@ func chooseStripeSessID(ord *model.Order, canBeNewSessID string) (string, bool) type createStripeSessionRequest struct { orderID string email string + customerID string successURL string cancelURL string trialDays int64 @@ -2815,9 +2818,17 @@ func createStripeSession(ctx context.Context, cl stripeClient, req createStripeS LineItems: req.items, } - // Email might not be given. - // This could happen while recreating a session, and the email was not extracted from the old one. - if req.email != "" { + // Different processes can supply different info about customer: + // - when customerID is present, it takes precedence; + // - when email is present: + // - first, search for customer; + // - fallback to using the email directly. + // Based on the rules above, if both are present, customerID wins. + switch { + case req.customerID != "": + params.Customer = &req.customerID + + case req.customerID == "" && req.email != "": if cust, ok := cl.FindCustomer(ctx, req.email); ok && cust.Email != "" { params.Customer = &cust.ID } else { diff --git a/services/skus/service_nonint_test.go b/services/skus/service_nonint_test.go index de3890fad..070fa1432 100644 --- a/services/skus/service_nonint_test.go +++ b/services/skus/service_nonint_test.go @@ -4564,7 +4564,84 @@ func TestCreateStripeSession(t *testing.T) { tests := []testCase{ { - name: "success_found_customer", + name: "success_cust_id", + given: tcGiven{ + cl: &xstripe.MockClient{ + FnCreateSession: func(ctx context.Context, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) { + if params.Customer == nil || *params.Customer != "cus_id" { + return nil, model.Error("unexpected") + } + + result := &stripe.CheckoutSession{ID: "cs_test_id"} + + return result, nil + }, + + FnFindCustomer: func(ctx context.Context, email string) (*stripe.Customer, bool) { + panic("unexpected_find_customer") + }, + }, + + req: createStripeSessionRequest{ + orderID: "facade00-0000-4000-a000-000000000000", + customerID: "cus_id", + successURL: "https://example.com/success", + cancelURL: "https://example.com/cancel", + trialDays: 7, + items: []*stripe.CheckoutSessionLineItemParams{ + { + Quantity: ptrTo[int64](1), + Price: ptrTo("stripe_item_id"), + }, + }, + }, + }, + exp: tcExpected{ + val: "cs_test_id", + }, + }, + + { + name: "success_cust_id_email", + given: tcGiven{ + cl: &xstripe.MockClient{ + FnCreateSession: func(ctx context.Context, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) { + if params.Customer == nil || *params.Customer != "cus_id" { + return nil, model.Error("unexpected") + } + + result := &stripe.CheckoutSession{ID: "cs_test_id"} + + return result, nil + }, + + FnFindCustomer: func(ctx context.Context, email string) (*stripe.Customer, bool) { + panic("unexpected_find_customer") + }, + }, + + req: createStripeSessionRequest{ + orderID: "facade00-0000-4000-a000-000000000000", + customerID: "cus_id", + email: "you@example.com", + successURL: "https://example.com/success", + cancelURL: "https://example.com/cancel", + trialDays: 7, + items: []*stripe.CheckoutSessionLineItemParams{ + { + Quantity: ptrTo[int64](1), + Price: ptrTo("stripe_item_id"), + }, + }, + }, + }, + exp: tcExpected{ + val: "cs_test_id", + }, + }, + + { + name: "success_email_found_customer", given: tcGiven{ cl: &xstripe.MockClient{ FnCreateSession: func(ctx context.Context, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) { @@ -4598,7 +4675,7 @@ func TestCreateStripeSession(t *testing.T) { }, { - name: "success_customer_not_found", + name: "success_email_customer_not_found", given: tcGiven{ cl: &xstripe.MockClient{ FnFindCustomer: func(ctx context.Context, email string) (*stripe.Customer, bool) { @@ -4636,7 +4713,7 @@ func TestCreateStripeSession(t *testing.T) { }, { - name: "success_no_customer_email", + name: "success_email_no_customer_email", given: tcGiven{ cl: &xstripe.MockClient{ FnFindCustomer: func(ctx context.Context, email string) (*stripe.Customer, bool) { @@ -4663,7 +4740,7 @@ func TestCreateStripeSession(t *testing.T) { }, { - name: "success_no_trial_days", + name: "success_email_no_trial_days", given: tcGiven{ cl: &xstripe.MockClient{}, From 90d61296c801826c5cd6a625c1d0a13e000f5b7f Mon Sep 17 00:00:00 2001 From: PavelBrm Date: Tue, 19 Nov 2024 21:07:07 +1300 Subject: [PATCH 2/3] test: fix test for setOrderTrialDays --- services/skus/service.go | 5 ++++- services/skus/service_nonint_test.go | 5 ++--- services/skus/xstripe/mock.go | 5 ++++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/services/skus/service.go b/services/skus/service.go index 360311839..4d92fb053 100644 --- a/services/skus/service.go +++ b/services/skus/service.go @@ -2573,13 +2573,16 @@ func (s *Service) recreateStripeSession(ctx context.Context, dbi sqlx.ExecerCont req := createStripeSessionRequest{ orderID: ord.ID.String(), email: email, - customerID: oldSess.Customer.ID, successURL: oldSess.SuccessURL, cancelURL: oldSess.CancelURL, trialDays: ord.GetTrialDays(), items: buildStripeLineItems(ord.Items), } + if oldSess.Customer != nil { + req.customerID = oldSess.Customer.ID + } + if req.email == "" { req.email = xstripe.CustomerEmailFromSession(oldSess) } diff --git a/services/skus/service_nonint_test.go b/services/skus/service_nonint_test.go index 070fa1432..76eb796f7 100644 --- a/services/skus/service_nonint_test.go +++ b/services/skus/service_nonint_test.go @@ -4388,7 +4388,7 @@ func TestService_recreateStripeSession(t *testing.T) { }, { - name: "success_email_from_session", + name: "success_email_cust_from_session", given: tcGiven{ ordRepo: &repository.MockOrder{ FnAppendMetadata: func(ctx context.Context, dbi sqlx.ExecerContext, id uuid.UUID, key, val string) error { @@ -4405,7 +4405,7 @@ func TestService_recreateStripeSession(t *testing.T) { ID: "cs_test_id_old", SuccessURL: "https://example.com/success", CancelURL: "https://example.com/cancel", - Customer: &stripe.Customer{Email: "you@example.com"}, + Customer: &stripe.Customer{ID: "cus_id", Email: "you@example.com"}, } return result, nil @@ -4473,7 +4473,6 @@ func TestService_recreateStripeSession(t *testing.T) { ID: "cs_test_id_old", SuccessURL: "https://example.com/success", CancelURL: "https://example.com/cancel", - Customer: &stripe.Customer{Email: "session@example.com"}, } return result, nil diff --git a/services/skus/xstripe/mock.go b/services/skus/xstripe/mock.go index 442ff3f57..803eb4943 100644 --- a/services/skus/xstripe/mock.go +++ b/services/skus/xstripe/mock.go @@ -15,7 +15,10 @@ type MockClient struct { func (c *MockClient) Session(ctx context.Context, id string, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) { if c.FnSession == nil { - result := &stripe.CheckoutSession{ID: id} + result := &stripe.CheckoutSession{ + ID: id, + Customer: &stripe.Customer{ID: "cus_id", Email: "customer@example.com"}, + } return result, nil } From f1e263c49079046b44e66ca05313249afc5774cc Mon Sep 17 00:00:00 2001 From: PavelBrm Date: Tue, 19 Nov 2024 21:25:47 +1300 Subject: [PATCH 3/3] fix: use customer id only when email is present on old session whilst recreating --- services/skus/service.go | 5 +- services/skus/service_nonint_test.go | 73 +++++++++++++++++++++++++++ services/skus/xstripe/xstripe.go | 10 ++++ services/skus/xstripe/xstripe_test.go | 49 ++++++++++++++++++ 4 files changed, 133 insertions(+), 4 deletions(-) diff --git a/services/skus/service.go b/services/skus/service.go index 4d92fb053..c56311d39 100644 --- a/services/skus/service.go +++ b/services/skus/service.go @@ -2573,16 +2573,13 @@ func (s *Service) recreateStripeSession(ctx context.Context, dbi sqlx.ExecerCont req := createStripeSessionRequest{ orderID: ord.ID.String(), email: email, + customerID: xstripe.CustomerIDFromSession(oldSess), successURL: oldSess.SuccessURL, cancelURL: oldSess.CancelURL, trialDays: ord.GetTrialDays(), items: buildStripeLineItems(ord.Items), } - if oldSess.Customer != nil { - req.customerID = oldSess.Customer.ID - } - if req.email == "" { req.email = xstripe.CustomerEmailFromSession(oldSess) } diff --git a/services/skus/service_nonint_test.go b/services/skus/service_nonint_test.go index 76eb796f7..dca5d150d 100644 --- a/services/skus/service_nonint_test.go +++ b/services/skus/service_nonint_test.go @@ -4455,6 +4455,79 @@ func TestService_recreateStripeSession(t *testing.T) { }, }, + { + name: "success_email_from_request_cust_without_email", + given: tcGiven{ + ordRepo: &repository.MockOrder{ + FnAppendMetadata: func(ctx context.Context, dbi sqlx.ExecerContext, id uuid.UUID, key, val string) error { + if key == "stripeCheckoutSessionId" && val == "cs_test_id" { + return nil + } + + return model.Error("unexpected") + }, + }, + cl: &xstripe.MockClient{ + FnSession: func(ctx context.Context, id string, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) { + result := &stripe.CheckoutSession{ + ID: "cs_test_id_old", + SuccessURL: "https://example.com/success", + CancelURL: "https://example.com/cancel", + Customer: &stripe.Customer{ID: "cus_id"}, + } + + return result, nil + }, + + FnFindCustomer: func(ctx context.Context, email string) (*stripe.Customer, bool) { + return nil, false + }, + + FnCreateSession: func(ctx context.Context, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) { + if params.Customer != nil { + return nil, model.Error("unexpected_customer") + } + + if *params.CustomerEmail != "request@example.com" { + return nil, model.Error("unexpected_customer_email") + } + + result := &stripe.CheckoutSession{ + ID: "cs_test_id", + PaymentMethodTypes: []string{"card"}, + Mode: stripe.CheckoutSessionModeSubscription, + SuccessURL: *params.SuccessURL, + CancelURL: *params.CancelURL, + ClientReferenceID: *params.ClientReferenceID, + Subscription: &stripe.Subscription{ + ID: "sub_id", + Metadata: map[string]string{ + "orderID": *params.ClientReferenceID, + }, + }, + AllowPromotionCodes: true, + } + + return result, nil + }, + }, + ord: &model.Order{ + ID: uuid.Must(uuid.FromString("facade00-0000-4000-a000-000000000000")), + Items: []model.OrderItem{ + { + Quantity: 1, + Metadata: datastore.Metadata{"stripe_item_id": "stripe_item_id"}, + }, + }, + }, + oldSessID: "cs_test_id_old", + email: "request@example.com", + }, + exp: tcExpected{ + val: "cs_test_id", + }, + }, + { name: "success_email_from_request", given: tcGiven{ diff --git a/services/skus/xstripe/xstripe.go b/services/skus/xstripe/xstripe.go index 688928726..e9f2a5580 100644 --- a/services/skus/xstripe/xstripe.go +++ b/services/skus/xstripe/xstripe.go @@ -56,3 +56,13 @@ func CustomerEmailFromSession(sess *stripe.CheckoutSession) string { // Default to empty, Stripe will ask the customer. return "" } + +func CustomerIDFromSession(sess *stripe.CheckoutSession) string { + // Return the customer id only if the customer is present AND it has email set. + // Without the email, the customer record is not fully formed, and does not suit the use case. + if sess.Customer != nil && sess.Customer.Email != "" { + return sess.Customer.ID + } + + return "" +} diff --git a/services/skus/xstripe/xstripe_test.go b/services/skus/xstripe/xstripe_test.go index 6af57ac4f..bcf367d49 100644 --- a/services/skus/xstripe/xstripe_test.go +++ b/services/skus/xstripe/xstripe_test.go @@ -65,3 +65,52 @@ func TestCustomerEmailFromSession(t *testing.T) { }) } } + +func TestCustomerIDFromSession(t *testing.T) { + tests := []struct { + name string + exp string + given *stripe.CheckoutSession + }{ + { + name: "nil_customer_no_email", + given: &stripe.CheckoutSession{}, + }, + + { + name: "customer_empty_email", + given: &stripe.CheckoutSession{ + Customer: &stripe.Customer{}, + }, + }, + + { + name: "customer_email_no_id", + given: &stripe.CheckoutSession{ + Customer: &stripe.Customer{ + Email: "me@example.com", + }, + }, + }, + + { + name: "customer_email_id", + given: &stripe.CheckoutSession{ + Customer: &stripe.Customer{ + ID: "cus_id", + Email: "me@example.com", + }, + }, + exp: "cus_id", + }, + } + + for i := range tests { + tc := tests[i] + + t.Run(tc.name, func(t *testing.T) { + actual := CustomerIDFromSession(tc.given) + should.Equal(t, tc.exp, actual) + }) + } +}