Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workflow needs changes state #6047

Merged

Conversation

jacobtoppm
Copy link
Member

@jacobtoppm jacobtoppm commented May 20, 2020

Changes the WorkflowState STATUS_REJECTED to STATUS_NEEDS_CHANGES, and adds the ability to resume a workflow in this state, rather than starting a new one, with the additional options to cancel a workflow, and to restart from scratch.

Removes task state copying between revisions (when set up in linear mode) to prevent swamping the database with unnecessary models, and replacing this by making WorkflowState's get_next_task method settings-aware.

Makes the linear mode (WAGTAIL_WORKFLOW_REQUIRE_REAPPROVAL_ON_EDIT=False) the default.

Adds the ability to cancel a workflow

Moves workflow actions into the action menu proper.

Adds tests for workflow cancellation and resumption.

Lock page to non-reviewers during workflow, but allow workflow cancellation from any user who would normally be able to (this allows the original author to stop a workflow when submitting accidentally, for instance).

@squash-labs
Copy link

squash-labs bot commented May 20, 2020

Manage this branch in Squash

Test this branch here: https://jacobtoppmworkflow-needs-chang-x6m5l.squash.io

@zerolab zerolab force-pushed the feature/workflow branch 2 times, most recently from ad7c74c to 6c06987 Compare May 29, 2020 11:37
@kaedroho kaedroho changed the base branch from feature/workflow to feature/workflow-pre-rebase-5 May 29, 2020 12:40
Copy link
Contributor

@kaedroho kaedroho left a comment

Choose a reason for hiding this comment

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

Looking good so far!

wagtail/admin/action_menu.py Outdated Show resolved Hide resolved
wagtail/admin/action_menu.py Show resolved Hide resolved
if WorkflowState.objects.filter(status=self.STATUS_IN_PROGRESS, page=self.page).exclude(pk=self.pk).exists():
raise ValidationError(_('There may only be one in progress workflow state per page.'))
if WorkflowState.objects.filter(Q(status=self.STATUS_IN_PROGRESS) | Q(status=self.STATUS_NEEDS_CHANGES), page=self.page).exclude(pk=self.pk).exists():
raise ValidationError(_('There may only be one in progress or needs changes workflow state per page.'))
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should mention "needs changes" here? As a user, I'd probably think of this state as just a variant of "in progress".

Maybe we should relabel it in STATUS_CHOICES to "In progress (needs changes)"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is a user actually going to see this message though? It seems like as a model level error, it's most relevant for developers, where the distinction is probably more relevant?

wagtail/core/models.py Show resolved Hide resolved
wagtail/admin/tests/test_workflows.py Outdated Show resolved Hide resolved
wagtail/admin/views/pages.py Show resolved Hide resolved
wagtail/admin/views/pages.py Outdated Show resolved Hide resolved
wagtail/admin/views/pages.py Show resolved Hide resolved
wagtail/core/models.py Outdated Show resolved Hide resolved
wagtail/core/models.py Outdated Show resolved Hide resolved
@jacobtoppm jacobtoppm requested a review from kaedroho June 5, 2020 16:25
Copy link
Contributor

@kaedroho kaedroho left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

…AGTAIL_WORKFLOW_REQUIRE_REAPPROVAL_ON_EDIT in get_next_task logic to determine whether task states created on previous revisions apply or not
…l workflow actions into normal action menu. Adjust page view to cope with new NEEDS_CHANGES state, and add placeholder banner message (which new UI is expected to replace).
…riginal user can cancel the workflow even if the page is locked to them at that workflow step (preparing for locking page to approval groups during workflow)
@jacobtoppm jacobtoppm force-pushed the workflow-needs-changes-state branch from fdeb26f to 5cac197 Compare June 10, 2020 17:17
@jacobtoppm jacobtoppm changed the base branch from feature/workflow-pre-rebase-5 to feature/workflow June 10, 2020 17:18
@jacobtoppm jacobtoppm merged commit d206a8d into wagtail:feature/workflow Jun 10, 2020
jacobtoppm added a commit that referenced this pull request Jun 15, 2020
Replace WorkflowState STATUS_REJECTED with STATUS_NEEDS_CHANGES, which does not end a workflow, and add cancellation and resubmit to rejector options. Make Workflows lock pages to non-reviewers. Make the linear mode of workflow the default.
zerolab pushed a commit that referenced this pull request Jun 26, 2020
Replace WorkflowState STATUS_REJECTED with STATUS_NEEDS_CHANGES, which does not end a workflow, and add cancellation and resubmit to rejector options. Make Workflows lock pages to non-reviewers. Make the linear mode of workflow the default.
gasman pushed a commit that referenced this pull request Jul 8, 2020
Replace WorkflowState STATUS_REJECTED with STATUS_NEEDS_CHANGES, which does not end a workflow, and add cancellation and resubmit to rejector options. Make Workflows lock pages to non-reviewers. Make the linear mode of workflow the default.
gasman pushed a commit to gasman/wagtail that referenced this pull request Jul 24, 2020
Replace WorkflowState STATUS_REJECTED with STATUS_NEEDS_CHANGES, which does not end a workflow, and add cancellation and resubmit to rejector options. Make Workflows lock pages to non-reviewers. Make the linear mode of workflow the default.
gasman pushed a commit that referenced this pull request Jul 24, 2020
Replace WorkflowState STATUS_REJECTED with STATUS_NEEDS_CHANGES, which does not end a workflow, and add cancellation and resubmit to rejector options. Make Workflows lock pages to non-reviewers. Make the linear mode of workflow the default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants