Add attribute filters in subcategories #2949
Conversation
saleor/product/filters.py
Outdated
| @@ -88,10 +88,10 @@ def __init__(self, *args, **kwargs): | |||
| super().__init__(*args, **kwargs) | |||
|
|
|||
| def _get_product_attributes_lookup(self): | |||
| return Q(product_types__products__category=self.category) | |||
| return Q(product_types__products__category=self.get_descendants(include_self=True)) | |||
There was a problem hiding this comment.
self is the ProductCategoryFilter instance, I think you've meant self.category?
There was a problem hiding this comment.
Yes, I'm gonna push improved commit in few minutes
|
@pitero92 Thanks for opening the PR! We care not only about the quality of our code but also about the quality of all the written part on Github 😉. It would be great if you could:
In case of any question feel free to ask. Thanks! |
pitero92
changed the title
pitero92
changed the title
pitero92
changed the title
|
@maarcingebala Thank You for your feedback. I've already corrected what you asked for. |
| @@ -0,0 +1,106 @@ | |||
| from collections import OrderedDict | |||
There was a problem hiding this comment.
Is this whole file used? I think it was added accidentally.
|
@pitero92 Thanks, now it looks much better! It seems that one of the Python tests is failing on CI, it needs to be fixed before we can merge the PR. |
| @@ -88,10 +88,10 @@ def __init__(self, *args, **kwargs): | |||
| super().__init__(*args, **kwargs) | |||
|
|
|||
| def _get_product_attributes_lookup(self): | |||
| return Q(product_types__products__category=self.category) | |||
| return Q(product_types__products__category=self.category.get_descendants(include_self=True)) | |||
There was a problem hiding this comment.
Two notes about both changes:
get_descentants()returns a Queryset, therefore you should use in lookup instead of=, which is suitable for single objects (like it was previously withself.category).- Both changed lines exceed the allowed characters limit per line (79). We could improve the formatting e.g. by extracting a variable
categories = self.category.get_descendants(include_self=True)
There was a problem hiding this comment.
Right, I'd also like to see a test for it
There was a problem hiding this comment.
@maarcingebala could You explain your 1st point?
This line should look something like return Q(product_types__products__category__in(categories))?
Where categories = self.category.get_descendants(include_self=True)
There was a problem hiding this comment.
@pitero92 Please refer to the Django's docs regarding the queryset methods: https://docs.djangoproject.com/en/2.1/ref/models/querysets/#in
Getting the category's descendants is correct.
filters.py
Outdated
|
|
||
| def _get_variant_attributes_lookup(self): | ||
| return Q(product_variant_types__products__category=self.category.get_descendants(include_self=True)) | ||
| return Q(product_variant_types__products__category__in=categories) |
There was a problem hiding this comment.
Categories will be undefined at this point
filters.py
Outdated
| @@ -83,12 +83,12 @@ def _get_attribute_choices(self, attribute): | |||
|
|
|||
|
|
|||
| class ProductCategoryFilter(ProductFilter): | |||
| categories = self.category.get_descendants(include_self=True) | |||
There was a problem hiding this comment.
I think you should take a different approach in resolving bugs.
I think that you could start the Saleor on your local instance and reproduce the situation listed in the issue to confirm that the bug appears in your local copy of Saleor.
Having a bug reproduced, you could make attempts to fix it, as in the more complex situation it might not be possible to fix it via the guessing game.
It's not better, as that would throw an error as self will be undefined at this point. You should define it categories in each function calling for attributes lookup.
|
Closing due to inactivity. |
|
Continued in #3535 |
This PR fixes a problem with availability of filters in subcategories
Now filters are avaible in subcategories
Fixes #2697