Integration with Rules module

Any new ideas on this issue? I can't help with this myself, because I do not know and use the rules module.

I had this working in D5 with Workflow-ng, Rules, Scheduler, and Node Import and have not been able to get it going in D6 (with Rules, Scheduler, Node Import). (I am just migrating to D6 after hoping that most kinks would be resolved by now :-)

When I try to assign the date values (Publish/Unpublish) to the CCK field, it keeps throwing a:

"The 'publish on' value does not match the expected format of 2010-07-20 22:46:09"
"The 'unpublish on' value does not match the expected format of 2010-07-20 22:46:09"

You can see the issue and code snippet here: http://groups.drupal.org/node/81904

Does scheduler integrate with Rules? It's not immediately clear from the project ...

At this point AFAIK, there is no general-purpose mechanism for scheduling 'actions' at a particular time. Allowing Scheduler to 'trigger' Rules would make it much more general purpose modules, without much code or loss of simplicity.

I cannot answer this question because I do not really know what rules is doing and I have never used it. Could you explain what it would mean to "integrate with rules".

From http://www.drupal.org/project/rules (features section):

Flexible scheduling system that allows scheduling any component / action.

This does not sound like

there is no general-purpose mechanism for scheduling 'actions' at a particular time

This would be really, useful I think.
There's a Rules API and some documentation for integrating other modules with it: http://drupal.org/node/298486
From what I've seen in other modules, what they apparently do is expose some of the modules functionality in some way to Rules in the form of triggers, conditions or actions.

Rules integration could be achieved very easily. It would simply be an event that is triggered when something is added to the scheduler table.

There might be a fringe case for wanting to know if something is removed from the table but I can't think of it now. The removal event might be redundant because you can watch for the publish and unpublish events that are already built-in.

Adds rules integration to the scheduler module.

Looks good so far.
What happens if rules is not installed/activated?

Did this ever go anywhere — is it still a viable option? If so, am happy to do some testing.

Hi Ken,
The version at the top of the issue says 1.6 so I'm not sure what the patch in #11 was created against, but probably it was -dev as 15th May 2011. I've not used rules but it has been asked for several times so maybe it should be progressed. Also I suppose the D7 version should really be implemented first, then ported to 6.

Have you tried implementing the patch and giving it a go?

Jonathan

The patch didn't work for me.

I've created a new version, updated for 7.x, and defined four rules which can be used to trigger events by Rules.

1. 'Node has been scheduled for publishing' will be triggered when a node is created or updated and contains an 'publish on' date
2. 'Node has been scheduled for unpublishing' will be triggered when a node is created or updated and contains an 'unpublish on' date
3. 'Scheduler has published a node' will be triggered when a node is published by scheduler during a cron run
4. 'Scheduler has unpublished a node' will be triggered when a node is unpublished by scheduler during a cron run

The $node and the two dates are passed as parameters to be available in Rules when creating the actions to perform.

The implementation of hook_rules_event_info is defined in a new file scheduler.rules.inc which is automatically included when needed. This is the preferred way as suggested in the Rules documentation. The other functions changed are scheduler_node_insert(), scheduler_node_update(), _scheduler_publish() and _scheduler_unpublish(). The patch is against the dev 1.1+7 of 25th May

There is more integration that might be needed for processing the other way round, ie exposing some of scheduler's functionality so that it can be executed by rules actions. When I've learnt more about Rules and Actions I will have a better idea on how to do that.

Jonathan

For those users (like me) who are new to Rules https://drupal.org/project/rules, here is what the above patch provides. When defining a rule, there are four new events which can be reacted on.

You have the two dates to use as replacement patterns in e-mails

