Static category view #1135

koradon · 2017-09-21T08:59:22Z

codecov-io · 2017-09-22T14:19:37Z

Codecov Report

Merging #1135 into master will increase coverage by 0.84%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1135      +/-   ##
==========================================
+ Coverage   68.89%   69.74%   +0.84%     
==========================================
  Files         113      115       +2     
  Lines        6163     6524     +361     
  Branches      751      835      +84     
==========================================
+ Hits         4246     4550     +304     
- Misses       1746     1800      +54     
- Partials      171      174       +3

Impacted Files	Coverage Δ
saleor/settings.py	`88.63% <ø> (+1.56%)`	⬆️
saleor/core/templatetags/shop.py	`90% <100%> (+10%)`	⬆️
saleor/product/filters.py	`100% <100%> (ø)`
saleor/core/permissions.py	`100% <100%> (ø)`	⬆️
saleor/core/utils/random_data.py	`80.82% <0%> (-1.13%)`	⬇️
saleor/dashboard/product/urls.py	`100% <0%> (ø)`	⬆️
saleor/dashboard/customer/views.py	`100% <0%> (ø)`	⬆️
saleor/dashboard/product/api.py	`58.13% <0%> (ø)`
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef4c648...ff20509. Read the comment docs.

Delete data-category from category template.

Add dynamically created filters for variant and product attributes.

Add filter for price. Add filters for product/variant attributes - not working properly. Add some template updates.

Refactor filters and views.

Change building product_attributes and variant_attributes to correct one.

Fix filters for product.attributtes and product.variants.

Change filters to be ordered by name. Use product_with_details in category_index view.

Add sort by to filters and template.

Fix category/index.html template. Add category/_items.html template to render product items in category/index.html. Add availability to product list.

Add tests for product/filters.py.

Delete RequestFactory from tests/test_product.py. Use clients instead.

mad-anne · 2017-09-28T08:45:16Z

tests/test_product.py

+                                      default_category):
+    products = models.Product.objects.all()
+    url = reverse('product:category', args=[default_category.slug,
+                                            default_category.pk])


For better readability we rather use kwargs in reverse function (same below).

mad-anne · 2017-09-28T08:50:00Z

tests/test_product.py

+    products = models.Product.objects.all()
+    url = reverse('product:category', args=[default_category.slug,
+                                            default_category.pk])
+    data = {u'price_0': [u'20'], u'price_1': [u'']}


I think you don't need to convert it to unicode, since you are using very basic data in tests (same above).

Refactor some code in product.filters.py, product.views.py, and in templates.

Refactor tests. Add Sort by links to category/index.html.

patrys

Most if not all files (including the test suite) should import unicode_literals. At least until we can drop support for Python 2 😄

patrys · 2017-09-28T11:59:52Z

saleor/core/templatetags/shop.py

+    else:
+        new_sort_by = field  # ascending sort
+    request_get['sort_by'] = new_sort_by
+    return '%s?%s' % (request.path, urlencode(request_get))


Missing from __future__ import unicode_literals

patrys · 2017-09-28T12:00:08Z

saleor/product/filters.py

@@ -0,0 +1,55 @@
+from collections import OrderedDict


Missing from __future__ import unicode_literals

Add missing imports for unicode_literals.

koradon · 2017-09-28T14:04:04Z

Its not done yet :)
But thx for trust.

Missing part is a frontend magic in category/index.html

Add toggle to filters in category/index.html. Add plain JS in template (so far).

Add some js

- Refactor category_index in product.views. - Translat and refactored choices for _sort_by.html. - Delete unused class in _sort_by.html.

Products_with_availability now runs only for products on current page. Products_paginated added to context. Update template.

Delete some duplicated queries in product.filter.py. Products_with_availability now runs only for products on current page. Add products_paginated to context. Update template.

Fis sort_by to have translated labels.

Refactor sort_menu.py.

Add order_by('name') to products_with_detail to have initial ordering by name ascending. Just like on demo site.

Add DEFAULT_SORT to product.filters.py for easy change of default sorting on the category page.

maarcingebala · 2017-10-06T09:49:15Z

saleor/product/filters.py

+        self.category = kwargs.pop('category')
+        super(ProductFilter, self).__init__(*args, **kwargs)
+        self.product_attributes, self.variant_attributes = \
+            self._get_attributes(self.category)


What's the point of passing the instance variable self.category as an argument to a instance method? The _get_attributes method shouldn't require this argument and use self.category internally.

