It would be great if workflows were exportable since they take a *really* long time to configure, if your site is of any complexity in terms of roles, workflow states, and content types.

According to this issue (#519686: Ubercart (product classes, attributes, options) features possibility), it would be possible to integrate with Features by supporting the hooks from CTools' export.inc.

Related issue for 5.x: #216731: workflow import/export.

Comments

askit’s picture

Desperately hope for workflow and worflow_fields import/export feature for D6...

pounard’s picture

This would be a killer feature!

mrfelton’s picture

+1 to this

rickvug’s picture

Issue tags: +CTools exportables

Tagging for tracking. IIRC there is a similar issue filed for Rules that has some code. I'm not sure if has made it in or not.

bricef’s picture

+1

Josh Waihi’s picture

Version: 6.x-1.1 » 6.x-1.3
Status: Active » Needs review
FileSize
15.11 KB

I've integrated workflow with features. Because workflow transitions are directly related to the roles available, features support for workflow requires that the roles already exists in the Drupal you try to enable the feature on. Workflow references roles by rid so its possible the wrong roles will inherit the transitions abilities if your roles between environments are out of sync.

This patch required database changes as features requires unique identifiers other that serials like cck, imagecache and cck all provide. So I've added machine_name to {workflows}, ref to {workflow_state} and a module column to both {workflows} and {workflow_states}. This enables features to reference a workflow and its states between environments.

Its important that before you go added workflow to your existing features, that you run update.php

I haven't fully tested this, so I am looking for feedback so I can fix the bugs. I hope it is helpful to people.

Josh Waihi’s picture

Title: Make workflows exportable (can integrate with Features by supporting CTools' export.inc) » Make workflows exportable (with features)
FileSize
16.03 KB

I found and fixed a few bugs already, I can now confirm this patch will add and remove states when applying the feature.

Boobaa’s picture

Subscribing - desperately needing it.

Josh Waihi’s picture

Status: Needs review » Needs work

I've been thinking about this A LOT. The problem is the workflow_states module. It directly relates to the worklfow_node table. States are objects you'll want to export but the node relations are not. However exporting an sid will have that effect if another workflow with the conflicting sid's is imported into the environment.

This isn't an issue with views, imagecache, cck, strongarm because none of those modules have to reference nids with the exception of explicit arguments or filters in Views but then that is intentional.

This is most likely the exact same reason why taxonomy isn't exportable.

Exportables will require a huge change to the workflow system. Potentially a 2.0 type release where each workflow and state have a unique machine name rather than a wid or sid. A state machine_name could be defined as unique when coupled with the workflow machine name so that states from different workflows could have the same machine name, thereby reducing import conflicts.

pounard’s picture

@Josh Waihi
Even without the nodes, workflows definitions should be exportable (the only dependency should the node type, right? Just like views and content).

Josh Waihi’s picture

@pounard no. because there is no unique identifier for a state. For example, if I have two sites, install workflow on both of them and create two different workflows, export both workflows and try to import them into a third site, it simply won't work because the wid and sid columns will contain conflicts since they are both serial columns that start from 1.

Instead, each workflow and each state requires a unique identifier that is not sid or wid but rather a machine_name or UUID. This becomes a key we can export on. State identifiers can be unique by reference to its workflow so that two states from seperate workflows could have the same identifier and still be classed as unique.

Summit’s picture

Subscribing, greetings, Martijn

pounard’s picture

@Josh Waihi
Views have a serial unique identifier on each site, but we can import/export them. It's because this identifier is loose on import/export operations and they use the machine name to ensure there is no conflicts.
All you have to do is to remove the serial identifier (or keep it only for database lookup in order to get best performances) and base all other code on machine names.

But the UUID is also a good idea. I used UUIDv4 identifiers to ensure nid/tid/uid synchronization between multiple sites in the YAMM module, but this need is also a bit different.

UUID is good, but it's also a 36 chars long big string (or 128 bits integer, depends of the representation you choose), it may impact performances to use it as every day identifier if you do varchar based database identifier instead of using integers (but I love UUID's).

Josh Waihi’s picture

Version: 6.x-1.3 » 6.x-1.x-dev
Assigned: Unassigned » Josh Waihi
Status: Needs work » Active
Issue tags: +drupal 7

Talking with the maintainer @jvandyk, its looks like workflow maybe converted to a field in Drupal 7 (which could mean porting to a cck field in Drupal 6). So this should be a feature implemented in Drupal 7 and backported to Drupal 6. I'm going to bump the version up to 6.x-1.x-dev but should really be 7.x-1.x-dev but there is no term for that yet.

Path forward from here:

  1. Convert Workflow to a field for D7
  2. Implement exportable in D7
  3. Backport workflow as cck field to D6 as 6.x-2.0 release
  4. Create upgrade path from 6.x-1.x -> 6.x-2.0 ->7.x-1.0
pounard’s picture

That's a solution, but something is bothering me, workflow state is not really data, it's a node state (like published, or sticky), and a node can't be under more than one workflow, it does not make any sense? Therefore using a field could be somehow confusing, plus it could be easy to have some piece of content in a incoherent state, don't you think?
And you can add the fact that workflow could play a role with node access and permissions, checking the worflow state using fields can be a performance killer due to the fields module overhead (I might be wrong here).

Josh Waihi’s picture

Status: Active » Postponed

we're getting off topic concerning exportables, we should continue this discussion over here: #732578: Drupal 7 version of Workflow, features

sdelbosc’s picture

Version: 6.x-1.x-dev » 6.x-1.4
Status: Postponed » Active
FileSize
17.82 KB

I have updated the patch given above to make it compatible with the last version of workflow module (6.x-1.4).

I have also made some changes, particularly in worfklow.features.inc :

  • Take tab roles and options into account
  • Implement hook_features_export_rebuild
  • Fix notices and warnings from coder

For me it is working pretty well. It would be nice if this patch could include workflow_access parameters.
Has anyone worked on something similar?

Josh Waihi’s picture

I did encouter a situation where I lost all workflow configuration and I couldn't figure out why. I don't know if it was the result of the patch I posted and an update from features, but I'm just saying, please wary.

Thanks very much for your feedback and contrib thought! :D

sdelbosc’s picture

To date I did not encounter such issue. I will update this post if I get it too.

In my previous post I have forgotten to thank you for this patch which is really useful for me.

brentratliff’s picture

subscribing

drhino’s picture

subscribe

rapsli’s picture

me too

rapsli’s picture

applied patch, I got two workflows in my system, but only one of them appears in the features module to be exported :(

rapsli’s picture

oky, it was a problem with the machine readable name. it was set to NULL, so there were two workflows with the same state. Else it seems to work just fine.

mani.atico’s picture

subscribing

kepford’s picture

subscribing

q0rban’s picture

subscribe

q0rban’s picture

Status: Active » Needs work

So quick code review here, take this all with a grain of salt b/c this is my first experience with workflow, so I'm not aware of the underlying complexities here.

+++ ./workflow.features.inc	2010-03-29 15:15:03.468625000 +0200
@@ -0,0 +1,264 @@
+      unset($state->sid);
+      unset($state->wid);

These can be combined unset($state->sid, $state->wid);

+function workflow_revert_default($config, $wid = FALSE) {

Seems like some of these functions should be in Workflow proper. Exportability and reverting to code should not be features dependent IMO.

+++ ./workflow.install	2010-03-29 15:27:35.624875000 +0200
@@ -54,11 +60,16 @@ function workflow_schema() {
+      'ref'    => array('type' => 'int', 'not null' => TRUE),

I know the other fields in here do not have a description, but I think it'd be helpful on this one. What is 'ref'?

+++ ./workflow.features.inc	2010-03-29 15:15:03.468625000 +0200
@@ -0,0 +1,264 @@
+    // Config already exists in database, we're just reverting back to the code.
+    // Use ref as the database reference.

I'm not familiar with Workflow too much, but is it too much to ask that default states not live in the database at all? IOW, workflow_load() should actually invoke hook_workflow_default().

Powered by Dreditor.

langworthy’s picture

subscribe

slosa’s picture

sub

franz’s picture

sub

franz’s picture

I used the patch from #17, it worked pretty well.

I came p with a solution for type mapping. It is actually pretty simple. The types mapped to the workflow are exported as an array with type => type items (only the types mapped to the workflow).

When reverting/importing, the solution for possible conflicts adopted is: If the type is not mapped to any workflow, map it. It if is, give up and display an error message when reverting/updating and do not change the mapping for that type.

I find it very usefull because I'm building features with node types inside, so this conflict is not even going to happen.

Anyways, patch attached, please test it.

I'll leave the needs work while I look into workflow_access parameters...

franz’s picture

Just added support for workflow_access (using role names as well). I realized that the workflow schema is wrong, I'll report on another issue.

I tested here and it's all working till now...

ryandekker’s picture

This wasn't adding dependencies automatically. I added the workflow module as a dependency regardless. I added a check for if the workflow is using workflow_access and add that as a dependency if it is. The $export array seems to be returning correctly, but it doesn't look like the workflow module is actually making it into the .info file as a dependency. workflow_access made it in fine for me.

Other than that, everything seems to be working to me.

franz’s picture

Status: Needs work » Needs review

Thanks! I guess we can at least change this do "needs review"

q0rban’s picture

Status: Needs review » Needs work
+++ ./workflow.admin.inc	2010-07-23 10:04:17.000000000 -0600
@@ -378,6 +384,11 @@ function workflow_state_add_form(&$form_
+  // Unique identifier for import/exporting.
+  $form['ref'] = array(
+    '#type' => 'hidden',
+    '#value' => time(),
+  );

time() is not unique. ;)

+++ ./workflow.module	2010-07-23 10:04:17.000000000 -0600
@@ -830,7 +845,13 @@ function workflow_workflow($op, $old_sta
 function workflow_load($wid) {
-  $workflow = db_fetch_object(db_query('SELECT * FROM {workflows} WHERE wid = %d', $wid));
+  if (!is_numeric($wid)) {
+    $where = "machine_name = '%s'";
+  }
+  else {
+    $where = 'wid = %d';
+  }
+  $workflow = db_fetch_object(db_query('SELECT * FROM {workflows} WHERE ' . $where, $wid));

This is bad form. We should create a separate function called workflow_load_by_name() or something.

+++ ./workflow.module	2010-07-23 10:04:17.000000000 -0600
@@ -1012,17 +1033,24 @@ function workflow_get_all() {
+    'machine_name' => strtolower(preg_replace('/[^\w\d]/', '_', $name)),

machine_name should be a field the user enters, not generated dynamically like it is here. We'll need to add validation to ensure the entered value is unique and matches safe chars.

+++ ./workflow.module	2010-07-23 10:04:17.000000000 -0600
@@ -1012,17 +1033,24 @@ function workflow_get_all() {
+    // Ref is a unique identifier used for exporting and importing workflow.
+    'ref' => $ref ? $ref : microtime(),

Once again, time isn't unique. We should add machine names to states as well. I'd like to separate the machine names of this work into a separate issue. Please see #867186: [EXPORTING] Add machine names to workflows and workflow states. Once that is done, it should be trivial to implement ctools export.inc for features exportability.

Powered by Dreditor.

franz’s picture

hi, q0rban

I don't see why go into ctools for exporting, considering everything is working fine without it. BTW, I'm using this on a dev site and so on it worked fine...

mrfelton’s picture

Not that I have put any effort into this development, but I would probably vote for integrating with ctools for the sake of standards. A lot of modules are relying on it for proving exportable support (amongst other things), as it provides that, and provides it well. Perhaps better to piggyback off the development effort and support that ctools can provide than to try and reinvent it here?

q0rban’s picture

@franz, ctools may end up not being an option for other reasons. I'm still working on the machine names patch, so once that's done I'll have a better idea of how best to export with features. :)

franz’s picture

mrfelton, I like to use ctools and all of that once in a while, but you must consider that it doesn't hurt the standards if you choose to be aside all of that. Ctools itself still has to adapt fully to core and hit the long road of being ported to D7 (and we don't really know how this is going to happen). I thinks making workflow exportable is a very simple thing and it's not because CTools make it possible trought it that we necessarilly have to use it. Again, "sake of standards" depends on which standards you choose to accept as so, and CTools is not any kind of "official" one. People use CTools for it's ease of use and for it's improvement (and some times substitution) of core features, not for it's standard.

pounard’s picture

@franz Agree.
@q0rban Disagree, CTools is not essential, even less with D7.
@mrfelton CTools is a NOT a standard, standard is core.

q0rban’s picture

@mrfelton CTools is a NOT a standard, standard is core.

CTools IS quickly becoming the standard for exportability with Features. Just look at Panels, Context, Boxes, Feeds, Strongarm, et al.

@q0rban Disagree, CTools is not essential, even less with D7.

I don't remember saying ctools was essential. What I did say is that it may end up not making sense to use CTools anyways, I'll have a better idea once the machine names patch is complete. :)

pounard’s picture

@q0rban

Don't forget that, in the module you listed:
* Features, Boxes, Feeds and Context and Strongarm are Development Seed modules
* Panels is a module from the author of CTools

What I mean is it does not make CTools a standard. Context module when you profile a bit shows that it suffers from some performances issues. Feeds does not uses that much CTools (only for some bits of code).

In the module you quoted I don't see at all a standard, but political choices made by a small number of individuals, without a comunity discussion about it. Don't forget that what defines Drupal is it's core, and standards are core only.

mrfelton’s picture

@franz, @pounard:

I think there is more than a small number of individuals looking to ctools to provide exportable support. My point was not to say that ctools was in core or going to become a part of core, but simply that ctools does exportable very well - what is the point in writing that code again, and having to support it yourself, when ctools already has huge developer and end user support base?

2 questions:

Q1) What do you gain by writing your own exportable support?
A1) I don't honestly know. You could try and argue that you avoid overhead. But exportable support should surely be an optional feature anyway (submodule). Enable it when you need it, disable the rest of the time.

Q2) What do you gain by using ctools?
A2) Developer support by some of the top devs in Drupal land. End user support by all the people that are using modules that rely on ctools.

Although, to be honest it doesn't bother me either way! just seems to make scene to take advantage of the work of others. That is one of the things that makes Drupal shine, IMO.

The Features 'movement' is quickly becoming the 'standard' way of creating deployable site features. Features is not 'the standard' or in core, but it is becoming A standard, and it is only by module authors adopting these 'standards' that the pace of development really picks up, and we end up with world class site building and developer tools like the Features module. I kinda see ctools in the same light.

pounard’s picture

@mrfelton

I must say I'm quite afraid of CTools, because it has some really weird behaviors on some environments (nothing critical though, but I don't like it by the experiences I had with it), so my judgment is somehow falsified by that.

Plus, it does not seems to have working D7 version right now (no D7 version at all if you look at the project page) while issue is flagged "Drupal 7". Some pieces of it have been moved to core, but not the exportable part.

Import/export code is really nothing to write, and in most case its complexity is proportional to module business itself, not the underlaying API used to achieve it, and because of this, in a lot of cases I would say that yes, you gain writing you own code, for numerous reasons:
1) The more you are module independent, the more porting will be easier (as soon as you add a module dependency, you are its maintainer slave), so relying on core is really good, on other modules, not that much.
2) It give you the power to achieve the task the way you want. Today, I have some problems, with a feature based site, I'm begining to deal with the Features module edges, and I'm starting to revert all features as "normal" modules to do tricky and custom stuff (way more efficient than using Features at the end), so, I would say in my case (not anyone's though) it's a lot better to have a custom import/export API (also which has good chances to remain simplier than CTools's one) which I can use directly without CTools of Features.

Beside of all that, I don't argue the fact this issue is about Features, this is good, but Features is not mandatory tied to CTools, and by controlling the features code integration, you will be able to do better stuff than with CTools, at the cost of maybe, more lines of code, but more portable code which remains dependency-free on which you keep the full power.

franz’s picture

@pounard, agreed

Again, this set of popular modules are not standard, and the fact they are becoming more and more used still doesn't move them into full compatibility with each other and/or with core. We are on the edge of launching Drupal 7, and so when I think of developing new features, I always think about the ease of porting, and that is something that would be seriously compromised if using CTools. And besides, the real "work on code" will have to be done anyways, like the machine-name issue. So, what's to point about submitting to CTools API and therefore insert your module inside a whole chain of complexity that may cause problems later? Features is a very nice module, and so is CTools, sometimes. But being independant from both is also really nice, as sometimes you just need to write custom code and don't want to deal with CTools/Features inner workings. Being able to export do features is an UP because it doesn't mean we will need it to export workflows.

BTW... Isn't there quite a fight between CTools and Context/Features/... devseedstuff ? How can that be considered any kind of standard? Do you understand that you need more than hype/popularity to become a standard? Next people will argue that Ubercart is a standard...

jide’s picture

subscribing

amelfe’s picture

FileSize
1.44 KB

I had this error :

Parse error: syntax error, unexpected T_STRING, expecting ')' in .../sites/all/modules/features/features.export.inc(593) : eval()'d code on line 373

It turns out that a quote in a role name caused the problem. I changed a line to fix it.

I also added a small improvement that suppresses content types associated to a workflow when reverting it.

amelfe’s picture

I also found out that workflow_access doesn't get reverted when the workflow is created
Here is the patch from #34 with the fix for workflow_access (my changes in #48 are also included)

shopdogg’s picture

+1

auzigog’s picture

Subscribing

q0rban’s picture

Version: 6.x-1.4 » 6.x-2.x-dev
Assigned: Josh Waihi » q0rban

There is a new 2.x branch of Workflow that will focus on implementing exportability. http://drupal.org/node/893230
Also, assigning this to me. Once machine names patch is finished, I will start working on this in the 2 branch.

q0rban’s picture

The machine names patch has now been committed #867186: [EXPORTING] Add machine names to workflows and workflow states, so I will shortly be picking this back up. If you are interested in helping out move this along, please download and test the 6.x-2.x branch of this module. DON'T test this out on your production site, and DO be sure you back up anything you care about beforehand. :)

Thanks!

q0rban’s picture

I'd like to get everyone to weigh in again on using Ctools for exportability. This won't mean that Workflow will require CTools. It will only require it for exportability. A simple Ctools++ or Ctools-- will suffice. Thanks!

My vote is:

Ctools++

;)

brentratliff’s picture

Ctools++

franz’s picture

Ctools--

rickvug’s picture

Ctools++

mrfelton’s picture

I think I made my case already.
Ctools+++

pounard’s picture

Ctools--

jide’s picture

I'm in the ctools-- gang.

q0rban’s picture

Ok, so far it seems pretty split down the middle. I'll begin work on this without CTools and see how it goes.

auzigog’s picture

I'm CTools++ but that doesn't exactly make a strong majority.

franz’s picture

FileSize
441 bytes

Latest patch does not apply to 2.x-dev, needs to be rerolled.

I have another addition to this: Author role name gets translated on some installations, breaking the feature. This patch is meant to be applied over the new features.inc file

Dp-Mtl’s picture

Thanks for all patchs and comments!

I find the latest patch does not do the exporting for the content of the table {workflow_type_map}, we still need hand configure to do. If add this export and import that would be perfect.

q0rban’s picture

Status: Needs work » Active

Hi all.. I would REALLY recommend you NOT use any of the patches above as what will end up getting committed is sure to have a different architecture. The above patches are likely to break your ability to upgrade. I hope you'll be patient with me as I actively work on a solution for this. :)

yhahn’s picture

My 2 cents:

We always recommend that you use the CTools export API for implementing exportables in your module. If there are design decisions in your module that make it difficult to implement the API, we recommend you alter your module's design decisions. There is no good reason to invent your own exportables API in your own project.

BTW... Isn't there quite a fight between CTools and Context/Features/... devseedstuff ? How can that be considered any kind of standard? Do you understand that you need more than hype/popularity to become a standard? Next people will argue that Ubercart is a standard...

It turns out a little research would show that every devseed module that has Features integration and supports exportables does so using the CTools export API.

q0rban’s picture

Looks like drupal.org will soon be using CTools itself, so hopefully that will give some credibility for it to those of you who don't believe it's a good idea. http://drupal.org/node/908260

netw3rker’s picture

@q0rban - not to nitpick, but I'm not sure how 5:3 in favor of ctools is 'split down the middle' :)

FWIW, there are *a lot* of people who are hurting for this functionality. There are also two ways to determine how to vote for this. if a vote of ctools-- gets the already reasonably working patches into the module faster, that's where my vote is. If the patches are being viewed as not good enough and a rewrite by you has to happen anyways, then ctools is probably the faster route to a release, so that would be a ctools++ vote. either way before this vote can be accurate, we kind of need some insight into why the work already done isn't good enough.

-Chris

brentratliff’s picture

Just my take. The original title of this feature request was "Make workflows exportable (with features)". The way to ensure the code will keep up with Features exportability is to use CTools. Features will always support what happens with changes to CTools. Rolling an independent export system is a short term fix that's likely to break when the API's inevitably change.

Shawn DeArmond’s picture

Count me in as one who is "hurting for this functionality". CTools is more than credible, it's fast becoming akin to core+. If it's easier to implement, then that's even better.

CTools++

If people aren't keen on workflow being dependent on ctools in its entirety, should we consider a separate "workflow_export" module which is dependent on ctools (and workflow, of course) and facilitates the features exporting?

franz’s picture

With features now stable, it is expected, as much as it is with CTools, that the API will not suffer more changes.

As with #36, sounds a quite reasonable point of view. I'm currently using this code and it has worked so far, but if there is too much to be done still, then perhaps CTools could be better. Although I think that the changes being made to Workflow have a lot of intersection between the two alternatives.

Anyways, this should get implemented in one way or the other. =]

