Orphaned assigned actions still triggered and cannot be removed

I just tried to verify this. I created an action 'redirect to url' and assigned it to a trigger. I then went and deleted the action under admin/settings/actions and the action was no longer under the admin/build/triggers screen. Looks like it was removed correctly. But, something interesting to know is that I still had a record in actions_aid for it. What exactly is the actions_aid table used for?

In the meantime, going to mark this is as need more info since I couldn't duplicate this.

Can you list the exact steps I can take to help reproduce this problem?

Ok I could confirm this. You need a module that provides a function via hook_action_info() that you can disable or uninstall.

1. Assign the module's action to a trigger.
2. Disable the module.
3. Confirm the action is still listed assigned to a trigger in admin/build/trigger.
4. Perform the trigger that would fire the action (should result in a function doesn't exists error)
5. Go to admin/build/trigger and attempt to unassign the action from the trigger
6. Confirm that the action is not removed (should also result in some undefined index notices)

Attached is a patch that performs a check to drupal_function_exists() for the action function and also allows for actions that no longer exist to be unassigned.
I've also attached the simple little module I used to test this.

Patch needs review.

Latest patch ready for testing and a summary of changes:
1. Fixed the incorrect link to 'admin/build/actions/orphan' to the correct url 'admin/settings/actions/orphan'
2. If an action is orphaned and no longer available (through say, a module disabled) and the current user has the permission 'administer actions', then a message is displayed about the orphaned action with a link to the url in the point above. This message will also be seen for orphaned actions on the admin/build/triggers or admin/settings/actions page.
3. If an action is orphaned but still assigned to a trigger, it can be successfully unassigned using the 'unassign' link in the action's trigger. Previous behavior: E_ALL notice errors during unassignment and action is never really unassigned.
4. If an action is orphaned and it is assigned to a trigger that occurs, you are going to get a fatal function not found error. When an action is called, it now checks to make sure that the action callback exists using drupal_function_exists.
5. Removed the $actions_in_code parameter from actions_synchronize because in every instance but one that the function is called, it is simply called as actions_synchronize(actions_list()). There is no point in having that parameter especially since the whole point is to re-synchronize the actions between what is available between module code and the actions database.

Ran the triggers and actions configuration test with 100% pass. Ready for review. Anyone?

Clarification about change 5 in #9 above:
I removed the $actions_in_code parameter and changed the first line of the actions_synchronize function to: $actions_in_code = array_list(TRUE);. Nearly every time the actions_synchronize function was called it was passed array_list() for the $actions_in_code parameter anyway. It seems logical and makes sense for "synchronize" function to always get a fresh list of the available actions.

I'm bumping this to critical since an orphaned action can still be executed, causing site failures, and cannot even be removed through the trigger interface.

Re-rolled to account for changes from #308526: Actions do not reset on sync.

I have looked through this patch and made a few changes. The patch introduced a drupal_set_message() into actions.inc. Since actions.inc is intended as a pure api file, I moved the drupal_set_message() into the calling code. I support the proposed removal of the first parameter of actions_synchronize(). This patch also adds a drupal_set_message() so that the user gets feedback when they actually click on the "Remove orphaned actions" link; formerly a message was written to watchdog to indicate that the orphaned actions had been deleted but there was no feedback to the user; they were left blinking at the Actions setting page with no feedback.

I like your improvements jvandyk, but I had a couple of concerns:
1. I agree about moving the orphaned actions message out of actions.inc, but that message also needs to get displayed on admin/settings/actions, so I added the code to system_actions_manage().
2. Removed the check for user_access('adminster actions') in trigger_assign() since the triggers page requires the 'administer actions' permission anyway. (see trigger_menu and trigger_access_check().
3. Thanks for the additions to documentation - I missed those!
4. Removed the orphaned core function system_action_delete_orphans_post() since it seemed silly not to just display the 'action x deleted' message in system_actions_remove_orphans(). The function system_action_delete_orphans_post() is not called by anything else in core - it would be best to get rid of it.

I'm posting this for review, and now I'm going to go write a test for orphaned actions.

Good changes. The patch works as expected.

eMPee584, thanks to your original patch, we're able to make some much needed improvements with regards to orphaned actions such as warning the user when there are orphaned actions and not running an action function if it doesn't exist. Also, see change 5 in comment #9 with regards to the actions_synchronize parameter.

I've been working on splitting my large patch into smaller ones. Working on another revision.

Please don't change the version. This needs to get fixed and accepted into 7.x first, then we can backport.

@#19) Any progress with that? If not, I'd be happy to split this up into bug fixing vs API change patches, just let me know.

In Drupal 6.12 I get the log message

One orphaned action (comment_unpublish_action) exists in the actions table. Remove orphaned actions

whenever I disable or enable the PHP filter module. Is this the same issue? I have not done anything with actions on my site.

Same message on D7 fresh install.

lilou: I'm assuming you're saying that with 7.x you're getting the same message I am. Does this Drupal issue pertain to that message? Does the patch from previous comments remove the message for you in 7.x?

I'd like to see if the test bot likes this.

This patch is quite old, but AFAICS, the (critical) problem still exists. Someone up for re-rolling this?

Not critical. Please read and understand Priority levels of Issues, thanks.

I'm not even sure this issue is even valid after all. Based on the latest patch, the registry is gone, and I believe since then, we also revamped the entire trigger/actions system.

Yes, not critical since another issue somewhere added the plain function_exists check so orphaned actions wouldn't cause a WSOD.

There is still a possibility of causing a white screen of death by assigning singleton actions from modules which are later disabled. This patch fixes that.

Here is a UI test to make sure this doesn't happen in the future. The white-screen will cause the test to fail until the above patch is committed of course.

The patch in #34 is still good. The test failed because the patch in this issue is the one that would fix it. Should the test have been submitted elsewhere?

Submitting them in the same patch would probably be optimal.

I've combined the fix and the test for the fix into one patch. Thanks for the advice cwgordon7.

I also wanted to note I've updated the trigger_test.module file to use 'label' rather then 'runs when' this stopped undefined index errors from happening. I think the expected array key must have changed at some point and this file hasn't caught up.

I've changed the broken unassign link next to orphaned actions on the triggers page to a link to remove orphaned actions. I used the exact text used in the t() in system.module so this won't add anything new for the translators.

Limiting comments to 80 characters and fixing spacing. I've changed static $actions to use drupal_static per tha_sun's recommendation.

part of this patch already in #601398: Simpletest does not allow assigning actions to triggers

Test assigns an action - right now this broken

+++ modules/trigger/trigger.admin.inc	26 Feb 2010 21:59:26 -0000
@@ -153,10 +153,20 @@ function trigger_assign_form($form, $for
+    // If action is defined unassign it otherwise offer to delete all orphaned
+    // actions.

Missing comma before "otherwise".

+++ modules/trigger/trigger.admin.inc	26 Feb 2010 21:59:26 -0000
@@ -153,10 +153,20 @@ function trigger_assign_form($form, $for
+        'link' => l(t('Remove orphaned actions'), "admin/config/system/actions/orphan"),

I'm not entirely sure where this path is pointing to. Does it even exist? If so, an inline comment may clarify the situation.

+++ modules/trigger/trigger.module	26 Feb 2010 21:59:26 -0000
@@ -610,7 +610,7 @@ function trigger_actions_delete($aid) {
-  if( $triggers ) {
+  if ( $triggers ) {

While slightly better than the original, there shouldn't be spaces between opening and closing parenthesis.

+++ modules/trigger/trigger.test	26 Feb 2010 21:59:26 -0000
@@ -299,3 +299,57 @@ class TriggerOtherTestCase extends Drupa
+      'description' => 'Test triggering an action that has since been removed.' ,

Please remove the space before the comma.

+++ modules/trigger/trigger.test	26 Feb 2010 21:59:26 -0000
@@ -299,3 +299,57 @@ class TriggerOtherTestCase extends Drupa
+    // Test 1: Assign an action from a disable-able module to a trigger, 

We can drop those "Test X:" prefixes everywhere.

Also: Trailing white-space here (and elsewhere).

+++ includes/actions.inc	26 Feb 2010 21:59:26 -0000
@@ -129,7 +129,12 @@ function actions_do($action_ids, $object
+      else {
+        $actions_result[$action_ids] = FALSE;
+      }

If this FALSE has a(ny) meaning, then an inline comment should explain. However, I guess it does not.

Powered by Dreditor.

I think I've addressed all of these. Thanks for your help.

Thanks!

+++ modules/trigger/trigger.test	26 Feb 2010 23:00:47 -0000
@@ -28,7 +28,7 @@ class TriggerContentTestCase extends Dru
-      // 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.

@@ -49,7 +49,7 @@ class TriggerContentTestCase extends Dru
-      // 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.

Now that you fixed unrelated comments, we need to fix them properly ;) Exceeding 80 chars.

Powered by Dreditor.

Unrelated comments now wrapped at 80 characters. :)

Great! another fix for static caches which lead to more stable simpletest

Thanks!

Wow, great work on some tweaky edge cases and test coverage, too! Committed to HEAD.

Need to get into 6.x too.

The patch as it is won't work for 6.x.

Wow, seems this issue has seen a lot of work. But I'm also still encountering this problem with Drupal 6. Is that a known issue?

I'm getting for example (when using Drush):

WD actions: 2 orphaned actions (comment_unpublish_action, backup_migrate_backup_action) exist in the actions table.[warning]
Remove orphaned actions
WD php: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for   [error]
the right syntax to use near '0, 'admin_menu'1)' at line 1
query: SELECT name, status FROM system WHERE name IN ('seochecklist', 'admin_menu', 'filefield_meta',
'filefield_paths', 'imagefield', 'nodereference', 'number', 'color', 'comment', 'dblog', 'admin_menu'0,
'admin_menu'1) in /home/username/drush/includes/drush.inc on line 352.

I see a similar issue to #53 above with D6+drush as well:

The following projects will be enabled: dblog
Do you really want to continue? (y/n): y
WD actions: One orphaned action (comment_unpublish_action) exists in the actions table. Remove orphaned actions                    [warning]
dblog was enabled successfully.                              

I think this issue could be closed because of commit #306611: Fatal error when trying to invoke non-existing action callbacks
http://drupal.org/cvs?commit=368938

Cleaning of orphaned actions is API and UI change which has low chances to be commited to D6

I found this on another post and thought I should share:
https://drupal.org/node/445922
tells you to do this
Just log in as the "admin" and go to http://yourwebsite.com/admin/settings/actions/orphan
When I did this, the Manage Actions page refreshed with the orphaned actions having been removed. This should probably be documented somewhere

@sher1

Thanks! This fixed it for me, too! :)

I saw the same advice as sher1 mentions in #56 elsewhere and visited admin/settings/actions/orphan but at first I didn't understand that it was an action when all I got was admin/settings/actions/manage without any notification of orphans removed (at least that's how I remember it).

Thanks for that dude, you helped me out :D

you can also do the same as #56 with drush:
drush php-eval "actions_synchronize(actions_list(), TRUE);"

#60 if you get an "could not be executed" error using the string from above use it this way:

drush -r /var/xyz/path_to_drupal/ php-
eval "actions_synchronize(actions_list(), TRUE);" -l my_domain.net

and the #56 solution does its job too. Thanks for pointing this out.

#56 worked very well... too bad i didn't find it sooner...

#56 also works with D7.

Visiting:
/admin/config/system/actions/orphan
in D7 did the trick for me

Subscribing.

What in the world is an orphaned action, and how did it get that way? I don't even use actions.

#56 worked for me... visit /admin/config/system/actions/orphan clears the orphaned actions.

#56 worked for me as well.