(I'm sure there is a way to set the label and description, I will find out how to do that)

and the two dates can be used in conditions to further filter on whether to react to the event

Here is an improved version, with more features. In addition to the four events listed above I have created two default 'reaction rules', to send an email to the owner of the content when it is published and unpublished by scheduler. The rules can be modified by admins, and tailored to your requirements. This is part of the 'rules reacting to scheduler' side of integration.

I have also worked on the other side, opening up schedulers functionality to be incorporated in custom rules. For this, I have created two components 'set publishing date for Scheduler module' and 'set unpublishing date for Scheduler module'.

This allows site admins to create their own reaction rules and have scheduler set the dates for publishing/unpublishing as actions which can be triggered. The $node and $date are passed as parameters, and the date can be set using any other field in the $node object.

There are now two new files - scheduler.rules.inc and scheduler.rules_defaults.inc, both of which are automatically included as and when Rules needs them.

Jonathan

Further things to consider for Rules Integration:

1. The nodes author object should be available via node:author, so I could remove the unnecessary $user=user_load() and the $account parameter from all the actions and the calling functions
2. Do we need new conditions that return 'node is scheduled for publishing' and 'node is scheduled for unpublishing'?
3. In Views Bulk Operations #1026072: Ability to schedule nodes in bulk using Views Bulk Operations the standard 'Publish Content' operation does not work if the node is currently scheduled, becuase the presence of the publish-on date (provided it is in the future) causes the status to be set back to 0 during scheduler_node_presave(). We could solve this
   • by modifying the standard component
   • or by somehow detecting the context in scheduler_node_presave()
   • or by creating our own 'publish now and remove scheduled date' component.
4. Also in VBO, is it possible to show the real number of nodes processed, not the number initially ticked, as per #2045068: The number of actions actually executed is not displayed

Here is an updated patch, against the new dev 1.1+10 dated 28th July.

Regarding (1) above I have removed the unnecessary calls to user_load() and passing $user to the rules because node:author is available by default. I have not addressed 2-4 yet, but these are relatively minor points.

Here is a re-rolled patch, against the new dev 1.1+15 of 25th Sept.

I've also removed a couple of changes from here which are repeated in #1566024: Coding Standards and Coder Review. This avoids a clash and allows both .module patches to be applied.

This is some great work and would be a fantastic addition to Scheduler. I'm sure it would come in very handy for a lot of people.

I do not have much experience with the Rules module myself, but will start by reviewing the code and write down any remarks I might have. I will test it later on.

+++ b/scheduler.moduleundefined
@@ -800,6 +821,11 @@ function _scheduler_publish() {
+    // Invoke the rule event to indicate that a node has been published by scheduler.

Comment exceeds 80 characters.

+++ b/scheduler.moduleundefined
@@ -800,6 +821,11 @@ function _scheduler_publish() {
+    if (module_exists('rules')) {
+      rules_invoke_event('scheduler_node_has_been_published_event', $n, $publish_on, $n->unpublish_on);
+    }

I'm not familiar with how Rules operates, but this seems to notify a listener, and passing some parameters to it. Isn't it redundant to include both the node ($n) and a property that is part of the same node object ($n->unpublish_on)? Is this perhaps necessary to display this parameter in the user interface?

edit: this is explained where this event is defined.

+++ b/scheduler.moduleundefined
@@ -873,6 +900,11 @@ function _scheduler_unpublish() {
+      // Invoke the rule event to indicate that a node has been unpublished by scheduler.

Comment exceeds 80 characters.

+++ b/scheduler.moduleundefined
@@ -873,6 +900,11 @@ function _scheduler_unpublish() {
+      if (module_exists('rules')) {
+        rules_invoke_event('scheduler_node_has_been_unpublished_event', $n, $n->publish_on, $unpublish_on);
+      }

Same question as above.

edit: this is explained where this event is defined.

+++ b/scheduler.rules.incundefined
@@ -0,0 +1,187 @@
+  $variables = array(
+    'node' => array('type' => 'node',
+                    'label' => t('Scheduled Content Node'),
+                    'description' => t('The node object representing the scheduled content'),
+                    ),

This lining up of array keys is very pretty, but alas not conforming to coding standards :)
This is occurring several times in the patch.

+++ b/scheduler.rules.incundefined
@@ -0,0 +1,187 @@
+      'help' => t('Here is some help. Where is this displayed?'),

This is a very good question :)

+++ b/scheduler.rules.incundefined
@@ -0,0 +1,187 @@
+      'help' => 'scheduler_help', // where is this used?

Same as above. This needs to be figured out before we can commit the patch.

+++ b/scheduler.rules.incundefined
@@ -0,0 +1,187 @@
+  return user_access('Schedule content publication');

This should use the machine name of the permission 'schedule (un)publishing of nodes' instead of the human readable string.

+++ b/scheduler.rules.incundefined
@@ -0,0 +1,187 @@
+  $node->unpublish_on += 0; // Ensure an empty or null value is converted to 0

Display comments above the affected code instead of inline. It is also more common to do typecasting by using (int).

+++ b/scheduler.rules.incundefined
@@ -0,0 +1,187 @@
+ * @param $node
+ *  The node object to be scheduled for unpublishing.

@param documentation should be indented with 2 spaces. This is occurring throughout the code.

+++ b/scheduler.rules_defaults.incundefined
@@ -0,0 +1,65 @@
+                           // Needs double quotes for the newlines to be evaluated. Not sure how this affects translatability.
+                           'message' => t("Dear [node:author:name],\r\nYour [node:type] '[node:title]' has been published on [site:name].\r\nThe link is [node:url]\r\n"),

I looked for some examples, and it is implemented in the same way in commerce_checkout_default_rules_configuration() so it seems fine. Wouldn't hurt to try translating it though.

Hi Pieter, thanks for the review. Here's what I've done:

1. Comments corrected to wrap at 80
2. Arrays changed to inline. If they are short they are contained in the one line. If long, the key is indented by 2 on the next line.
3. In hook_rules_event_info() and hook_rules_action_info() the 'help' callbacks are actually optional. These do not seem to be used or displayed anywhere. I have checked the Rules own examples and their functions defined do not exist. I have removed them from the code as we do not need them.
4. Similarly the access callbacks are optional and we do not need them. If the user can get to admin/config/workflow/rules then there is no reason why we should further restrict them from using our rules.
   See http://drupalcontrib.org/api/drupal/contributions%21rules%21rules.api.ph... for more on both of these optional items
5. Ensuring a null or empty value is converted to 0. I was using +=0 and you suggested casting via (int).
   However the php manual http://www.php.net/manual/en/language.types.integer.php#language.types.i... does not recommend using (int) for any input other than strings or booleans. So I have changed it to explicitly check is_numeric() and set to 0 if not.
6. @param indentation fixed.

I have not added the two new files to the files[] array in .info as it seems Rules automatically detects and uses the files. Do you think that is ok?

Here's a new patch against 1.1+20 and an interdiff file for you to see the changes.

Jonathan

It's looking very good now. Using +0 to cast is fine for me, it works :) If the rules integration works without announcing the files in the info file then that is good too.

Similarly the access callbacks are optional and we do not need them. If the user can get to admin/config/workflow/rules then there is no reason why we should further restrict them from using our rules.

Are you sure about this? It seems like a privilege separation violation. It is not necessarily always the case that people that have rights to create rules automatically have the right to schedule nodes for publication. For example if an organisation has a "webmaster" role that can do regular maintenance tasks including making rules, but the publication of nodes is strictly reserved to the "chief editor" role.

I suppose we don't need tests for this? At least I don't feel comfortable writing them as I have very little experience with Rules ;)

Thanks for the review. Yes I am pleased with this work.

It is not necessarily always the case that people that have rights to create rules automatically have the right to schedule nodes for publication.

I agree with you. However, my understanding of the access callbacks in hook_rules_event_info() and hook_rules_action_info() are to allow contrib modules to make even tighter restrictions on who can create and configure rules using the components and default rules that the contrib module provides. I was saying that if a user has enough admin permission to get to the admin/config/workflow/rules page which configures rules, then we do not additionally need to restrict them. Specifically we are not checking if the 'webmaster' has the authority to schedule nodes, because as you say, those roles may be separate.

If you want to test this with Rules be aware that a bug was introduced in Rules 2.5 #1568134: Enabling or disabling a contrib rule makes it disappear which makes using the contrib rules rather difficult! The dev version 2.5+2 dated 7th October was when this bug was fixed.

Jonathan

Needed re-roll after 1.1+30, to make it easier if anyone else comes along to patch and test this enhancement.

@Pieter, to answer your question about tests, no I do not think we could write any meaningful tests for Rules Integration. I think good old fashioned manual testing is all we can do with this.

Added link to Rules module http://www.drupal.org/project/rules

The changes in patch 29 are:

1. Two more conditions added, for use in building rules: 'node is scheduled for publishing' and 'node is scheduled for unpublishing' - this answers point 2 in #19
2. Two more components added 'Remove scheduled publishing date' and 'Remove scheduled unpublishing date' for use in rules and Views Bulks Operations
3. Added 'Scheduler' as the tag for all our rules and components
4. Improved the naming of the conditions to be more precise, as these are displayed in the Rules debug log
5. Improved the labels and descriptions to assist when building Rules
6. Fixed a couple of typos in the comments

There are no changes to the .module code - all the above are in the two new files.

In scheduler.rules.inc, fix typos where the item keys did not match the changes in scheduler.rules_default.inc. I also improved the text labels.

In scheduler.rules_default.inc, in "1. Component to set the publishing date on a node", I have now removed the action to unpublish the node as this caused an unwanted node_save (along with all the associated hooks). The unpublishing can be achieved by using the core component instead.

No changes to scheduler.module

Patch against 1.1+39

[drupal failed to add the new comment first time, although I did get the e-mail notification, and patch 31 did get attached. Trying again. The patch does not seem to be showing here. See the issue summary to get the patch.]

The patch file, although attached, is not queued for testing. Something obviously did go wrong with the node save. Cannot find how to request it to be sent for testing, so uploading a new file.

Rerolled against latest HEAD.

Going to test the functionality manually. It's quite a lot to go through, so I am going to have to keep track of what I've already done. I will edit this post as I make progress.

Events

• ☑ On saving a node that is scheduled for publishing ^*1
• ☑ On saving a node that is scheduled for unpublishing ^*1
• ☑ After a node has been published by Scheduler
• ☑ After a node has been unpublished by Scheduler

Actions

• ☒ Set publishing date ^*2
• ☒ Set unpublishing date ^*2
• ☑ Remove publishing date
• ☑ Remove unpublishing date

Conditions

• ☑ Scheduled publishing is enabled for this content type
• ☑ Scheduled unpublishing is enabled for this content type
• ☑ The node is scheduled for publishing
• ☑ The node is scheduled for unpublishing

Default rules

• ☑ Send e-mail when content is published by Scheduler
• ☑ Send e-mail when content is unpublished by Scheduler

Default components

• ☑ Set scheduled publishing date
• ☑ Set scheduled unpublishing date
• ☑ Remove scheduled publishing date
• ☑ Remove scheduled unpublishing date

Questions / remarks

1. The "On saving a node that is scheduled for publishing / unpublishing" event fires both when a new node is created and when an existing node is updated. Would it be useful to split these in two events, so we have one that fires when a new node is created, and another when updated?
   This would be similar to the node events "After saving new content" and "After updating existing content".
2. When the "Set publishing / unpublishing date" action is executed this saves the dates in the scheduler table, even for content types that do not have scheduling enabled. This should only be done for content types that support scheduling.

Thanks for doing this testing. I will address the problems, but not post a new patch until you have tested all the items listed.

Is there any way I can test the components through the Rules interface? I tried executing them but can't figure out how to choose a node to perform the action on. "node:nid:2" doesn't work :)

Yes, even though the components were built so that the the power of VBO could be utilised, they can be tested solely through rules without installing VBO. To act on the node being edited just select 'node' from the drop-down list, as in:

Or using your question, if you want to test a specific node id, switch to 'direct input mode' and enter the nid:

The components check that the requested node can be scheduled (or is scheduled, if using the 'remove' versions) so you do not need to add those conditions in the rules if using components rather than actions. But if your rule also shows a message giving details of the new date or whatever, then you may want the conditions added to the rules as well, so that the message is not shown when the action cannot actually be performed.

Jonathan

Thanks! I finished the manual testing, so setting this to Needs work as per #34. Very nice work overall, I'm very excited about this addition to Scheduler!

Yes, this is going to be a very powerful addition to Scheduler.

Here is a patch addressing both of the issues you found. Good testing, thanks. I have added a message when attempting to set a scheduled date on a node that is not enabled. Also the message explains what to do to remedy the problem.

I have not done the same message when removing a date, we just avoid running the code but do it quietly.

Jonathan

I was playing around with the Rules integration some more yesterday evening. Creating and exporting rules turns out to be very easy, so I created a test rule for each component and exported them into our test module. Started poking around a bit to see how much work it would be to write tests. I'm not sure yet if I will finish this but am attaching my work in progress anyway, we can maybe use it for a followup issue.

@Jonathan, thanks for the update, will review the patch immediately, I have some free time now.

1. +++ b/scheduler.rules.inc
   @@ -114,13 +124,18 @@ function scheduler_rules_action_info() {
   +    drupal_set_message(t('Scheduled publishing is not enabeld for %type content. To prevent this message add the condition "Scheduled publishing is enabled" to your rule.', array('%type' => node_type_get_name($node->type))), 'warning');
   

   +++ b/scheduler.rules.inc
   @@ -132,9 +147,14 @@ function scheduler_set_publish_date_action($node, $date) {
   +    drupal_set_message(t('Scheduled unpublishing is not enabeld for %type content. To prevent this message add the condition "Scheduled unpublishing is enabled" to your rule.', array('%type' => node_type_get_name($node->type))), 'warning');
   

   Typo: "enabeld".

Don't bother rolling a new patch for this, I will fix it before committing.

Code looks good. I'm going to do a quick functional test now.

Tested manually and everything works as expected.

However, I think that it would maybe be better to use the watchdog to log events in which scheduling is applied to nodes that do not support it. The messages might be shown to regular visitors if an action they took created or updated a node in the background. They will not understand it, and might think something went wrong.

Yes, ok I was in two minds whether to show the message. A watchdog row is better, so I will make that change and re-patch.

The tests look interesting, but I think we should get the main code in and committed without extra delays. Then as you say, the tests can be added in a separate issue.

Jonathan

Changed the drupal_set_message to watchdog call, added for all four of the events. Now that we have more space for the text I expanded the message and provided a link to the appropriate content type settings page. Also fixed the typo.

Did not include any changes to .test as I think that should be in a separate issue. Patch against 1.1+49.

Great! It's all good now. Committed to 7.x-1.x: commit e2dcf53e5.

Very nice to finally have this in, more than 3 years after it was originally requested, excellent work!!

Fabulous.

more than 3 years after it was originally requested

Yes, but only five months since I started working on it, and only two months since the first of your excellent reviews, so thank you to us! ;-)

It appears not to be included in the latest dev?

Hi Yuri,

The latest dev release from the project main page is stuck at 19th December due to Drupal.org release problems
#2132327: Can't create release: git repository tag appears in repo browser, but not vcs tables
#2125405: Not all commits showing up

If you want the actual latest dev you can get it from the git repository http://drupalcode.org/project/scheduler.git

Hope that helps. I might ask Pieter to put a notice to this effect on the front page.

Jonathan