diefenbach / django-lfs

An online-shop based on Django
http://www.getlfs.com
BSD 3-Clause "New" or "Revised" License
622 stars 222 forks source link

Live search shows non-active products/variants #93

Closed jzuschlag closed 11 years ago

jzuschlag commented 11 years ago

The following should heal the problem:

diff --git a/lfs/search/views.py b/lfs/search/views.py index 0934a39..5c64c81 100644 --- a/lfs/search/views.py +++ b/lfs/search/views.py @@ -31,7 +31,7 @@ def livesearch(request, template_name="lfs/search/livesearch_results.html"): Q(sku_manufacturer__icontains=q) & \ Q(sub_type__in=(STANDARD_PRODUCT, PRODUCT_WITH_VARIANTS, VARIANT))

'- temp = Product.objects.filter(query) '+ temp = Product.objects.filter(query).filter(active=True) total = len(temp) products = temp[0:5]

diefenbach commented 11 years ago

Which version is that?

"active = True" is already a part of the query

https://github.com/diefenbach/django-lfs/blob/master/lfs/search/views.py#L27

and non-active products are not found in 0.7 and dev for me.

jzuschlag commented 11 years ago

Yeah, it is part of the query, but it doesn't work. I found it by inicidence with the following change:

    query = Q(active=True) & \
            Q(name__icontains=q) | \
            Q(sku__icontains=q) | \
            Q(manufacturer__name__icontains=q) | \
            Q(sku_manufacturer__icontains=q) & \
            Q(sub_type__in=(STANDARD_PRODUCT, PRODUCT_WITH_VARIANTS, VARIANT))

    temp = Product.objects.filter(query)

It finds inaktive sku's/products. But even without the search of sku I could find an inaktive product through manufacturer search. Just try the following in the test shop:

Now search the manufacturer. You will find the product in the live search. Klick on it and try to add it to the cart. And bang!

I would prefer to remove it from query and to do it like in the normal search.

diefenbach commented 11 years ago

Thanks.

I think this is because of missing parentheses.

I'll fix this.

jzuschlag commented 11 years ago

Yes, the parentheses are missing. I overlooked it at first glance. And there is a second '&' (sku_manufacturer). Maybe there is another pair of parentheses missing. But doesn't make sense somehow.

But why do we do it in livesearch() and search() differently? We could do a bit of refactoring.

diefenbach commented 11 years ago

Sure, if you find some time just do it and provide a PR.

I, for myself, would prefer the approach within livesearch.