Item14048: multiple critical improvements to WorkflowPlugin

pencil
Priority: Urgent
Current State: Closed
Released In: n/a
Target Release:
Applies To: Extension
Component: WorkflowPlugin
Branches: Item14048 master
Reported By: MichaelDaum
Waiting For:
Last Change By: MichaelDaum
This is a big patch to WorkflowPlugin. I know this is bad and should better be split up in one patch per fix/added feature. Please have mercy as this is quite thorough and needs review as a whole thing.

Add an "Allow View" column

For now the plugin can only restrict edit rights based on the controlled topic being in a certain state. However, key for proper workflow control is more to restrict view in order to protect content that is not yet approved for open access.

Use proper access control

Controlled topics can be edit-protected based on the workflow state it is in. Alas, edit rights are NOT enforced via Foswiki's own ACL mechanisms. Instead the plugin implements its own veto-like blocking on an edit action. As a consequence none of the other parts of the system can properly react on it.

This is even more important for view protection for unapproved content. Using proper ACLs for edit and view control is mandatory not to leak any such information to the broader audience, not via view, or any other subsystem such as %IF, %SEARCH, DBCachePlugin or SolrPlugin etc. These all do respect Foswiki ACLs but have no clue what WorkflowPlugin would assert based on a workflow state.

The patch changes this by maintaining ACLs in the afterSaveHandler() thus properly setting edit and view rights. All other systems thus naturally follow restrictions coming from workflow information.

Admins can always switch transitions

In some cases users lock themselves out or are unable to transition a topic into a new state. So admins need to be able to help out. This policy follows standard ACLs.

Integration into SolrPlugin

In case SolrPlugin is installed, index the workflow state of a controlled topic.

Add support for transition comments

Allow people to store a comment when switching states. These are stored together with the rest of the workflow history and made accessible via WORKFLOWHISTORY or WORKFLOWSTATE.

Flaws in WORKFLOWHISTORY

Basically, workflow history stores one record per transition and stores info about the reached target state. That's fine but as a consequence the starting state is not recorded into the workflow history. History only starts with the first transition. One could argure that there is one inherent "transition" not recorded when entering the starting state.

Another flaw is that the actual transition that led to the target state is not recorded in the history.

Current format is

META:WORKFLOWHISTORY{
   name="<revision>" 
   author="<wikiname>" 
   comment="<string>" 
   date="<epoch seconds>" 
   state="<state name>"
}%

There should be additional info describing the action that led to this state

META:WORKFLOWHISTORY{
   name="<revision>" 
   author="<wikiname>" 
   comment="<string>" 
   date="<epoch seconds>" 
   prevstate="<source state name>"
   action="<transition name>"
   state="<target state name>"
}%

According formatting features have to be added to the %WORKFLOWHISTORY macro.

Improve formatting of WORKFLOWHISTORY

This adds the "fantastic four" parameters header, format, separator and footer to ease programming the output in wiki apps. I.e. the way workflow history was formatted before - using a format string in a preference variable called WORKFLOWHISTORYFORMAT was particularly cumbersome.

  • header: defaults to empty
  • footer: defaults to empty
  • separator: defaults to empty
  • format: defaults to the prev setting in WORKFLOWHISTORYFORMAT, or <br> $state -$date otherwise
  • include: regex to only render history entries whose state matches
  • exclude: regex suppress matching history entries

format now takes additional variables:

  • $date, $day, $dow, $email, $epoch, ..., $year: date info of item in history
  • $comment: transition comment
  • $index: numeric index of item in history
  • $count: number of items in the workflow history

There currently is no way to find out what revision the topic was in before reaching the current state. This is not necessarily $rev -1 as a topic can run through multiple revisions while remaining in the same workflow state. The workflow history does record this information properly (with the exception of the first workflow state not being recorded to the history list; see "Flaws in WORKFLOWHISTORY").

For example a typical history could look like this:

% META:WORKFLOWHISTORY{name="2" author="..." ...}%
% META:WORKFLOWHISTORY{name="4" author="..." ...}%
% META:WORKFLOWHISTORY{name="5" author="..." ...}%
% META:WORKFLOWHISTORY{name="10" author="..." ...}%
% META:WORKFLOWHISTORY{name="13" author="..." ...}%

There is no way to format a history describing the transaction that has happened just for the fact that there is no way to access the previous history entry in the list. (Note that arithmetics (via %CALCULATE) are no solution: the number of topic revisions between two workflow states can be arbitrarily high.)