darrenphillips’s picture

CTools sounds great to me. I'm very much looking forward to this being done.

Not sure if this is the right place but I have 2 patches for Workflow 2.

workflow.view.inc.patch - This fixes an issue I found where the state column is being referenced instead of the new label column. Without this fix I get a SQL unknown column error when I add the workflow name as a field to a view.

workflow.actions.inc.patch - This is also a bug in Workflow 1. When creating a workflow rule change action changing the comment works but the changed comment is never displayed in the form as workflow_history is used for the default value instead of workflow_comment.

netw3rker’s picture

@darrenphillips you should open another issue for those problems in the workflow module's issue queue and attach your patches there. Also, see: http://drupal.org/patch/submit and "naming your patch" for the filename conventions ;)

Good to see you are writing patches already! Maybe we'll get to work together again soon!
-Chris

pcambra’s picture

suscribe

irakli’s picture

q0rban, what's the state of 2.x branch? I saw the machine name patch, so that looks fixed, but there's no actual implementation of CTools Exportable hooks that I could find on 2.x branch. Am I missing something?

Also, can you please create a dev release of the 2.x branch?

Thanks

q0rban’s picture

Hi Irakli.

Currently the 2.x branch is very unstable, hence no release on the page. :) The object structure needs to be completely changed in order for exportability to work properly. Currently I have been working in my own local git repository on this, and have not committed work to CVS in case my work ends up changing due to the complexity of this module.

All that said, I would LOVE to get another developer on this. I can create a temporary repository on github with my latest work that you can send pull requests to if you are interested. Let me know how you'd like to proceed. :)

irakli’s picture

q0rban,

I think the basic integration with CTools exportables API is complete at my github fork. I was able to stay out of workflow.inc and workflow.admin.inc for the most part of it.

At the integration point, I fetched your changes from upstream and did minimal edit to workflow_load_all() to add default ctools-sxported workflows to the list of the ones you were already fetching from DB and third-party modules. I also fixed couple of very minor bugs.

In the workflow.admin.inc I did minor code changes to expose current state of a workflow object (Normal, Default or Overriden) and to change titles of operations accordingly.

If you pull the code right now, you should not get any merge conflicts.

Can you, please, review the code and pull it into your github branch before we proceed? Would save us the headache of merge conflicts.

Thank you

betz’s picture

subscribing

irakli’s picture

Status: Active » Reviewed & tested by the community

q0rban,

the Features/CTools exportability is complete: http://github.com/inadarei/drupal-workflow We've spent a lot of time testing/debugging it and all basic things seem to work pretty well, as far as exporting the main workflow object and exporting workflow/node_type_mapping goes. All the related edit screens have also been debugged and there are no obvious problems remaining.

At this point, I think it would be very helpful if you can pull it in, on github, and maybe even sync with CVS 2.x branch so more people can try it and help us find the remaining bugs, if any.

