From 3ae281d03226446a8748ff075e4f082f61bc36a9 Mon Sep 17 00:00:00 2001 From: Angie Byron Date: Sun, 7 Mar 2010 07:22:42 +0000 Subject: #306540 by halstead, Dave Reid, jvandyk, eMPee584: Fixed Orphaned assigned actions still triggered and cannot be removed. --- modules/trigger/tests/trigger_test.module | 4 +- modules/trigger/trigger.admin.inc | 19 +++++++-- modules/trigger/trigger.module | 10 ++--- modules/trigger/trigger.test | 71 ++++++++++++++++++++++++++++--- 4 files changed, 87 insertions(+), 17 deletions(-) (limited to 'modules/trigger') 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.')); + } +} -- cgit v1.2.3