Add additional check for workflow being removed on remove_workflow view. Fix bug in add_to_page when models are missing. Move workflow edit fbv to cbv. Change permissions checks to use policy

wagtail/admin/templates/wagtailadmin/workflows/edit.html

@@ -20,9 +20,7 @@
 
 {% block content %}
 
-    {% block header %}
-        {% include "wagtailadmin/shared/header.html" with tabbed=1 merged=1 %}
-    {% endblock %}
+    {% include "wagtailadmin/shared/header.html" with title=view.page_title icon=view.header_icon %}
 
     <form action="{% block form_action %}{{ view.edit_url }}{% endblock %}"{% if is_multipart %} enctype="multipart/form-data"{% endif %} method="POST" novalidate>
         {% csrf_token %}

wagtail/admin/templates/wagtailadmin/workflows/listing/_list_workflow_pages.html

@@ -4,7 +4,7 @@
 
 {% block page_navigation %}
     <td class="remove-workflow" valign="top">
-        <form action="{% url 'wagtailadmin_workflows:remove' page.id %}" method="post">
+        <form action="{% url 'wagtailadmin_workflows:remove' page.id workflow.pk %}" method="post">
             {% csrf_token %}
             <input type="hidden" name="next" value="{{ request.path }}">
             <button type="submit" class="button button-secondary button-small">{% trans 'Remove' %}</button>

wagtail/admin/urls/workflows.py

@@ -7,7 +7,8 @@ app_name = 'wagtailadmin_workflows'
 urlpatterns = [
     path('', workflows.Index.as_view(), name='index'),
     path('add/', workflows.Create.as_view(), name='add'),
-    path('edit/<int:pk>/', workflows.edit, name='edit'),
-    path('remove/<int:pk>/', workflows.remove_workflow, name='remove'),
+    path('edit/<int:pk>/', workflows.Edit.as_view(), name='edit'),
+    path('remove/<int:page_pk>/', workflows.remove_workflow, name='remove'),
+    path('remove/<int:page_pk>/<int:workflow_pk>/', workflows.remove_workflow, name='remove'),
     path('add_to_page/<int:workflow_pk>/', workflows.add_to_page, name='add_to_page'),
 ]

wagtail/admin/views/workflows.py

@@ -59,6 +59,47 @@ class Create(CreateView):
         return context
 
 
+class Edit(EditView):
+    permission_policy = workflow_permission_policy
+    model = Workflow
+    page_title = _("Edit workflow")
+    template_name = 'wagtailadmin/workflows/edit.html'
+    success_message = _("Workflow '{0}' updated.")
+    add_url_name = 'wagtailadmin_workflows:add'
+    edit_url_name = 'wagtailadmin_workflows:edit'
+    index_url_name = 'wagtailadmin_workflows:index'
+    header_icon = 'placeholder'
+    edit_handler = None
+    MAX_PAGES = 5
+
+    def get_edit_handler(self):
+        if not self.edit_handler:
+            self.edit_handler = self.model.get_edit_handler()
+            self.edit_handler = self.edit_handler.bind_to(request=self.request, instance=self.get_object())
+        return self.edit_handler
+
+    def get_form_class(self):
+        return self.get_edit_handler().get_form_class()
+
+    def get_form(self, form_class=None):
+        form = super().get_form(form_class)
+        self.edit_handler = self.edit_handler.bind_to(form=form)
+        return form
+
+    def get_paginated_pages(self):
+        pages = Page.objects.filter(workflowpage__workflow=self.get_object())
+        pages.paginator = Paginator(pages, self.MAX_PAGES)
+        page_number = int(self.request.GET.get('p', 1))
+        paginated_pages = pages.paginator.page(page_number)
+        return paginated_pages
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context['edit_handler'] = self.edit_handler
+        context['pages'] = self.get_paginated_pages()
+        return context
+
+
 def edit(request, pk):
     if not request.user.is_superuser:
         raise PermissionDenied
@@ -78,7 +119,6 @@ def edit(request, pk):
         form = form_class(instance=workflow)
 
     edit_handler = edit_handler.bind_to(form=form)
-    pages.paginator = Paginator(pages, 5)
     page_number = int(request.GET.get('p', 1))
     page = pages.paginator.page(page_number)
 
@@ -95,21 +135,23 @@ def edit(request, pk):
         'paginator': pages.paginator,
     })
 
+
 @require_POST
-def remove_workflow(request, pk):
+def remove_workflow(request, page_pk, workflow_pk=None):
+    # Remove a workflow from a page (specifically a single workflow if workflow_pk is set)
     # Get the page
-    page = get_object_or_404(Page, id=pk).specific
+    page = get_object_or_404(Page, id=page_pk)
 
     # Check permissions
-    if not request.user.is_superuser:
+    if not workflow_permission_policy.user_has_permission(request.user, 'remove_from_page'):
         raise PermissionDenied
 
-    # Unlock the page
-    if page.workflow:
-        page.workflow = None
-        page.save()
-
-        messages.success(request, _("Workflow unassigned from Page '{0}'.").format(page.get_admin_display_title()))
+    if hasattr(page, 'workflowpage'):
+        # If workflow_pk is set, this will only remove the workflow if it its pk matches - this prevents accidental
+        # removal of the wrong workflow via a workflow edit page if the page listing is out of date
+        if not workflow_pk or workflow_pk == page.workflowpage.workflow.pk:
+            page.workflowpage.delete()
+            messages.success(request, _("Workflow removed from Page '{0}'.").format(page.get_admin_display_title()))
 
     # Redirect
     redirect_to = request.POST.get('next', None)
@@ -128,10 +170,12 @@ class AddWorkflowToPageForm(forms.Form):
 
     def clean(self):
         page = self.cleaned_data.get('page')
-        if page:
-            existing_workflow = self.cleaned_data.get('page').workflowpage.workflow
+        try:
+            existing_workflow = page.workflowpage.workflow
             if not self.errors and existing_workflow != self.cleaned_data['workflow'] and not self.cleaned_data['overwrite_existing']:
                 self.add_error('page', ValidationError(_("This page already has workflow '{0}' assigned. Do you want to overwrite the existing workflow?").format(existing_workflow), code='needs_confirmation'))
+        except AttributeError:
+            pass
 
     def save(self):
         page = self.cleaned_data['page']
@@ -143,8 +187,10 @@ class AddWorkflowToPageForm(forms.Form):
 
 
 def add_to_page(request, workflow_pk):
-    if not request.user.is_superuser:
+
+    if not workflow_permission_policy.user_has_permission(request.user, 'add_to_page'):
         raise PermissionDenied
+
     workflow = get_object_or_404(Workflow, pk=workflow_pk)
     form_class = AddWorkflowToPageForm