Thank you!

betz’s picture

q0rban, i would like to test out irakli's work, could you merge?

q0rban’s picture

No I'm not doing any commits to the 2 branch in CVS until I've actually reviewed his work. Feel free to test with his code from github, but DON'T do this with any data you care about, as I make no guarantees about schema in the 2.x version yet. :)

alayham’s picture

Issue tags: +Features integration

sub

FiNeX’s picture

subscribing

MarcusF’s picture

Subscriping

mthart’s picture

subscribe

chia’s picture

subscribe

rmontero’s picture

Subscribing

irakli’s picture

@q0rban,

any update on this?

Thank you.

q0rban’s picture

I've begun reviewing your work Irakli and cleaning up on github. I will be working on this any spare time I get, but there's a lot to go through. We need someone to write some simpletests! Any volunteers? :)

irakli’s picture

Hey James,

I noticed that you have pulled the exportability changes. That's great. However, I noticed that you then went ahead and changed some significant parts of the code. Which is fine, obviously, except now I checked out your version and it's broken - throws errors, has leftover dsms and does not recognize exported workflows etc.

Since the last version I committed to the forked branch was relatively stable, I was hoping at least that level of stability could be maintained so that we could keep using the new workflow and concentrate on getting bugs fixed. Alas, it's back to non-usable state again.