To do so the %WORKFLOWHISTORY macro is extended by additional variables

  • $prevname: revision id of previous history entry
  • $prevwikiname: wiki name of previous workflow transition
  • $prevstate: name of the previous state
  • $prevcomment: comment of the previous history entry
  • $prevdate, $prevday, ... date info of previous entry in history

This basically adds a prev prefix to all existing format tokens.

Improve formatting of WORKFLOWSTATE

This adds the "fantastic four" parameters header, format, separator and footer to ease programming the output in wiki apps. I.e. does it bundle in a consistent way and thus supersedes some of the other WORKFLOW* macros that refer to specific information of a workflow state.

New parameters are:

  • state: specify the state to render info of
  • hidenull: suppress output in case state is not matching the current state
  • format: format string to render state info, defaults to $state

format takes the following variables:

  • $web: web name of the controlled topic
  • $topic: topic name of the controlled topic
  • $state: of the controlled topic
  • $message: workflow message of the state the topic is in (replaces WORKFLOWSTATEMESSAGE)
  • $rev: revision the controlled topic is in in the given state (replaces WORKFLOWLASTREV)
  • $user: last user that switched the topic into the given state (replaces WORKFLOWLASTUSER)
  • $time: time the topic has been switched into the given state (replaces WORKFLOWLASTTIME)
  • $comment: transition comment when the topic has been switched into its state
  • $numactions: number of possible transitions from this state on
  • $actions: comma-separated list of possible follow-up actions
  • $allowed, $allowedit: value of the "Allow edit" column for this state
  • $allowview: value of the "Allow view" column for this state

Deprecate WORKFLOWEDITTOPIC, WORKATTACHTOPIC macros

Edit access is properly controlled by ACLs now so that any edit or attach link can be properly rendered or hidden using %IF

Deprecate WORKFLOWSTATEMESSAGE, WORKFLOWLASTREV, WORKFLOWLASTUSER, WORKFLOWLASTTIME

As these are available in a consistent way via %WORKFLOWSTATE!{format="..."}% now.

Re-Notify on last transition

Transitions allow to send emails to people in the "Notify" column of a workflow transition. This column can compute recipients dynamicaly such as reading a formfield value from an attached DataForm, e.g a formfield "!ResponsiblePerson". However when the ResponsiblePerson changes but the transition already fired, there is no way to re-send the notification email again.

To implement a "re-notify" button in such a case we need to separate out the notification mechanism from the changeState api that it is currently part of, and then add a rest handler that triggers the notification again (and only the notification part).

However as things are right now this is not possible as the actual action that lead to the current workflow state is lost. History does not record it. So this feature depends on fixing "Flaws of WORKFLOWHISTORY" first, and then have a new "notify" rest handler that reads out the current state, the last action and then re-sends notifications.

Misc notes

WORKFLOWTRANSITION is rather unflexible in case you'd prefer a somewhat different user interfaces switching states. This now possible using the extended formatting capabilities of the WORKFLOWSTATE macro.

no, noautolink and literal tags haven't been always removed from formatted output -> fixed.

Some object constructors have been fixed from new Package(..) to Package->new(...) style, not at least for consistency.

Patch fully specifies REST security required in newer Foswiki engines.

This work is part of an effort to bring workflows into the WikiWorkbenchContrib framework.

-- MichaelDaum - 11 Apr 2016

Comments

Good stuff. None of the changes described rings any alarm bells.

-- Main.CrawfordCurrie - 11 Apr 2016 - 19:36

More flaws and shortcomings documented, i.e. with regards to the way the workflow history is recorded and retrieved.

-- MichaelDaum - 19 Apr 2016

Not seeing anything there that worries me - it's all good stuff. The previous state stuff especially is very much needed.

-- Main.CrawfordCurrie - 19 Apr 2016 - 10:41

Will check in more fixes.

-- MichaelDaum - 20 Jan 2017

The branch is merged, but note that compatibility with 1.1.9 is broken when validation is disabled due to the core not checking {Validation}{Method} in Rest.pm. However I'm closing this to stop you checking any more into it - I can't cope with these mega-patches, sorry, I'd prefer you checked in small fixes to master.

-- Main.CrawfordCurrie - 25 Jan 2017 - 09:45

It simply is no small fix.

-- MichaelDaum - 25 Jan 2017

I've parked my last changes that were left behind after this item branch has been closed without having talked to me. I know that they are not merging well with the latter changes that happend to master. Communication on this task has been very poor, and I owe my share of this problem. Sorry, but that's all I have to say for now. I am not feeling like I'll be able to continue working on this plugin.

-- MichaelDaum - 18 Sep 2017
 
Topic revision: r12 - 18 Sep 2017, MichaelDaum
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy