Control Live Discount Behavior for Users, Groups, and Anonymous Users

[DWDBE]

…Users

[Copilot]

Pull request overview

This PR adds more granular control over Live Integration discount/price behavior based on the current user (including inherited group settings) and introduces a configuration switch to disable ERP-controlled discounts specifically for anonymous users.

Changes:

• Introduces user/group-aware checks for disabling live prices via a new User extension method.
• Adds DisableErpDiscountsForAnonymousUsers setting and threads a per-user ErpControlsDiscount decision through order XML generation and response processing.
• Updates package/version references (notably Dynamicweb.Core/project version) toward 10.21.x.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/XmlGenerators/OrderXmlGenerator.cs	Uses generator settings’ ErpControlsDiscount when excluding discount lines from hash generation.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/UI/Commands/DownloadOrderXmlCommand.cs	Computes user-specific ERP discount control before generating downloadable order XML.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/TemplatesHelper.cs	Switches live-price disable checks to group-aware extension method.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Products/ProductPriceProvider.cs	Switches live-price disable checks to group-aware extension method.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Products/ProductManager.cs	Switches live-price disable checks to group-aware extension method.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/OrderHandler.cs	Threads per-user erpControlsDiscount through request/response processing; adds IsUserErpDiscountAllowed.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/NotificationSubscribers/VariantListBeforeRender.cs	Switches live-price disable checks to group-aware extension method.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/LiveIntegrationAddIn.cs	Adds UI-exposed setting for disabling ERP discounts for anonymous users.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Helpers.cs	Switches live-price disable checks to group-aware extension method.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Extensions/UserExtensions.cs	New extension method to treat group “live prices disabled” as disabling live prices for the user.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Dynamicweb.Ecommerce.DynamicwebLiveIntegration.csproj	Bumps project/package versions (partial) toward 10.21.x.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Configuration/Settings.cs	Adds DisableErpDiscountsForAnonymousUsers setting and maps it in UpdateFrom.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Configuration/ISettings.cs	Adds DisableErpDiscountsForAnonymousUsers to settings contract.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Configuration/Global.cs	Switches live-price disable checks to group-aware extension method.
src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration.Examples/Dynamicweb.Ecommerce.DynamicwebLiveIntegration.Examples.csproj	Aligns example project package versions to 10.21.5.

Comments suppressed due to low confidence (2)

src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Configuration/ISettings.cs:344

• Interface members should not declare an explicit public modifier for a simple property signature, and this file otherwise omits access modifiers on interface members. This public is inconsistent at best and can be a compile error depending on the language version. Drop the public keyword here to match the rest of the interface.

        /// <summary>
        /// When enabled anonymous users will receive discounts calculated by DynamicWeb instead of retrieving them from the ERP via Live Integration
        /// </summary>
        /// <value><c>true</c> if [disable ERP discounts calculation for anonymous users]; otherwise, <c>false</c>.</value>        
        public bool DisableErpDiscountsForAnonymousUsers { get; set; }

src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Dynamicweb.Ecommerce.DynamicwebLiveIntegration.csproj:30

• The project version and Dynamicweb.Core package are bumped to 10.21.x, but the other Dynamicweb.* dependencies remain at 10.4.0. Mixing these versions is very likely to cause compile/runtime incompatibilities (API surface differences, transitive dependency conflicts). Please align Dynamicweb.CoreUI, Dynamicweb.DataIntegration, Dynamicweb.Ecommerce, and Dynamicweb.Ecommerce.UI to the same 10.21.x baseline (or otherwise pin/validate compatible versions).

	<ItemGroup>
		<PackageReference Include="Dynamicweb.Core" Version="10.21.5" />
		<PackageReference Include="Dynamicweb.CoreUI" Version="10.4.0" />
		<PackageReference Include="Dynamicweb.DataIntegration" Version="10.4.0" />
		<PackageReference Include="Dynamicweb.Ecommerce" Version="10.4.0" />
		<PackageReference Include="Dynamicweb.Ecommerce.UI" Version="10.4.0" />		

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[MatthiasSort]

I double-checked this and I think the new per-user/group/anonymous ERP discount logic makes the existing cart response cache unsafe.

In OrderHandler.UpdateOrder / GetResponse we now compute erpControlsDiscount = IsUserErpDiscountAllowed(settings, user) and thread it into XML generation and response processing, but GetResponse still caches by Helpers.OrderIdentifier(order) only. OrderIdentifier is built from order id/currency/order lines and does not include the effective discount mode or customer identity.

Before this PR that was fine because ErpControlsDiscount was effectively global. With this PR the same cart can legitimately switch between erpControlsDiscount = true/false when the customer logs in or out, or belongs to a group with discounts disabled. In that case we can reuse an ERP response generated under the previous mode and then process it under the current mode, which can mix ERP-priced totals with the Dynamicweb discount flow.

I think the response cache key needs to include the effective discount mode, and probably customer identity as well, or the cache should be bypassed for this path. The order hash cache looks similar and may need the same treatment.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (6)

src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Extensions/UserExtensions.cs:12

