Create a pagination object for view. Create todos for the issue (#325)

Update todo

Create tests for pagination

Fix todo

Review fixes

Review fixes

shopelectro/settings/base.py

@@ -295,6 +295,7 @@
 )
 
 TOP_PRODUCTS = [291, 438, 1137, 2166, 2725, 2838, 3288, 3884, 3959, 2764]
+CATEGORY_STEP_MULTIPLIERS = [12, 15, 24, 25, 48, 50, 60, 100]
 
 # Reduce retail product prices by PRICE_REDUCER.
 # It is required to make prices on shopelectro.ru and se78.ru unique.

shopelectro/tests/tests_views.py

@@ -9,7 +9,7 @@
 from itertools import chain
 from operator import attrgetter
 from xml.etree import ElementTree as ET
-from urllib.parse import urlparse, quote
+from urllib.parse import urlencode, urlparse, quote
 
 from bs4 import BeautifulSoup
 from django.conf import settings
@@ -26,11 +26,14 @@
 CANONICAL_HTML_TAG = '<link rel="canonical" href="{path}">'
 
 
-def reverse_category_url(
-    category: Category, tags: TagQuerySet=None, sorting: int=None
+def reverse_catalog_url(
+    url: str,
+    route_kwargs: dict,
+    tags: TagQuerySet=None,
+    sorting: int=None,
+    query_string: dict=None,
 ) -> str:
-    route_kwargs = {'slug': category.page.slug}
-
+    query_string = f'?{urlencode(query_string)}' if query_string else ''
     if tags:
         # PyCharm's option:
         # noinspection PyTypeChecker
@@ -39,28 +42,43 @@ def reverse_category_url(
     if sorting is not None:
         route_kwargs['sorting'] = sorting
 
-    return reverse('category', kwargs=route_kwargs)
+    return f'{reverse(url, kwargs=route_kwargs)}{query_string}'
+
+
+def get_page_number(response):
+    return response.context['paginated_page'].number
 
 
 def json_to_dict(response: HttpResponse) -> dict():
     return json.loads(response.content)
 
 
 class CatalogPage(TestCase):
+    """
+    @todo #302 Divide the CatalogPage test class into parts related to the features.
+    """
 
     fixtures = ['dump.json']
 
     def setUp(self):
         self.category = Category.objects.root_nodes().select_related('page').first()
         self.tags = Tag.objects.order_by(*settings.TAGS_ORDER).all()
 
-    def get_category_page(self, category=None, tags=None):
+    def get_category_page(
+        self,
+        category: Category=None,
+        tags: TagQuerySet=None,
+        sorting: int=None,
+        query_string: dict=None,
+    ):
         category = category or self.category
-        return self.client.get(reverse_category_url(category, tags))
+        return self.client.get(reverse_catalog_url(
+            'category', {'slug': category.page.slug}, tags, sorting, query_string,
+        ))
 
     def test_category_page_contains_all_tags(self):
         """Category contains all Product's tags."""
-        response = self.client.get(reverse('category', args=(self.category.page.slug, )))
+        response = self.get_category_page()
 
         tags = set(chain.from_iterable(map(
             lambda x: x.tags.all(), Product.objects.get_by_category(self.category)
@@ -73,18 +91,22 @@ def test_category_page_contains_all_tags(self):
 
     def test_has_canonical_meta_tag(self):
         """Test that CategoryPage should contain canonical meta tag."""
-        url = reverse_category_url(self.category)
-        response = self.client.get(url)
+        response = self.get_category_page()
         self.assertEqual(response.status_code, 200)
-        self.assertContains(response, CANONICAL_HTML_TAG.format(path=url))
+        self.assertContains(
+            response,
+            CANONICAL_HTML_TAG.format(path=response.request['PATH_INFO']),
+        )
 
     def test_tags_page_has_no_canonical_meta_tag(self):
         """Test that CategoryTagsPage should not contain canonical meta tag."""
         # ignore CPDBear
-        url = reverse_category_url(self.category, self.tags)
-        response = self.client.get(url)
+        response = self.get_category_page(tags=self.tags)
         self.assertEqual(response.status_code, 200)
-        self.assertNotContains(response, CANONICAL_HTML_TAG.format(path=url))
+        self.assertNotContains(
+            response,
+            CANONICAL_HTML_TAG.format(path=response.request['PATH_INFO']),
+        )
 
     def test_paginated_tags_page_has_no_canonical_meta_tag(self):
         """
@@ -94,10 +116,12 @@ def test_paginated_tags_page_has_no_canonical_meta_tag(self):
         should not contain canonical meta tag.
         """
         # ignore CPDBear
-        url = reverse_category_url(self.category, self.tags, sorting=1)
-        response = self.client.get(url)
+        response = self.get_category_page(tags=self.tags, sorting=1)
         self.assertEqual(response.status_code, 200)
-        self.assertNotContains(response, CANONICAL_HTML_TAG.format(path=url))
+        self.assertNotContains(
+            response,
+            CANONICAL_HTML_TAG.format(path=response.request['PATH_INFO'])
+        )
 
     def test_contains_product_with_certain_tags(self):
         """Category page contains Product's related by certain tags."""
@@ -170,6 +194,72 @@ def test_product_tag_linking(self):
         for link in property_links:
             self.assertContains(response, link)
 
+    def test_pagination_numbering(self):
+        page_number = 1
+        response = self.get_category_page(query_string={'page': page_number})
+        self.assertEqual(get_page_number(response), page_number)
+
+    def test_pagination_products_count(self):
+        """
+        @todo #302:30m Implement test case for pagination logic.
+         Products number changes in depend on page number.
+         If step=24 and page number=2, then products quantity is 48.
+         If step=24 and page number=2 and total products quantity is 40, then products quantity is 40.
+        """
+
+    def test_pagination_step(self):
+        """CategoryPage should contain `pagination_step` products count in list."""
+        pagination_step = 25
+        response = self.get_category_page(query_string={'step': pagination_step})
+        self.assertEqual(len(response.context['product_image_pairs']), pagination_step)
+
+
+class LoadMore(TestCase):
+
+    fixtures = ['dump.json']
+    DEFAULT_LIMIT = 48
+
+    def setUp(self):
+        self.category = Category.objects.root_nodes().select_related('page').first()
+
+    def load_more(
+        self,
+        category: Category=None,
+        tags: TagQuerySet=None,
+        offset: int=0,
+        # uncomment after implementation urls for load_more with pagination
+        # limit: int=0,
+        sorting: int=0,
+        query_string: dict=None,
+    ) -> HttpResponse:
+        category = category or self.category
+        route_kwargs = {
+            'category_slug': category.page.slug,
+            'offset': offset,
+            # uncomment after implementation urls for load_more with pagination
+            # 'limit': limit,
+        }
+        return self.client.get(reverse_catalog_url(
+            'load_more', route_kwargs, tags, sorting, query_string,
+        ))
+
+    def test_pagination_numbering_first_page(self):
+        self.assertEqual(get_page_number(self.load_more()), 1)
+
+    def test_pagination_numbering_last_page(self):
+        offset = Product.objects.get_by_category(self.category).count() - 1
+        self.assertEqual(
+            get_page_number(self.load_more(offset=offset)),
+            offset // self.DEFAULT_LIMIT + 1,
+        )
+
+    def test_pagination_numbering_rest_page(self):
+        offset = self.DEFAULT_LIMIT + 1
+        self.assertEqual(
+            get_page_number(self.load_more(offset=offset)),
+            2,
+        )
+
 
 class SitemapXML(TestCase):
     """

shopelectro/urls.py

@@ -41,6 +41,7 @@ def cache_page(arg):  # Ignore PyFlakesBear
 ]
 
 catalog_urls = [
+    # "category" group
     url(r'^categories/(?P<slug>[\w-]+)/$',
         cached_2h(views.CategoryPage.as_view()), name='category'),
     url(r'^categories/(?P<slug>[\w-]+)/tags/(?P<tags>[\w-]+)/$',
@@ -49,12 +50,14 @@ def cache_page(arg):  # Ignore PyFlakesBear
         views.CategoryPage.as_view(), name='category'),
     url(r'^categories/(?P<slug>[\w-]+)/(?P<sorting>[0-9]*)/tags/(?P<tags>[\w-]+)/$',
         views.CategoryPage.as_view(), name='category'),
+    # "load more" group
     url(r'categories/(?P<category_slug>[\w-]+)/load-more/'
         r'(?P<offset>[0-9]+)/(?P<sorting>[0-9]*)/$',
         views.load_more, name='load_more'),
     url(r'categories/(?P<category_slug>[\w-]+)/load-more/'
         r'(?P<offset>[0-9]+)/(?P<sorting>[0-9]*)/tags/(?P<tags>[\w-]+)/$',
         views.load_more, name='load_more'),
+    # rest of urls
     url(r'^no-images/$', views.ProductsWithoutImages.as_view(),
         name='products_without_images'),
     url(r'^no-text/$', views.ProductsWithoutText.as_view(),

shopelectro/views/catalog.py

@@ -1,7 +1,8 @@
 from functools import partial
 
 from django.conf import settings
-from django.http import HttpResponse, HttpResponseForbidden
+from django.core.paginator import Paginator
+from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden
 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
@@ -15,11 +16,11 @@
 from shopelectro.views.helpers import set_csrf_cookie
 
 PRODUCTS_ON_PAGE_PC = 48
-PRODUCTS_ON_PAGE_MOB = 10
+PRODUCTS_ON_PAGE_MOB = 12
 
 
 def get_products_count(request):
-    """Get Products count for response context depends on the `user_agent`."""
+    """Calculate max products list size from request. List size depends on device type."""
     mobile_view = get_user_agent(request).is_mobile
     return PRODUCTS_ON_PAGE_MOB if mobile_view else PRODUCTS_ON_PAGE_PC
 
@@ -113,16 +114,20 @@ class CategoryPage(catalog.CategoryPage):
 
     def get_context_data(self, **kwargs):
         """Add sorting options and view_types in context."""
-        context = super(CategoryPage, self).get_context_data(**kwargs)
-        products_on_page = get_products_count(self.request)
-
-        # tile is default view_type
+        context = super().get_context_data(**kwargs)
+        products_on_page = int(self.request.GET.get(
+            'step', get_products_count(self.request),
+        ))
+        page_number = int(self.request.GET.get('page', 1))
         view_type = self.request.session.get('view_type', 'tile')
-
-        category = context['category']
-
         sorting = int(self.kwargs.get('sorting', 0))
         sorting_option = config.category_sorting(sorting)
+        category = context['category']
+        if (
+            page_number < 1 or
+            products_on_page not in settings.CATEGORY_STEP_MULTIPLIERS
+        ):
+            raise Http404('Page does not exist.')
 
         all_products = (
             models.Product.objects
@@ -165,37 +170,55 @@ def template_context(page, tag_titles, tags):
         page.get_template_render_context = partial(
             template_context, page, tag_titles, tags)
 
-        products = all_products.get_offset(0, products_on_page)
+        paginated_page = Paginator(all_products, products_on_page).page(page_number)
+        total_products = all_products.count()
+        products = paginated_page.object_list
+        if not products:
+            raise Http404('Page without products does not exist.')
 
         return {
             **context,
             'product_image_pairs': merge_products_and_images(products),
             'group_tags_pairs': group_tags_pairs,
-            'total_products': all_products.count(),
+            'total_products': total_products,
+            'products_count': (page_number - 1) * products_on_page + products.count(),
+            'paginated_page': paginated_page,
             'sorting_options': config.category_sorting(),
+            'limits': settings.CATEGORY_STEP_MULTIPLIERS,
             'sort': sorting,
             'tags': tags,
             'view_type': view_type,
             'skip_canonical': bool(tags),
         }
 
 
-def load_more(request, category_slug, offset=0, sorting=0, tags=None):
+def load_more(request, category_slug, offset=0, limit=0, sorting=0, tags=None):
     """
     Load more products of a given category.
 
     :param sorting: preferred sorting index from CATEGORY_SORTING tuple
     :param request: HttpRequest object
     :param category_slug: Slug for a given category
     :param offset: used for slicing QuerySet.
-    :return:
+    :return: products list in html format
     """
-    products_on_page = get_products_count(request)
-
+    products_on_page = limit or get_products_count(request)
+    offset = int(offset)
+    if offset < 0:
+        return HttpResponseBadRequest('The offset is wrong. An offset should be greater than or equal to 0.')
+    if products_on_page not in settings.CATEGORY_STEP_MULTIPLIERS:
+        return HttpResponseBadRequest(
+            'The limit number is wrong. List of available numbers:'
+            f' {", ".join(map(str, settings.CATEGORY_STEP_MULTIPLIERS))}'
+        )
+    # increment page number because:
+    # 11 // 12 = 0, 0 // 12 = 0 but it should be the first page
+    # 12 // 12 = 1, 23 // 12 = 1, but it should be the second page
+    page_number = (offset // products_on_page) + 1
     category = get_object_or_404(models.CategoryPage, slug=category_slug).model
     sorting_option = config.category_sorting(int(sorting))
 
-    products = (
+    all_products = (
         models.Product.objects
         .prefetch_related('page__images')
         .select_related('page')
@@ -207,19 +230,21 @@ def load_more(request, category_slug, offset=0, sorting=0, tags=None):
             slug__in=models.Tag.parse_url_tags(tags)
         )
 
-        products = (
-            products
+        all_products = (
+            all_products
             .filter(tags__in=tag_entities)
             # Use distinct because filtering by QuerySet tags,
             # that related with products by many-to-many relation.
             .distinct(sorting_option.lstrip('-'))
         )
 
-    products = products.get_offset(int(offset), products_on_page)
+    paginated_page = Paginator(all_products, products_on_page).page(page_number)
+    products = paginated_page.object_list
     view = request.session.get('view_type', 'tile')
 
     return render(request, 'catalog/category_products.html', {
         'product_image_pairs': merge_products_and_images(products),
+        'paginated_page': paginated_page,
         'view_type': view,
         'prods': products_on_page,
     })

templates/catalog/category.html

@@ -32,7 +32,7 @@ <h1 class="category-title">{{ page.display_h1|capfirst }}</h1>
 
       <p class="products-showed">
         Показано товаров
-        <span class="font-bold js-products-showed-count"> {{ product_image_pairs|length }} </span>
+        <span class="font-bold js-products-showed-count"> {{ products_count }} </span>
         из <span class="font-bold js-total-products">{{ total_products }}</span>.
       </p>
 
@@ -46,7 +46,7 @@ <h1 class="category-title">{{ page.display_h1|capfirst }}</h1>
                 data-url="{{ category.url }}" type="button">Загрузить ещё</button>
         <p class="products-showed-text">
           Показано товаров
-          <span class="js-products-showed-count">{{ product_image_pairs|length }}</span> из
+          <span class="js-products-showed-count">{{ products_count }}</span> из
           <span class="js-total-products">{{ total_products }}</span>
         </p>
       </div>

templates/catalog/category_products.html

@@ -2,6 +2,12 @@
 {% load se_extras %}
 {% load user_agents %}
 
+{% comment %} @todo #302:60m Implement pagination buttons for seo purposes.
+ reate buttons with django templates on backend and hide them with js on frontend.
+ Load_more button should reniew canonical url for current page.
+ See the parent task for details.
+{% endcomment %}
+
 {% for product, image in product_image_pairs %}
   <div class="product-card col-xs-6 col-md-4" productId="{{ product.id }}"
        itemscope itemtype="http://schema.org/Product">