Right now, I need to fix a minor bug and i don't know where to fix it, because you have moved on from my fork and the state of your branch is not currently usable.

Would appreciate if we can stabilize code-base. Especially since that would allow this module to actually be used "in the field", getting it better tested and feeding bug-fixes back into the module.

Thanks

q0rban’s picture

Irakli, yes, I'm sorry, I got pulled of this work yet again. :(

Go ahead and keep working on your fork, I will end up having to start again from wherever you leave off, but my work on this has been tabled until January at least.

irakli’s picture

Thank you.

Appreciate it.

TravisCarden’s picture

subscribing

NicolasH’s picture

subscribing

ndwilliams3’s picture

irakli
When installing your fork version (op2.3.03) from github on a fresh install drupal 6.19 i get the following error when creating a workflow.

user warning: Unknown column 'tm.type' in 'field list' query: SELECT tm.type, w.name, w.label, wt.tid, wt.state_name, wt.target_state_name FROM workflow_type_map tm LEFT JOIN workflows w ON tm.workflow_name = w.name LEFT JOIN workflow_states ws ON w.name = ws.workflow_name LEFT JOIN workflow_transitions wt ON ws.name = wt.state_name WHERE wt.target_state_name IS NOT NULL ORDER BY tm.type, ws.weight in ...\sites\all\modules\contrib\workflow\workflow.module on line 561.
q0rban’s picture

Please keep in mind comment #81. I make no guarantees that any of the work that has been done in either branches will end up getting committed to Workflow 2.x. It is UNSTABLE in the truest sense of the word, so DON'T use either irakli's work or my work on any site you care about. Seriously.

ndwilliams3’s picture

related to trigger.module. no error when disabled.

ndwilliams3’s picture

the version is used in the latest Openpublish release. Not using openpublish, but could not figure out how it was able to have the feature export. After comparing the releases and finding this post I discovered that is was another version. What are the plans with irakli version in reguards to the 1.x branch. With it being used in Openpublish, would it be safe to say that there would be an upgrade path if the schema changes?

q0rban’s picture

Status: Reviewed & tested by the community » Needs work

Workflow module will not be addressing any schema changes between Irakli's version and the final 2.x release. I assume Openpublish will write updates in their own custom module to address the schema changes, but Irakli could better speak to that. :)

Also, putting this back to needs work, as I still need to vet Irakli's work completely.

digi24’s picture

I think that in Iraklis version (todays git) there should be something like ctools_include('export'); to avoid errors when the ctool functions have not been defined yet.

And a small question, regarding the version above: is there an ui to actually export the definitions (I do not see it), or do I have to do this in code?

scottatdrake’s picture

subscribing

jhedstrom’s picture

I'm testing Irakli's version on github, and it seems to store workflow states and transitions just fine.

@digi24, the UI is provided by the Features module.

jhedstrom’s picture

I ran into the issue caused by not having ctools_include calls before certain ctools function calls. Sent a pull request to irakli on github.

irakli’s picture

Jonathan,

just committed the pull request.

Thank you!

