From edfd17a1f17be5f915cce1495e9e1ff75fdfa2bc Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Thu, 25 Jun 2020 14:35:05 +0100 Subject: [PATCH] Reduce usage of edit handlers for workflow management (#6172) * Use plain old Django forms for tasks * Revert "Add ExcludeFieldsOnEditMixin for edit handlers, disabling specific fields when bound to an existing instance rather than creating a new one" This reverts commit 62961b74bb67340fe7d5a2c216021d84ea6902f0. * Move Workflow edit handler creation into Workflow forms Only advantage of having it the way it was before is that allows overriding the edit handler in subclasses. But nobody will be doing this with workflows. I've added a note into the code pointing out that we might want to rethink using edit handlers if: - It gets easier to style formsets without using InlinePanel - We want to allow customisation of the form (the use of edit handlers should be considered an internal implementation detail at the moment) * Make task name readonly but groups editable * Update wagtail/admin/templates/wagtailadmin/workflows/task_chooser/includes/create_form.html Co-authored-by: Dan Braghis * Update wagtail/admin/forms/workflows.py Co-authored-by: Dan Braghis Co-authored-by: Dan Braghis --- docs/advanced_topics/custom_tasks.rst | 53 ++++++++++----- wagtail/admin/edit_handlers.py | 54 +-------------- wagtail/admin/forms/workflows.py | 59 ++++++++++++++++- .../wagtailadmin/workflows/create_task.html | 10 ++- .../wagtailadmin/workflows/edit_task.html | 10 ++- .../task_chooser/includes/create_form.html | 4 +- wagtail/admin/tests/test_workflows.py | 17 +++-- wagtail/admin/views/workflows.py | 66 +++---------------- wagtail/core/models.py | 9 +++ 9 files changed, 147 insertions(+), 135 deletions(-) diff --git a/docs/advanced_topics/custom_tasks.rst b/docs/advanced_topics/custom_tasks.rst index 64ff5c1b7b..07c15d7905 100644 --- a/docs/advanced_topics/custom_tasks.rst +++ b/docs/advanced_topics/custom_tasks.rst @@ -26,7 +26,7 @@ All custom tasks must be models inheriting from ``wagtailcore.Task``. In this se Subclassed Tasks follow the same approach as Pages: they are concrete models, with the specific subclass instance accessible by calling ``Task.specific()``. -You can now add any custom fields. To make these editable in the admin, they must also be added as panels. +You can now add any custom fields. To make these editable in the admin, add the names of the fields into the `admin_form_fields` attribute: For example: @@ -36,18 +36,17 @@ For example: from django.conf import settings from django.db import models - from wagtail.admin.edit_handlers import FieldPanel from wagtail.core.models import Task class UserApprovalTask(Task): user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True, blank=False) - panels = Task.panels + [FieldPanel('user')] + admin_form_fields = Task.admin_form_fields + ['user'] -Any fields that shouldn't be edited after task creation - for example, anything that would fundamentally change the meaning of the task in any history logs - -can be added to ``exclude_on_edit``. For example: +Any fields that shouldn't be edited after task creation - for example, anything that would fundamentally change the meaning of the task in any history logs - +can be added to ``admin_form_readonly_on_edit_fields``. For example: .. code-block:: python @@ -55,24 +54,48 @@ can be added to ``exclude_on_edit``. For example: from django.conf import settings from django.db import models - from wagtail.admin.edit_handlers import FieldPanel from wagtail.core.models import Task class UserApprovalTask(Task): user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True, blank=False) - panels = Task.panels + [FieldPanel('user')] + admin_form_fields = Task.admin_form_fields + ['user'] # prevent editing of ``user`` after the task is created - exclude_on_edit = {'user'} + # by default, this attribute contains the 'name' field to prevent tasks from being renamed + admin_form_readonly_on_edit_fields = Task.admin_form_readonly_on_edit_fields + ['user'] + + +Wagtail will choose a default form widget to use based on the field type. But you can override the form widget using the `admin_form_widgets` attribute: + + +.. code-block:: python + + # /models.py + + from django.conf import settings + from django.db import models + from wagtail.core.models import Task + + from .widgets import CustomUserChooserWidget + + + class UserApprovalTask(Task): + user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True, blank=False) + + admin_form_fields = Task.admin_form_fields + ['user'] + + admin_form_widgets = { + 'user': CustomUserChooserWidget, + } Custom TaskState models ~~~~~~~~~~~~~~~~~~~~~~~ You might also need to store custom state information for the task: for example, a comment left by an approving user. -Normally, this is done on an instance of ``TaskState``, which is created when a page starts the task. However, this can +Normally, this is done on an instance of ``TaskState``, which is created when a page starts the task. However, this can also be subclassed equivalently to ``Task``: .. code-block:: python @@ -93,7 +116,6 @@ Your custom task must then be instructed to generate an instance of your custom from django.conf import settings from django.db import models - from wagtail.admin.edit_handlers import FieldPanel from wagtail.core.models import Task, TaskState @@ -104,13 +126,10 @@ Your custom task must then be instructed to generate an instance of your custom class UserApprovalTask(Task): user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True, blank=False) - panels = Task.panels + [FieldPanel('user')] + admin_form_fields = Task.admin_form_fields + ['user'] task_state_class = UserApprovalTaskState - # prevent editing of ``user`` after the task is created - exclude_on_edit = {'user'} - Customising behaviour ~~~~~~~~~~~~~~~~~~~~~ @@ -148,7 +167,7 @@ For example: ``Task.on_action(task_state, user, action_name)``: This performs the actions specified in ``Task.get_actions(page, user)``: it is passed an action name, eg ``approve``, and the relevant task state. By default, -it calls ``approve`` and ``reject`` methods on the task state when the corresponding action names are passed through. +it calls ``approve`` and ``reject`` methods on the task state when the corresponding action names are passed through. For example, let's say we wanted to add an additional option: cancelling the entire workflow: @@ -251,7 +270,7 @@ Next, you need to instantiate the notifier, and connect it to the ``task_submitt def register_signal_handlers(): task_submitted.connect(user_approval_task_submission_email_notifier, dispatch_uid='user_approval_task_submitted_email_notification') -``register_signal_handlers()`` should then be run on loading the app: for example, by adding it to the ``ready()`` method in your ``AppConfig`` +``register_signal_handlers()`` should then be run on loading the app: for example, by adding it to the ``ready()`` method in your ``AppConfig`` (and making sure this config is set as ``default_app_config`` in ``/__init__.py``). .. code-block:: python @@ -267,4 +286,4 @@ Next, you need to instantiate the notifier, and connect it to the ``task_submitt def ready(self): from .signal_handlers import register_signal_handlers - register_signal_handlers() \ No newline at end of file + register_signal_handlers() diff --git a/wagtail/admin/edit_handlers.py b/wagtail/admin/edit_handlers.py index 0671f97194..0d7f0dfca7 100644 --- a/wagtail/admin/edit_handlers.py +++ b/wagtail/admin/edit_handlers.py @@ -14,7 +14,7 @@ from taggit.managers import TaggableManager from wagtail.admin import compare, widgets from wagtail.core.fields import RichTextField -from wagtail.core.models import GroupApprovalTask, Page, Task, Workflow +from wagtail.core.models import Page from wagtail.core.utils import camelcase_to_underscore, resolve_model_string from wagtail.utils.decorators import cached_classmethod @@ -786,58 +786,6 @@ Page.settings_panels = [ Page.base_form_class = WagtailAdminPageForm -# Similarly, set up wagtailcore.Workflow to have edit handlers -Workflow.panels = [ - FieldPanel("name", heading=gettext_lazy("Give your workflow a name")), - InlinePanel("workflow_tasks", heading=gettext_lazy("Add tasks to your workflow")), -] -Task.panels = [ - FieldPanel("name", heading=gettext_lazy("Give your task a name")), -] -GroupApprovalTask.panels = Task.panels + [FieldPanel('groups', heading=gettext_lazy("Choose approval groups"))] -# do not allow editing of group post creation - this could lead to confusing history if a group is changed after tasks -# are started/completed -GroupApprovalTask.exclude_on_edit = {'groups'} - -Workflow.base_form_class = WagtailAdminModelForm -Task.base_form_class = WagtailAdminModelForm - - -class ExcludeFieldsOnEditMixin: - """A mixin for edit handlers, which disables fields listed in a model's 'exclude_on_edit' attribute when binding - to an existing instance - editing rather than creating""" - - def bind_to(self, *args, **kwargs): - new = super(ExcludeFieldsOnEditMixin, self).bind_to(*args, **kwargs) - # when binding to an existing instance with a pk - ie editing - set those fields in the form to disabled - if new.form and new.instance and new.instance.pk is not None and hasattr(new.model, 'exclude_on_edit'): - for field in new.model.exclude_on_edit: - try: - new.form.fields[field].disabled = True - except KeyError: - continue - return new - - -class VaryOnEditObjectList(ExcludeFieldsOnEditMixin, ObjectList): - pass - - -@cached_classmethod -def get_simple_edit_handler(cls): - """ - Get the EditHandler to use in the Wagtail admin when editing this class, constructing an ObjectList from the contents of cls.panels. - """ - if hasattr(cls, 'edit_handler'): - edit_handler = cls.edit_handler - else: - edit_handler = VaryOnEditObjectList(cls.panels, base_form_class=cls.base_form_class) - return edit_handler.bind_to(model=cls) - - -Workflow.get_edit_handler = get_simple_edit_handler -Task.get_edit_handler = get_simple_edit_handler - @cached_classmethod def get_edit_handler(cls): diff --git a/wagtail/admin/forms/workflows.py b/wagtail/admin/forms/workflows.py index add6bda7ab..e0ae865410 100644 --- a/wagtail/admin/forms/workflows.py +++ b/wagtail/admin/forms/workflows.py @@ -1,10 +1,13 @@ from django import forms -from django.core.exceptions import ValidationError +from django.core.exceptions import ImproperlyConfigured, ValidationError from django.utils.functional import cached_property from django.utils.translation import ugettext as _ from django.utils.translation import ugettext_lazy as __ from wagtail.admin import widgets +from wagtail.admin.edit_handlers import FieldPanel, InlinePanel, ObjectList +from wagtail.admin.forms import WagtailAdminModelForm +from wagtail.admin.widgets.workflows import AdminTaskChooser from wagtail.core.models import Page, Task, Workflow, WorkflowPage from wagtail.core.utils import get_model_string @@ -141,3 +144,57 @@ class BaseWorkflowPagesFormSet(forms.BaseInlineFormSet): WorkflowPagesFormSet = forms.inlineformset_factory( Workflow, WorkflowPage, form=WorkflowPageForm, formset=BaseWorkflowPagesFormSet, extra=1, can_delete=True, fields=['page'] ) + + +class BaseTaskForm(forms.ModelForm): + pass + + +def get_task_form_class(task_model, for_edit=False): + """ + Generates a form class for the given task model. + + If the form is to edit an existing task, set for_edit to True. This applies + the readonly restrictions on fields defined in admin_form_readonly_on_edit_fields. + """ + fields = task_model.admin_form_fields + + form_class = forms.modelform_factory( + task_model, + form=BaseTaskForm, + fields=fields, + widgets=getattr(task_model, 'admin_form_widgets', {}) + ) + + if for_edit: + for field_name in getattr(task_model, 'admin_form_readonly_on_edit_fields', []): + if field_name not in form_class.base_fields: + raise ImproperlyConfigured( + "`%s.admin_form_readonly_on_edit_fields` contains the field " + "'%s' that doesn't exist. Did you forget to add " + "it to `%s.admin_form_fields`?" + % (task_model.__name__, field_name, task_model.__name__) + ) + + form_class.base_fields[field_name].disabled = True + + return form_class + + +def get_workflow_edit_handler(): + """ + Returns an edit handler which provides the "name" and "tasks" fields for workflow. + """ + # Note. It's a bit of a hack that we use edit handlers here. Ideally, it should be + # made easier to reuse the inline panel templates for any formset. + # Since this form is internal, we're OK with this for now. We might want to revisit + # this decision later if we decide to allow custom fields on Workflows. + + panels = [ + FieldPanel("name", heading=_("Give your workflow a name")), + InlinePanel("workflow_tasks", [ + FieldPanel('task', widget=AdminTaskChooser(show_clear_link=False)), + ], heading=_("Add tasks to your workflow")), + ] + edit_handler = ObjectList(panels, base_form_class=WagtailAdminModelForm) + return edit_handler.bind_to(model=Workflow) diff --git a/wagtail/admin/templates/wagtailadmin/workflows/create_task.html b/wagtail/admin/templates/wagtailadmin/workflows/create_task.html index da7b168064..32def3eb00 100644 --- a/wagtail/admin/templates/wagtailadmin/workflows/create_task.html +++ b/wagtail/admin/templates/wagtailadmin/workflows/create_task.html @@ -25,7 +25,15 @@
{% csrf_token %} - {% block form %}{{ edit_handler.render_form_content }}{% endblock %} +
    + {% for field in form %} + {% if not field.is_hidden %} + {% include "wagtailadmin/shared/field_as_li.html" %} + {% else %} + {{ field }} + {% endif %} + {% endfor %} +
{% block footer %}