Using the features module, I can't export a permissions set if it includes a permission defined by workbench_moderation.

Here's the error on the target site:

Notice: Undefined index: moderate content from csnmont_important to needs_review in user_role_grant_permissions() (line 3031 of /srv/aegir/platforms/drupal-7.8-build-2011.09.29-staging/modules/user/user.module).

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null: INSERT INTO {role_permission} (rid, permission, module) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => 3 [:db_insert_placeholder_1] => moderate content from csnmont_important to needs_review [:db_insert_placeholder_2] => ) in user_role_grant_permissions() (line 3033 of /srv/aegir/platforms/drupal-7.8-build-2011.09.29-staging/modules/user/user.module).

Cheers,

Albert.

Comments

stevector’s picture

Status: Active » Postponed (maintainer needs more info)

I'm guessing you've defined Workbench Moderations states on the generating site that don't exist on the target site. This would cause a mismatch because the permissions are just a a concatenation of state names. Is that correct?

Workbench Moderation states are not yet exportable.

alberto56’s picture

Title: Allow exporting of permissions with the Features module » Make states exportable with the Features module

@stevector thanks for the timely response!

In fact, I manually reproduced the states on the target site before exporting the permissions, but I recognize this is an error-prone technique (maybe a typo prevented the permissions from being exported), so I abandoned the idea. I finally settled on manually reproducing everything on my target site with a checklist.

That being said, I am changing the title of this request to start a discussion on a more global solution which will allow the exporting of states using the features module.

Thanks for your work,

Albert.

alberto56’s picture

Status: Postponed (maintainer needs more info) » Active
robeano’s picture

Yes, I would like to see that we have the ability to export wb_moderation configuration including States. Discussion is underway.

robeano’s picture

Tagging and relating to a higher level architecture issue: #1316314: Workflow Architecture for Workbench Moderation

stevector’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
rogical’s picture

+1

Anonymous’s picture

Patch to Workbench Moderation is coming soon to integrate with Features.

Anonymous’s picture

Priority: Normal » Major
Status: Active » Needs review
StatusFileSize
new6.5 KB

Status: Needs review » Needs work

The last submitted patch, Workbench_Moderation-Features-1314508-8.patch, failed testing.

Anonymous’s picture

Status: Needs work » Needs review
StatusFileSize
new6.38 KB

Status: Needs review » Needs work

The last submitted patch, Workbench_Moderation-Features-1314508-8.patch, failed testing.

ericduran’s picture

I'm cleaning this patch up now.

ericduran’s picture

Status: Needs work » Needs review
StatusFileSize
new5.65 KB

And here we go.

This patch does a couple of things different then the previous patch. I try using all the workbench_moderation internal functions instead of messing with the table.

ericduran’s picture

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

Oh the test is going to fail, This is for the 1.x-dev version. Switching version now if it's ok with @stevector hope that's ok :-D

soomp’s picture

i've high hopes for this - thanks!

ericduran’s picture

FYI, I'm already using this is in a project :). I'll follow up with any issues I run into and make sure to update the patch if required. But for now this is working quiet well.

ericduran’s picture

Title: Make states exportable with the Features module » Make states/transitions exportable with the Features module

Updating the title to reflect the actual patch.

whitingx’s picture

Status: Needs review » Reviewed & tested by the community

Tested this and it works as required for exporting workflow transition states and them importing them as part of a Feature.

Very useful, good work @ericduran.

christianadamski’s picture

I could be wrong but: the .features.inc file is not included in the .module and the patch does modify the .info. I needed to add the file manually to the .info file to see an effect.

ericduran’s picture

@ChristianAdamski not sure why you would need to modify the info file. Feature doesn't use the registry system so that shouldn't affect it. At least since the last time I checked.

autopoietic’s picture

Status: Reviewed & tested by the community » Needs review

I was quite sure that the patch at #14 had worked for me previously, but I have tried several times today in a blank D7 buildkit installation to create a basic feature containing workbench states and transitions and each time it has fallen down. I have applied the patch against workbench_moderation-7.x-1.1, 7.x-1.x-dev and 7.x-2.x-dev

The features ui only gives the option to store states or transitions once, if I try to recreate the feature the options are no longer available. Clearing the cache makes them visible once more, but with the same limitation.

The second problem I encounter is that the Features UI only picks up the first state/transition checkbox I tick, and does not list any additional changes.

Finally on submission to download the newly created feature I encounter

An illegal choice has been detected. Please contact the site administrator.

I am trying hard to stick to a configuration in code development process, so I really need this functionality in workbench_moderation and would be really grateful if anyone could offer advice.

a1russell’s picture

Did you try using dev versions of the entity and ctools modules?

christianadamski’s picture

I can confirm the behaviour autopoietic described.

autopoietic’s picture

