[KrzysztofPajak]

Type: bugfix

Issue

Payment, discount, shipping and widget plugins resolved services with HttpContext!.RequestServices (CashOnDeliveryPaymentProvider, BrainTreePaymentProvider, ShoppingCartDiscountRule, ByWeightShippingCalcPlugin, WidgetsGoogleAnalyticsViewComponent). Outside an HTTP request, for example from the API module, scheduled tasks or a background mediator handler, they threw NullReferenceException, and their dependencies were hidden.

Two of these were workarounds for real DI cycles:

• OrderCalculationService → IPaymentService → IPaymentProvider → IOrderCalculationService. The percentage handling fee needs the cart subtotal.
• PricingService → DiscountHandlerService → DiscountValidationService → DiscountProviderLoader → DiscountProvider → ShoppingCartDiscountRule → IPricingService.

Solution

• New IShoppingCartSubTotalService / ShoppingCartSubTotalService in Grand.Business.Checkout. The subtotal calculation and GetOrderSubtotalDiscount moved there unchanged, and OrderCalculationService.GetShoppingCartSubTotal delegates to it. Payment providers depend on the new service, so the cycle is gone. The IPaymentProvider contract is unchanged.
• New GetShoppingCartItemsSubTotalQuery with a handler in Grand.Business.Catalog. ShoppingCartDiscountRule sends it through IMediator, so the handler and IPricingService are resolved only at send time. The rule now reads the cart of request.Customer instead of WorkContext.CurrentCustomer, and a request without a customer does not meet the requirement. The query carries the customer and store, and the handler prices each item for them (IPricingService.GetUnitPrice with an explicit customer, store and currency), so the cart and its prices always belong to the same customer. The sum is in the primary store currency, the currency the spent-amount requirement is entered in; before, a shopper paying in another currency was compared against the requirement in the wrong currency.
• The remaining dependencies (ICurrencyService, IContextAccessor, IPaymentTransactionService, IShippingByWeightService, IPricingService in ByWeight, and the GA widget's order, country, product and category services) are constructor-injected. They were never part of a cycle.
• Fix: the percentage handling fee in Cash on delivery and BrainTree was converted to the working currency twice. The subtotal is already in the working currency, so only the fixed fee is converted now.
• BrainTree keeps IHttpContextAccessor because the card form is stored in the session. Computing the fee no longer needs it.
• Fix: the Google Analytics ecommerce script threw for an order item whose product had since been deleted; the item is now sent with an empty name and category.
• Not changed: Authentication.OpenIdConnect still uses context.RequestServices in its endpoint delegate and event handler. There, that is the request's own container.

Breaking changes

• The OrderCalculationService constructor takes IShoppingCartSubTotalService instead of IPricingService, ICheckoutAttributeParser and IProductService. GetOrderSubtotalDiscount (protected virtual) moved to ShoppingCartSubTotalService. A plugin that subclasses OrderCalculationService or constructs it by hand has to follow.
• OrderCalculationService.GetShoppingCartSubTotal is no longer virtual. Payment providers read the subtotal from IShoppingCartSubTotalService, so an override in OrderCalculationService would make the order total and the payment fee disagree. A subclass that overrides it stops compiling; move the customization to ShoppingCartSubTotalService, whose GetShoppingCartSubTotal and GetOrderSubtotalDiscount are virtual.
• The constructors of CashOnDeliveryPaymentProvider, BrainTreePaymentProvider, ShoppingCartDiscountRule, ByWeightShippingCalcPlugin and WidgetsGoogleAnalyticsViewComponent changed. This only matters to code that subclasses or constructs them.
• A plugin built against the old OrderCalculationService constructor has to be rebuilt.

Testing

1. dotnet build GrandNode.slnx
2. dotnet test src/Tests/Grand.Business.Checkout.Tests: 245 passed
3. dotnet test src/Tests/Grand.Business.Catalog.Tests: 576 passed, 1 skipped
4. dotnet test src/Tests/Grand.Modules.Tests: 375 passed. New tests:
   • CashOnDeliveryPaymentProviderTests and BrainTreePaymentProviderTests: the provider is built from DI with no HttpContext; percentage fee without a second conversion; fixed fee converted; CancelPayment.
   • ShoppingCartDiscountRuleTests: cart of the request customer and store, passed on to the query; no customer.
   • PluginDependencyCycleTests: the real core services next to the real plugins, built with ValidateOnBuild. Putting IPricingService back into ShoppingCartDiscountRule makes this test fail. ShippingProviders_DoNotDependBackOnShipping covers ShippingService with ByWeight, real pricing, discount services and the standard rules.
5. GetShoppingCartItemsSubTotalQueryHandlerTests in Grand.Business.Catalog.Tests: items priced for the request customer and store in the primary currency, missing products skipped.

After the review follow-up (38beba0), re-run: Grand.Modules.Tests Plugins folder (166 passed), GetShoppingCartItemsSubTotalQueryHandlerTests, OrderCalculationService tests (10 passed), and a build of Widgets.GoogleAnalytics. The full solution build and full test suites were not re-run.

Not done: manual check of the storefront cart and checkout.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.