This patch does two major things, and also fixes one important bug. I know, the bug fix should probably be separate, and I can probably post that one separately if you need, just let me know.

First, this patch adds two fields to the history table: old_sid and comment. old_sid is filled automatically, and 'comment' is added to the workflow form. It is optional. A history record written if there is a comment, even if there is no transition; it is not written if there is neither a comment nor a transition. (This allows an author and editor to have a dialogue via the comments).

Future feature: My hacked version of workflow for assignblame also had these comments emailed when they were written, but it was based upon roles and transitions. All that really needs to be done here, I suspect, is to get the comment information available to the email action, but we also need to be able to take an action on 'no state change' (which is much like 'state change back to itself'). Not sure how to accomplish this, but it is necessary for a publishing site to work smoothly.

Bug fix:

When checking permissions for a transition, 'author' was not being added to $user->roles, thus preventing my nodes from getting a state.

Bug fix #2:

If there is no valid transition worfklow_form_alter hides the form element, which is fine; however, it did this by checking to see if there were only 2 choices. If the current state is '(creation)', and only one state was valid, this hides the form element and locks the node into a permanently bad state. I added a line to check to see if (creation) was the current node state, and if it is, it'll accept just 1 transition in order to allow a user to fix the bad state, if possible.

Comments

merlinofchaos’s picture

Ha, I lost a paragraph.

Feature #2: It adds a tab to pages which checks permissions based on role. That tab can only do workflow transitions. Good for giving review privileges without edit privileges.

merlinofchaos’s picture

StatusFileSize
new8.97 KB

Updated; adds a node_save during the transition, because na_arbitrator only rewrites the node_access stuff that workflow_access relies on during save.

Ideally workflow_access needs a way to hook into the transition change so that it can know to rewrite the access records; but that's for later.

moshe weitzman’s picture

i have not looked at any code, but that node_save() might be unwanted if it is called from within an ongoing node_save(). our node save process still can't survive recursion. there are a couple new node hooks which help wiht this problem and might be useful here. but sometimes you really want recursion in which case find the issue that JVD opened about it.

or maybe a transition doesn't happen during node save

merlinofchaos’s picture

The transition I added the node_save to happens specifically outside of the actual node save process; it is on the workflow tab page.

moshe weitzman’s picture

There is talk of a new ticketin system based on CCK and workflow. This patch would help a lot with emailing comments when ticket state changes. WHats the status here?

JVD - you listening?

jvandyk’s picture

Yeah, I'm listening. Will try to get to this this weekend.

merlinofchaos’s picture

I should be available for much of the weekend if you need a hand. Not sure if the patch will still apply.

jvandyk’s picture

Status: Needs review » Fixed

Committed. Thanks!

Anonymous’s picture

Status: Fixed » Closed (fixed)