From 9f68373a17b7730d2e9a7c56d36740151b731ec8 Mon Sep 17 00:00:00 2001
From: duker <duker33@gmail.com>
Date: Thu, 6 Sep 2018 07:20:48 +0300
Subject: [PATCH] #550  Minor self-review fixes

---
 shopelectro/context.py           | 23 +++++++++++++++--------
 shopelectro/tests/tests_views.py |  2 ++
 shopelectro/views/catalog.py     |  4 +---
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/shopelectro/context.py b/shopelectro/context.py
index 7383cae5..0b77f94b 100644
--- a/shopelectro/context.py
+++ b/shopelectro/context.py
@@ -1,10 +1,15 @@
 """
 Contains view context classes.
-Context is part of the View abstract layer in MTV paradigm.
+Context is our own concept.
+It's part of the View abstract layer in MTV paradigm.
+Data flow looks like this:
+Urls -> Context -> View.
+So, by concept, View receives context directly
+without accessing url args.
 It stands between urls and view functions.
 
 Every context class is used via objects composition.
-Code example to create tagged:
+Code example to create tagged category:
 
 >>> from django.http import HttpRequest
 >>> url_kwargs = {'slug': 'my-page'}
@@ -40,6 +45,7 @@ def directed_field(self):
         return self.direction + self.field
 
 
+# @todo #550:30m Split to ProductImagesContext and ProductBrandContext
 @lru_cache(maxsize=64)
 def merge_products_context(products: QuerySet):
     assert isinstance(products, QuerySet)
@@ -50,7 +56,8 @@ def merge_products_context(products: QuerySet):
 
     # @todo #550:30m Create Product.get_brand method
     #  We use `brands.get(product)` below.
-    #  So, `Product.get_brand` is what we are needed here.
+    #  So, `Product.get_brand` or `ProductQuerySet.get_brand`
+    #  is what we are needed here.
     #  We can take care of performance
     #  with `prefetch_related('tags')` approach.
     #  Also create queries count test.
@@ -156,6 +163,7 @@ def get_context_data(self):
         #  Depends on updating to python3.7
         view_type = self.request.session.get('view_type', 'tile')
 
+        # @todo - create product.get_brand instead
         group_tags_pairs = (
             models.Tag.objects
             .filter_by_products(self.products)
@@ -219,13 +227,13 @@ def get_context_data(self):
 
 
 class DBTemplate(AbstractPageContext):
-    """Processes some page data fields as templates with each own context."""
+    """Processes some page data fields as templates with their own context."""
 
     @property
     @lru_cache(maxsize=1)
     def page(self):
         page = self.super.page
-        context = self.get_super_context_data()
+        context = self.get_super_context_data_cached()
 
         def template_context(page, tag_titles, tags):
             return {
@@ -244,14 +252,13 @@ def template_context(page, tag_titles, tags):
         return page
 
     @lru_cache(maxsize=1)
-    def get_super_context_data(self):
-        """Just for cache."""
+    def get_super_context_data_cached(self):
         return self.super.get_context_data()
 
     @lru_cache(maxsize=1)
     def get_context_data(self):
         return {
-            **self.get_super_context_data(),
+            **self.get_super_context_data_cached(),
             'page': self.page,
         }
 
diff --git a/shopelectro/tests/tests_views.py b/shopelectro/tests/tests_views.py
index c9b2fdf2..4fa53849 100644
--- a/shopelectro/tests/tests_views.py
+++ b/shopelectro/tests/tests_views.py
@@ -81,8 +81,10 @@ def test_merge_product_cache(self):
         """Context merging should cached."""
         products = Product.objects.all()[:2]
         with self.assertNumQueries(7):
+            # N db queries without before cached
             merge_products_context(products)
         with self.assertNumQueries(0):
+            # no db queries after cached
             merge_products_context(products)
 
 
diff --git a/shopelectro/views/catalog.py b/shopelectro/views/catalog.py
index 3e97f214..358c3ea6 100644
--- a/shopelectro/views/catalog.py
+++ b/shopelectro/views/catalog.py
@@ -159,14 +159,12 @@ def get_context_data(self, **kwargs):
         }
 
 
-# TODO - split to ProductImagesContext and ProductBrandContext
 @lru_cache(maxsize=64)
 def merge_products_context(products):
     images = Image.objects.get_main_images_by_pages(
         models.ProductPage.objects.filter(shopelectro_product__in=products)
     )
 
-    # TODO - create product.get_brand instead
     brands = (
         models.Tag.objects
         .filter_by_products(products)
@@ -202,7 +200,7 @@ def get_context_data(self, **kwargs):
         }
 
 
-# TODO make load_more one of the context classes
+# @todo #550:60m Make `catalog.views.load_more` one of the context classes
 def load_more(request, category_slug, offset=0, limit=0, sorting=0, tags=None):
     """
     Load more products of a given category.