Workbench moderation is currently causing subscriptions_content_node_insert() to run with "$inserted" as TRUE when updating a node causing subscription notices to be sent out when subscription settings are set to not send out subscription notices during updates.

Ideally this issue would be fixed in workbench_moderate, however, after investigating I didn't notice a good clean way to do so since the final call to "subscriptions_content_node_update" is being kicked off by workbench_moderation's 'workbench_moderation_store' which calls node_save. At some point in the scheme of things is also calling a node_load which in turn sets the drupal_static variable "subscriptions_content_unpublished_nids" with the current node's nid.

** Note workbench_moderate relies on this final node_save call to save the current published revision as the rid being used in the node table.

CommentFileSizeAuthor
#7 2027767-3-subscription-add_workbench_moderation_support.patch1.98 KBpattersonc
#3 2027767-2-subscription-add_workbench_moderation_support.patch1.91 KBgrndlvl
#1 2027767-1-subscription-add_workbench_moderation_support.patch1.07 KBgrndlvl

Comments

grndlvl’s picture

grndlvl commented
Status: Active » Needs review
StatusFileSize
new 2027767-1-subscription-add_workbench_moderation_support.patch1.07 KB
salvis’s picture

salvis
he/his
commented

Thank you for your analysis and patch, grndlvl.

Any workbench_moderation.module users out here? Please review!

grndlvl’s picture

grndlvl commented
Status: Needs review » Needs work
StatusFileSize
new 2027767-2-subscription-add_workbench_moderation_support.patch1.91 KB

OK, the above patch didn't actually work in all cases.

After spending more time on this I came up with another solution. I am not positive it is the best
solution, but I cannot spend anymore time on this at the moment. I was on the wrong path
and this seemed to work so I am moving on at the moment.

salvis’s picture

salvis
he/his
commented
Status: Needs work » Needs review

Thank you for the updated patch and stating your situation.

Let's see what the testbot says, and hopefully some workbench_moderation.module users will pick up the ball.

shaneonabike’s picture

shaneonabike commented

@salvis: nice work

I tested this out and it works great! I also like the fact that when it's set to Published it then adds it appropriately to any queues which was a concern I had. So far testing this out the patch seems good.

Maybe a few others can verify that it's good though...

salvis’s picture

salvis
he/his
commented

This looks straight-forward enough. I only have two minor things:

  1. +++ b/subscriptions_content.module
@@ -248,13 +248,47 @@ function subscriptions_content_node_load(array $nodes, array $types) {
+ *    Returns TRUE if the node is unpublished by workbench_moderation standards

    Missing trailing dot. 

  2. +++ b/subscriptions_content.module
@@ -248,13 +248,47 @@ function subscriptions_content_node_load(array $nodes, array $types) {
+  $states = db_query("SELECT state FROM {workbench_moderation_node_history} WHERE nid = :nid",array(':nid' => $node->nid))->fetchCol();

    Please rewrite this using DBTNG db_select(). (Also, missing space.)

    I know that db_query() usually works just fine and the docs say it's fine, but we should try to avoid it where we can. The reason is that it gives headaches to people trying to run Drupal on weird database systems like Oracle, which feature odd reserved names and other quirks. "state" might well be a reserved word for Oracle...

    With DBTNG queries the Oracle driver can navigate all these quirks, but it's a pain with db_query().

After that, this is RTBC.

pattersonc’s picture

pattersonc commented
Issue summary: View changes
StatusFileSize
new 2027767-3-subscription-add_workbench_moderation_support.patch1.98 KB

Fixing minor issues mentioned by @salvis.

  • salvis committed 9ca60bd on 7.x-1.x 
    #2027767 by grndlvl, pattersonc, salvis: Added Workbench moderation...
salvis’s picture

salvis
he/his
commented
Status: Needs review » Fixed

Streamlined a little and committed to the -dev version.

Thanks to grndlvl, ShaneOnABike, and pattersonc!

Status: Fixed » Closed (fixed)

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