paulmckibben’s picture

Subscribe

tayzlor’s picture

using the master branch from the git repo the views fields (particularly the 'current state') appear to not work / be broken?
The label displays for me but not the actual state.

dixon_’s picture

subscribing

Thanks for all the work on this! As said before, very needed :)

@q0rban @irakli Would it be possible to get irakli's work Github over to drupal.org now when we have our fabulous Git migration in place? :)

RogerB’s picture

Subscribing

R.J. Steinert’s picture

R.J. Steinert’s picture

Over in #660958: ctools export intergration (features support) for the Node Relationships module they are considering rewriting it as a CCK Field Widget which would negate their need to have a CTools integration. Over the weekend as I spent time working on patches for Irakli's branch on GitHub I couldn't help but get the feeling that if Workflow was rewritten today that it would be written as a CCK Field Widget that extends CCK Field Type Text.

If I'm understanding the problem correctly, this would allow the Workflow module not only to drop a lot of its storage and CTools integration code, but also the Views handlers. I'm new to the Workflow module so I'm wondering what the Workflow veterans have to say about this idea.

q0rban’s picture

Hi rjstatic!

I couldn't help but get the feeling that if Workflow was rewritten today that it would be written as a CCK Field Widget that extends CCK Field Type Text.

This conversation has occurred before, but unfortunately I can't find the issues now. My belief is that fields should only be used for content, and using them for other use-cases as a shortcut to completion is probably not the best idea for the long term health of this project.

I apologize to all on the this thread for the lack of progress here. I was put on this issue as a deliverable on one of our projects, but unfortunately (paid) work on this has now been postponed.

I must have somehow miscommunicated to Irakli about what was remaining to be completed, and so his branch of work is still in a 'needs work' state. I am feeling it will just be simpler to pick up from where I left off (https://github.com/q0rban/drupal-workflow) prior to Phase 2 graciously picking up the slack. The branch there is broken, albeit in ways that I understand. ;) The big portions are done there (the database switch to machine names, an initial exportable workflow object, etc), and the remaining work is in going through the UI and ensuring that the new API is being implemented properly. If someone wants to pick that up and run with it, I'd be more than happy to sync up and try to steer you in the right direction.

Thanks everyone, and again my apologies, especially to Irakli and others who've put sweat into this.

R.J. Steinert’s picture

@q0rban sorry to hear your project got postponed.

Irakli's branch definitely still needs work but the core pieces I need right now are working as of today (exportables, views state filter, views state field, workflow access). Looking at the history of commits, it looks like irakli rebased a bit when he first started but all of q0rban's commits are in there. There's not much for pending patches here in the issue queue so I think we should be good to move irakli's branch over as the 6.x-2.x branch.

I'll look around for those threads on implementing Workflow as a CCK Field Filter.

Cyberwolf’s picture

Subscribing.

R.J. Steinert’s picture

On the topic of different possible storage methods for Workflow, I would like to add using the Drupal variables table as an option. Each workflow could be stored in the Drupal variable table as an individual variable. This approach has the following advantages:

1. Drupal variables are already exportable.
2. I think it might be less of a paradigm shift to that of switching to Workflow as a CCK field.
3. Integration with the Spaces module would be possible for customizing workflows per Space. (Spaces stores Drupal variable overrides on a per space level)

Thoughts?

Emilio Davis’s picture

Patch in #49 wasnt working in postgres, fixed it and tested against 6.15.

aiphes’s picture

suscribe.do you know if a new version or dev one of workflow will be released when this feature will reviewed ?

WorldFallz’s picture

I'm going to be needing this functionality in the very near future and would like to be able to test and submit patches-- can we please get an update on the plan for this feature?

Should I be using a patched module with the patch from 115 (which seems based on the very old patch in 49)?

The 6.x-2.x branch?

The github fork (updated last 5/2011)?

Some thing else?

I'm tempted to just use the irakli's github version, but then I might not be able to contribute anything back here. I'm fine with that for my project, but it would be a waste not to be able to use my time on this for the drupal.org version.

q0rban’s picture

If you are interested in working on this, the best place to start would be from my github branch. There may be parts of irakli's work that makes it into the final patch, but not enough to warrant starting from that repo.

WorldFallz’s picture

thanks for the update q0rban -- when you say 'broken in ways you understand' above, can you give me an overview?

Bastlynn’s picture

Good idea, I'll take a look at the work done so far once I get the rest of the issue queue under control.

Bastlynn’s picture

netw3rker’s picture

I find it very frustrating that this issue has now reached and passed its 2yr anniversary w/o any resolution when a reasonably working patch has been around for well over a year.

I think for the sake of forward progress we should consider the patch available in #34 to be the best possible approach, commit it and move forward letting people test, revise and patch as needed.

If that patch does not adequately meet the requirements set forth in this issue - workflows need to be managed by features. please list the specific reasons why it does not meet that requirement. If it does, it's time to commit it, and either apply the changes in the github repo to a new major version of workflow, or abandon it.

Sorry if this starts a battle, but Long and short, it's time to get the discussions going.

Bastlynn’s picture

Please recall I got maintainer access about two days ago and have *quite* a lot to catch up on.

This work is already on my short list to be reviewed on priority out of the collection of issues on hand. If you want to speed that along, rerolling the patch against the changes I put into the 7.x branch today to divide workflow into submodules would be great so I can appraise the changes on the branch I'm currently operating on. Once that version is stable I can switch back to 6 to backport with a good feel for how the update path will go.

netw3rker’s picture

@Bastlynn

That's fantastic news. I'll take a look at the 7.x branch & see how much effort a reroll will be. This is much easier when workflow is on my radar, but I'll put some time in tonight/tomorrow for this issue.

Bastlynn’s picture

Version: 6.x-2.x-dev » 7.x-1.x-dev

I should be making a commit sometime tonight The refactor has been committed to get all the scattered SQL throughout the module(s) into one location (there was another issue on that floating around from the 5.x days). Yay refactoring! ;)