old code remainings, done

maarcingebala · 2017-10-06T09:52:08Z

saleor/product/filters.py

+    def __init__(self, *args, **kwargs):
+        self.category = kwargs.pop('category')
+        super(ProductFilter, self).__init__(*args, **kwargs)
+        self.product_attributes, self.variant_attributes = \


I think we've already talked about this - we don't use backslashes to break lines in our code.

maarcingebala · 2017-10-06T10:32:07Z

saleor/product/filters.py

+            self._get_attributes(self.category)
+        self._add_product_attributes_filters()
+        self._add_product_variants_attributes_filters()
+        self.filters = OrderedDict(sorted(self.filters.items()))


As a result of each of the three lines above the instance variable self.filters is modified. I think that instance variables should represent state of the instance and here we have to call these three particular lines to have it initialized properly. Instead, the first two functions should return some structures, then join them together, sort and and the end the result should be assigned to self.filters.

functions now updates self.filters instead of modifing it

maarcingebala · 2017-10-06T10:44:44Z

saleor/product/templatetags/sort_by.py

+register = template.Library()
+
+
+@register.inclusion_tag('category/_sort_by.html', takes_context=True)


I think the idea of context was misunderstood here. The context that you get in first argument when using takes_context=True is the template context as of when the tag was called. What a inclusion tag should return is the context for the snippet it is rendering, not the whole context of the template. I don't see any need for the context in this tag. Here you should simply return a dictionary with attributes data.

done, but we need to pass request all the way through get_sort_by_url

maarcingebala · 2017-10-06T10:46:01Z

saleor/product/templatetags/sort_menu.py

+    context['sort_by_choices'] = attributes
+    context['arrow_down'] = \
+        (context['request'].GET.get('sort_by', DEFAULT_SORT).startswith('-'))
+    return context


Again, instead of returning whole context you received as first argument, you should construct your own dictionary with data required by _sort_menu.html snippet.

same as above

maarcingebala · 2017-10-06T10:47:21Z

saleor/product/views.py

+           'products': products_and_availability,
+           'products_paginated': products_paginated,
+           'sort_by_choices': SORT_BY_FIELDS,
+           'show_pagination': len(products) > PAGINATE_BY}


len(products) makes additional queries and evaluates the whole queryset of products. This makes the pagination pointless.

maarcingebala · 2017-10-06T10:49:32Z

templates/category/index.html

+            </div>
+            <div class="row">
+              <div class="m-auto">
+                {% if show_pagination %}


{% if products_paginated.has_other_pages %} is much better way of checking this and doesn't make any queries.

Change everything from last review.

Combine sort_by and sort_menu to one template and templatetag. Move templatetags to shop.py. Delete sort_menu.py and _sort_menu.html. Update index.html with new templatetags.

Deleted _sort_by.html and sort_by templatetag from shop.py. Updated index.html and category_index view.

koradon requested a review from maarcingebala September 21, 2017 08:59

patrys added the in progress label Sep 21, 2017

maarcingebala mentioned this pull request Sep 26, 2017

Replace Relay with Apollo Client #1139

Closed

koradon added 13 commits September 27, 2017 15:57

Initial commit

d5ea00c

Delete data-category from category template.

Install django-filter

f3a4a75

Add filters

c8b0780

Add dynamically created filters for variant and product attributes.

Add filters

0deb80f

Add filter for price. Add filters for product/variant attributes - not working properly. Add some template updates.

Refactor filters

7a8cb00

Refactor filters and views.

Fix params in get_attributes_choices

b1969f5

Change ProductFilter

216acc5

Change building product_attributes and variant_attributes to correct one.

Fix filters

58bfffd

Fix filters for product.attributtes and product.variants.

Change filters order

e9baa47

Change filters to be ordered by name. Use product_with_details in category_index view.

Add sort by to filters

c64499a

Add sort by to filters and template.

Fix template

6405d58

Fix category/index.html template. Add category/_items.html template to render product items in category/index.html. Add availability to product list.

Add tests

efaf3ca

Add tests for product/filters.py.

Delete RequestFactory

bd9b761

Delete RequestFactory from tests/test_product.py. Use clients instead.

koradon force-pushed the static_category_view branch from d3f9787 to bd9b761 Compare September 27, 2017 14:00

maarcingebala requested a review from mad-anne September 28, 2017 08:18

mad-anne reviewed Sep 28, 2017

View reviewed changes

koradon added 2 commits September 28, 2017 11:59

Refactor filters

