#550 Cleanup code

shopelectro/context.py

@@ -9,24 +9,20 @@
 """
 
 import typing
+from abc import ABC, abstractmethod
 from functools import lru_cache, partial
 
 from django import http
 from django.db.models import QuerySet
 from django.conf import settings
 from django.core.paginator import Paginator, InvalidPage
-from django.shortcuts import render, get_object_or_404
-from django.views.decorators.http import require_POST
 from django_user_agents.utils import get_user_agent
 
-from catalog.views import catalog
 from images.models import Image
-from pages import views as pages_views
 
 from shopelectro import models
-from shopelectro.views.helpers import set_csrf_cookie
 
-from pages.models import ModelPage, Page
+from pages.models import ModelPage
 
 
 class SortingOption:
@@ -49,7 +45,12 @@ def merge_products_context(products: QuerySet):
         models.ProductPage.objects.filter(shopelectro_product__in=products)
     )
 
-    # TODO - create product.get_brand instead
+    # @todo #550:30m Create Product.get_brand method
+    #  We use `brands.get(product)` below.
+    #  So, `Product.get_brand` is what we are needed here.
+    #  We can take care of performance
+    #  with `prefetch_related('tags')` approach.
+    #  Also create queries count test.
     brands = (
         models.Tag.objects
         .filter_by_products(products)
@@ -66,34 +67,32 @@ class ObjectsComposition:
 
     super: 'ObjectsComposition' = None
 
-    # TODO - resolve objects good piping
     def __or__(self, other: 'ObjectsComposition'):
         other.super = self
         return other
 
 
-class AbstractContext(ObjectsComposition):
+class AbstractContext(ObjectsComposition, ABC):
 
     super: 'AbstractContext' = None
 
     def __init__(
         self,
-        # TODO slug maybe part of url_kwargs
-        slug='',
         url_kwargs: typing.Dict[str, str]=None,
         request: http.HttpRequest=None
     ):
         """
         :param url_kwargs: Came from `urls` module.
         :param request: Came from `urls` module
         """
-        self.slug_ = slug
+        if url_kwargs:
+            assert 'slug' in url_kwargs
         self.url_kwargs_ = url_kwargs or {}
         self.request_ = request
 
     @property
     def slug(self) -> str:
-        return self.slug_ or self.super.slug
+        return self.url_kwargs['slug']
 
     @property
     def url_kwargs(self) -> typing.Dict[str, str]:
@@ -108,12 +107,12 @@ def request(self) -> http.HttpRequest:
     def page(self):
         return ModelPage.objects.get(slug=self.slug)
 
+    @abstractmethod
     def get_context_data(self) -> typing.Dict[str, typing.Any]:
-        raise NotImplementedError
+        ...
 
 
-# TODO make it ABC or smth
-class ProductsListContext(AbstractContext):
+class ProductsListContext(AbstractContext, ABC):
 
     super: 'ProductsListContext' = None
 
@@ -135,7 +134,8 @@ def products(self) -> QuerySet:
 
     def get_context_data(self):
         """Add sorting options and view_types in context."""
-        # TODO take from dataclass or smth
+        # @todo #550:15m Take `view_type` value from dataclass.
+        #  Depends on updating to python3.7
         view_type = self.request.session.get('view_type', 'tile')
 
         group_tags_pairs = (
@@ -147,7 +147,7 @@ def get_context_data(self):
         return {
             'products_data': merge_products_context(self.products),
             'group_tags_pairs': group_tags_pairs,
-            # TODO add comment about view_type
+            # can be `tile` or `list`. Defines products list layout.
             'view_type': view_type,
         }
 
@@ -157,7 +157,7 @@ class TaggedCategoryContext(ProductsListContext):
     def get_sorting_index(self):
         return int(self.url_kwargs.get('sorting', 0))
 
-    # TODO - test if it's cached
+    # @todo #550:30m Test if context.TaggedCategory.get_tags cached.
     def get_tags(self) -> typing.Optional[models.TagQuerySet]:
 
         @lru_cache(maxsize=64)
@@ -181,7 +181,8 @@ def products(self):
                 .filter(tags__in=tags)
                 # Use distinct because filtering by QuerySet tags,
                 # that related with products by many-to-many relation.
-                # TODO - try to rm this sorting staff
+                # @todo #550:60m Try to rm sorting staff from context.TaggedCategory.
+                #  Or explain again why it's impossible. Now it's not clear from comment.
                 .distinct(sorting_option.field)
             )
         return products
@@ -192,7 +193,9 @@ def get_context_data(self):
         return {
             **context,
             'tags': tags,
-            # TODO write comment about skip_canonical
+            # Category's canonical link is `category.page.get_absolute_url`.
+            # So, this link always contains no tags.
+            # That's why we skip canonical link on tagged category page.
             'skip_canonical': bool(tags),
         }
 
@@ -215,8 +218,7 @@ def template_context(page, tag_titles, tags):
 
         tags = context['tags']
         if tags:
-            # TODO - move to QuerySet
-            tag_titles = models.serialize_tags_to_title(tags)
+            tag_titles = tags.as_title()
             page.get_template_render_context = partial(
                 template_context, page, tag_titles, tags
             )
@@ -258,14 +260,10 @@ def get_context_data(self):
 
 class PaginationCategoryContext(ProductsListContext):
 
-    # TODO - move it to settings
-    PRODUCTS_ON_PAGE_PC = 48
-    PRODUCTS_ON_PAGE_MOB = 12
-
     def get_products_count(self):
         """Calculate max products list size from request. List size depends on device type."""
         mobile_view = get_user_agent(self.request).is_mobile
-        return self.PRODUCTS_ON_PAGE_MOB if mobile_view else self.PRODUCTS_ON_PAGE_PC
+        return settings.PRODUCTS_ON_PAGE_MOB if mobile_view else settings.PRODUCTS_ON_PAGE_PC
 
     def get_paginated_page_or_404(self, per_page, page_number):
         try:

shopelectro/models.py

@@ -256,6 +256,44 @@ def get_brands(self, products: List[Product]) -> Dict[Product, 'Tag']:
             if product in brand.products.all()
         }
 
+    def as_string(
+        self,
+        field_name: str,
+        type_delimiter: str,
+        group_delimiter: str,
+    ) -> str:
+        """
+        :param field_name: Only field's value is used to represent tag as string.
+        :param type_delimiter:
+        :param group_delimiter:
+        :return:
+        """
+        if not self:
+            return ''
+
+        group_tags_map = self.get_group_tags_pairs()
+
+        _, tags_by_group = zip(*group_tags_map)
+
+        return group_delimiter.join(
+            type_delimiter.join(getattr(tag, field_name) for tag in tags_list)
+            for tags_list in tags_by_group
+        )
+
+    def as_title(self) -> str:
+        return self.as_string(
+            field_name='name',
+            type_delimiter=settings.TAGS_TITLE_DELIMITER,
+            group_delimiter=settings.TAG_GROUPS_TITLE_DELIMITER
+        )
+
+    def as_url(self) -> str:
+        return self.as_string(
+            field_name='slug',
+            type_delimiter=settings.TAGS_URL_DELIMITER,
+            group_delimiter=settings.TAG_GROUPS_URL_DELIMITER
+        )
+
 
 class TagManager(models.Manager.from_queryset(TagQuerySet)):
 
@@ -320,43 +358,6 @@ def parse_url_tags(tags: str) -> list:
         ))
 
 
-def serialize_tags(
-    tags: TagQuerySet,
-    field_name: str,
-    type_delimiter: str,
-    group_delimiter: str,
-) -> str:
-    if not tags:
-        return ''
-
-    group_tags_map = tags.get_group_tags_pairs()
-
-    _, tags_by_group = zip(*group_tags_map)
-
-    return group_delimiter.join(
-        type_delimiter.join(getattr(tag, field_name) for tag in tags_list)
-        for tags_list in tags_by_group
-    )
-
-
-def serialize_tags_to_url(tags: TagQuerySet) -> str:
-    return serialize_tags(
-        tags=tags,
-        field_name='slug',
-        type_delimiter=settings.TAGS_URL_DELIMITER,
-        group_delimiter=settings.TAG_GROUPS_URL_DELIMITER
-    )
-
-
-def serialize_tags_to_title(tags: TagQuerySet) -> str:
-    return serialize_tags(
-        tags=tags,
-        field_name='name',
-        type_delimiter=settings.TAGS_TITLE_DELIMITER,
-        group_delimiter=settings.TAG_GROUPS_TITLE_DELIMITER
-    )
-
-
 class ExcludedModelTPageManager(PageManager):
 
     def get_queryset(self):

shopelectro/settings/base.py

@@ -487,3 +487,6 @@ def get_robots_content():
 
 # products with price lower this value should not be presented at gm.yml price
 PRICE_GM_LOWER_BOUND = 200.0
+
+PRODUCTS_ON_PAGE_PC = 48
+PRODUCTS_ON_PAGE_MOB = 12

shopelectro/tests/tests_views.py

@@ -19,7 +19,7 @@
 from django.urls import reverse
 from django.utils.translation import ugettext as _
 
-from shopelectro.models import Category, Product, Tag, TagGroup, TagQuerySet, serialize_tags_to_url
+from shopelectro.models import Category, Product, Tag, TagGroup, TagQuerySet
 from shopelectro.views.catalog import merge_products_context
 from shopelectro.views.service import generate_md5_for_ya_kassa, YANDEX_REQUEST_PARAM
 
@@ -38,7 +38,7 @@ def reverse_catalog_url(
     if tags:
         # PyCharm's option:
         # noinspection PyTypeChecker
-        tags_slug = serialize_tags_to_url(tags)
+        tags_slug = tags.as_url()
         route_kwargs['tags'] = tags_slug
     if sorting is not None:
         route_kwargs['sorting'] = sorting

shopelectro/views/catalog.py

@@ -190,9 +190,7 @@ def get_products(self) -> QuerySet:
     def get_context_data(self, **kwargs):
         """Add sorting options and view_types in context."""
         context_ = (
-            context.CategoryContext(
-                self.object.slug, self.kwargs, self.request
-            )
+            context.CategoryContext(self.kwargs, self.request)
             | context.TaggedCategoryContext()
             | context.SortingCategoryContext()
             | context.PaginationCategoryContext()