Refactor LinkBlock to use a StreamBlock for the different link types.

.gitignore

@@ -1,5 +1,6 @@
 *.egg-info
 __pycache__
 .cache
+.pytest_cache
 build/
 dist/

tests/test_blocks.py

@@ -1,16 +1,28 @@
 import datetime
+import pytest
 from unittest.mock import patch
 
 from django.core.exceptions import ValidationError
 from freezegun import freeze_time
 from phonenumber_field.phonenumber import PhoneNumber
-import pytest
+
 from wagtail.wagtailcore.models import Page
 from wagtail_extensions.blocks import (DepartmentBlock, LinkBlock,
                                        OpeningTimeBlock, OpeningTimesBlock,
                                        PhoneBlock)
 
 
+@pytest.mark.django_db
+@pytest.fixture
+def page():
+    # Homepage is created by Wagtail's initial migrations
+    # But let's create our own child page for testing with.
+    homepage = Page.objects.get(url_path='/home/')
+    page = Page(title='A test page', slug="test")
+    homepage.add_child(instance=page)
+    return page
+
+
 def test_department_block_clean_invalid():
     department = DepartmentBlock()
     with pytest.raises(ValidationError):
@@ -33,51 +45,45 @@ def test_department_block_to_python_strip_empty_phonenumbers():
     assert value['phones'] == ['+447528712345']
 
 
-@pytest.mark.django_db
-def test_link_block_clean_just_page():
-    link = LinkBlock()
-    link.clean({'text': 'A link', 'page': 1})
+def test_link_block_with_url():
+    block = LinkBlock()
+    value = block.to_python({
+        'link': [{'type': 'url', 'value': '/hello/'}]
+    })
+    html = block.render(value)
+    assert html.strip() == '<a href="/hello/">/hello/</a>'
 
 
-def test_link_block_clean_just_url():
-    link = LinkBlock()
-    link.clean({'text': 'A link', 'absolute_url': 'https://foo.com'})
-
-
-def test_link_block_clean_both_page_and_url():
-    link = LinkBlock()
-    with pytest.raises(ValidationError):
-        link.clean({
-            'text': 'A link',
-            'page': '1',
-            'absolute_url': 'https://foo.com',
-        })
+def test_link_block_with_url_and_text():
+    block = LinkBlock()
+    value = block.to_python({
+        'text': 'Hello World',
+        'link': [{'type': 'url', 'value': '/hello/'}]
+    })
+    html = block.render(value)
+    assert html.strip() == '<a href="/hello/">Hello World</a>'
 
 
 @pytest.mark.django_db
-def test_link_block_get_context_page():
-    link = LinkBlock()
-    ctx = link.get_context({'page': Page.objects.get(pk=2)})
-    assert ctx['url'] == "/"
+def test_link_block_with_page(page):
+    block = LinkBlock()
+    value = block.to_python({
+        'link': [{'type': 'page', 'value': page.pk}]
+    })
 
-
-def test_link_block_get_context_absolute_url():
-    link = LinkBlock()
-    ctx = link.get_context({'absolute_url': 'http://test.com/'})
-    assert ctx['url'] == "http://test.com/"
+    html = block.render(value)
+    assert html.strip() == '<a href="{}">{}</a>'.format(page.url, page.title)
 
 
 @pytest.mark.django_db
-def test_link_block_get_context_page_and_absolute_url():
-    link = LinkBlock()
-    ctx = link.get_context({'page': Page.objects.get(pk=2), 'absolute_url': 'http://test.com/'})
-    assert ctx['url'] == "/"
-
-
-def test_link_block_get_context_no_url():
-    link = LinkBlock()
-    ctx = link.get_context({})
-    assert ctx['url'] == None
+def test_link_block_with_page_and_text(page):
+    block = LinkBlock()
+    value = block.to_python({
+        'text': 'Hello World',
+        'link': [{'type': 'page', 'value': page.pk}]
+    })
+    html = block.render(value)
+    assert html.strip() == '<a href="{}">Hello World</a>'.format(page.url)
 
 
 @freeze_time("2017-01-01")

wagtail_extensions/blocks.py

@@ -14,6 +14,7 @@
 from phonenumber_field import phonenumber
 from phonenumber_field.formfields import PhoneNumberField
 from wagtail.wagtailcore import blocks
+from wagtail.wagtaildocs.blocks import DocumentChooserBlock
 from wagtail.wagtailimages.blocks import ImageChooserBlock
 from wagtailgeowidget.blocks import GeoBlock
 
@@ -33,36 +34,37 @@ def get_prep_value(self, value):
 
 
 class LinkBlock(blocks.StructBlock):
-    text = blocks.CharBlock(required=False)
-    page = blocks.PageChooserBlock(required=False)
-    absolute_url = blocks.CharBlock(label="Url", required=False)
-
-    class Meta:
-        template = 'wagtail_extensions/blocks/link.html'
-
-    def clean(self, value):
-        if value.get('page') and value.get('absolute_url'):
-            errors = {
-                'page': ErrorList([
-                    ValidationError('Either a page or url must be defined'),
-                ]),
-            }
-            raise ValidationError('There is a problem with this link', params=errors)
 
-        return super().clean(value)
+    text = blocks.CharBlock(required=False)
+    link = blocks.StreamBlock([
+        ('page', blocks.PageChooserBlock()),
+        ('document', DocumentChooserBlock()),
+        ('url', blocks.CharBlock(label="URL (absolute or relative)"))
+    ], min_num=1, max_num=1)
 
     def get_context(self, value, parent_context=None):
         ctx = super().get_context(value, parent_context=parent_context)
-        if value.get('page'):
-            url = value['page'].url
-        elif value.get('absolute_url'):
-            url = value.get('absolute_url')
+
+        link_item = value['link'][0]
+        if link_item.block_type in ('page', 'document'):
+            link_url = link_item.value.url
+            link_text = link_item.value.title
         else:
-            url = None
-        ctx['url'] = url
+            link_url = link_item.value      # raw url
+            link_text = link_item.value
+
+        # If text is set then it takes precedence
+        if value.get('text') is not None:
+            link_text = value['text']
+        ctx['link_url'] = link_url
+        ctx['link_text'] = link_text
         return ctx
 
 
+    class Meta:
+        template = 'wagtail_extensions/blocks/link.html'
+
+
 class CarouselItemBlock(blocks.StructBlock):
 
     image = ImageChooserBlock()

wagtail_extensions/templates/wagtail_extensions/blocks/carousel_item.html

@@ -2,11 +2,13 @@
 
 {% image item.image fill-1600x600 as item_image %}
 <div class="carousel-item {% if forloop.first %}active{% endif %}">
-    <a {% if item.link.url %}href="{{ item.link.url }}"{% endif %} class="item-inner" style="background-image: url({{ item_image.url }});">
+    {% with link=item.link.link.0 %}
+    <a {% if link %}href="{% firstof link.value.url link.value %}"{% endif %} class="item-inner" style="background-image: url({{ item_image.url }});">
         {% if item.caption %}
         <div class="carousel-caption">
             <h3>{{ item.caption }}</h3>
         </div>
         {% endif %}
     </a>
+    {% endwith %}
 </div>

wagtail_extensions/templates/wagtail_extensions/blocks/link.html

@@ -1,9 +1 @@
-{% if url %}
-    <a href="{{ url }}">
-        {% if value.page %}
-            {% firstof value.text value.page.title %}
-        {% else %}
-            {% firstof value.text url %}
-        {% endif %}
-    </a>
-{% endif %}
+<a href="{{ link_url }}">{{ link_text }}</a>