I recommend some refactoring in AdjustShippingRate (src\Libraries\Nop.Services\Orders\OrderTotalCalculationService.cs) - IsFreeShipping() is called once at top, but again in GetShoppingCartAdditionalShippingCharge(cart);
I think we can just short circuit at the top. Here's the rewrite:
public virtual decimal AdjustShippingRate(decimal shippingRate,
IList<ShoppingCartItem> cart, out Discount appliedDiscount)
{
if (IsFreeShipping(cart))
return decimal.Zero;
//additional shipping charges
decimal additionalShippingCharge = GetShoppingCartAdditionalShippingCharge(cart);
var adjustedRate = shippingRate + additionalShippingCharge;
//discount
var customer = cart.GetCustomer();
appliedDiscount = null;
decimal discountAmount = GetShippingDiscount(customer, adjustedRate, out appliedDiscount);
adjustedRate = adjustedRate - discountAmount;
if (adjustedRate < decimal.Zero)
adjustedRate = decimal.Zero;
if (_shoppingCartSettings.RoundPricesDuringCalculation)
adjustedRate = Math.Round(adjustedRate, 2);
return adjustedRate;
}
Also, during checkout process (src\Presentation\Nop.Web\Controllers\CheckoutController.cs), GetShippingOptions is called twice - first to get all methods to show customer (CheckoutController.ShippingMethod), and then when customer selects a method called again to get all options for the method-name chosen CheckoutController.SelectShippingMethod. I.e. it calls the selected option's shipping rate calculation method again to get all of its options to then match the selected name to get the specific option (rate/description). I can understand that only the name is being passed in on the url during the action, but couldn't you store the entire list of shipping options in the _workContext during CheckoutController.ShippingMethod? This could save a bit of time when external carrier APIs are called.