That should make it easier to start modifying schema for a proper re-roll since we won't have to go hunting outside of the DB functions.

I'll dig into what's at git hub and get up to date on CTools and the like. I've never done an integration with CTools before so advice is welcome. :)

jdwfly’s picture

I thought this is worth mentioning because I too am also needing this functionality. A google search found a Workflows Features module in alfababy's sandbox. I just tested it out and it seemed to work as expected for me. It probably needs more testing but from my tests and a cursory glance at the code it seems well laid out. Maybe it would be a good idea just to promote that to a full module and anyone needing this functionality could install it. Just a thought...

Bastlynn’s picture

I'll take a look, I can almost promise that it's not going to match the D7 version exactly though, so there will probably be extra work to it either way you cut it.

Bastlynn’s picture

Assigned: Unassigned » q0rban

First pass at this is up:

http://drupal.org/commitlog/commit/198/3a39fc997ad0dda5e9da77ecf3c45aec7...

It's very very simplified compared to the version of this effort up on github, providing just the CRUD functions to translate between the table data and the export format. Looking over the tables, I noticed that workflows name field is kept unique by the code so I made it a unique key to further enforce this. Same applies to workflow_states state field. Given this, we have an effective machine name for these two tables already - just not formally enforced, so I've recycled those as the machine name in this pass - I expect this will be a temporary thing as I dig into CTools further and get feedback on this effort.

Stuff I can already see I need to add on round two:
workflow_access
and role_export by role name not role id

Edit: CTools approach wasn't working - see post below where I fell back to using Features API instead.

Bastlynn’s picture

Assigned: q0rban » Unassigned
Bastlynn’s picture

Bug found: workflow_states->state is only unique within a workflow, not across all. So that needs to be addressed. Since we're not using anything but the export / import features here, I think we can treat workflow-states as a non unique additional table.

Bastlynn’s picture

Fixed on the latest update from today (11/25/11). State names are unique within workflows and that's the only context our import / export interacts with them in, so all's well so long as we hold wid/state unique.

Bastlynn’s picture

The biggest need i have here is for confirmation that this *works* with your own Features configs. If someone is using Features, please let me know if this is working for you. :)

Bastlynn’s picture

Assigned: q0rban » Unassigned
Status: Needs work » Needs review

After experiencing significant difficulty trying to do this the CTools way, and given our UI does not use CTools admin panel, or other CTools features, I've decided to go for the Features API based approach. Simpler, cleaner, and working. Should anyone still want to follow up on the CTools approach to features integration under 7.x, feel free to roll a patch against the latest version of the code accounting for the refactoring that's been done so far.

The new features integration has now been tested on my machine, and is functioning quite well.
It's in the latest commit on 7.x-dev - I would like to hear confirmation from someone else that this piece of the update is now working.

jct’s picture

I agree that the features route would be preferable -- workflow would be a nice addition to a number of features-based distros (I'm interested in rolling something for OpenAtrium). Any chance of a backport into the 6.x version?

Bastlynn’s picture

Once we have 7.x stable and released we can start back porting into 6.x - all the more reason to get this one out and tested, right? ;)

Bastlynn’s picture

Status: Needs review » Reviewed & tested by the community

This has been working reliably on the sites I've been using it on.
And I've not gotten any reports that others aren't seeing it work. So - I'm calling this one done on the 7.x branch.

Reroll for D6 to come soonlike.

hefox’s picture

Status: Reviewed & tested by the community » Needs work

There's several code standard issues (spacing, too long comments); a quick run through coder will pick those up.

  $saved_workflows = module_invoke($module, 'workflow_features_default_workflow');

Better:

  $saved_workflows = features_get_default('workflow', $module);

(since you're using features anyway; current version means no drupal_alter so can't alter the workflow defination!).

Following features naming schema, the default would be workflow_default_workflows. There's no need to name space it with features.

Features has role exportables using rolename. How about replacing rids with roles on exporting, then translate to corresponding rids on importing?

I originally looked at this interest of backporting the current patch, but there wasn't a patch -- just a serious of commits against -dev mixed with some other patches. Just commiting to -dev for such a big project is not good for collaborating/getting good reviews of a patch! Also, the commits messages are against standard and not as informative as they could be; see http://drupal.org/node/52287 for commit message best practices. (The commit message of "." was especially uninformative).

hefox’s picture

The exported workflow object contains various references to IDs (sid, workflow ids, rids, etc.), so won't work on separate install.

Working on a patch for this, and other issues.

hefox’s picture

Status: Needs work » Needs review
FileSize
9.82 KB
hefox’s picture

Status: Needs review » Needs work
+++ b/workflow.features.inc
@@ -1,5 +1,11 @@
+define('WORKFLOW_FEATURES_AUTHOR_NAME', 'workflow_features_author_name');

This is sort of a hack for changing transition roles to a role names. Can't just do 'author' cause that seems like a reasonable role name, 'workflow_features_author_name' does not seem like a role that anyone would have. This could use something different.

