Discounts Bug

Hi,

We would like to report a bug that we have noticed with the discounts in nopCommerce 2.4.

To reproduce it:

1. Create a Discount that is Assigned to Categories.
2. Assign the discount to a category.
3. Go back and edit the Discount and change its type to be Assigned to Product Variants and save it.
4. Go back to the category, which was previously assigned the discount and click the Discount tab. It would say that there are not Discounts that could be assigned to a category.
5. Open your nopCommerce 2.4 database and go to the Discount_AppliedToCategories table and you will see that despite the fact that the discount type was changed and it should have removed the mapping to the category the record that maps the discount to the category still exists.

This is a problem when calculating the final price for products in this category as the record that remains in the database will make nopCommerce consider the discount applied to the category while in reality it is not.

On a separate issued, there are no entities for the Discount_AppliedToCategories and Discount_AppliedToProductVariants tables and thus we are not able to write more complex linq queries that involve joins to these tables. Would be possible to add entities for these tables in the default object context?

Many Thanks

7Spikes wrote:
Although a record still exists in the [Discount_AppliedToCategories] table, this discount is not applied. Try testing it one more time. You can also open \Libraries\Nop.Services\Catalog\PriceCalculationService.cs file and have a look at 'GetAllowedDiscounts' method to ensure that they're not returned.

var productCategories = _categoryService.GetProductCategoriesByProductId(productVariant.ProductId);
            if (productCategories != null)
            {
                foreach (var productCategory in productCategories)
                {
                    var categoryDiscounts = productCategory.Category.AppliedDiscounts;
                    foreach (var discount in categoryDiscounts)
                    {
                        if (_discountService.IsDiscountValid(discount, customer) &&
                            discount.DiscountType == DiscountType.AssignedToCategories &&
                            !allowedDiscounts.ContainsDiscount(discount))
                            allowedDiscounts.Add(discount);
                    }
                }
            }

7Spikes wrote:
Right. There should not be any mapping entity. This is how EF works. But 'ProductVariant' entity has 'AppliedDiscounts' property and 'Category' entity also has 'AppliedDiscounts' property. Use them for writing queries. EF will convert them to appropriate SQL commands

Hi Andrei,

Thanks for the info!

Yes I can see now that the PriceCalculationService does make this check for the DiscountType when determining the discounts for a category. But the PriceCalculationService is not what we are interested in.
We are actually implementing some custom logic where we just need to know whether a discount on a category exists and we have a method DiscountAppliedToCategoryExists and we thought we could just implement it like this:

var query = from d in DiscountRepository.Table
            where
                d.AppliedToCategories.FirstOrDefault(
                    x => x.Id == categoryId) != null
            select d;

Discount discount = query.FirstOrDefault();

return discount != null;

The fact that the record remains in the database makes this return incorrect results. We can and will of course change this method to check the discount type, but it did seem odd that there will be a mapped discount to a category in the backend when the discount is not actually mapped in the business layer.

As to the entities for Discount_AppliedToCategories and Discount_AppliedToProductVariants tables I believe we do have a problem, because just like the above method we have a DiscountAppliedToProductVariantsInCategoryExists method that just checks whether a discount exists for at least one ProductVariant in a given category, a method that goes like this:

var productVariantQuery = (from pv in ProductVariantRepository.Table
                        join p in ProductRepository.Table
                            on pv.ProductId equals p.Id
                        join pc in ProductCategoryRepository.Table
                            on p.Id equals pc.ProductId
                        where pc.CategoryId == categoryId
                        select pv).GroupBy(x => x.ProductId).Select(
                            y => y.FirstOrDefault());

int[] productVariantIds =
    productVariantQuery.ToList().Select(x => x.Id).ToArray();

var discountQuery = from d in DiscountRepository.Table
                    where d.AppliedToProductVariants.Any()
                    select d;

IList<Discount> discounts = discountQuery.ToList();
bool discountExists = false;
foreach (var discount in discounts)
{
    if (
        discount.AppliedToProductVariants.Select(x => x.Id).Intersect(
            productVariantIds).Any())
    {
        discountExists = true;
        break;
    }
}

return discountExists;

This code is quite awkward and not really optimized if there are many ProductVariants in the category. If there was a  Discount_AppliedToProductVariants entity this code would have been just a simple linq query with one inner join. Is there a reason why there could not be entities for these tables? There is an entity for the Product_Category_Mapping table for example.

Thanks

7Spikes wrote:
I see. I've just created a work item here. But I'm sure that the provided source code can be modified to check the discount type (DiscountType.AssignedToCategories). I'm a bit busy with the new version preparations, so don't have time to write and test the right query. Drop me a PM once the new version is released and I'll help you.

Yes, no problem! Whenever there is time for this as it is not a big priority obviously.
Actually it is the second problem with the entities we are mostly after.
But will drop you a PM later when you have time.

Thanks

Fixed. Please see changeset b3d5ab0eb1bf

Hi Andrei,

Thanks for this!
Regarding the other problem is there any chance the entities for these tables could be added for the next release of nopCommerce?

Discount_AppliedToCategories
Discount_AppliedToProductVariants

As described previously we believe the lack of these entities is problematic for LINQ queries. It is not something of a huge priority though.

Thanks

It's not possible (not supported by Entity Framework). You should use existing navigation properties

how can we use this query

SELECT d.DiscountAmount,d.DiscountPercentage,p.Name,* from Product p  
INNER JOIN Discount_AppliedToProducts dp on p.Id = dp.Product_Id
inner JOIN Discount d on p.Id = d.Id

from code side to get discount in home page ?

where i can use this code for get data ?

i search for product discount Ribbon in home page

You're SQL is not quite right; it should be
... JOIN Discount d on dp.Discount_Id = d.Id

But in any case, rather than using SQL, you should (as per Andrei above), try to use navigation properties (i.e. Linq)

(also, regading "...product discount Ribbon .." - makes it's not clear whether you are trying to show ALL discounted products, or just the discount applied for specific home page products)