I have now updated entity and ctools modules to dev as suggested (#23) but workbench_moderation still does not provide features integration.

autopoietic’s picture

The attached patch for 7.x-1.x-dev adds a specific path for workbench_moderation.features.inc to workbench_moderation_features_api in workbench_moderation.module.

This appears to fix the issue for me - does this help you ChristianAdamski?

christianadamski’s picture

I can confirm that autopoietic's patch works fine with 1.x-dev. My problems were resolved.

I might add: I used patch #14 with version 1.1, not 1.x-dev

acbramley’s picture

Status: Needs review » Reviewed & tested by the community

Patch #27 worked perfectly, tested with exporting, modifying, and reverting.

dixon_’s picture

I can confirm the RTBC.

TidalWave’s picture

Status: Reviewed & tested by the community » Needs work

I tested the patch and at first this works perfectly,
however once you create an feature with all states, transitions and user permissions (for those transitions).
You'll end up with errors
Notice: Undefined index: moderate content from draft to published in user_role_grant_permissions() (line 3032 of /public_html/sandbox/modules/user/user.module).

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null: INSERT INTO {role_permission} (rid, permission, module) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => 6 [:db_insert_placeholder_1] => moderate content from draft to published [:db_insert_placeholder_2] => ) in user_role_grant_permissions() (line 3034 of /public_html/sandbox/modules/user/user.module).

From what I can determine with tests and code checks, it's because the permissions aren't available at time of importing the transitions.
user_role_grant_permission will try to check the permission 'moderate content from draft to published' which is an added transition.
Even though I thought the static function of the states & transitions was the problem, using a Drupal static and resetting them after rebuild/revert wasn't the sollution.
Anybody has another idea?

By creating a feature for all the states & transitions and 1 for the permissions, you can work around this problem.

acrollet’s picture

Status: Needs work » Needs review
StatusFileSize
new5.6 KB

I had the same problem as Tidalwave in #30, and after some amount of frustration, found that by applying the patch at in comment 10 of #1530386: Avoid unnecessary cache rebuilds and improve installation performance, I can create a feature with custom states, transitions and permissions based on the custom transitions, and be able to enable it. That said, after enabling such a feature, the states, transitions and permissions will all show as over-ridden initially, but it's possible to revert the states/transitions first and then the permissions. My sense is that we may have hit on an edge case in features, in that a module with with custom components and permissions relying on those components may not be properly enabled by features. I say this because the features implementation in this patch appears to be by the book, as far as I can tell. If someone knows of another module that does something similar and works completely, perhaps we could look at their approach? Otherwise I say we should get this patch into workbench_moderation and then file an issue against features.

I'm also attaching a patch for review that improves (imo) the comments and file structure, and removes some duplicated code between the rebuild/revert hooks, but is functionally identical to that in #26.

Status: Needs review » Needs work
acrollet’s picture

Status: Needs work » Needs review
StatusFileSize
new5.6 KB

re-formatted patch.

Status: Needs review » Needs work
acrollet’s picture

Status: Needs work » Needs review
StatusFileSize
new5.45 KB

ugh, was doing things backwards - one more try.

whitingx’s picture

Status: Needs review » Reviewed & tested by the community

Can confirm this patch works well, thanks for this.

gold’s picture

Umm... Looking at the attachments test result for workbench_moderation-featuresint-state-transition-1314508-22.patch it claims to have passed. Where did you experience the failure? Just curious as some the tests may need to be rewritten to properly do what they are meant to do.

...and ignore me. Miss read "33" for "22" which is what I was looking into.

alfaguru’s picture

Component: Miscellaneous » Code
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new6.37 KB

I've extended this patch to include resets of the static variables (amended to be drupal statics) as mentioned in comment 30. The rationale is that if you load more than one module at a time, such as when doing a simpletest run or in an installation profile, a features module may have successfully updated the db with new transitions but the workbench_moderation permissions hook fails to return them because the previous state is still loaded in the static variable.

This amended patch fixes that issue. I don't know if it also makes it possible to use the transition permissions in the same features module as there still may be a problem with the order of processing, ie. the permission features may be rebuilt before the transitions are stored, but this can still be worked round by having two modules and enforcing their dependencies.

richard.thomas’s picture

I can confirm that the patch in #38 works for me where #35 didn't, namely including exported states/transitions and exported permissions in the same install profile in different modules. Including them in the same module still did not work but it is a usable workaround to simply split out the permissions.

SebCorbin’s picture

Status: Needs review » Reviewed & tested by the community

#38 works good for me

Elvar’s picture

Status: Reviewed & tested by the community » Needs work

#38 worked for me aswell, until i upgraded panopoly from beta6 ~> rc1. I narrowed it down to this commit http://drupalcode.org/project/panopoly_admin.git/commitdiff/d8fadc799f9b...
Which introduces defaultconfig, the error is the exact same. Any ideas?

