Repository: netlify/gocommerce
Pull request #190 (2019-09-16 11:36:22)

Remove deprecated payments through stripe charges
**- Summary**

The Stripe payment provider was recently migrated to use the new PaymentIntent API.
This is required for EU countries as of Sept 14th 2019.
That change was behind a feature flag.
This change makes PaymentIntents the default and removes the flag.

**- Test plan**

n/a

**- Description for the changelog**

Default to PaymentIntents for Stripe



Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index e45b633..f081132 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -5,8 +5,6 @@ please read the [code of conduct](CODE_OF_CONDUCT.md).
 
 ## Setup
 
-> Install Go and Glide https://github.com/Masterminds/glide
-
 ```sh
 $ git clone https://github.com/netlify/gocommerce
 $ cd gocommerce
diff --git a/api/order.go b/api/order.go
index dd032b0..5ce55f6 100644
--- a/api/order.go
+++ b/api/order.go
@@ -145,7 +145,8 @@ func (a *API) ClaimOrders(w http.ResponseWriter, r *http.Request) error {
 	}
 
 	log.Info("Finished updating")
-	return sendJSON(w, http.StatusNoContent, "")
+	w.WriteHeader(http.StatusNoContent)
+	return nil
 }
 
 // ReceiptView renders an HTML receipt for an order
diff --git a/api/payments.go b/api/payments.go
index 77548c3..d5ac4b5 100644
--- a/api/payments.go
+++ b/api/payments.go
@@ -535,8 +535,7 @@ func createPaymentProviders(c *conf.Configuration) (map[string]payments.Provider
 	provs := map[string]payments.Provider{}
 	if c.Payment.Stripe.Enabled {
 		p, err := stripe.NewPaymentProvider(stripe.Config{
-			SecretKey:         c.Payment.Stripe.SecretKey,
-			UsePaymentIntents: c.Payment.Stripe.UsePaymentIntents,
+			SecretKey: c.Payment.Stripe.SecretKey,
 		})
 		if err != nil {
 			return nil, err
diff --git a/conf/configuration.go b/conf/configuration.go
index 1371d3f..48281c1 100644
--- a/conf/configuration.go
+++ b/conf/configuration.go
@@ -68,8 +68,6 @@ type Configuration struct {
 			Enabled   bool   `json:"enabled"`
 			PublicKey string `json:"public_key" split_words:"true"`
 			SecretKey string `json:"secret_key" split_words:"true"`
-
-			UsePaymentIntents bool `json:"use_payment_intents" split_words:"true"`
 		} `json:"stripe"`
 		PayPal struct {
 			Enabled  bool   `json:"enabled"`
diff --git a/docker-compose.yml b/docker-compose.yml
index cfa8d72..fbf71f1 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -19,6 +19,8 @@ services:
       - 8080:8080
     networks:
       - db
+    depends_on:
+      - db
 
   db:
     image: mysql:5.7
diff --git a/models/line_item.go b/models/line_item.go
index 50ed44b..80ab3ac 100644
--- a/models/line_item.go
+++ b/models/line_item.go
@@ -285,7 +285,7 @@ func (i *LineItem) Process(config *conf.Configuration, userClaims map[string]int
 		i.AddonItems[index].Price = lowestPrice.cents
 	}
 
-	order.Downloads = i.MissingDownloads(order, meta)
+	order.Downloads = append(order.Downloads, i.MissingDownloads(order, meta)...)
 
 	return i.calculatePrice(userClaims, meta.Prices, order.Currency)
 }
@@ -338,10 +338,10 @@ func (i *LineItem) FetchMeta(siteURL string) (*LineItemMetadata, error) {
 // MissingDownloads returns all downloads that are not yet listed in the order
 func (i *LineItem) MissingDownloads(order *Order, meta *LineItemMetadata) []Download {
 	downloads := []Download{}
-	for _, download := range meta.Downloads {
+	for _, metaDownload := range meta.Downloads {
 		alreadyCreated := false
 		for _, d := range order.Downloads {
-			if d.URL == download.URL {
+			if d.URL == metaDownload.URL {
 				alreadyCreated = true
 				break
 			}
@@ -349,11 +349,14 @@ func (i *LineItem) MissingDownloads(order *Order, meta *LineItemMetadata) []Down
 		if alreadyCreated {
 			continue
 		}
-		download.ID = uuid.NewRandom().String()
-		download.OrderID = order.ID
-		download.Title = i.Title
-		download.Sku = i.Sku
-		downloads = append(downloads, download)
+		orderDownload := metaDownload
+		orderDownload.ID = uuid.NewRandom().String()
+		orderDownload.OrderID = order.ID
+		orderDownload.Sku = i.Sku
+		if orderDownload.Title == "" {
+			orderDownload.Title = i.Title
+		}
+		downloads = append(downloads, orderDownload)
 	}
 	return downloads
 }
diff --git a/payments/stripe/stripe.go b/payments/stripe/stripe.go
index b721578..c9f6dee 100644
--- a/payments/stripe/stripe.go
+++ b/payments/stripe/stripe.go
@@ -17,8 +17,6 @@ import (
 
 type stripePaymentProvider struct {
 	client *client.API
-
-	usePaymentIntents bool
 }
 
 type stripeBodyParams struct {
@@ -28,8 +26,7 @@ type stripeBodyParams struct {
 
 // Config contains the Stripe-specific configuration for payment providers.
 type Config struct {
-	SecretKey         string `mapstructure:"secret_key" json:"secret_key"`
-	UsePaymentIntents bool   `mapstructure:"use_payment_intents" json:"use_payment_intents"`
+	SecretKey string `mapstructure:"secret_key" json:"secret_key"`
 }
 
 // NewPaymentProvider creates a new Stripe payment provider using the provided configuration.
@@ -40,8 +37,6 @@ func NewPaymentProvider(config Config) (payments.Provider, error) {
 
 	s := stripePaymentProvider{
 		client: &client.API{},
-
-		usePaymentIntents: config.UsePaymentIntents,
 	}
 	s.client.Init(config.SecretKey, nil)
 	return &s, nil
@@ -62,16 +57,6 @@ func (s *stripePaymentProvider) NewCharger(ctx context.Context, r *http.Request,
 		return nil, err
 	}
 
-	if !s.usePaymentIntents {
-		log.Warning(`Deprecation Warning: Payment Intents are not enabled. Stripe requires those after Sep 14th 2019. Enable by setting "payment.stripe.use_payment_intents" to true`)
-		if bp.StripeToken == "" {
-			return nil, errors.New("Stripe requires a stripe_token for creating a payment")
-		}
-		return func(amount uint64, currency string, order *models.Order, invoiceNumber int64) (string, error) {
-			return s.chargeDeprecated(bp.StripeToken, amount, currency, order, invoiceNumber)
-		}, nil
-	}
-
 	if bp.StripePaymentMethodID == "" {
 		return nil, errors.New("Stripe requires a stripe_payment_method_id for creating a payment intent")
 	}
@@ -94,30 +79,6 @@ func prepareShippingAddress(addr models.Address) *stripe.ShippingDetailsParams {
 	}
 }
 
-func (s *stripePaymentProvider) chargeDeprecated(token string, amount uint64, currency string, order *models.Order, invoiceNumber int64) (string, error) {
-	stripeAmount := int64(amount)
-	stripeDescription := fmt.Sprintf("Invoice No. %d", invoiceNumber)
-	ch, err := s.client.Charges.New(&stripe.ChargeParams{
-		Amount:      &stripeAmount,
-		Source:      &stripe.SourceParams{Token: &token},
-		Currency:    &currency,
-		Description: &stripeDescription,
-		Shipping:    prepareShippingAddress(order.ShippingAddress),
-		Params: stripe.Params{
-			Metadata: map[string]string{
-				"order_id":       order.ID,
-				"invoice_number": fmt.Sprintf("%d", invoiceNumber),
-			},
-		},
-	})
-
-	if err != nil {
-		return "", err
-	}
-
-	return ch.ID, nil
-}
-
 func (s *stripePaymentProvider) chargePaymentIntent(paymentMethodID string, amount uint64, currency string, order *models.Order, invoiceNumber int64) (string, error) {
 	params := &stripe.PaymentIntentParams{
 		PaymentMethod: stripe.String(paymentMethodID),
@@ -176,10 +137,6 @@ func (s *stripePaymentProvider) NewPreauthorizer(ctx context.Context, r *http.Re
 }
 
 func (s *stripePaymentProvider) NewConfirmer(ctx context.Context, r *http.Request, log logrus.FieldLogger) (payments.Confirmer, error) {
-	if !s.usePaymentIntents {
-		return nil, errors.New("Cannot confirm a payment if not using payment intents")
-	}
-
 	return s.confirm, nil
 }
 


Test patch:
diff --git a/api/order_test.go b/api/order_test.go
index 9d56e1c..31790fc 100644
--- a/api/order_test.go
+++ b/api/order_test.go
@@ -217,7 +217,7 @@ func TestOrderCreate(t *testing.T) {
 					Claims: map[string]string{
 						"email": test.Data.testUser.Email,
 					},
-					Percentage: 15,
+					Percentage:   15,
 					ProductTypes: []string{"Book"},
 				},
 			},
@@ -249,6 +249,68 @@ func TestOrderCreate(t *testing.T) {
 		assert.Equal(t, uint64(15), discountItem.Percentage)
 		assert.Equal(t, uint64(0), discountItem.Fixed)
 	})
+
+	t.Run("MultipleItemsWithDownloads", func(t *testing.T) {
+		test := NewRouteTest(t)
+
+		site := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			switch r.URL.Path {
+			case "/i/believe/i/can/fly":
+				fmt.Fprint(w, productMetaFrame(
+					`{
+						"sku": "123-i-can-fly-456",
+						"downloads": [{"title": "First Download", "url": "/assets/first-download"}],
+						"prices": [{"currency": "USD", "amount": "3.00"}]
+					}`,
+				))
+				return
+			case "/its/not/about/the/money":
+				fmt.Fprintf(w, productMetaFrame(
+					`{
+						"sku": "not-about-the-money",
+						"downloads": [{"title": "Second Download", "url": "/assets/second-download"}],
+						"prices": [{"currency": "USD", "amount": "5.00"}]
+					}`,
+				))
+				return
+			}
+			w.WriteHeader(http.StatusNotFound)
+		}))
+		defer site.Close()
+		test.Config.SiteURL = site.URL
+
+		body := strings.NewReader(`{
+			"email": "info@example.com",
+			"shipping_address": {
+				"name": "Test User",
+				"address1": "Branengebranen",
+				"city": "Berlin", "country": "Germany", "zip": "94107"
+			},
+			"line_items": [
+				{"path": "/i/believe/i/can/fly", "quantity": 1},
+				{"path": "/its/not/about/the/money", "quantity": 1}
+			]
+		}`)
+		token := test.Data.testUserToken
+		recorder := test.TestEndpoint(http.MethodPost, "/orders", body, token)
+
+		order := &models.Order{}
+		extractPayload(t, http.StatusCreated, recorder, order)
+		assert.Len(t, order.Downloads, 2)
+		for _, dl := range order.Downloads {
+			fmt.Printf("dl: %+v\n", dl)
+			switch dl.Sku {
+			case "123-i-can-fly-456":
+				assert.Equal(t, "First Download", dl.Title)
+				assert.Equal(t, "/assets/first-download", dl.URL)
+			case "not-about-the-money":
+				assert.Equal(t, "Second Download", dl.Title)
+				assert.Equal(t, "/assets/second-download", dl.URL)
+			default:
+				t.Errorf("Unknown download item: %+v", dl)
+			}
+		}
+	})
 }
 
 func TestOrderCreateNewUser(t *testing.T) {
diff --git a/api/payments_test.go b/api/payments_test.go
index 0763c2a..140fbc6 100644
--- a/api/payments_test.go
+++ b/api/payments_test.go
@@ -439,8 +439,6 @@ func TestPaymentCreate(t *testing.T) {
 			for name, card := range tests {
 				t.Run(name, func(t *testing.T) {
 					test := NewRouteTest(t)
-					test.Config.Payment.Stripe.UsePaymentIntents = true
-
 					callCount := 0
 					stripe.SetBackend(stripe.APIBackend, NewTrackingStripeBackend(func(method, path, key string, params stripe.ParamsContainer, v interface{}) error {
 						switch path {
@@ -518,46 +516,6 @@ func TestPaymentCreate(t *testing.T) {
 				})
 			}
 		})
-		t.Run("Charges", func(t *testing.T) {
-			test := NewRouteTest(t)
-
-			callCount := 0
-			stripe.SetBackend(stripe.APIBackend, NewTrackingStripeBackend(func(method, path, key string, params stripe.ParamsContainer, v interface{}) error {
-				switch path {
-				case "/v1/charges":
-					payload := params.GetParams()
-					assert.Equal(t, test.Data.firstOrder.ID, payload.Metadata["order_id"])
-					assert.Equal(t, "1", payload.Metadata["invoice_number"])
-					callCount++
-					return nil
-				default:
-					t.Fatalf("unknown Stripe API call to %s", path)
-					return &stripe.Error{Code: stripe.ErrorCodeURLInvalid}
-				}
-			}))
-			defer stripe.SetBackend(stripe.APIBackend, nil)
-
-			test.Data.firstOrder.PaymentState = models.PendingState
-			rsp := test.DB.Save(test.Data.firstOrder)
-			require.NoError(t, rsp.Error, "Failed to update order")
-
-			params := &stripePaymentParams{
-				Amount:      test.Data.firstOrder.Total,
-				Currency:    test.Data.firstOrder.Currency,
-				StripeToken: "123456",
-				Provider:    payments.StripeProvider,
-			}
-
-			body, err := json.Marshal(params)
-			require.NoError(t, err)
-
-			recorder := test.TestEndpoint(http.MethodPost, "/orders/first-order/payments", bytes.NewBuffer(body), test.Data.testUserToken)
-
-			trans := models.Transaction{}
-			extractPayload(t, http.StatusOK, recorder, &trans)
-			assert.Equal(t, models.PaidState, trans.Status)
-			assert.Equal(t, 1, callCount)
-		})
 	})
 }
 
@@ -576,8 +534,6 @@ func TestPaymentConfirm(t *testing.T) {
 	for name, testParams := range tests {
 		t.Run(name, func(t *testing.T) {
 			test := NewRouteTest(t)
-			test.Config.Payment.Stripe.UsePaymentIntents = true
-
 			callCount := 0
 			stripe.SetBackend(stripe.APIBackend, NewTrackingStripeBackend(func(method, path, key string, params stripe.ParamsContainer, v interface{}) error {
 				if path == fmt.Sprintf("/v1/payment_intents/%s/confirm", stripePaymentIntentID) {