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

2 files changed, 47 insertions, 32 deletions

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);
     }
   }