#585868 by sun: Provide a generic way for actions to denote that they change a property.

5 files changed, 114 insertions, 96 deletions

diff --git a/modules/comment/comment.module b/modules/comment/comment.module
index 07fd233e3..a8c014bf5 100644
--- a/modules/comment/comment.module
+++ b/modules/comment/comment.module
@@ -2336,18 +2336,27 @@ function comment_action_info() {
       'label' => t('Publish comment'),
       'type' => 'comment',
       'configurable' => FALSE,
-      'triggers' => array('comment_insert', 'comment_update'),
+      'behavior' => array('changes_property'),
+      'triggers' => array('comment_presave', 'comment_insert', 'comment_update'),
     ),
     'comment_unpublish_action' => array(
       'label' => t('Unpublish comment'),
       'type' => 'comment',
       'configurable' => FALSE,
-      'triggers' => array('comment_insert', 'comment_update'),
+      'behavior' => array('changes_property'),
+      'triggers' => array('comment_presave', 'comment_insert', 'comment_update'),
     ),
     'comment_unpublish_by_keyword_action' => array(
       'label' => t('Unpublish comment containing keyword(s)'),
       'type' => 'comment',
       'configurable' => TRUE,
+      'behavior' => array('changes_property'),
+      'triggers' => array('comment_presave', 'comment_insert', 'comment_update'),
+    ),
+    'comment_save_action' => array(
+      'label' => t('Save comment'),
+      'type' => 'comment',
+      'configurable' => FALSE,
       'triggers' => array('comment_insert', 'comment_update'),
     ),
   );
