(A beta breaker.)

Users __who used__ the old actions support in beta3 and upgrade to the new "dev" will encounter 'http error 200'. These are caused by PHP errors such as:

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

That's because Actions tries to call action callbacks that we no longer provide:

flag_action_email
flag_action_delete
flag_action_unpublish

CommentFileSizeAuthor
#22 circ_db_prefix.diff1.82 KBmooffie
#15 better_detect_actions_two.diff743 bytesmooffie
#12 diag.diff2.75 KBmooffie
#3 remove_old_actions.diff2.03 KBmooffie

Comments

mooffie’s picture

mooffie commented

It's not easy to fix this.

The actions_synchronize() function can delete orphan actions, but we shouldn't use it, as it may delete actions not belonging to us, whose modules the user temporarily disabled. Besides, the Actions module may not be enabled.

We can't just do action_delete(), because Actions may not be enabled. So we'll have to do double work: both action_delte() and SQL.

mooffie’s picture

mooffie commented
Assigned: Unassigned » mooffie
mooffie’s picture

mooffie commented
Status: Active » Needs review
StatusFileSize
new remove_old_actions.diff2.03 KB

Here's a patch.

archetwist’s picture

archetwist commented

I know you have many tasks and limited time but this patch (if I understand the code correctly) is another reason to provide an upgrade path for users of the beta3 version. It wouldn't be a pleasant surprise to loose all the configured actions and the ability to restore them (by reverting back to beta3, for example).

One other thing. Does this patch fix the issue with e-mails not being sent? I've configured new Send tokenized e-mail actions using the dev version but no message is sent.

Thanks.

mooffie’s picture

mooffie commented

to provide an upgrade path

How are we going to upgrade an 'email' action? We don't know that 'Token actions' will be installed.

Nate, what are your thoughts on this?

I've configured new "Send tokenized e-mail" actions using the dev version but no message is sent.

I haven't checked this yet. Please open a new issue (Things to check: 1. do other actions work? 2. if you're not using a token in the 'recipient', does the mail get sent?).

archetwist’s picture

archetwist commented

How are we going to upgrade an 'email' action? We don't know that 'Token actions' will be installed.

In this case, I think a big warning "Backup your data" will suffice ;) .