mstef’s picture

#38 looks good to me.

mstef’s picture

I'm not sure if this is directly related to this patch or not, but I'm experiencing a very strange error when enabling a feature that contains permissions for moderation states within the feature..

In the feature:

features[workbench_moderation_states][] = draft
features[workbench_moderation_states][] = needs_review
features[workbench_moderation_states][] = published
features[workbench_moderation_transitions][] = draft:needs_review
features[workbench_moderation_transitions][] = draft:published
features[workbench_moderation_transitions][] = needs_review:draft
features[workbench_moderation_transitions][] = needs_review:published
features[user_permission][] = moderate content from draft to needs_review
features[user_permission][] = moderate content from draft to published
features[user_permission][] = moderate content from needs_review to draft
features[user_permission][] = moderate content from needs_review to published

Error:

WD php: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null: INSERT INTO {role_permission} (rid,     [error]
permission, module) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array
(
    [:db_insert_placeholder_0] => 4
    [:db_insert_placeholder_1] => moderate content from draft to published
    [:db_insert_placeholder_2] => 
)
 in user_role_grant_permissions() (line 3054 of /var/www/d7/modules/user/user.module).
Cannot modify header information - headers already sent by (output started at /var/drush/includes/drush.inc:596) bootstrap.inc:1212                 [warning]
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null: INSERT INTO {role_permission} (rid, permission, module) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array
(
    [:db_insert_placeholder_0] => 4
    [:db_insert_placeholder_1] => moderate content from draft to published
    [:db_insert_placeholder_2] => 
)
 in user_role_grant_permissions() (line 3054 of /var/www/d7/modules/user/user.module).
Drush command terminated abnormally due to an unrecoverable error. 

Feature diff, despite numerous reverts:

Component: user_permission
      'module' => 'workbench_moderation',
      'name' => 'moderate content from draft to published',
<     'roles' => array(
<       0 => 'site-wide content moderator',
<     ),
    ),
    'moderate content from needs_review to draft' => array(
      'module' => 'workbench_moderation',
      'name' => 'moderate content from needs_review to draft',
<     'roles' => array(
<       0 => 'site-wide content moderator',
<     ),
    ),
    'moderate content from needs_review to published' => array(
      'module' => 'workbench_moderation',
      'name' => 'moderate content from needs_review to published',
<     'roles' => array(
<       0 => 'site-wide content moderator',
<     ),
    ),
    'post comments' => array(
      'module' => 'workbench_moderation',
      'name' => 'view all unpublished content',
<     'roles' => array(
<       0 => 'site-wide content moderator',
<     ),
    ),
alfaguru’s picture

Yes, because the states and transitions are created AFTER any permissions covered by the features module you need to create permissions relating to transitions separately. The simplest workround is to create a second feature with the permissions you need and make it depend on the first.

mstef’s picture

Yea I had a good feeling that was the issue.. but why can't the feature revert then. After it's installed, the states and permissions should be available. Right?

And don't you think there should be a way to avoid this?

alfaguru’s picture

@mike if you want answers to those questions I think you'll have to address them to whoever wrote the Features module.

mstef’s picture

Oh, I see you already mentioned this in the comment containing the patch. Sorry for the duplicate info then.

mstef’s picture

Have you looked into exactly what's happening and potentially a way to avoid the issue? I understand what you said but I still don't understand why it wouldn't be able to revert -- the exports from workbench_moderation are in the system by then. I also don't know why the module field on the database insert would be NULL, if the export provides the module name. And I also don't understand why this wouldn't be happening with nodes -- it's potentially the same situation where the permissions don't exist until the nodes do..

I'll see if I can find some more information..

BrockBoland’s picture

Updated the patch in #38 to implement two more hooks: COMPONENT_features_enable_feature() for both workbench_moderation_states and workbench_moderation_transitions components.

I have a Feature module with states and transitions defined. When I enable that module through the interface (at admin/modules), those states and transitions are created just fine. But, when I enable the module using module_enable() from an update hook on another module, the states and transitions are NOT created. For me, this causes errors like those reported in #43 when I later try to enable the Features module that includes user permissions for those states and transitions.

I still don't know why it matters which way the module is enabled, but since the COMPONENT_features_enable_feature() hook was not implemented, I gave it a shot and it fixed my problem.

Full patch attached, along with an interdiff from #38.

BrockBoland’s picture

Actually, I think I just figured out why it matters: Features alters the module list form to do a full rebuild, but doesn't behave this way on a programmatic enable of modules.

/**
 * Implements hook_form_alter() for system_modules form().
 */
function features_form_system_modules_alter(&$form) {
  features_rebuild();
}

