#306540 by halstead, Dave Reid, jvandyk, eMPee584: Fixed Orphaned assigned actions still triggered and cannot be removed.

4 files changed, 87 insertions, 17 deletions

diff --git a/modules/trigger/tests/trigger_test.module b/modules/trigger/tests/trigger_test.module
index ad7c4ec0e..5f0ab6878 100644
--- a/modules/trigger/tests/trigger_test.module
+++ b/modules/trigger/tests/trigger_test.module
@@ -63,12 +63,12 @@ function trigger_test_trigger_info() {
   return array(
     'node' => array(
       'node_triggertest' => array(
-        'runs when' => t('A test trigger is fired'),
+        'label' => t('A test trigger is fired'),
       ),
     ),
     'trigger_test' => array(
       'trigger_test_triggertest' => array(
-        'runs when' => t('Another test trigger is fired'),
+        'label' => t('Another test trigger is fired'),
       ),
     ),
   );
diff --git a/modules/trigger/trigger.admin.inc b/modules/trigger/trigger.admin.inc
index 7568bbb2b..8e462fe98 100644
--- a/modules/trigger/trigger.admin.inc
+++ b/modules/trigger/trigger.admin.inc
@@ -153,10 +153,21 @@ function trigger_assign_form($form, $form_state, $module, $hook, $label) {
   $form[$hook]['assigned']['#type'] = 'value';
   $form[$hook]['assigned']['#value'] = array();
   foreach ($actions as $aid => $info) {
-    $form[$hook]['assigned']['#value'][$aid] = array(
-      'label' => $info['label'],
-      'link' => l(t('unassign'), "admin/structure/trigger/unassign/$module/$hook/" . md5($aid)),
-    );
+    // If action is defined unassign it, otherwise offer to delete all orphaned
+    // actions.
+    if (actions_function_lookup(md5($aid))) {
+      $form[$hook]['assigned']['#value'][$aid] = array(
+        'label' => $info['label'],
+        'link' => l(t('unassign'), "admin/structure/trigger/unassign/$module/$hook/" . md5($aid)),
+      );
+    }
+    else {
+      // Link to system_actions_remove_orphans() to do the clean up.
+      $form[$hook]['assigned']['#value'][$aid] = array(
+        'label' => $info['label'],
+        'link' => l(t('Remove orphaned actions'), "admin/config/system/actions/orphan"),
+      );
+    }
   }
 
   $form[$hook]['parent'] = array(
diff --git a/modules/trigger/trigger.module b/modules/trigger/trigger.module
index 4d8190bba..2bdfec24d 100644
--- a/modules/trigger/trigger.module
+++ b/modules/trigger/trigger.module
@@ -609,13 +609,13 @@ function trigger_actions_delete($aid) {
  * Retrieves and caches information from hook_trigger_info() implementations.
  */
 function _trigger_get_all_info() {
-  static $triggers = NULL;
-  if( $triggers ) {
-    return $triggers;
+  $triggers = &drupal_static(__FUNCTION__);
+
+  if (!isset($triggers)) {
+    $triggers = module_invoke_all('trigger_info');
+    drupal_alter('trigger_info', $triggers);
   }
 
-  $triggers = module_invoke_all('trigger_info');
-  drupal_alter('trigger_info', $triggers);
   return $triggers;
 }
 
diff --git a/modules/trigger/trigger.test b/modules/trigger/trigger.test
index fce5d460e..2eeea20cd 100644
--- a/modules/trigger/trigger.test
+++ b/modules/trigger/trigger.test
@@ -8,7 +8,7 @@ class TriggerContentTestCase extends DrupalWebTestCase {
   public static function getInfo() {
     return array(
       'name' => 'Trigger content (node) actions',
-      'description' => 'Perform various tests with content actions.' ,
+      'description' => 'Perform various tests with content actions.',
       'group' => 'Trigger',
     );
   }
@@ -28,7 +28,8 @@ class TriggerContentTestCase extends DrupalWebTestCase {
       $hash = md5($action);
       $info = $this->actionInfo($action);
 
-      // Test 1: Assign an action to a trigger, then pull the trigger, and make sure the actions fire.
+      // Assign an action to a trigger, then pull the trigger, and make sure
+      // the actions fire.
       $test_user = $this->drupalCreateUser(array('administer actions'));
       $this->drupalLogin($test_user);
       $edit = array('aid' => $hash);
@@ -49,7 +50,8 @@ class TriggerContentTestCase extends DrupalWebTestCase {
       $this->assertTrue($loaded_node->$info['property'] == $info['expected'], t('Make sure the @action action fired.', array('@action' => $info['name'])));
       // Leave action assigned for next test
 
-      // Test 2: There should be an error when the action is assigned to the trigger twice.
+      // There should be an error when the action is assigned to the trigger
+      // twice.
       $test_user = $this->drupalCreateUser(array('administer actions'));
       $this->drupalLogin($test_user);
       $edit = array('aid' => $hash);
@@ -58,7 +60,7 @@ class TriggerContentTestCase extends DrupalWebTestCase {
       $this->drupalPost('admin/structure/trigger/node', $edit, t('Assign'));
       $this->assertRaw(t('The action you chose is already assigned to that trigger.'), t('Check to make sure an error occurs when assigning an action to a trigger twice.'));
 
-      // Test 3: The action should be able to be unassigned from a trigger.
+      // The action should be able to be unassigned from a trigger.
       $this->drupalPost('admin/structure/trigger/unassign/node/node_presave/' . $hash, array(), t('Unassign'));
       $this->assertRaw(t('Action %action has been unassigned.', array('%action' => ucfirst($info['name']))), t('Check to make sure the @action action can be unassigned from the trigger.', array('@action' => $info['name'])));
       $assigned = db_query("SELECT COUNT(*) FROM {trigger_assignments} WHERE aid IN (:keys)", array(':keys' => $content_actions))->fetchField();
@@ -118,7 +120,7 @@ class TriggerCronTestCase extends DrupalWebTestCase {
   public static function getInfo() {
     return array(
       'name' => 'Trigger cron (system) actions',
-      'description' => 'Perform various tests with cron trigger.' ,
+      'description' => 'Perform various tests with cron trigger.',
       'group' => 'Trigger',
     );
   }
@@ -190,7 +192,7 @@ class TriggerOtherTestCase extends DrupalWebTestCase {
   public static function getInfo() {
     return array(
       'name' => 'Trigger other actions',
-      'description' => 'Test triggering of user, comment, taxonomy actions.' ,
+      'description' => 'Test triggering of user, comment, taxonomy actions.',
       'group' => 'Trigger',
     );
   }
@@ -299,3 +301,60 @@ class TriggerOtherTestCase extends DrupalWebTestCase {
     $this->assertTrue(variable_get($action_id, FALSE), t('Check that creating a taxonomy term triggered the action.'));
   }
 }
+
+/**
+ * Test that orphaned actions are properly handled.
+ */
+class TriggerOrphanedActionsTestCase extends DrupalWebTestCase {
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Trigger orphaned actions',
+      'description' => 'Test triggering an action that has since been removed.',
+      'group' => 'Trigger',
+    );
+  }
+
+  function setUp() {
+    parent::setUp('trigger', 'trigger_test');
+  }
+
+  /**
+   * Test logic around orphaned actions.
+   */
+  function testActionsOrphaned() {
+    $action = 'trigger_test_generic_any_action';
+    $hash = md5($action);
+
+    // Assign an action from a disable-able module to a trigger, then pull the
+    // trigger, and make sure the actions fire.
+    $test_user = $this->drupalCreateUser(array('administer actions'));
+    $this->drupalLogin($test_user);
+    $edit = array('aid' => $hash);
+    $this->drupalPost('admin/structure/trigger/node', $edit, t('Assign'));
+
+    // Create an unpublished node.
+    $web_user = $this->drupalCreateUser(array('create page content', 'edit own page content', 'access content', 'administer nodes'));
+    $this->drupalLogin($web_user);
+    $edit = array();
+    $langcode = LANGUAGE_NONE;
+    $edit["title"] = '!SimpleTest test node! ' . $this->randomName(10);
+    $edit["body[$langcode][0][value]"] = '!SimpleTest test body! ' . $this->randomName(32) . ' ' . $this->randomName(32);
+    $this->drupalPost('node/add/page', $edit, t('Save'));
+    $this->assertRaw(t('!post %title has been created.', array('!post' => 'Basic page', '%title' => $edit["title"])), t('Make sure the Basic page has actually been created'));
+
+    // Action should have been fired.
+    $this->assertTrue(variable_get('trigger_test_generic_any_action', FALSE), t('Trigger test action successfully fired.'));
+
+    // Disable the module that provides the action and make sure the trigger
+    // doesn't white screen.
+    module_disable(array('trigger_test'));
+    $loaded_node = $this->drupalGetNodeByTitle($edit["title"]);
+    $edit["body[$langcode][0][value]"] = '!SimpleTest test body! ' . $this->randomName(32) . ' ' . $this->randomName(32);
+    $this->drupalPost("node/$loaded_node->nid/edit", $edit, t('Save'));
+
+    // If the node body was updated successfully we have dealt with the
+    // unavailable action.
+    $this->assertRaw(t('!post %title has been updated.', array('!post' => 'Basic page', '%title' => $edit["title"])), t('Make sure the Basic page can be updated with the missing trigger function.'));
+  }
+}