2c6e45f

Refactor some code in product.filters.py, product.views.py, and in templates.

Refactor tests

f12ea5d

Refactor tests. Add Sort by links to category/index.html.

patrys requested changes Sep 28, 2017

View reviewed changes

Add import for unicode_literals

cb9876d

Add missing imports for unicode_literals.

patrys approved these changes Sep 28, 2017

View reviewed changes

koradon requested review from salwator and dominik-zeglen September 28, 2017 14:11

koradon added 2 commits September 28, 2017 16:31

Add toggle to filters

7f85c93

Add toggle to filters in category/index.html. Add plain JS in template (so far).

Add JS

b8dc743

Add some js

salwator approved these changes Oct 4, 2017

View reviewed changes

Final touch

0d26508

- Refactor category_index in product.views. - Translat and refactored choices for _sort_by.html. - Delete unused class in _sort_by.html.

koradon removed the request for review from dominik-zeglen October 4, 2017 09:35

dominik-zeglen and others added 6 commits October 4, 2017 16:29

Revert product grid from 4 to 3 columns

bb29809

Fix filter visual bug

e07cdf4

Fix bug where price filter is not hiding when it is supposed to

e7d6987

Fix usage of products_with_availability

abf4833

Products_with_availability now runs only for products on current page. Products_paginated added to context. Update template.

Fix performance

de3ec9b

Delete some duplicated queries in product.filter.py. Products_with_availability now runs only for products on current page. Add products_paginated to context. Update template.

Hide paginator when products fit in one page

534b31d

dominik-zeglen force-pushed the static_category_view branch from ef26b4a to 534b31d Compare October 5, 2017 11:37

koradon and others added 10 commits October 5, 2017 14:51

Fix imports

f36359c

Fix pep8

4b60e1b

Made sorting menu dynamic

fa59bba

Fix sort_by

420aed7

Fis sort_by to have translated labels.

Fix bug where two dashes were added to attribute instead of one

cdc7daa

Make codeclimate happy

903ffba

Refactor sort_menu.py

85f8669

Refactor sort_menu.py.

Add order_by to initial product geter

1b12449

Add order_by('name') to products_with_detail to have initial ordering by name ascending. Just like on demo site.

Drop relay dependencies

7b607fb

Add DEFAULT_SORT to filters

9bd307e

Add DEFAULT_SORT to product.filters.py for easy change of default sorting on the category page.

maarcingebala requested changes Oct 6, 2017

View reviewed changes

koradon added 5 commits October 6, 2017 13:25

Review changes

d03a11f

Change everything from last review.

Make codeclimate happy

52de1c6

Rename private methods in ProductFilter

0d11f7d

Further refactoring of sort_by and sort_menu

0ba195f

Combine sort_by and sort_menu to one template and templatetag. Move templatetags to shop.py. Delete sort_menu.py and _sort_menu.html. Update index.html with new templatetags.

Further refactoring of sort_by

71c993f

Deleted _sort_by.html and sort_by templatetag from shop.py. Updated index.html and category_index view.

maarcingebala approved these changes Oct 6, 2017

View reviewed changes

Change product column width in products grid

ff20509

maarcingebala merged commit d65b80f into master Oct 6, 2017

maarcingebala deleted the static_category_view branch October 6, 2017 14:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Static category view #1135

Static category view #1135

koradon commented Sep 21, 2017

codecov-io commented Sep 22, 2017 •

edited

mad-anne Sep 28, 2017

mad-anne Sep 28, 2017

patrys left a comment

patrys Sep 28, 2017

koradon Sep 28, 2017

patrys Sep 28, 2017

koradon Sep 28, 2017

koradon commented Sep 28, 2017

maarcingebala Oct 6, 2017

koradon Oct 6, 2017

maarcingebala Oct 6, 2017

koradon Oct 6, 2017

maarcingebala Oct 6, 2017

koradon Oct 6, 2017

maarcingebala Oct 6, 2017

koradon Oct 6, 2017

maarcingebala Oct 6, 2017

koradon Oct 6, 2017

maarcingebala Oct 6, 2017

koradon Oct 6, 2017

maarcingebala Oct 6, 2017

koradon Oct 6, 2017

		register = template.Library()


		@register.inclusion_tag('category/_sort_by.html', takes_context=True)

Static category view #1135

Static category view #1135

Conversation

koradon commented Sep 21, 2017

codecov-io commented Sep 22, 2017 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koradon commented Sep 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Sep 22, 2017 •

edited