@@ -2364,18 +2373,18 @@ function comment_action_info() {
  * @ingroup actions
  */
 function comment_publish_action($comment, $context = array()) {
-  if (isset($comment->cid)) {
-    $cid = $comment->cid;
+  if (isset($comment->comment)) {
     $subject = $comment->subject;
+    $comment->status = COMMENT_PUBLISHED;
   }
   else {
     $cid = $context['cid'];
     $subject = db_query('SELECT subject FROM {comment} WHERE cid = :cid', array(':cid', $cid))->fetchField();
+    db_update('comment')
+      ->fields(array('status' => COMMENT_PUBLISHED))
+      ->condition('cid', $cid)
+      ->execute();
   }
-  db_update('comment')
-    ->fields(array('status' => COMMENT_PUBLISHED))
-    ->condition('cid', $cid)
-    ->execute();
   watchdog('action', 'Published comment %subject.', array('%subject' => $subject));
 }
 
@@ -2390,18 +2399,18 @@ function comment_publish_action($comment, $context = array()) {
  * @ingroup actions
  */
 function comment_unpublish_action($comment, $context = array()) {
-  if (isset($comment->cid)) {
-    $cid = $comment->cid;
+  if (isset($comment->comment)) {
     $subject = $comment->subject;
+    $comment->status = COMMENT_NOT_PUBLISHED;
   }
   else {
     $cid = $context['cid'];
     $subject = db_query('SELECT subject FROM {comment} WHERE cid = :cid', array(':cid', $cid))->fetchField();
+    db_update('comment')
+      ->fields(array('status' => COMMENT_NOT_PUBLISHED))
+      ->condition('cid', $cid)
+      ->execute();
   }
-  db_update('comment')
-    ->fields(array('status' => COMMENT_NOT_PUBLISHED))
-    ->condition('cid', $cid)
-    ->execute();
   watchdog('action', 'Unpublished comment %subject.', array('%subject' => $subject));
 }
 
@@ -2411,9 +2420,7 @@ function comment_unpublish_action($comment, $context = array()) {
  * @param $comment
  *   A comment object.
  * @param $context
- *   An array providing more information about the context of the call to this action.
- *   Unused here, since this action currently only supports the insert and update ops of
- *   the comment hook, both of which provide a complete $comment object.
+ *   Keyed array. Must contain the id of the comment if $comment is not passed.
  *
  * @ingroup actions
  * @see comment_unpublish_by_keyword_action_form()
@@ -2422,10 +2429,7 @@ function comment_unpublish_action($comment, $context = array()) {
 function comment_unpublish_by_keyword_action($comment, $context) {
   foreach ($context['keywords'] as $keyword) {
     if (strpos($comment->comment, $keyword) !== FALSE || strpos($comment->subject, $keyword) !== FALSE) {
-      db_update('comment')
-        ->fields(array('status' => COMMENT_NOT_PUBLISHED))
-        ->condition('cid', $comment->cid)
-        ->execute();
+      $comment->status = COMMENT_NOT_PUBLISHED;
       watchdog('action', 'Unpublished comment %subject.', array('%subject' => $comment->subject));
       break;
     }
@@ -2460,6 +2464,14 @@ function comment_unpublish_by_keyword_action_submit($form, $form_state) {
 }
 
 /**
+ * Action to save a comment.
+ */
+function comment_save_action($comment) {
+  comment_save($comment);
+  watchdog('action', 'Saved comment %title', array('%title' => $comment->subject));
+}
+
+/**
  * Implement hook_ranking().
  */
 function comment_ranking() {
diff --git a/modules/node/node.module b/modules/node/node.module
index f7cafdbd5..ba326824a 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -2947,76 +2947,56 @@ function node_action_info() {
       'type' => 'node',
       'label' => t('Publish content'),
       'configurable' => FALSE,
-      'behavior' => array('changes_node_property'),
-      'triggers' => array('node_presave', 'comment_insert', 'comment_update'),
+      'behavior' => array('changes_property'),
+      'triggers' => array('node_presave', 'comment_insert', 'comment_update', 'comment_delete'),
     ),
     'node_unpublish_action' => array(
       'type' => 'node',
       'label' => t('Unpublish content'),
       'configurable' => FALSE,
-      'behavior' => array('changes_node_property'),
-      'triggers' => array(
-        'node_presave',
-        'comment_insert',
-        'comment_update',
-        'comment_delete'
-      ),
+      'behavior' => array('changes_property'),
+      'triggers' => array('node_presave', 'comment_insert', 'comment_update', 'comment_delete'),
     ),
     'node_make_sticky_action' => array(
       'type' => 'node',
       'label' => t('Make content sticky'),
       'configurable' => FALSE,
-      'behavior' => array('changes_node_property'),
-      'triggers' => array('node_presave', 'comment_insert', 'comment_update'),
+      'behavior' => array('changes_property'),
+      'triggers' => array('node_presave', 'comment_insert', 'comment_update', 'comment_delete'),
     ),
     'node_make_unsticky_action' => array(
       'type' => 'node',
       'label' => t('Make content unsticky'),
       'configurable' => FALSE,
-      'behavior' => array('changes_node_property'),
-      'triggers' => array(
-        'node_presave',
-        'comment_insert',
-        'comment_update',
-        'comment_delete'
-      ),
+      'behavior' => array('changes_property'),
+      'triggers' => array('node_presave', 'comment_insert', 'comment_update', 'comment_delete'),
     ),
     'node_promote_action' => array(
       'type' => 'node',
       'label' => t('Promote content to front page'),
       'configurable' => FALSE,
-      'behavior' => array('changes_node_property'),
-      'triggers' => array('node_presave', 'comment_insert', 'comment_update'),
+      'behavior' => array('changes_property'),
+      'triggers' => array('node_presave', 'comment_insert', 'comment_update', 'comment_delete'),
     ),
     'node_unpromote_action' => array(
       'type' => 'node',
       'label' => t('Remove content from front page'),
       'configurable' => FALSE,
-      'behavior' => array('changes_node_property'),
-      'triggers' => array(
-        'node_presave',
-        'comment_insert',
-        'comment_update',
-        'comment_delete'
-      ),
+      'behavior' => array('changes_property'),
+      'triggers' => array('node_presave', 'comment_insert', 'comment_update', 'comment_delete'),
     ),
     'node_assign_owner_action' => array(
       'type' => 'node',
       'label' => t('Change the author of content'),
       'configurable' => TRUE,
-      'behavior' => array('changes_node_property'),
-      'triggers' => array(
-        'node_presave',
-        'comment_insert',
-        'comment_update',
-        'comment_delete',
-      ),
+      'behavior' => array('changes_property'),
+      'triggers' => array('node_presave', 'comment_insert', 'comment_update', 'comment_delete'),
     ),
     'node_save_action' => array(
       'type' => 'node',
       'label' => t('Save content'),
       'configurable' => FALSE,
-      'triggers' => array('comment_delete', 'comment_insert', 'comment_update'),
+      'triggers' => array('comment_insert', 'comment_update', 'comment_delete'),
     ),
     'node_unpublish_by_keyword_action' => array(
       'type' => 'node',
diff --git a/modules/system/system.api.php b/modules/system/system.api.php
index 020e8a846..05ed97d28 100644
--- a/modules/system/system.api.php
+++ b/modules/system/system.api.php
@@ -2405,13 +2405,16 @@ function hook_file_mimetype_mapping_alter(&$mapping) {
  *   - 'triggers': An array of the events (that is, hooks) that can trigger this
  *     action. For example: array('node_insert', 'user_update'). You can also
  *     declare support for any trigger by returning array('any') for this value.
- *   - 'behavior': (optional) machine-readable array of behaviors of this
- *     action, used to signal additional actions that may need to be triggered.
- *     Currently recognized behaviors by Trigger module:
- *     - 'changes_node_property': If an action with this behavior is assigned to
- *       a trigger other than 'node_presave', any node save actions also
- *       assigned to this trigger are moved later in the list. If a node save
- *       action is not present, one will be added.
+ *   - 'behavior': (optional) A machine-readable array of behaviors of this
+ *     action, used to signal additionally required actions that may need to be
+ *     triggered. Currently recognized behaviors by Trigger module:
+ *     - 'changes_property': If an action with this behavior is assigned to a
+ *       trigger other than a "presave" hook, any save actions also assigned to
+ *       this trigger are moved later in the list. If no save action is present,
+ *       one will be added.
+ *       Modules that are processing actions (like Trigger module) should take
+ *       special care for the "presave" hook, in which case a dependent "save"
+ *       action should NOT be invoked.
  */
 function hook_action_info() {
   return array(
@@ -2419,12 +2422,20 @@ function hook_action_info() {
       'type' => 'comment',
       'label' => t('Unpublish comment'),
       'configurable' => FALSE,
-      'triggers' => array('comment_insert', 'comment_update'),
+      'behavior' => array('changes_property'),
+      'triggers' => array('comment_presave', 'comment_insert', 'comment_update'),
     ),
     'comment_unpublish_by_keyword_action' => array(
       'type' => 'comment',
       'label' => t('Unpublish comment containing keyword(s)'),
       'configurable' => TRUE,
+      'behavior' => array('changes_property'),
+      'triggers' => array('comment_presave', 'comment_insert', 'comment_update'),
+    ),
+    'comment_save_action' => array(
+      'type' => 'comment',
+      'label' => t('Save comment'),
+      'configurable' => FALSE,
       'triggers' => array('comment_insert', 'comment_update'),
     ),
   );
diff --git a/modules/trigger/trigger.admin.inc b/modules/trigger/trigger.admin.inc
index 15b9fd39f..f716c84a4 100644
--- a/modules/trigger/trigger.admin.inc
+++ b/modules/trigger/trigger.admin.inc
@@ -83,17 +83,16 @@ function trigger_unassign($form, $form_state, $module, $hook = NULL, $aid = NULL
  * Submit callback for trigger_unassign() form.
  */
 function trigger_unassign_submit($form, &$form_state) {
-  $form_values = $form_state['values'];
-  if ($form_values['confirm'] == 1) {
-    $aid = actions_function_lookup($form_values['aid']);
+  if ($form_state['values']['confirm'] == 1) {
+    $aid = actions_function_lookup($form_state['values']['aid']);
     db_delete('trigger_assignments')
-      ->condition('hook', $form_values['hook'])
+      ->condition('hook', $form_state['values']['hook'])
       ->condition('aid', $aid)
       ->execute();
     $actions = actions_get_all_actions();
     watchdog('actions', 'Action %action has been unassigned.',  array('%action' => check_plain($actions[$aid]['label'])));
     drupal_set_message(t('Action %action has been unassigned.', array('%action' => $actions[$aid]['label'])));
-    $form_state['redirect'] = 'admin/structure/trigger/' . $form_values['module'];
+    $form_state['redirect'] = 'admin/structure/trigger/' . $form_state['values']['module'];
   }
   else {
     drupal_goto('admin/structure/trigger');
@@ -206,49 +205,56 @@ function trigger_assign_form_validate($form, $form_state) {
 /**
  * Submit function for trigger_assign_form().
  */
-function trigger_assign_form_submit($form, $form_state) {
-  $form_values = $form_state['values'];
-
-  if (!empty($form_values['aid'])) {
-    $aid = actions_function_lookup($form_values['aid']);
-    $weight = db_query("SELECT MAX(weight) FROM {trigger_assignments} WHERE hook = :hook", array(
-      ':hook' => $form_values['hook'],
-    ))->fetchField();
+function trigger_assign_form_submit($form, &$form_state) {
+  if (!empty($form_state['values']['aid'])) {
+    $aid = actions_function_lookup($form_state['values']['aid']);
+    $weight = db_query("SELECT MAX(weight) FROM {trigger_assignments} WHERE hook = :hook", array(':hook' => $form_state['values']['hook']))->fetchField();
 
+    // Insert the new action.
     db_insert('trigger_assignments')
       ->fields(array(
-        'hook' => $form_values['hook'],
+        'hook' => $form_state['values']['hook'],
         'aid' => $aid,
         'weight' => $weight + 1,
       ))
       ->execute();
-    // If this action changes a node property, we need to save the node
-    // so the change will persist.
 
+    // If we are not configuring an action for a "presave" hook and this action
+    // changes an object property, then we need to save the object, so the
+    // property change will persist.
     $actions = actions_list();
-    if (isset($actions[$aid]['behavior']) && in_array('changes_node_property', $actions[$aid]['behavior']) && ($form_values['hook'] != 'node_presave') && ($form_values['hook'] != 'comment_presave')) {
-      // Delete previous node_save_action if it exists, and re-add a new one at
-      // a higher weight.
-      $save_post_action_assigned = db_query("SELECT aid FROM {trigger_assignments} WHERE hook = :hook AND aid = :aid", array(
-        ':hook' => $form_values['hook'],
-        ':aid' => 'node_save_action',
-      ))->fetchField();
+    if (strpos($form_state['values']['hook'], 'presave') === FALSE && isset($actions[$aid]['behavior']) && in_array('changes_property', $actions[$aid]['behavior'])) {
+      // Determine the corresponding save action name for this action.
+      $save_action = strtok($aid, '_') . '_save_action';
+      // If no corresponding save action exists, we need to bail out.
+      if (!isset($actions[$save_action])) {
+        throw new Exception(t('Missing/undefined save action (%save_aid) for %aid action.', array('%save_aid' => $aid, '%aid' => $aid)));
+      }
+      // Delete previous save action if it exists, and re-add it using a higher
+      // weight.
+      $save_action_assigned = db_query("SELECT aid FROM {trigger_assignments} WHERE hook = :hook AND aid = :aid", array(':hook' => $form_state['values']['hook'], ':aid' => $save_action))->fetchField();
 
-      if ($save_post_action_assigned) {
+      if ($save_action_assigned) {
         db_delete('trigger_assignments')
-          ->condition('hook', $form_values['hook'])
-          ->condition('aid', 'node_save_action')
+          ->condition('hook', $form_state['values']['hook'])
+          ->condition('aid', $save_action)
           ->execute();
       }
       db_insert('trigger_assignments')
         ->fields(array(
-          'hook' => $form_values['hook'],
-          'aid' => 'node_save_action',
+          'hook' => $form_state['values']['hook'],
+          'aid' => $save_action,
           'weight' => $weight + 2,
         ))
         ->execute();
-      if (!$save_post_action_assigned) {
-        drupal_set_message(t("You have added an action that changes a the property of some content. Your 'Save content' action has been moved later in the list so that the property change will be saved."));
+
+      // If no save action existed before, inform the user about it.
+      if (!$save_action_assigned) {
+        drupal_set_message(t('The %label action has been appended, which is required to save the property change.', array('%label' => $actions[$save_action]['label'])));
+      }
+      // Otherwise, just inform about the new weight.
+      else {
+        drupal_set_message(t('The %label action was moved to save the property change.', array('%label' => $actions[$save_action]['label'])));
       }
     }
   }
diff --git a/modules/trigger/trigger.module b/modules/trigger/trigger.module
index 3c15e5a4b..97c07cd38 100644
--- a/modules/trigger/trigger.module
+++ b/modules/trigger/trigger.module
@@ -106,7 +106,10 @@ function trigger_trigger_info() {
        ),
      ),
      'comment' => array(
-         'comment_insert' => array(
+       'comment_presave' => array(
+         'label' => t('When either saving a new comment or updating an existing comment'),
+       ),
+       'comment_insert' => array(
          'label' => t('After saving a new comment'),
        ),
        'comment_update' => array(
@@ -349,6 +352,13 @@ function _trigger_normalize_comment_context($type, $comment) {
 }
 
 /**
+ * Implement hook_comment_presave().
+ */
+function trigger_comment_presave($comment) {
+  _trigger_comment($comment, 'comment_presave');
+}
+
+/**
  * Implement hook_comment_insert().
  */
 function trigger_comment_insert($comment) {
@@ -407,7 +417,6 @@ function _trigger_comment($a1, $hook) {
       actions_do($aid, $objects[$type], $context);
     }
     else {
-      $a1 = (object) $a1;
       actions_do($aid, $a1, $context);
     }
   }