+++ b/workflow.features.inc
@@ -24,20 +30,25 @@ function workflow_features_export($data, &$export, $module_name = '') {
 function workflow_features_export_render($module, $data) {
-  $code = array();
+  $translatables = $code = array();
+  $code[] = '  $workflows = array();';
+  $code[] = '';
+

Changed around this area so the export looks more like a the other feature exports.

+  foreach (features_get_default('workflow', $module) as $key => $workflow) {

+ foreach (features_get_default('workflow', $module) as $key => $workflow) {

changed this for the above reasons stated in a previous comment.

...
+  $states = isset($workflow->states) ? $workflow->states : array();
+  $transitions = isset($workflow->transitions) ? $workflow->transitions : array();
+  $node_types = isset($workflow->node_types) ? $workflow->node_types : array();
+  unset($workflow->states, $workflow->transitions, $workflow->node_types);
+

Changed this cause the amount of lines setting the variables was taking seemed a bit excessive.

@@ -114,35 +126,45 @@ function workflow_update_workflows_full_object($workflow) {
-    // workflow_update_workflow_transitions checks preexisting transitions by sid, target_sid.
+    $transition->sid = $active_states[$transition->state];
+    $transition->target_sid = $active_states[$transition->target_state];

Changed target_sid/sid to target_state/state when they are state names to be a little less confusing.

+++ b/workflow.features.inc
@@ -114,35 +126,45 @@ function workflow_update_workflows_full_object($workflow) {
-    $node_type->wid = $workflow->wid;
+    $node_type = (object) array(
+      'type' => $node_type,
+      'wid' => $workflow->wid
+    );

Changed node_type export to just be type. Node type mapping is purely wid to type, so no need to have an array of information there. Simplifies the

Ignore comment #138, turns out the reference to WID were just excessive and didn't matter, so the export would have worked fine.

Existing exports of this won't work cause changed the hook name to hook_workflow_default_workflows. On a site with it already installed, do a features update and should update it since component key hasn't changed.

hefox’s picture

Status: Needs work » Needs review

Didn't mean to reset status

hefox’s picture

Status: Needs review » Needs work

tab_roles needs to be changed to rolenames and back again

q0rban’s picture

I think you guys would do well to look at the work that was done on github for this. Looks like you are addressing issues that were already addressed there. :)

Bastlynn’s picture

@q0rban - I did take a look at the code there but in combination with the changes in place for the upgrade to 7 I wasn't able to reuse as much of your code as I would have liked. From the looks of the patches hefox is set up this is mostly a final polish run for style, and tightening up the current code for features support.

@hefox - Thank you for advice. I suspect I owe you an apology for how I first took it (thankfully I didn't reply immediately) but coming back to the review, I now see what you were going for with the idea of putting all the changes into a patch before applying when it comes to large pieces like this. It *would* make it easier to reroll for 6 to have that sort of summary. Mea culpa, at the time I was just wanting to get it done with. I'll keep that in mind for future chunks of work like that.

I'll have more time for a deeper review of what you're looking at later tonight - so expect another reply then.

Bastlynn’s picture

Er. Doubleposting ftw?

Bastlynn’s picture

Status: Needs work » Fixed

The update to features looks good, polished out the last of things near as I can tell. I've committed the patch to dev. Let me know if there's any others I should expect. :)

thedavidmeister’s picture

Version: 7.x-1.x-dev » 6.x-2.x-dev
Status: Fixed » Needs work

as mentioned above #134, #135, #136 a D6 backport of the D7 work is still expected Bastlynn.

thedavidmeister’s picture

Version: 6.x-2.x-dev » 6.x-1.x-dev
aiphes’s picture

any idea of disponibility of dev version with this feature on D6 ?

hefox’s picture

Status: Needs work » Patch (to be ported)

Needs work implies there's a 6.x patch; patch (to be ported) says there's a patch in another version to be ported to a different version

thedavidmeister’s picture

ah, thanks hefox. I wasn't actually sure what the right way to do this was, I was even considering opening a duplicate thread for the 6.x branch and referencing this issue, glad you updated it.

NBZ4live’s picture

+1
Subscribed

Kristen Pol’s picture

Hmmm... this is marked for 6.x but I just tried for 7.x and definitely the role ids aren't working though it looked like they were discussed that they needed to be fixed in #138. I'm not sure what to do as I don't want to change the version number since the 6.x patch is still in the works. I might need to abandon Workflow because of this issue as we are heavily tied to Features.

What I found is that when the features module was copied from one site to the other, then:

1) the workflows were created but the workflow ids are all different (which is to be expected, but makes me worry about how it will interact with other things on the site... views, rules, etc. as well as dealing with custom code)

2) the roles that were selected were based on role id rather than name and because roles are created via Features then the role ids were different on the different sites and it didn't work

3) the permissions (to change from/to different states) weren't copied over (I think due to the role id issues in 2)

NancyDru’s picture

It doesn't seem that anyone is really working on a patch.

igor.ro’s picture

FileSize
2.59 KB

We should use en roles name, because of features import and export pages could have different names (because of settings for current user)

Here is the patch

orjantorang’s picture

FileSize
3.06 KB

I made a module enabling Workflow to Features, I didn't what to patch.

igor.ro’s picture

@orjantorang this module have some critical problems.
First of all it does not allow to export one workflow. Only all in once.
Also you truncate whole table that will drop other workflows, you did manually.

In other words it is not acceptable on most project and is not drupal way.

entrepreneur27’s picture

Hi

I am keen to be able to export as a feature the workflow I just set up. I am using D7, and find that when I export the workflow and move from testsite to production various things appear to get screwed up, and it looks very much as though it relates to the issue of ids discussed in #153.

As far as I can see the issues affect both roles and the states themselves that also seem to be referenced by an id that changes across installation.

I am looking at the patch in #155 but am unsure if this is for D7 or D6. Can you clarify please @igor.ro?

And has it been tested as yet?

While on this topic, I am unsure of the etiquette, but would like to add a vote for the importance of a functioning export capability from this excellent module.

igor.ro’s picture

Hello, @entrepreneur27

Patch #155 is drupal 7. I use it on one of my projects. Works fine for me.

johnv’s picture

Title: Make workflows exportable (with features) » Make workflows exportable with Features
Component: Code » Features
Issue summary: View changes
johnv’s picture

Title: Make workflows exportable with Features » Make workflows exportable with Features (D6)
Status: Patch (to be ported) » Closed (won't fix)

I wish there was a nicer way to clear the issue queue from 'D6-issues that are fixed in D7' then a "won't fix for D6."

@igor.ro, please open a new issue if your patch is still needed in 7.x-2.x version.