Merge pull request #755 from OpenBazaar/shipping

Refactor shipping costs

Author: cpacia

api/jsonapi.go

@@ -1406,6 +1406,13 @@ func (i *jsonAPIHandler) GETListing(w http.ResponseWriter, r *http.Request) {
 				}
 			}
 		}
+		if sl.Listing.Metadata != nil && sl.Listing.Metadata.Version == 1 {
+			for _, so := range sl.Listing.ShippingOptions {
+				for _, ser := range so.Services {
+					ser.AdditionalItemPrice = ser.Price
+				}
+			}
+		}
 
 		out, err := m.MarshalToString(sl)
 		if err != nil {

api/jsonapi_data_test.go

@@ -425,22 +425,7 @@ const listingJSON = `{
 				"price": 150000000,
 				"estimatedDelivery": "2-3 days"
 			}
-		],
-		"shippingRules": {
-			"ruleType": "QUANTITY_DISCOUNT",
-			"rules": [
-				{
-					"price": 10000000,
-					"minRange": 5,
-					"maxRange": 10
-				},
-				{
-					"price": 200000,
-					"minRange": 11,
-					"maxRange": 20
-				}
-			]
-		 }
+		]
 	}
 	],
 	"taxes": [
@@ -626,22 +611,7 @@ const listingUpdateJSON = `{
 				"price": 150000000,
 				"estimatedDelivery": "2-3 days"
 			}
-		],
-		"shippingRules": {
-			"ruleType": "QUANTITY_DISCOUNT",
-			"rules": [
-				{
-					"price": 10000000,
-					"minRange": 5,
-					"maxRange": 10
-				},
-				{
-					"price": 200000,
-					"minRange": 11,
-					"maxRange": 20
-				}
-			]
-		 }
+		]
 	}
 	],
 	"taxes": [

core/listings.go

@@ -26,7 +26,7 @@ import (
 )
 
 const (
-	ListingVersion           = 1
+	ListingVersion           = 2
 	TitleMaxCharacters       = 140
 	ShortDescriptionLength   = 160
 	DescriptionMaxCharacters = 50000
@@ -948,41 +948,6 @@ func validateListing(listing *pb.Listing, testnet bool) (err error) {
 		if len(shippingOption.Regions) > MaxCountryCodes {
 			return fmt.Errorf("Number of shipping regions is greater than the max of %d", MaxCountryCodes)
 		}
-		if shippingOption.ShippingRules != nil {
-			if len(shippingOption.ShippingRules.Rules) == 0 {
-				return errors.New("At least on rule must be specified if ShippingRules is selected")
-			}
-			if len(shippingOption.ShippingRules.Rules) > MaxListItems {
-				return fmt.Errorf("Number of shipping rules is greater than the max of %d", MaxListItems)
-			}
-			if shippingOption.ShippingRules.RuleType > pb.Listing_ShippingOption_ShippingRules_COMBINED_SHIPPING_SUBTRACT {
-				return errors.New("Unknown shipping rule")
-			}
-			if shippingOption.ShippingRules.RuleType == pb.Listing_ShippingOption_ShippingRules_FLAT_FEE_WEIGHT_RANGE && listing.Item.Grams == 0 {
-				return errors.New("Item weight must be specified when using FLAT_FEE_WEIGHT_RANGE shipping rule")
-			}
-			if (shippingOption.ShippingRules.RuleType == pb.Listing_ShippingOption_ShippingRules_COMBINED_SHIPPING_ADD || shippingOption.ShippingRules.RuleType == pb.Listing_ShippingOption_ShippingRules_COMBINED_SHIPPING_SUBTRACT) && len(shippingOption.ShippingRules.Rules) > 1 {
-				return errors.New("Selected shipping rule type can only have a maximum of one rule")
-			}
-			if len(shippingOption.ShippingRules.Rules) > MaxListItems {
-				return fmt.Errorf("Shipping rules exceeds max of %d", MaxListItems)
-			}
-			for i, rule := range shippingOption.ShippingRules.Rules {
-				if (shippingOption.ShippingRules.RuleType == pb.Listing_ShippingOption_ShippingRules_FLAT_FEE_QUANTITY_RANGE ||
-					shippingOption.ShippingRules.RuleType == pb.Listing_ShippingOption_ShippingRules_FLAT_FEE_WEIGHT_RANGE ||
-					shippingOption.ShippingRules.RuleType == pb.Listing_ShippingOption_ShippingRules_QUANTITY_DISCOUNT) && rule.MaxRange <= rule.MinRange {
-					return errors.New("Shipping rule max range cannot be less than or equal to the min range")
-				}
-				for x, checkRule := range shippingOption.ShippingRules.Rules {
-					if x == i {
-						continue
-					}
-					if (rule.MinRange >= checkRule.MinRange && rule.MinRange <= checkRule.MaxRange) || (rule.MaxRange <= checkRule.MaxRange && rule.MaxRange >= checkRule.MinRange) {
-						return errors.New("Shipping rule ranges must not overlap")
-					}
-				}
-			}
-		}
 		if len(shippingOption.Services) == 0 && shippingOption.Type != pb.Listing_ShippingOption_LOCAL_PICKUP {
 			return errors.New("At least one service must be specified for a shipping option when not local pickup")
 		}

core/order.go

@@ -774,21 +774,21 @@ func (n *OpenBazaarNode) CalculateOrderTotal(contract *pb.RicardianContract) (ui
 	}
 
 	// Add in shipping costs
-	type combinedShipping struct {
-		quantity int
-		price    uint64
-		add      bool
-		modifier uint64
+	type itemShipping struct {
+		primary               uint64
+		secondary             uint64
+		quantity              uint32
+		shippingTaxPercentage float32
+		version               uint32
 	}
-	var combinedOptions []combinedShipping
+	var is []itemShipping
 
-	var shippingTotal uint64
+	// First loop through to validate and filter out non-physical items
 	for _, item := range contract.BuyerOrder.Items {
 		listing, ok := physicalGoods[item.ListingHash]
 		if !ok { // Not physical good no need to calculate shipping
 			continue
 		}
-		var itemShipping uint64
 		// Check selected option exists
 		shippingOptions := make(map[string]*pb.Listing_ShippingOption)
 		for _, so := range listing.ShippingOptions {
@@ -827,11 +827,17 @@ func (n *OpenBazaarNode) CalculateOrderTotal(contract *pb.RicardianContract) (ui
 		if err != nil {
 			return 0, err
 		}
-		shippingPrice := uint64(item.Quantity) * shippingSatoshi
-		itemShipping += shippingPrice
-		shippingTaxPercentage := float32(0)
+
+		var secondarySatoshi uint64
+		if service.AdditionalItemPrice > 0 {
+			secondarySatoshi, err = n.getPriceInSatoshi(listing.Metadata.PricingCurrency, service.AdditionalItemPrice)
+			if err != nil {
+				return 0, err
+			}
+		}
 
 		// Calculate tax percentage
+		var shippingTaxPercentage float32
 		for _, tax := range listing.Taxes {
 			regions := make(map[pb.CountryCode]bool)
 			for _, taxRegion := range tax.TaxRegions {
@@ -843,98 +849,41 @@ func (n *OpenBazaarNode) CalculateOrderTotal(contract *pb.RicardianContract) (ui
 			}
 		}
 
-		// Apply shipping rules
-		if option.ShippingRules != nil {
-			for _, rule := range option.ShippingRules.Rules {
-				switch option.ShippingRules.RuleType {
-				case pb.Listing_ShippingOption_ShippingRules_QUANTITY_DISCOUNT:
-					if item.Quantity >= rule.MinRange && item.Quantity <= rule.MaxRange {
-						rulePrice, err := n.getPriceInSatoshi(listing.Metadata.PricingCurrency, rule.Price)
-						if err != nil {
-							return 0, err
-						}
-						itemShipping -= rulePrice
-					}
-				case pb.Listing_ShippingOption_ShippingRules_FLAT_FEE_QUANTITY_RANGE:
-					if item.Quantity >= rule.MinRange && item.Quantity <= rule.MaxRange {
-						itemShipping -= shippingPrice
-						rulePrice, err := n.getPriceInSatoshi(listing.Metadata.PricingCurrency, rule.Price)
-						if err != nil {
-							return 0, err
-						}
-						itemShipping += rulePrice
-					}
-				case pb.Listing_ShippingOption_ShippingRules_FLAT_FEE_WEIGHT_RANGE:
-					weight := listing.Item.Grams * float32(item.Quantity)
-					if uint32(weight) >= rule.MinRange && uint32(weight) <= rule.MaxRange {
-						itemShipping -= shippingPrice
-						rulePrice, err := n.getPriceInSatoshi(listing.Metadata.PricingCurrency, rule.Price)
-						if err != nil {
-							return 0, err
-						}
-						itemShipping += rulePrice
-					}
-				case pb.Listing_ShippingOption_ShippingRules_COMBINED_SHIPPING_ADD:
-					itemShipping -= shippingPrice
-					rulePrice, err := n.getPriceInSatoshi(listing.Metadata.PricingCurrency, rule.Price)
-					rulePrice += uint64(float32(rulePrice) * shippingTaxPercentage)
-					shippingSatoshi += uint64(float32(shippingSatoshi) * shippingTaxPercentage)
-					if err != nil {
-						return 0, err
-					}
-					cs := combinedShipping{
-						quantity: int(item.Quantity),
-						price:    shippingSatoshi,
-						add:      true,
-						modifier: rulePrice,
-					}
-					combinedOptions = append(combinedOptions, cs)
-
-				case pb.Listing_ShippingOption_ShippingRules_COMBINED_SHIPPING_SUBTRACT:
-					itemShipping -= shippingPrice
-					rulePrice, err := n.getPriceInSatoshi(listing.Metadata.PricingCurrency, rule.Price)
-					rulePrice += uint64(float32(rulePrice) * shippingTaxPercentage)
-					shippingSatoshi += uint64(float32(shippingSatoshi) * shippingTaxPercentage)
-					if err != nil {
-						return 0, err
-					}
-					cs := combinedShipping{
-						quantity: int(item.Quantity),
-						price:    shippingSatoshi,
-						add:      false,
-						modifier: rulePrice,
-					}
-					combinedOptions = append(combinedOptions, cs)
-				}
-			}
-		}
-		// Apply tax
-		itemShipping += uint64(float32(itemShipping) * shippingTaxPercentage)
-		shippingTotal += itemShipping
+		is = append(is, itemShipping{
+			primary:               shippingSatoshi,
+			secondary:             secondarySatoshi,
+			quantity:              item.Quantity,
+			shippingTaxPercentage: shippingTaxPercentage,
+			version:               listing.Metadata.Version,
+		})
 	}
 
-	// Process combined shipping rules
-	if len(combinedOptions) > 0 {
-		lowestPrice := int64(-1)
-		for _, v := range combinedOptions {
-			if int64(v.price) < lowestPrice || lowestPrice == -1 {
-				lowestPrice = int64(v.price)
+	var shippingTotal uint64
+	if len(is) == 1 {
+		shippingTotal = (is[0].primary * uint64(((1+is[0].shippingTaxPercentage)*100)+.5) / 100)
+		if is[0].quantity > 1 {
+			if is[0].version == 1 {
+				shippingTotal += (is[0].primary * uint64(((1+is[0].shippingTaxPercentage)*100)+.5) / 100) * uint64((is[0].quantity - 1))
+			} else if is[0].version == 2 {
+				shippingTotal += (is[0].secondary * uint64(((1+is[0].shippingTaxPercentage)*100)+.5) / 100) * uint64((is[0].quantity - 1))
+			} else {
+				return 0, errors.New("Unknown listing version")
 			}
 		}
-		shippingTotal += uint64(lowestPrice)
-		for _, o := range combinedOptions {
-			modifier := o.modifier
-			modifier *= (uint64(o.quantity) - 1)
-			if o.add {
-				shippingTotal += modifier
-			} else {
-				shippingTotal -= modifier
+	} else if len(is) > 1 {
+		var highest uint64
+		var i int
+		for x, s := range is {
+			if s.primary > highest {
+				highest = s.primary
+				i = x
 			}
+			shippingTotal += (s.secondary * uint64(((1+s.shippingTaxPercentage)*100)+.5) / 100) * uint64(s.quantity)
 		}
+		shippingTotal -= (is[i].primary * uint64(((1+is[i].shippingTaxPercentage)*100)+.5) / 100)
+		shippingTotal += (is[i].secondary * uint64(((1+is[i].shippingTaxPercentage)*100)+.5) / 100)
 	}
-
-	total += shippingTotal
-	return total, nil
+	return total + shippingTotal, nil
 }
 
 func (n *OpenBazaarNode) getPriceInSatoshi(currencyCode string, amount uint64) (uint64, error) {