Rename the email tokens, especially %event and %eventurl

Two approaches:

"php": queries for all records in {signup}, converts the two email template fields via PHP, and if the new versions are different than what's in the DB for each record, does a separate query to update each record.

"sql": makes one crazy query that does all the conversion all at once on all rows using some nested REPLACE() string manipulation functions as part of the query.

I suspect the SQL one is better, especially for sites with a really big {signup} table, but I'm really not sure. Also, according to the docs, REPLACE() works the same on both pgsql and mysql, but I haven't tested that.

Rerolled after http://drupal.org/cvs?commit=150472

I realized that with both patches, we need to replace %eventurl before we replace %event, or you end up with %node_titleurl. ;)

If we go with the PHP version, it probably needs work to convert it to the update.php "batch" functionality (such as it is). However, I suspect the SQL version is good enough, and certainly the less complicated approach.

I got davidstrauss's opinions on this in IRC, and the nested REPLACE() is the clear winner. For the record, a few technical details about why:

davidstrauss: dww: the best approach would be nested REPLACE() done in batches of 1000 rows at a time
davidstrauss: nested REPLACE() is going to be far faster than the php+sql version
davidstrauss: just make sure it won't take too long on large datasets
dww: davidstrauss: greggles said he'd do it as separate queries instead of the nested one -- is that just superstition? ;)
davidstrauss: dww: far faster to nest
davidstrauss: dww: it's simply an issue of temporal locality
davidstrauss: dww: if you iterate over the entire set X times, you won't have good caches
davidstrauss: dww: if you iterate once over the set and perform X operations per item, then you only read/write once
davidstrauss: and the other operations happen in memory
davidstrauss: and, most likely, in high-speed caches

So, there you have it, nested REPLACE() is the clear win. Now I just need to batch-ify it anyway, to make sure I only do 1000 rows at a time.

This does it in batches of at most 2000 nids at a time. Since we don't know how many rows in the {signup} table each 2000 nids will be (probably much less than 2000), and since there's no easy cross-db way to know what's the last nid we updated if we just do a LIMIT/OFFSET query, this seems like the most fool-proof approach, even if it means we end up doing lots of small queries we might not need. It's what davidstrauss told me to do, at least, and that's gotta count for something. ;)

Also, at his suggestion, using %user_mail instead of %user_email, to be a little more consistent with core (it's {user}.mail for example).

New patch using the following:

$replacements = array(
  '%eventurl' => '%node_url',
  '%event' => '%node_title',
  '%time' => '%node_start_time',
  '%username' => '%user_name',
  '%useremail' => '%user_mail',
  '%info' => '%user_signup_info',
);

%node_start_time and %user_signup_info seem less ambiguous than %node_time and %user_info...

Thanks for the testing....

I was surprised it didn't update the tokens in use within the default confirmation and reminder e-mail fields, but then I suppose we don't normally mess with added content :)

Huh? That's exactly what this update is supposed to be doing... What do you mean "it didn't update the tokens in use within the default confirmation and reminder e-mails fields"? Can you post an example of an email template from after the upgrade that still contains the old tokens? This is very worrisome. :(

I find the new token names a lot less useable, but then I'm using event :)

Heh. ;)

Using drupal jargon (node) seems particularly nasty.

Well, I was trying to make these generic, unambiguous, and self-documenting... I know "node" is considered uncool in the UI, but it's still all over the place in Drupal. I don't know how to "un-node" Drupal.

'%eventurl' => '%url',

URL of what? The node? The user?

'%event' => '%title',

That's probably safe.

'%time' => '%start_time',

Again, probably safe, but still potentially a bit ambiguous.

I suppose ideally we'd want a substitution like:
$content_type . '_' . $title
so that whatever the signed up content is was used as the token (event_title, concert_title etc) but I don't know how workable that is in practice.

Yeah, that could be cool. Sadly, it's not really workable at all. :( Think about the site-wide global settings for this at admin/settings/signup. There's no node there, so there's no way to know what node type to use to replace the tokens with. It also makes the DB update much more treacherous...

Anyway, I'm still open to finalizing a better name for these tokens. But, I'm most worried that you didn't see everything replaced during the upgrade. Evil. Can you post more details about that?

Thanks!
-Derek

Bah, I'm an idiot. My update code was starting from 0 and going up, but I forgot that we use nid 0 to store the global defaults...
Try this one. ;)

Yeah, that's caused by http://drupal.org/cvs?commit=151782. This one applies cleanly.

Any final thoughts on the new proposed names before I commit this?

Thanks!
-Derek

And here's a version of the same patch that applies against HEAD for the D6 port.

Committed to HEAD and DRUPAL-5--2. Yay. ;)

EVIL... I was testing this on D6 and noticing that it never seemed to work. Much interesting debugging later, and I realized we were getting nailed there since in D6, db_query() now supports a '%n' placeholder, and all those '%node_title' etc tokens were confusing everything. Yikes. To be safe, all of our '%' in our token string literals should really be '%%' so that db_query doesn't mess with them.

Committed that to HEAD and DRUPAL-5--2. Phew.