Skip to content

Commit

Permalink
#345 Implement crumbs (#346)
Browse files Browse the repository at this point in the history
* #345  Simplify breadcrumbs logic

* #345  Rm model.Page.get_index method

* #345  Implement breadcrumbs functions

* #345  Rm redundant "select_related" field

* #345  Minor logic fixes

* #345  Review#1 fix. Create two public methods instead of one with configuration
  • Loading branch information
duker33 authored Sep 23, 2019
1 parent 202424c commit 6350027
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 132 deletions.
47 changes: 18 additions & 29 deletions pages/logic/page.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,25 @@ def __init__(self, model: models.Page):
# But "model" is Django standard.
self.model = model

def breadcrumbs(self) -> 'Breadcrumbs':
pass
def __str__(self):
return f'<logic.Page: {str(self.model)}>'

def __repr__(self):
return f'<logic.Page: {str(self.model)}>'

@property
def siblings(self) -> models.PageQuerySet:
return self.model.parent.children.exclude(id=self.model.id)


# @todo #343:60m Implement Breadcrumbs class.
# Use it instead of monolithic logic at the `breadcrumbs_with_siblings`.
# Create Breadcrumb class or named tuple to specify crumb data structure.
class Breadcrumbs:
def __init__(self, page_model: models.Page):
self.model = page_model

def query(self, include_self: bool) -> models.PageQuerySet:
return (
self.model
.get_ancestors(include_self)
.select_related(self.model.related_model_name)
.active()
)

def list(self, include_self=False) -> typing.List[typing.Tuple[str, str]]:
"""Breadcrumbs list consists of current page ancestors."""
return [
(crumb.display_menu_title, crumb.url)
for crumb in self.query(include_self).iterator()
]

def list_with_self(self) -> list:
return self.list(include_self=True)
return self.model.parent.children.active().exclude(id=self.model.id)

@property
def breadcrumbs(self) -> 'Pages':
return Pages(self.model.get_ancestors(include_self=True).active())


class Pages:
def __init__(self, models: typing.Iterable[models.Page]):
self.models = models

def __iter__(self):
for model in self.models:
yield Page(model)
4 changes: 0 additions & 4 deletions pages/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,6 @@ class Meta:
verbose_name=_('page template')
)

@classmethod
def get_index(cls):
return Page.objects.filter(type=cls.CUSTOM_TYPE, slug=cls.INDEX_PAGE_SLUG).first()

@property
def url(self):
return self.get_absolute_url()
Expand Down
38 changes: 26 additions & 12 deletions pages/templates/pages/breadcrumbs.html
Original file line number Diff line number Diff line change
@@ -1,19 +1,33 @@
{% load pages_extras %}

<div class="breadcrumbs-wrapper" itemscope itemtype="https://schema.org/BreadcrumbList">
{% for name, url in crumbs_list %}
{% if url %}
<span class="breadcrumbs-item" itemprop="itemListElement" itemscope itemtype="https://schema.org/ListItem">
<a href="{{ base_url }}{{ url }}" itemprop="item" class="breadcrumbs-link">
<span itemprop="name">{{ name }}</span>
</a>
<meta itemprop="position" content="{{ forloop.counter }}">
{% if not forloop.last %}
<span class="breadcrumbs-separator">{{ separator }}</span>
{% endif %}
</span>
{% for logic_page in breadcrumbs %}
<span class="breadcrumbs-item" itemprop="itemListElement" itemscope itemtype="http://schema.org/ListItem">
{% if not forloop.last %}
<a href="{{ logic_page.model.url }}" itemscope itemtype="http://schema.org/Thing" itemprop="item"
class="breadcrumbs-link">
<span itemprop="name">{{ logic_page.model.display_menu_title }}</span>
</a>
{% else %}
<span>{{ name }}</span>
<span itemprop="name">{{ logic_page.model.display_menu_title }}</span>
{% endif %}

{% if show_siblings and logic_page.siblings %}
<ul class="breadcrumbs-siblings-links list-white">
{% for page in logic_page.siblings %}
<li>
<a href="{{ page.url }}" class="list-white-link">
{{ page.display_menu_title }}
</a>
</li>
{% endfor %}
</ul>
{% endif %}

