With both the latest release version (7.x-2.5) and the latest dev version, I am unable to get Workflows to import or revert properly.
Here's what I'm seeing:
- States lose their SID on import, and end up receiving a new SID that's based on the sorting order of the states. So, if you created states and then later re-ordered them, on import they get completely new SID values that break any of the transition or role permissions you assigned before export.
- Except for the "(author)" pseudo-role, transition permissions for all other roles get lost.
- Any features containing workflows are always in an "Overridden" state on activation.
- Features shows that the
participate in workflow
permission is for some reason being overridden by Workflow. - Attempting to revert a workflow results in the error shown below, followed by the workflow disappearing from the site.
Notice: Trying to get property of non-object in workflow_features_pipe_field_base_alter() (line 116 of modules/workflow/workflow.features.inc).
Warning: array_flip(): Can only flip STRING and INTEGER values! in EntityAPIControllerExportable->load() (line 742 of contrib/entity/includes/entity.controller.inc).
Warning: array_flip(): Can only flip STRING and INTEGER values! in EntityAPIController->load() (line 219 of contrib/entity/includes/entity.controller.inc).
Warning: array_flip(): Can only flip STRING and INTEGER values! in EntityAPIControllerExportable->load() (line 742 of contrib/entity/includes/entity.controller.inc).
Warning: array_flip(): Can only flip STRING and INTEGER values! in EntityAPIController->load() (line 219 of contrib/entity/includes/entity.controller.inc).
I am using "Role Export" to keep the role IDs uniform across installs.
Here's a copy of my workflow export:
/**
* Implements hook_default_Workflow().
*/
function haw_listing_default_Workflow() {
$workflows = array();
// Exported workflow: 'project_listing'
$workflows['project_listing'] = entity_import('Workflow', '{
"name" : "project_listing",
"tab_roles" : [],
"options" : [],
"states" : {
"1" : {"sid":"1","wid":"1","weight":"-50","sysid":"1","state":"(creation)","status":"1","name":"(creation)"},
"2" : {"sid":"2","wid":"1","weight":"-19","sysid":"0","state":"Open for bids","status":"1","name":"open_for_bids"},
"3" : {"sid":"3","wid":"1","weight":"-18","sysid":"0","state":"Expired","status":"1","name":"expired"},
"4" : {"sid":"4","wid":"1","weight":"-17","sysid":"0","state":"In progress","status":"1","name":"in_progress"},
"5" : {"sid":"5","wid":"1","weight":"-16","sysid":"0","state":"On hold","status":"1","name":"on_hold"},
"6" : {"sid":"6","wid":"1","weight":"-15","sysid":"0","state":"Completed","status":"1","name":"completed"},
"7" : {"sid":"7","wid":"1","weight":"-14","sysid":"0","state":"Canceled","status":"1","name":"canceled"}
},
"transitions" : {
"1" : {"tid":"1","sid":"1","target_sid":"2","roles":{"-1":-1,"30037204":30037204,"79959846":79959846},"wid":"1","name":"1_2","label":"Publish listing"},
"36" : {"tid":"36","sid":"1","target_sid":"4","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"1_4","label":"Start project"},
"3" : {"tid":"3","sid":"2","target_sid":"2","roles":{"-1":-1,"30037204":30037204,"79959846":79959846,"263062362":263062362},"wid":"1","name":"2_2","label":"Remain open for bids"},
"4" : {"tid":"4","sid":"2","target_sid":"3","roles":{"-1":-1,"30037204":30037204,"79959846":79959846},"wid":"1","name":"2_7","label":"Expire listing"},
"5" : {"tid":"5","sid":"2","target_sid":"4","roles":{"-1":-1,"30037204":30037204,"79959846":79959846},"wid":"1","name":"2_3","label":"Start project"},
"40" : {"tid":"40","sid":"2","target_sid":"5","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"2_5","label":"Place project on hold"},
"41" : {"tid":"41","sid":"2","target_sid":"6","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"2_6","label":"Complete project"},
"42" : {"tid":"42","sid":"2","target_sid":"7","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"2_7","label":"Cancel project"},
"43" : {"tid":"43","sid":"3","target_sid":"2","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"3_2","label":"Re-open for bids"},
"10" : {"tid":"10","sid":"3","target_sid":"3","roles":{"-1":-1,"30037204":30037204,"79959846":79959846,"263062362":263062362},"wid":"1","name":"7_7","label":"Remain expired"},
"44" : {"tid":"44","sid":"3","target_sid":"4","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"3_4","label":"Start project"},
"45" : {"tid":"45","sid":"3","target_sid":"5","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"3_5","label":"Place project on hold"},
"46" : {"tid":"46","sid":"3","target_sid":"6","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"3_6","label":"Complete project"},
"47" : {"tid":"47","sid":"3","target_sid":"7","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"3_7","label":"Cancel project"},
"48" : {"tid":"48","sid":"4","target_sid":"2","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"4_2","label":"Re-open for bids"},
"16" : {"tid":"16","sid":"4","target_sid":"4","roles":{"-1":-1,"30037204":30037204,"79959846":79959846,"263062362":263062362},"wid":"1","name":"3_3","label":"Remain in progress"},
"17" : {"tid":"17","sid":"4","target_sid":"5","roles":{"-1":-1,"30037204":30037204,"79959846":79959846},"wid":"1","name":"3_4","label":"Place project on hold"},
"18" : {"tid":"18","sid":"4","target_sid":"6","roles":{"-1":-1,"30037204":30037204,"79959846":79959846},"wid":"1","name":"3_5","label":"Complete project"},
"19" : {"tid":"19","sid":"4","target_sid":"7","roles":{"-1":-1,"30037204":30037204,"79959846":79959846},"wid":"1","name":"3_6","label":"Cancel project"},
"50" : {"tid":"50","sid":"5","target_sid":"2","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"5_2","label":"Re-open for bids"},
"21" : {"tid":"21","sid":"5","target_sid":"4","roles":{"-1":-1,"30037204":30037204,"79959846":79959846},"wid":"1","name":"4_3","label":"Resume project"},
"22" : {"tid":"22","sid":"5","target_sid":"5","roles":{"-1":-1,"30037204":30037204,"79959846":79959846,"263062362":263062362},"wid":"1","name":"4_4","label":"Remain on hold"},
"23" : {"tid":"23","sid":"5","target_sid":"6","roles":{"-1":-1,"30037204":30037204,"79959846":79959846},"wid":"1","name":"4_5","label":"Complete project"},
"24" : {"tid":"24","sid":"5","target_sid":"7","roles":{"-1":-1,"30037204":30037204,"79959846":79959846},"wid":"1","name":"4_6","label":"Cancel project"},
"52" : {"tid":"52","sid":"6","target_sid":"2","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"6_2","label":"Re-open for bids"},
"54" : {"tid":"54","sid":"6","target_sid":"4","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"6_4","label":"Restart project"},
"55" : {"tid":"55","sid":"6","target_sid":"5","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"6_5","label":"Restart project and place on hold"},
"28" : {"tid":"28","sid":"6","target_sid":"6","roles":{"-1":-1,"30037204":30037204,"79959846":79959846,"263062362":263062362},"wid":"1","name":"5_5","label":"Remain completed"},
"56" : {"tid":"56","sid":"6","target_sid":"7","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"6_7","label":"Move to canceled"},
"57" : {"tid":"57","sid":"7","target_sid":"2","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"7_2","label":"Re-open for bids"},
"59" : {"tid":"59","sid":"7","target_sid":"4","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"7_4","label":"Restart project"},
"60" : {"tid":"60","sid":"7","target_sid":"5","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"7_5","label":"Restart project and place on hold"},
"61" : {"tid":"61","sid":"7","target_sid":"6","roles":{"30037204":30037204,"79959846":79959846},"wid":"1","name":"7_6","label":"Move to completed"},
"34" : {"tid":"34","sid":"7","target_sid":"7","roles":{"-1":-1,"30037204":30037204,"79959846":79959846,"263062362":263062362},"wid":"1","name":"6_6","label":"Remain canceled"}
},
"label" : "Project listing",
"typeMap" : [],
"wid_original" : "1",
"system_roles" : {
"-1" : "(author)",
"1" : "anonymous user",
"2" : "authenticated user",
"30037204" : "administrator",
"259479786" : "manage all properties",
"219102880" : "manage own properties",
"79959846" : "manage all project listings",
"244667557" : "manage all project bids",
"263062362" : "manage own project listings",
"160177598" : "manage all profiles",
"230455779" : "manage own main profile",
"59661863" : "manage own contractor profile",
"199746617" : "write reviews on contractors",
"262615644" : "bid on projects",
"139118367" : "access contractor application",
"20566584" : "administer users",
"245154260" : "read private messages for all users",
"76210658" : "manage deleted content",
"6462655" : "search for contractors",
"60039842" : "search project listings",
"238777296" : "manage all contractor reviews",
"110079917" : "review all customer surveys"
}
}');
return $workflows;
}
Comment | File | Size | Author |
---|---|---|---|
#43 | workflow-features_import-2484297-43-7.patch | 24.8 KB | klaasvw |
#41 | workflow-features_import-2484297-41-7.patch | 24.68 KB | bleedev |
#19 | features_import-2484297-19.patch | 23.91 KB | vasi |
#19 | interdiff-2484297-10-19.txt | 20.8 KB | vasi |
#18 | interdiff-2484297-18-10.txt | 1.29 KB | bartvig |
Comments
Comment #1
GuyPaddock CreditAttribution: GuyPaddock commentedTypo fix.
Comment #2
GuyPaddock CreditAttribution: GuyPaddock commentedComment #3
GuyPaddock CreditAttribution: GuyPaddock commentedComment #4
GuyPaddock CreditAttribution: GuyPaddock commentedAlso important question: if all of the states and the workflows have machine names, why does the export even use
sid
andwid
values? Serial IDs are notoriously fragile in exportables.Comment #5
GuyPaddock CreditAttribution: GuyPaddock commentedAdding related issues.
Comment #6
GuyPaddock CreditAttribution: GuyPaddock at RedBottle Design, LLC commentedUpdated issue summary.
Comment #7
GuyPaddock CreditAttribution: GuyPaddock at RedBottle Design, LLC for House at Work commentedAttached is a patch that refactors how Workflow imports and exports workflows via features.
The way I've implemented this patch allows Features to install and revert based on both the old export format (i.e. the one that included serial IDs) as well as the new format (i.e. the one without any serial IDs, using machine names for tying transitions to states). If you have existing features that use the legacy Workflow export format, I'd suggest applying this patch, reverting the workflow from the feature, then re-exporting the feature to get a copy that is no longer "Overridden". Basically, the only way to clear the Overridden status with an older feature is to generate a new export with machine names instead of numeric IDs, but the patch can still import the older format so you don't lose your workflow in the process.
The approach of this patch
Like what was discussed in #2286559: [META] Make Workflow's subobjects things exportable (a.k.a. machine names)., the goal was to eliminate serial IDs entirely from the feature export and instead to fall back on machine names. Unlike the approach "Artusamak" took in that issue, though, my changes only affect import and export, not how Workflow internally associates workflows, states, transitions, and roles. Internally, numeric IDs are still used, which provides the best performance and avoids breaking the Workflow API for other modules. That addresses issues #1 and #3 in my OP.
As for issue #2 in my OP, the problem here seems to be that if the roles are bundled with the workflow in the same feature, the roles were being installed after the workflow so Workflow couldn't find them during role remapping. The fix here was to use
hook_features_api_alter()
to invoke Workflow's code last.Issue #4 in my original post turned out to be a "feature" (no pun intended) of Workflow that is addressed/removed in #2484431: Do NOT automatically enable roles to participate in workflows.
Finally, I took a look at why revert wasn't working, and basically the issue was that it was commented out / never finished. The default Entity API implementation of
revert()
was being used instead, which literally just causes the workflow entity to be deleted as in #5 in my post (the error messages related to array_flip were caused by the workflow being missing but still referenced by a field).Testers welcome! This is a significant change, but for the first time I can successfully install and revert workflows! It even gets back to the "Default" state!
Old Export Format
New Export Format
Comment #8
kaareThis approach looks promising! I'm currently experimenting with changing transition permissions between dev -> staging. But I bumped into a problem: On the development site, where the feature was originally created, using drush to update the feature containing the workflow definition (
drush -y fu example_workflow
) would actually revert the feature instead of update it. I had to update the feature using the browser.But otherwise it works beautifully! Using machine name to export workflow states to features is the future!
Comment #9
vasi CreditAttribution: vasi at Evolving Web commentedI found a small bug in the patch, on a fresh install adding a brand new workflow yields:
It looks like we need an isset() check, I'll reroll the patch.
Comment #10
vasi CreditAttribution: vasi at Evolving Web commentedRerolled with the isset added.
Comment #11
vasi CreditAttribution: vasi at Evolving Web commentedHmm, now I'm seeing a more important bug: If I have two feature modules with an exported workflow in each, when I enable the second module, I end up with two copies of all its transitions! I'm not sure what's going on, I'll look into it.
Comment #12
vasi CreditAttribution: vasi at Evolving Web commentedHmm. So this is all happening after the normal load from defaults. Instead our workflow gets saved a second time during the cache flush, here's the call stack where the problem occurs:
When we get to the "Delete all existing transitions" step, the old workflow is fetched from the entity cache, and for some reason the cached workflow has no transitions. So we fail to delete the old transitions, but then go ahead and recreate them all.
Comment #13
vasi CreditAttribution: vasi at Evolving Web commentedAnd I can't for the life of me figure out why the wrong transitions are in the cache. Ideas, anyone?
Comment #14
vasi CreditAttribution: vasi at Evolving Web commentedFor now, I'm using a little sandbox project that I put together: https://www.drupal.org/sandbox/evolvingweb-vasi/2493673
It basically just takes over export of workflows, and does similar things to GuyPaddock's patch.
Comment #15
johnv@vasi,
in Workflow::save(), try replacing $this->states by public function getStates($all, $reset = TRUE)
This will bypass buffers.
Comment #16
bartvig CreditAttribution: bartvig commentedThe patch works brilliantly. However, I have some workflow states with long names which will make workflow transition names too long, because the column is only 32 bytes long.
I've added a patch that changes workflows machine names to varchar 64 (which is now 255) and workflow transition machine names to varchar 255 (which is now 32). This way we can be sure that the workflows names aren't too long when concatenated and '_to_' added.
Comment #17
johnv@bartvig, please open a new issue with your patch. Thanks.
Comment #18
bartvig CreditAttribution: bartvig commentedWell, the patch is related to patch #10, and my patch was wrong as it should have included patch #10.
I've included the full patch and the interdiff.
Comment #19
vasi CreditAttribution: vasi at Evolving Web commentedOk, I've fixed up GuyPaddock's patch a bit. It basically uses the same export code, but completely changes the way revert (import) works. Highlights:
Some changes:
I've tested a variety of situations, but I'd love to see more testing. Thanks so much to GuyPaddock for the export code, that helped a ton.
Comment #20
hwasem CreditAttribution: hwasem commentedWe are also having this problem trying to use features for version control on our dev team.
I attempted to use #19 patch to our workflow-7.x-2.5 install. I was able to apply the patch and create a feature, but upon rebuilding/importing that feature, received the following error:
Reverting feature module 'our_feature' dependencies...
Fatal error: Call to a member function getName() on a non-object in /build/sites/all/modules/workflow/includes/Entity/Workflow.php on line 226
I tried to apply patch 18 using the dev version of workflow, but that threw errors in using the workflow (not in features).
I finally got a combination of workflow-7.x-2.5 + patch #18 to work for us. I was able to apply the patch, export and the rebuild/import the feature AND keep workflow functioning at every step.
Comment #21
johnv@hwasem, can you post the resulting patch?
Comment #22
hwasem CreditAttribution: hwasem commentedHi johnv,
I used patch #18 as is, I just applied it to the recommended workflow version 2.5 instead of the dev branch. If there is anything else you need in way of a patch, I'm not sure how to produce that. I'm happy to do so, I'll need some guidance on what you need.
Thanks,
Heidi
Comment #23
mansspams CreditAttribution: mansspams at Wunder commentedPatch #19 works well here.
Comment #24
mansspams CreditAttribution: mansspams at Wunder commentedIn fact, if it works for me and author, maybe this would be better status.
@hwasem please test with dev branch.
Comment #25
Manuel Garcia CreditAttribution: Manuel Garcia at Appnovation commentedSeems to work here for transitions.
In our case however the exported workflow field is broken when I revert, because of wid changing. Apparently you can work around this by updating the
wid' => 1,
' line of your export code in hook_field_default_field_bases. Not a very practical workaround, but it can save you should it ever happen in production.Comment #26
Manuel Garcia CreditAttribution: Manuel Garcia at Appnovation commentedNot sure if I should post this comment on #2152435: [EXPORTING] Export of Workflow Field using machine_name, not $wid, I agree that we should fix it on this side of the code rather, in order not to break existing sites.
Using workflow field, after a feature revert, even if you fix the bad wid on the field definition, the sid on workflow_states table change, so all the data on existing nodes (and their revisions), gets corrupted, since it is refering to workflow states sid's that no longer exist. This translates into:
This is obviously a deal breaker for workflow field and features right now, you end up with a broken site on your first feature revert.
So for workflow field and features to work, all I can think of is that we should NOT be changing the wid or the sid, (if that's a possibility). Or perhaps rework workflow field to store machine names of workflows and workflow states, instead of wid and sid.
@mansspams you say it works for you, are you using workflow field?
Comment #27
netw3rker CreditAttribution: netw3rker commented@Manuel Garcia,
using the dev version of workflow and applying patch #19, I'm able to properly export and enable/import a feature that has a workflow, workflow_access settings and includes a workflow_field base and its instances.
Before this patch I encountered the problem you (and this issue) described, the SID's were constantly changing. Interestingly, this was because my local environment where the workflow was developed has db auto incrementing set to 1 (so each new SID is incremented by one), but on the Acquia environment this was deployed to, the DB has auto incrementing set to 5 (each SID will be 5 higher than the previous). This means a) the feature will never reach a default state, and b) each time it is reverted the SID's will get recreated a step higher.
This posed a particular problem because workflow_access correctly stores the the workflow and state by machine name in the feature, so it never looks like there's a problem, and no overrides are shown. However in the database table, the access settings are stored by SID. When the workflow is reverted, the SID's move forward in the primary table and suddenly the access permissions tables foreign keys don't line up with their primary key complements.
To your questions:
After applying the patch in #19, the export now has machine names for the states, and correctly imports them, and does not rebuild the SID each time. This fixes all of my problems, and workflow_access remains happy afterwards.
I checked the workflow_field base that I exported, and noted this:
What is interesting to me is that the "allowed values string" contains a string representation of the settings from my local instance where I created the workflow. Those key values are totally different on the destination environment, but the feature somehow thinks this is just fine. Presumably that's because of the "allowed_values_function" that is being used, which must supersede the string.
The other thing I noticed is what you are specifically concerned about and cited the other issue about: The WID value is the numeric WID of the workflow. This is definitely a problem, and really just tells me that I got lucky that this workflow I enabled/imported is the first and only workflow, and therefore this field lines up with the desired destination workflow. Had I declared a workflow in a different feature that was enabled before my current one, these fields would reference the wrong workflow entirely.
That is definitely a problem, but maybe not one for this issue, which is tackling the problem of SID's rotating on each revert. I would say that fully functional features support in the workflow module will certainly require both of these issues to go in before things are 100% ready, but each one can probably be separately handled.
With that being said, I'm going to +1 the "RTBC tag" for this patch, and ask/beg the maintainers to commit it!
Comment #28
Manuel Garcia CreditAttribution: Manuel Garcia at Appnovation commentedThanks a lot @netw3rker for your in depth explanation!
Let's get this one in then, and focus next on #2152435: [EXPORTING] Export of Workflow Field using machine_name, not $wid
Comment #30
johnvThis is committed. Thanks guys!
Comment #31
Manuel Garcia CreditAttribution: Manuel Garcia at Appnovation commentedGreat news @johnv thanks!
Comment #32
GuyPaddock CreditAttribution: GuyPaddock at RedBottle Design, LLC for House at Work commentedOkay, just tried this, and it's looking better, but I was curious about one thing: why does the latest patch still export the serial wid + sids?
Won't this just lead to features being constantly flagged as overridden between installs if they don't have exactly the same wid + sids?
That was why my patch dropped them entirely.
Comment #33
johnvShould we set this to 'needs work' or 'active' again?
I understand that exporting/importing now works as long as ths id's are equal in both systems.
Then, still to be done:
- export workflow with machine_name
- #2152435: [EXPORTING] Export of Workflow Field using machine_name, not $wid
- separate exports of workflow and workflow_field (they were combined the last year, leading to several comments here and elsewhere.)
Comment #34
johnvThe exporting of workflow and Workflow Field definition is now decoupled. See #2375261-7: [EXPORTING] Workflow exported to new feature even if it already exists in an other feature
Comment #36
yoruvo CreditAttribution: yoruvo commentedI believe that this remains a significant issue, as both Workflow Field instances and Workflow State view filters refer to specific SIDs, and these references are broken whenever the Workflow component of a feature is reverted.
The solution from comment #19 cannot be applied to the current code base.
Comment #37
SpleshkaI confirm this is still an issue. I wasn't able to deploy workflow config with Features - the feature is always overriden and the workflow doesn't get created on the target site.
Comment #38
miaoulafrite CreditAttribution: miaoulafrite commentedSame here with version 7.x-2.8
Comment #39
GuyPaddock CreditAttribution: GuyPaddock at RedBottle Design, LLC for House at Work commentedWhile we're at it, can we also stop including the list of all roles in the role map and instead only include roles actually referenced by the workflow? We have a problem where any time a new role is added to the site, all the workflows get flagged as overridden because of the role map adding that to their export.
Comment #40
miaoulafrite CreditAttribution: miaoulafrite commentedCould we use the "export" functionality provided by this module and use that to be included thanks to a custom module? Would it be possible at all? If so, how?
Just thinking as a workaround as for now this module is not suitable for production...
EDIT: we actually cannot, as the "Import" functionality is broken too AFAICT
Comment #41
bleedev CreditAttribution: bleedev at Phase2 commented#19 re-rolled for 7.x-2.8
Comment #42
jlscott CreditAttribution: jlscott as a volunteer commentedJust to comment that patch #41 solved my problem with exporting/importing workflow data to/from a feature.
Thanks @bleedev.
Comment #43
klaasvw CreditAttribution: klaasvw as a volunteer and at Randstad Digital commentedRerolling against the latest 7.x-2.x
This also fixes a bug introduced by #41 where an install of a feature containing an exported workflow gives the following error:
Comment #44
harivenuvHi klaasvw,
Thank for the patch. Some simple issue there in PHP 5.4 version array declaration is changed.
So kindly check my patch.
Comment #45
Tess Bakker@harivenu_zyxware .. array() is still needed, because workflow supports Drupal and Drupal supports PHP 5.3 and PHP 5.3 doesn't support Short syntax for arrays :( .. array() is still valid in even PHP 7.0
Because of that, patch #43 is the better than #44, see also interdiff
Comment #46
Tess BakkerUsed patch #43 on a prototype site and it worked perfectly with Workflow (dev) and Workflow Access. Noticed only that using the module 'role_export' could break the feature, but not a show stopper.
Didn't check the code.
Comment #48
johnvQuiet a big patch! Thanks all. Patch #43 was committed.