Trigger upgrade path: enabled triggers removed due to hook changes from 6.x to 7.x

I think this is a bug since the upgrade is destroying data.

At first glance the test coverage looks great. I'd like to see the latest results of the test by themselves exposed on QA, so I'll try to upload a patch for that shortly.

Couple minor notes:

+++ b/modules/simpletest/tests/upgrade/update.trigger.testundefined
@@ -1,23 +1,23 @@
-      'group' => 'Upgrade path',
+      'group' => 'update path',

I think "Update path" should be capitalized here.

+++ b/modules/trigger/trigger.installundefined
@@ -105,3 +105,33 @@ function trigger_update_7002() {
+ * Renames taxonomy to taxonomy_term.

This is user-facing text, so I'd say something a little clearer. (Also update hooks get imperative verbs for the same reason; see http://drupal.org/node/1354#hookimpl.)

Perhaps "Update taxonomy hooks to new names." Something along those lines?

Leaving at NR since I haven't looked at the tests closely yet.

Attached just cleans up the minor points above and includes the latest tests separately to expose the taxonomy failures.

Oh, for the next reroll, the inline comments also need to end in periods. :)

+
+/**
+ * Update taxonomy hook records to new names.
+ */
+function trigger_update_7003() {

Taxonomy vs trigger

Taxonomy vs trigger

No, actually. This is the trigger module updating the names of the taxonomy hooks in its database records to their Drupal 7 names. So the doc is correct.

Tagging.

Attached just tweaks the update hook doc slightly to avoid confusion like @aspilicious's in #11.

Not sure what happened to the interdiff upload; one more time.

+++ b/modules/trigger/trigger.installundefined
@@ -105,3 +105,33 @@ function trigger_update_7002() {
+    $pattern = array(
...
+      'cron_run'
...
+    $replacement = array(
...
+      'cron'

Hrm. I was about to update the doc here to mention cron too, but there is no hook_cron_run() in D6 or D7, nor can I find this grepping D6. Can you clarify where the value 'cron_run' would be coming from?

That's not a Drupal hook, it's a trigger hook, see http://api.drupal.org/api/drupal/modules%21trigger%21trigger.module/func... and http://api.drupal.org/api/drupal/modules%21system%21system.module/functi....

Yes, the hook is defined in system.module but called in trigger.module ;)

Downgrading this.

While some data gets lost during the upgrade path it's easy to just configure the triggers again - there's nothing irreversible here.

Also it's fairly minor compared to the configuration changes that usually need to be made when doing a major version upgrade.

Drupalcon Sydney

Too adventurous for our first attempt. Needs re-roll due to PSR-0

Have re-rolled the patch. Please review!

Thank you @welly

Thanks for your patch, but it looks like there are some missing changes in other files?

We need the complete patch including the changes made in:

modules/simpletest/tests/upgrade/drupal-6.trigger.database.php
modules/simpletest/tests/upgrade/drupal-7.trigger.database.php
modules/simpletest/tests/upgrade/update.trigger.test
modules/simpletest/tests/upgrade/upgrade.trigger.test
modules/trigger/trigger.install b/modules/trigger/trigger.install

(Note: credit no_angel and damienwhaley too? http://core.drupalofficehours.org/task/1471)

Another re-roll...

@vijaycs84,

If possible, could you add an interdiff.txt (http://drupal.org/node/1488712, http://xjm.drupalgardens.com/blog/interdiffs-how-make-them-and-why-they-...) so the patch can be reviewed a little easier?