• IsLiveIntegrationPricesDisabled dereferences user on the first line (user.IsLivePricesDisabled) without a null guard. While most current callers null-check before invocation, this is an extension method that can be called on a null receiver (((User)null).IsLiveIntegrationPricesDisabled()). Consider returning false when user is null to make the helper safe and consistent with the surrounding null-tolerant call patterns.

        internal static bool IsLiveIntegrationPricesDisabled(this User user)
        {
            if (user.IsLivePricesDisabled)
                return true;

src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/Extensions/UserExtensions.cs:29

• The cache short-circuits on the cached group-result only, but the first branch (user.IsLivePricesDisabled) is re-evaluated on each call. That is fine, but the cache key DynamicwebLiveIntegrationIsLivePricesDisabled{user.ID} is process-/request-scoped via Context.Current.Items, so any cache invalidation on group membership changes within the same request will not be picked up. More importantly, when Context.Current is null (e.g. background jobs, scheduled tasks) the lookup falls through and recomputes ancestor groups on every call — consider documenting this or adding a fallback in-memory cache, since GetAncestorGroups() can be expensive when called per product/orderline.

            var key = $"DynamicwebLiveIntegrationIsLivePricesDisabled{user.ID}";
            var cacheValue = Context.Current?.Items?[key];
            if (cacheValue is not null)
            {
                return Converter.ToBoolean(cacheValue);
            }
            else
            {
                var groups = user.GetAncestorGroups();
                bool result = groups.Any(g => g.IsLivePricesDisabled);
                if (Context.Current?.Items is not null)
                {
                    Context.Current.Items[key] = result;
                }
                return result;
            }

src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/OrderHandler.cs:1419

• IsUserErpDiscountAllowed is called from the hot order-processing path (e.g. PostOrderToErp) and calls user.GetAncestorGroups() plus a LINQ scan every time, with no caching equivalent to the one introduced in UserExtensions.IsLiveIntegrationPricesDisabled. For consistency and to avoid repeated ancestor-group lookups when the same order/user is processed multiple times in a request, consider caching the result similarly (or extracting a shared helper that handles both the user-level and group-level disabled flags with caching).

        internal static bool IsUserErpDiscountAllowed(Settings settings, User user)
        {
            if(!settings.ErpControlsDiscount)
                return false;

            if (user is null)
            {
                return !settings.DisableErpDiscountsForAnonymousUsers;
            }

            if(user.IsLiveDiscountsDisabled)
                return false;

            var groups = user.GetAncestorGroups();
            if (groups.Any(g => g.IsLiveDiscountsDisabled))
                return false;

            return true;
        }

src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/OrderHandler.cs:1411

• Missing space after if keyword on lines 1403 and 1411 (if(!settings.ErpControlsDiscount) / if(user.IsLiveDiscountsDisabled)). The surrounding file and codebase consistently use if (...) with a space. Please match the existing style.

            if(!settings.ErpControlsDiscount)
                return false;

            if (user is null)
            {
                return !settings.DisableErpDiscountsForAnonymousUsers;
            }

            if(user.IsLiveDiscountsDisabled)

src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/OrderHandler.cs:1401

• IsUserErpDiscountAllowed is now internal, making it part of the assembly-internal API and reachable from DownloadOrderXmlCommand. Consider adding XML documentation describing the parameters (especially the contract that a null user is treated as anonymous) and the relationship with Settings.ErpControlsDiscount / Settings.DisableErpDiscountsForAnonymousUsers, matching the doc-comment style used throughout this file.

        internal static bool IsUserErpDiscountAllowed(Settings settings, User user)

src/Dynamicweb.Ecommerce.DynamicwebLiveIntegration/OrderHandler.cs:1419

• The PR introduces two new behaviors (group-level IsLivePricesDisabled inheritance via IsLiveIntegrationPricesDisabled, and per-user/group/anonymous control of ERP-controlled discounts via IsUserErpDiscountAllowed). I see no new unit tests added for either method. Given the number of conditional branches (settings flag off, anonymous user with/without DisableErpDiscountsForAnonymousUsers, user-level disabled, group-level disabled) and the fact that these flags can flip pricing/discount math on live orders, this code deserves test coverage. If the project does not currently have a unit-test project, please ignore.

        internal static bool IsUserErpDiscountAllowed(Settings settings, User user)
        {
            if(!settings.ErpControlsDiscount)
                return false;

            if (user is null)
            {
                return !settings.DisableErpDiscountsForAnonymousUsers;
            }

            if(user.IsLiveDiscountsDisabled)
                return false;

            var groups = user.GetAncestorGroups();
            if (groups.Any(g => g.IsLiveDiscountsDisabled))
                return false;

            return true;
        }

[MatthiasSort]

I re-checked this after the latest changes on 11b3eb5 and I still see the same cache issue.

The relevant bits are still unchanged:

• GetResponse(...) caches by Helpers.OrderIdentifier(order).
• Helpers.OrderIdentifier(order) still only uses order id / currency / order lines.
• GetOrderHashCacheKey(...) still keys off Helpers.GetCurrentExtranetUser() or settings.AnonymousUserKey.

So the PR now has request-specific erpControlsDiscount behavior, but neither the response-cache key nor the order-hash key includes that effective discount mode or the actual order customer identity.

Unless I am missing another invalidation path, I think this is still vulnerable when the same cart is recalculated under a different user context.