Volume Discount validation bug

[nethrelm]

I was looking at creating a customization to the way volume discounts work, and I saw that there were two services that can be implemented: IDiscountCalculator and IVolumeDiscountProvider. That's great, but there is one problem I found... I shouldn't need to do a custom implementation of IDiscountCalculator with my changes since the only thing I needed to change was the logic in the IVolumeDiscountProvider.IsValidForUser method. The thing is, the default implementation of IDiscountCalculator calls VolumeDiscount.IsValidForUser instead of IVolumeDiscountProvider.IsValidForUser! This is a bit of a DRY problem anyway and should be fixed ASAP so that a custom implementation of IVolumeDiscountProvider actually does what it is supposed to do.

The easiest fix would be to simply replace this (CommerceBuilder.Marketing.VolumeDiscount, line 34):

Code: Select all

        public virtual bool IsValidForUser(User user)
        {
            if (user == null) return false;
            if (this.Groups.Count == 0) 
                return true;

            foreach (Group group in this.Groups)
            {
                if (user.IsInGroup(group.Id))
                    return true;
            }

            return false;
        }

with this:

Code: Select all

        public virtual bool IsValidForUser(User user)
        {
            IVolumeDiscountProvider discountProvider = AbleContext.Resolve<IVolumeDiscountProvider>();
            return discountProvider.IsValidForUser(user, this);
        }

[nethrelm]

Any info on this? There is a similar issue with IPaymentMethodProvider and its UserHasAccess method. The PaymentMethod object also has this method and duplicates (DRY!) the code in the default PaymentMethodProvider.UserHasAccess implementation. I did not find any references that call the PaymentMethod version of this method, so that's good, but the fact that the method itself does not just return the result from the registered IPaymentMethodProvider is bad. There is no point in using IoC if you aren't resolving and calling the service interfaces, or if parts of the library circumvent the services.

[mazhar]

During works for Gold we did some refactoring and created some services. In order to keep the codes backward compatible we left the methods on entity objects and instead invoked the service methods from within them as you suggested for VolumeDiscount.IsValidForUser. We may have overlooked some locations by mistake like the ones you pointed out. We will review the codes and make appropriate updates for future.