I can't decide if that's a bug that should be addressed in Features, or if the correct solution is just to implement the hooks as I did in that last patch.

BrockBoland’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1314508-49-interdiff.diff, failed testing.

BrockBoland’s picture

Note: this latest patch is ONLY for people also using patch from #1891534: Add a label for transitions

If you're using transition labels from that other issue, you might also want to export them to Features, because that's exactly what I want. This patch is a minor update to the one in #49 to add the new transition_label field.

If are you are NOT using the patch from #1891534, DO NOT USE THIS PATCH.

trisketonni’s picture

Status: Needs work » Needs review
wbobeirne’s picture

#49 no longer works with the latest dev branch. Rerolling...

realityloop’s picture

Patch against 1.3 stable (for use with our own makefiles)

realityloop’s picture

#56 was a bad patch :/

This patch is against 1.3 stable (for use with our own makefiles)

elvis2’s picture

Is there a plan to roll this feature (exportable via features) into the workbench moderation module?

RyanPrice’s picture

It is still in the needs review state. It needs to be tested by the community to confirm it is working and then set to fixed. At that point the maintainers are more likely to look at rolling it into the dev branch.

azinck’s picture

#55 is working well for us.

zenlan’s picture

#57 tested and working for me

1. Created my new Feature to include:
all default WM states plus one new custom state
default WM transitions minus 1, plus 4 new custom transitions
2 user roles, each with different WM permissions
all WM permissions, plus a few node edit permissions
completely new View to replace the default WM View
Panels Page variant to handle user page redirection to new WM View

2. Completely uninstalled Workbench Moderation 7.x-1.3
3. Re-installed Workbench Moderation 7.x-1.3
4. Enabled my new Feature
5. Checked - all components have the correct configuration!

Previously, without the states and transitions, this Feature was impossible to re-enable and revert, all permissions had to be reconfigured, making our deployments during testing very tedious and error-prone. Looks like this is going to resolve those problems.

Thanks so much to all, this is a great help!

RyanPrice’s picture

Status: Needs review » Reviewed & tested by the community

Switching the status hoping to get it merged. Thanks for testing.

timwood’s picture

#57 tested and works for me. Thanks to realityloop for creating the patch against 1.3 stable!

elvis2’s picture

#57 works for me. I have used it extensively.

lorique’s picture

I found an issue.

I exporting a feature which requires a number of new transitions and states, which are permission controlled so only people with certain roles can move between the transitions. All this is workbench_moderation core stuff and works great. With this export it doesn't take into account the required order of execution, which is required because when a transition or state is removed or changed so is the permission for that transition or state. This is all well and fine, but the default order of execution in the export goes:

1) User permissions
2) States
3) Transitions

But when features tries to revert the permissions, a permission is now missing, causing an SQL error and thus preventing the revert from continuing with the revert. I fixed this locally by putting the user permissions last in my feature info file, and that fixes the order of execution problem. But this needs to be handled somewhere, and i imagine its in the revert process somewhere or in the output of the code.

Steps to reproduce:
1) Create a feature with a permission on a state or transition.
2) Delete the state or transition.
3) Try to revert the feature and watch it break.

I don't know if this is a features thing, or a specifically a workbench moderation thing, but since it was from the comment 57 patch the issue came up I'm posting it here.

BrockBoland’s picture

This is addressed in #44. Basically, that's just the way Features does it, and we don't have any control over it (aside from patching Features, which someone may or may not be doing). The workaround is to export the permissions into a second Feature module that depends on the first, which will ensure that the states/transitions are created before it attempts to set the permissions.

acbramley’s picture

Yup, the issue in #65 crops up from time to time with features for me as well (for stuff unrelated to workbench too), maybe something for the features issue queue?

acbramley’s picture

Any chance of getting this committed? I've been using patch #57 for months on multiple projects now

kristen pol’s picture

Bump :)

dave reid’s picture

Revised patch that moves workbench_moderation_features_api() and fixes missing @file in workbench_moderation.features.inc.

dave reid’s picture

Status: Reviewed & tested by the community » Fixed
mautumn’s picture

I just want to say a big thanks to the developer/s and all those who tested this. I was aware that 7.1.3. didn't support features, however, just before writing an update hook to achieve the same thing I thought I would see if this capability had been added since I last looked. And low and behold it has, and I tried it, and it works for both WF states and transitions. So it saved me a few hours work. Thanks very much. Good job.

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes
StatusFileSize
new7.25 KB

Hi,

I sent a patch with two to three correction in foreach loops.

Best Regards.
Valentino

Anonymous’s picture

Sorry,

I did not post the correct file

alberto56’s picture

@Valdoe because the current issue is fixed, you'll probably want to open a new issue with a patch against HEAD, otherwise your patch will go unnoticed.

Cheers,

Albert.