<meta itemprop="position" content="{{ forloop.counter }}">
{% if not forloop.last %}
<span class="breadcrumbs-separator">{{ separator }}</span>
{% endif %}
</span>
{% endfor %}
</div>
33 changes: 0 additions & 33 deletions pages/templates/pages/breadcrumbs_with_siblings.html

This file was deleted.

69 changes: 17 additions & 52 deletions pages/templatetags/pages_extras.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,65 +8,30 @@
register = template.Library()


@register.inclusion_tag('pages/breadcrumbs.html')
def breadcrumbs(page: Page, separator='', base_url=''):
index = page.get_index()

crumbs_list = (
(index.display_menu_title, index.url) if index else ('Main', '/'),
*page.get_ancestors_fields('display_menu_title', 'url', include_self=False),
(page.display_menu_title, '')
)

def _base_breadcrumbs(page: Page, separator='', *, show_siblings=False):
return {
'crumbs_list': crumbs_list,
'breadcrumbs': [
# @todo #345:60m Refold catalog pages in DB.
# In both fixtures and local DB.
# Implement this pages structure:
# - each(category_roots).parent == CustomPage.get('catalog')
# - CustomPage.get('catalog').parent == CustomPage.get('index')
logic.Page(model=CustomPage.objects.get(slug='')), # index page
*list(logic.Page(model=page).breadcrumbs)
],
'separator': separator,
'base_url': base_url,
'show_siblings': show_siblings,
}


@register.inclusion_tag('pages/breadcrumbs_with_siblings.html')
def breadcrumbs_with_siblings(
page: Page, separator='', base_url='', include_self=False
):
def get_ancestors_crumbs() -> list:
ancestors_query = (
page
.get_ancestors(include_self)
.select_related(page.related_model_name)
.active()
)

if not ancestors_query.exists():
return []

catalog, *ancestors = ancestors_query

return [
(catalog.display_menu_title, catalog.url, []),
*[
(
crumb.display_menu_title,
crumb.url,
list(logic.Page(page).siblings)
) for crumb in ancestors
],
]

index = page.get_index()
@register.inclusion_tag('pages/breadcrumbs.html')
def breadcrumbs(page: Page, separator=''):
return _base_breadcrumbs(page, separator, show_siblings=False)

crumbs_list = [
(index.display_menu_title, index.url, []) if index else ('Main', '/', []),
*get_ancestors_crumbs(),
(page.display_menu_title, '', logic.Page(page).siblings)
]

return {
'index_slug': index.url if index else '/',
'crumbs_list': crumbs_list,
'separator': separator,
'base_url': base_url,
}
@register.inclusion_tag('pages/breadcrumbs.html')
def breadcrumbs_with_siblings(page: Page, separator=''):
return _base_breadcrumbs(page, separator, show_siblings=True)


@register.inclusion_tag('pages/accordion.html')
Expand Down
4 changes: 2 additions & 2 deletions tests/catalog/test_views.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
"""
Defines tests for views in Catalog app
"""
from django.test import TestCase
from django.core.urlresolvers import reverse
from django.test import TestCase

from pages.models import CustomPage

from tests.catalog.models import MockCategory, MockProduct


Expand Down Expand Up @@ -87,6 +86,7 @@ def test_product_page(self):
response = self.client.get(self.test_product.url)
self.assertEqual(response.status_code, 200)

# @todo #345:30m Test crumbs siblings.
def test_category_crumbs(self):
"""Category should have valid crumbs"""
page = self.test_last.page
Expand Down

3 comments on commit 6350027

@0pdd
Copy link
Collaborator

@0pdd 0pdd commented on 6350027 Sep 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Puzzle 343-4255fdd0 disappeared from pages/logic/page.py, that's why I closed #345. Please, remember that the puzzle was not necessarily removed in this particular commit. Maybe it happened earlier, but we discovered this fact only now.

@0pdd
Copy link
Collaborator

@0pdd 0pdd commented on 6350027 Sep 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Puzzle 345-5d25d7b3 discovered in tests/catalog/test_views.py and submitted as #347. Please, remember that the puzzle was not necessarily added in this particular commit. Maybe it was added earlier, but we discovered it only now.

@0pdd
Copy link
Collaborator

@0pdd 0pdd commented on 6350027 Sep 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Puzzle 345-e32a958e discovered in pages/templatetags/pages_extras.py and submitted as #348. Please, remember that the puzzle was not necessarily added in this particular commit. Maybe it was added earlier, but we discovered it only now.

Please sign in to comment.