A spin-off of #296473: Replace Actions/Trigger Support.

While Workflow-NG/Rules will provide the ultimate flexibility in setting up flagging rules, I want to maintain a simpler approach to actions that leverages the available actions in core without the overhead of the Rules UI. This patch wipes out our current Actions integration, other than flag_hook_info() which remains in flag.actions.inc.

This moves all our Actions integration to a separate module "flag_actions.module". Considering that users will probably use either Flag actions or Rules, it makes sense to move everything to a module that can be turned off in the case that they are going to need Rules. This module provides two "conditions" for firing actions: the event (flag or unflag) and the count threshold.

Screenshots of the UI are attached.

CommentFileSizeAuthor
#8 flag_better_actions5.patch38.42 KBquicksketch
#8 flag_better_actions6.patch39.7 KBquicksketch
flag_better_actions.patch39.21 KBquicksketch
flag_actions_list.png30.67 KBquicksketch
flag_actions_edit.png35.18 KBquicksketch

Comments

quicksketch’s picture

quicksketch
he/him
commented

Mooffie: I recalled why the $flag_action variable was an object previously. When performing token replacements, the argument calls for an object. However, this differs from the $context argument needed for Actions (which is an array). The $flag_action object was made for token replacement, but just so happens to work well for the $context argument for actions. This patch switches it back to an object rather than array.

fago’s picture

fago
Primary language German
Location Vienna
commented

Perhaps I can help you with some notes regarding rules:

rules also supports firing core actions - perhaps you can reuse these actions with rules. (I had no close look at it, so I don't know if it makes sense.) - for more about that see http://drupal.org/node/299055

If your motivation for this is just the better integrated UI - you could also build upon rules, build an own UI and programatically configure the rules. (as you prefer, I just want to let you know.)

mooffie’s picture

mooffie commented

(Nate, I'm examining the patch right now.)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

subscribe.

mooffie’s picture

mooffie commented
Status: Needs review » Reviewed & tested by the community

Nate, this is superb!

Much, much better than what we now have.

I have only minor comments (but you can commit the code as-is):

- I wish you worked in E_ALL mode. (There are only about five or six places where the code genrates a warning. None of them because of a bug.)

- flag_actions_check() can be removed.

- The comment flag's get_valid_actions() allows for some node actions too. But only a 'comment' context is passed to the action's callback (in flag_actions_do()). (You may decide to simply discard node actions. The Rules patch has a "give me all contexts" method.)

- FWIW, for PHP4 compatibility, "$context[$action->type] = $object" should be changed to "$context[$action->type] = &$object". Or, simpler, this line could be moved outside of the loop. (So that if two or more actions are registered, and both modify a node, both will get saved.)

- BTW, the "send tokenized e-mail" action doesn't yet exist there.

- Will you make a D5 version of it?

I'm for commiting this right away. Excellent work.

mooffie’s picture

mooffie commented

(@fago: Rules is certainly superior to Actions.)

quicksketch’s picture

quicksketch
he/him
commented
Status: Reviewed & tested by the community » Patch (to be ported)

FYI, I'm still working on backporting this to Drupal 5.

quicksketch’s picture

quicksketch
he/him
commented
Status: Patch (to be ported) » Fixed
StatusFileSize
new flag_better_actions6.patch39.7 KB
new flag_better_actions5.patch38.42 KB

Looks like I was pretty close to finishing this before heading to DrupalCon. It just required a few small changes to get a multistep form working in Drupal 5, but now it's all rolling along just fine.

Moofie: I've addressed the problems you outlined above,
- All notices are now corrected, I'll be sure to work in E_ALL moving forward.
- flag_actions_check() is now removed.
- For PHP4 compatibility... I just replaced "= $object" with "= &$object" as you suggested.
- The "send tokenized e-mail" action is included in the "Token actions" module, it's not provided by Flag.

As for:
- The comment flag's get_valid_actions() allows for some node actions too.

I was aware of this but I didn't account for it yet. Let's take a stab at it in a later patch. I've committed what we've got so we can roll out a new beta.

mooffie’s picture

mooffie commented

Great. I'll test this later today.

archetwist’s picture

archetwist commented
Status: Fixed » Needs work

Not there yet. I've got two problems with the latest dev version.

1) There is no upgrade path. After updating the module all actions created using the beta3 version are lost.

2) The following error message appears when trying to use a flag that has a Send tokenized e-mail action assigned to it. Of course, no e-mail message is sent.

Fatal error: Call to undefined function flag_action_email() in [...]/includes/actions.inc on line 99

mooffie’s picture

mooffie commented
Status: Needs work » Fixed

Archetwist, I've just noticed that too. I've opened a new issue:

#305391: beta3 actions still registered

(I'm marking this "fixed". Once a patch got commited it can't go back to "needs work". Instead, new issues must be opened.)

1) There is no upgrade path. After updating the module all actions created using the beta3 version are lost.

I'd say we shouldn't waste time on this. It could be nice to tend to this, of course, but there are so many other tasks we need to do.

archetwist’s picture

archetwist commented

Once a patch got commited it can't go back to "needs work". Instead, new issues must be opened.

Good to know. Sorry for messing with this issue ;) .

mooffie’s picture

mooffie commented

The comment flag's get_valid_actions() allows for some node actions too. But only a 'comment' context is passed to the action

This was fixed in #307250: Make actions get the right object

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.