Please open a new issue (Things to check: 1. do other actions work? 2. if you're not using a token in the 'recipient', does the mail get sent?).

Will do. Thanks.

mooffie’s picture

mooffie commented
Status: Needs review » Fixed

Committed.
http://drupal.org/cvs?commit=139127
http://drupal.org/cvs?commit=139126

I think a big warning "Backup your data" will suffice

We'll do that.

mooffie’s picture

mooffie commented

Archetwist,

I've configured new "Send tokenized e-mail" actions using the dev version but no message is sent.

I've just checked this, and this action do work for me. Perhaps there's some problem with the token you use for the recipient. Put that token in the body of the email, and type a fixed email address for the recipient. This way you'll find out to what string the token expands to.

Note that there's one bug I'm aware of:

When you're flagging a comment, the event won't expose the node it belongs to, so node tokens, e.g. [node-author-email], won't work. I hope to fix that very soon.

archetwist’s picture

archetwist commented

Apparently, there is something wrong with the Token module (or my website's configuration). I've been trying to use the [author-mail] token (in the aforementioned action which I've created using the standard Drupal interface) but it's treated literally (doesn't expand). Here is a support request I've opened.

archetwist’s picture

archetwist commented
Status: Fixed » Active

I've installed the newest dev version and executed the update.php script (no queries were executed). All the old actions are still active and I get the infamous error message.

mooffie’s picture

mooffie commented

and executed the update.php

Go there again. Open the "Select versions" fieldset. Under "flag module" you'll see "No updates available". Click this drop-down. Do you see "6000"?

(no queries were executed)

We can't know.

My upgrade path won't report any quries it might have executed. I'm not to blame.

I'll have to put some diagnostic messages there.

(Do you happen to have an 'actions_registry' table in your database?)

mooffie’s picture

mooffie commented
StatusFileSize
new diag.diff2.75 KB

I added diagnostic messages to the upgrade path. Note that it takes hours for D.O. to repackage the 'dev'.

You should revisit 'update.php'. In the flag module's dropdown you could change "No updates available" to "6000", then submit the form.

mooffie’s picture

mooffie commented
Status: Active » Postponed (maintainer needs more info)
archetwist’s picture

archetwist commented

We can't know.

I just cited the message I got from update.php.

Do you happen to have an 'actions_registry' table in your database?

I do. It's empty.

You should revisit 'update.php'. In the flag module's dropdown you could change "No updates available" to "6000", then submit the form.

Result:

Update #6000
Cleanup of Actions tables is unneeded.

mooffie’s picture

mooffie commented
StatusFileSize
new better_detect_actions_two.diff743 bytes

Do you happen to have an 'actions_registry' table in your database?

I do. It's empty.

Ahhh!! That's it!

The short story: there are two versions of Actions, "1.x" and "2.x".

The upgrade path looked for an "actions_registry" table to determine if the old Actions 1.x is installed, and, in this case, to bail out.

I changed the logic to detect Actions 2.x (or Trigger) in a safer way now.

(Either wait several hours or apply the patch; you can do it by hand: the change is simple enough. Then go to "upddate.php", select "6000", and submit.)

mooffie’s picture

mooffie commented

Archetwist, the help you give me, us, in this queue, is invaluable! Thanks.

archetwist’s picture

archetwist commented

I am glad my reports are of good use :) , though I'm afraid this issue is still unresolved. After applying your modifications to the flag.install file I get the same Cleanup of Actions tables is unneeded message.

Additional info:

db_table_exists('actions_assignments') returns 0 even though the table is present:

--
-- Table structure for table `actions_assignments`
--

CREATE TABLE IF NOT EXISTS `actions_assignments` (
  `hook` varchar(32) NOT NULL default '',
  `op` varchar(40) NOT NULL default '',
  `aid` varchar(255) NOT NULL default '',
  `weight` int(11) NOT NULL default '0',
  PRIMARY KEY  (`hook`,`op`,`aid`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8;
mooffie’s picture

mooffie commented

db_table_exists('actions_assignments') returns 0 even though the table is present:

-- Table structure for table `actions_assignments`

You say the table is there in the database? Could you please double check that you've looked up in the right database? (I myself have 7 Drupal databases right now, so it's possible to err.) Because it's weird that db_table_exists() should fail.

Another question, more important: do you have a 'trigger_assignments' table in your database?

archetwist’s picture

archetwist commented

Could you please double check that you've looked up in the right database?

I even triple checked it. It's been quite a shock for me, too :) .

do you have a 'trigger_assignments' table in your database?

I do.

I don't know if this is relevant but I'm using the multi-site feature and share some tables between multiple databases. However, actions_assignments is not one of these tables.

mooffie’s picture

mooffie commented

Archetwist,

1. What does print db_prefix_tables('{trigger_assignments}'); print?

2. Does db_table_exists('node') returns TRUE?

archetwist’s picture

archetwist commented

1. archekrk_drupalorgpl.trigger_assignments

2. It returns FALSE...

mooffie’s picture

mooffie commented
StatusFileSize
new circ_db_prefix.diff1.82 KB

archekrk_drupalorgpl.trigger_assignments

Turns out db_table_exists() doesn't work if there's a dot in $db_prefix:
#286986: MySQL: SHOW TABLES LIKE database.tablename doesn't work

I tweaked the conditional a bit.

Again, either wait a couple of hours, or apply the patch.

archetwist’s picture

archetwist commented

Finally! The patch works perfectly.

mooffie’s picture

mooffie commented
Status: Postponed (maintainer needs more info) » Fixed

Archetwist, thank you for all the tests and reports!

===

I've configured new "Send tokenized e-mail" [...]

[...]
Note that there's one bug I'm aware of:
When you're flagging a comment, the event won't [...]

I fixed this yesterday.

archetwist’s picture

archetwist commented

Good teamwork!

Anonymous’s picture

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

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