diff --git a/server/basicknowledge/models.py b/server/basicknowledge/models.py index 24e975ed..020477f6 100644 --- a/server/basicknowledge/models.py +++ b/server/basicknowledge/models.py @@ -1,6 +1,6 @@ from django.db import models from django.utils.text import slugify -from wagtail.admin.panels import FieldPanel +from wagtail.admin.panels import FieldPanel, TitleFieldPanel from wagtail.fields import RichTextField, StreamField from wagtail.images.blocks import ImageChooserBlock @@ -110,7 +110,7 @@ class BasicKnowledge(StrictHierarchyPage): ) content_panels = [ - FieldPanel("title", classname="full title"), + TitleFieldPanel("title", classname="full title"), FieldPanel("new_type"), FieldPanel("intro"), FieldPanel("contents"), diff --git a/server/books/models/book.py b/server/books/models/book.py index 352c1165..1a19a2db 100644 --- a/server/books/models/book.py +++ b/server/books/models/book.py @@ -1,6 +1,6 @@ import logging -from wagtail.admin.panels import FieldPanel, TabbedInterface, ObjectList +from wagtail.admin.panels import TabbedInterface, ObjectList, TitleFieldPanel from core.wagtail_utils import StrictHierarchyPage, get_default_settings @@ -13,7 +13,7 @@ class Book(StrictHierarchyPage): verbose_name_plural = 'Bücher' content_panels = [ - FieldPanel('title', classname="full title") + TitleFieldPanel('title', classname="full title") ] edit_handler = TabbedInterface([ diff --git a/server/books/models/chapter.py b/server/books/models/chapter.py index 9332d30d..c13cc4e9 100644 --- a/server/books/models/chapter.py +++ b/server/books/models/chapter.py @@ -1,7 +1,7 @@ import logging from django.db import models -from wagtail.admin.panels import FieldPanel, TabbedInterface, ObjectList +from wagtail.admin.panels import FieldPanel, TabbedInterface, ObjectList, TitleFieldPanel from core.wagtail_utils import StrictHierarchyPage, get_default_settings from users.models import SchoolClass @@ -18,7 +18,7 @@ class Chapter(StrictHierarchyPage, GraphqlNodeMixin): description = models.TextField(blank=True) content_panels = [ - FieldPanel("title", classname="full title"), + TitleFieldPanel("title", classname="full title"), FieldPanel("description", classname="full description"), ] diff --git a/server/books/models/contentblock.py b/server/books/models/contentblock.py index 14132123..2c427d49 100644 --- a/server/books/models/contentblock.py +++ b/server/books/models/contentblock.py @@ -3,6 +3,7 @@ from wagtail.admin.panels import ( FieldPanel, TabbedInterface, ObjectList, + TitleFieldPanel, ) from wagtail.blocks import StreamBlock from wagtail.fields import StreamField @@ -140,7 +141,7 @@ class ContentBlock(StrictHierarchyPage, GraphqlNodeMixin): type = models.CharField(max_length=100, choices=TYPE_CHOICES, default=NORMAL) content_panels = [ - FieldPanel("title", classname="full title"), + TitleFieldPanel("title", classname="full title"), FieldPanel("type"), FieldPanel("contents"), ] diff --git a/server/books/models/module.py b/server/books/models/module.py index 745a85f1..9b43a953 100644 --- a/server/books/models/module.py +++ b/server/books/models/module.py @@ -1,14 +1,16 @@ -from django import forms +from core.constants import DEFAULT_RICH_TEXT_FEATURES +from core.wagtail_utils import StrictHierarchyPage, get_default_settings from django.db import models from django.utils import timezone from django.utils.translation import gettext as _ -from wagtail.admin.forms import WagtailAdminPageForm -from wagtail.admin.panels import FieldPanel, TabbedInterface, ObjectList -from wagtail.fields import RichTextField - -from core.constants import DEFAULT_RICH_TEXT_FEATURES -from core.wagtail_utils import StrictHierarchyPage, get_default_settings from users.models import SchoolClass +from wagtail.admin.panels import ( + FieldPanel, + ObjectList, + TabbedInterface, + TitleFieldPanel, +) +from wagtail.fields import RichTextField EXACT = "exact" @@ -51,20 +53,23 @@ class ModuleCategory(models.Model): return f"{self.name}" -class ModulePageForm(WagtailAdminPageForm): - def clean(self): - cleaned_data = super().clean() - if "slug" in self.cleaned_data: - page_slug = cleaned_data["slug"] - if not Module._slug_is_available(page_slug, self.instance): - self.add_error( - "slug", - forms.ValidationError( - _("The slug '%(page_slug)s' is already in use") - % {"page_slug": page_slug} - ), - ) - return cleaned_data +# Commented out since that check is not necessary if a slug is chosen that is already in use +# a new one will be generated +# TODO: remove after pullrequest is merged +# class ModulePageForm(WagtailAdminPageForm): +# def clean(self): +# cleaned_data = super().clean() +# if "slug" in self.cleaned_data and "id" in self.cleaned_data: +# page_slug = cleaned_data["slug"] +# if not Module._slug_is_available(page_slug, self.parent_page, self.instance): +# self.add_error( +# "slug", +# forms.ValidationError( +# _("The slug '%(page_slug)s' is already in use") +# % {"page_slug": page_slug} +# ), +# ) +# return cleaned_data class Module(StrictHierarchyPage): @@ -97,7 +102,7 @@ class Module(StrictHierarchyPage): solutions_enabled_for = models.ManyToManyField(SchoolClass) content_panels = [ - FieldPanel("title", classname="full title"), + TitleFieldPanel("title", classname="full title"), FieldPanel("meta_title", classname="full title"), FieldPanel("level"), FieldPanel("category"), @@ -106,7 +111,8 @@ class Module(StrictHierarchyPage): FieldPanel("teaser"), FieldPanel("intro"), ] - base_form_class = ModulePageForm + # TODO remove after pullrequest is merged + # base_form_class = ModulePageForm edit_handler = TabbedInterface( [ObjectList(content_panels, heading="Content"), get_default_settings()] @@ -181,11 +187,44 @@ class Module(StrictHierarchyPage): return f"{self.meta_title} - {self.title}" @staticmethod - def _slug_is_available(slug, page): - # modeled after `Page._slug_is_available` - modules = Module.objects.filter(slug=slug).not_page(page) + def _slug_is_available(slug, parent_page, page=None): + """ - return not modules.exists() + # modeled after `Page._slug_is_available` + + Determine whether the given slug is available for use on a child page of + parent_page. If 'page' is passed, the slug is intended for use on that page + (and so it will be excluded from the duplicate check). + """ + if parent_page is None: + # the root page's slug can be whatever it likes... + return True + + modules = Module.objects.all() + if page: + modules = modules.not_page(page) + + return not modules.filter(slug=slug).exists() + + def _get_autogenerated_slug(self, base_slug): + # modeled after `Page._get_autogenerated_slug` + candidate_slug = base_slug + suffix = 1 + parent_page = self.get_parent() + + while not self._slug_is_available(candidate_slug, parent_page, self): + # try with incrementing suffix until we find a slug which is available + suffix += 1 + candidate_slug = "%s-%d" % (base_slug, suffix) + return candidate_slug + + def full_clean(self, *args, **kwargs): + super().full_clean(*args, **kwargs) + + # Always create a slug if it is not available + # todo: do we really want to do this? this will silently change a slug if the users sets one that already exists, which probably isn't what they expect + if not self._slug_is_available(self.slug, self.get_parent(), self): + self.slug = self._get_autogenerated_slug(self.slug) class RecentModule(models.Model): diff --git a/server/books/models/topic.py b/server/books/models/topic.py index 1c32749e..458497c1 100644 --- a/server/books/models/topic.py +++ b/server/books/models/topic.py @@ -1,7 +1,7 @@ import logging from django.db import models -from wagtail.admin.panels import FieldPanel, TabbedInterface, ObjectList +from wagtail.admin.panels import FieldPanel, TabbedInterface, ObjectList, TitleFieldPanel from wagtail.fields import RichTextField from core.constants import DEFAULT_RICH_TEXT_FEATURES @@ -22,7 +22,7 @@ class Topic(StrictHierarchyPage): instructions = models.CharField(max_length=255, blank=True, null=True, default=None) content_panels = [ - FieldPanel('title', classname="full title"), + TitleFieldPanel('title', classname="full title"), FieldPanel('order'), FieldPanel('teaser'), FieldPanel('vimeo_id'), diff --git a/server/books/tests/test_chapter.py b/server/books/tests/test_chapter.py new file mode 100644 index 00000000..2bccce7f --- /dev/null +++ b/server/books/tests/test_chapter.py @@ -0,0 +1,30 @@ +from books.factories import ModuleFactory +from books.models import Chapter +from core.tests.base_test import SkillboxTestCase +from django.test.client import Client + + +class TestChapterCreation(SkillboxTestCase): + """Test created for Issue MS-932""" + + def setUp(self) -> None: + self.createDefault() + self.module = ModuleFactory(slug="my-module") + self.Client = Client() + self.client.login(username="admin", password="test") + + def test_create_chapter_creates_slug_automatically(self): + new_chapter = Chapter(title="New Chapter") + self.module.add_child(instance=new_chapter) + new_chapter.save() + self.assertEqual("new-chapter", new_chapter.slug) + + def test_create_chapter_creates_slug_automatically_if_existing(self): + new_chapter = Chapter(title="New Chapter") + self.module.add_child(instance=new_chapter) + new_chapter.save() + self.assertEqual("new-chapter", new_chapter.slug) + new_chapter2 = Chapter(title="New Chapter") + self.module.add_child(instance=new_chapter2) + new_chapter2.save() + self.assertEqual("new-chapter-2", new_chapter2.slug) diff --git a/server/books/tests/test_module_creation.py b/server/books/tests/test_module_creation.py new file mode 100644 index 00000000..91356e32 --- /dev/null +++ b/server/books/tests/test_module_creation.py @@ -0,0 +1,32 @@ +from django.test import TestCase, RequestFactory +from unittest import skip +from graphene.test import Client +from graphql_relay import to_global_id + +from api.schema import schema +from api.utils import get_object +from books.models import ContentBlock, Chapter +from books.factories import ModuleFactory, ModuleLevelFactory, TopicFactory +from core.factories import UserFactory +from users.models import User + + +class TestModuleCreation(TestCase): + """ + Since the modules url in the frontend is not /topic/module but /module the slug has to be unique. + This test checks if the slug is generated correctly. + """ + + def test_create_new_module_generates_slug(self): + topic = TopicFactory(title="Berufslehre") + module = ModuleFactory(title="Modul 1", parent=topic) + self.assertEqual("modul-1", module.slug) + + def test_create_new_module_different_topic(self): + topic = TopicFactory(title="Berufslehre") + module = ModuleFactory(title="Modul 1", parent=topic) + + topic2 = TopicFactory(title="Geld und Macht") + module2 = ModuleFactory(title="Modul 1", parent=topic2) + self.assertEqual("modul-1", module.slug) + self.assertEqual("modul-1-2", module2.slug, ) diff --git a/server/core/templates/wagtailadmin/pages/listing/_list.html b/server/core/templates/wagtailadmin/pages/listing/_list.html deleted file mode 100644 index 69d831d0..00000000 --- a/server/core/templates/wagtailadmin/pages/listing/_list.html +++ /dev/null @@ -1,124 +0,0 @@ -{# This template is overwritten to create a custom cms ui for the model "chapter" to improve navigation experience.#} -{# See MS-538#} - -{% load i18n %} -{% load l10n %} -{% load wagtailadmin_tags %} - - {% if show_ordering_column or show_bulk_actions %} - - {% endif %} - - {% if show_parent %} - - {% endif %} - - - - - - {% block pre_parent_page_headers %} - {% endblock %} - - {% if parent_page %} - {% page_permissions parent_page as parent_page_perms %} - - - - - - - - {% endif %} - - {% block post_parent_page_headers %} - {% endblock %} - - - {% if pages %} - {% trans "Select page" as checkbox_aria_label %} - {% for page in pages %} - {% page_permissions page as page_perms %} - - {% if show_ordering_column %} - - {% elif show_bulk_actions %} - {% include "wagtailadmin/bulk_actions/listing_checkbox_cell.html" with obj_type="page" obj=page aria_labelledby_prefix="page_" aria_labelledby=page.pk|unlocalize aria_labelledby_suffix="_title" %} - {% endif %} - - - {% if show_parent %} - - {% endif %} - - - - - - {% block page_navigation %} - {% endblock %} - - - {% endfor %} - {% else %} - {% block no_results %}{% endblock %} - {% endif %} - -
- {% block parent_page_title %} - {% endblock %} - {% if parent_page.latest_revision_created_at %} -
- {% blocktrans with time_period=parent_page.latest_revision_created_at|timesince %}{{ time_period }} - ago{% endblocktrans %}
{% endif %}		 - {% if not parent_page.is_root %} - {{ parent_page.content_type.model_class.get_verbose_name }} - {% endif %} - - {% if not parent_page.is_root %} - {% include "wagtailadmin/shared/page_status_tag.html" with page=parent_page %} - {% endif %} -
{% if orderable and ordering == "ord" %} -
{% trans 'Drag' %}
{% endif %}		 - - {% block page_title %} - {% endblock %} - {% if page.content_type.model == 'chapter' %} -
-
    - {% for c in page.get_children %} - {% if not c.specific.user_created and not c.specific.contentblocksnapshot %} -
  • - {% if page_perms.can_edit %} - {{ c.get_admin_display_title }} - - {% else %} - {{ c.get_admin_display_title }} - {% endif %} -
    • - {% endif %} - - {% endfor %} -
-
- {% endif %} - 		- {% block page_parent_page_title %} - {% with page.get_parent as parent %} - {% if parent %} - {{ parent.specific_deferred.get_admin_display_title }} - {% endif %} - {% endwith %} - {% endblock %} - {% if page.latest_revision_created_at %} -
- {% blocktrans with time_period=page.latest_revision_created_at|timesince %}{{ time_period }} - ago{% endblocktrans %}
{% endif %}		{{ page.content_type.model_class.get_verbose_name }} - {% include "wagtailadmin/shared/page_status_tag.html" with page=page %} -
diff --git a/server/core/templates/wagtailadmin/pages/listing/_page_title_explore.html b/server/core/templates/wagtailadmin/pages/listing/_page_title_explore.html new file mode 100644 index 00000000..b80bf3f4 --- /dev/null +++ b/server/core/templates/wagtailadmin/pages/listing/_page_title_explore.html @@ -0,0 +1,60 @@ +{% load i18n wagtailadmin_tags %} + +{# The title field for a page in the page listing, when in 'explore' mode #} + +
+ {% if page.is_site_root %} + {% if perms.wagtailcore.add_site or perms.wagtailcore.change_site or perms.wagtailcore.delete_site %} + {% icon name="site" classname="initial" + %} + {% endif %} + {% endif %} + + {% if page_perms.can_edit %} + + {% if not page.is_site_root and not page.is_leaf %}{% icon name="folder" classname="initial" %}{% endif %} + {{ page.get_admin_display_title }} + + {% else %} + {% if not page.is_site_root and not page.is_leaf %}{% icon name="folder" classname="initial" %}{% endif %} + {{ page.get_admin_display_title }} + {% endif %} + + {% if show_locale_labels %} + {% status page.locale.get_display_name classname="w-status--label" %} + {% endif %} + + + + + {% include "wagtailadmin/pages/listing/_privacy_indicator.html" with page=page %} + {% include "wagtailadmin/pages/listing/_locked_indicator.html" with page=page %} +
+ + + + + + +{% if page.content_type.model == 'chapter' %} +
+ +
+{% endif %} diff --git a/server/core/wagtail_utils.py b/server/core/wagtail_utils.py index 06f1ac17..06950734 100644 --- a/server/core/wagtail_utils.py +++ b/server/core/wagtail_utils.py @@ -2,6 +2,7 @@ from django.contrib import admin from wagtail.admin.panels import CommentPanel from wagtail.admin.panels import FieldPanel, ObjectList from wagtail.models import Page +from wagtail.admin.widgets.slug import SlugInput class StrictHierarchyPage(Page): @@ -41,4 +42,4 @@ def wagtail_parent_filter(parent_cls, child_cls): def get_default_settings(): - return ObjectList([FieldPanel("slug"), CommentPanel()], heading="Settings") + return ObjectList([FieldPanel("slug", widget=SlugInput), CommentPanel()], heading="Settings")