Orphan node types lose bodies on upgrade

Also to note #866410: node_update_7006() relies on has_body

Found this out the hard way.

I would say the body label should be "Body". That's what almost everyone uses so it will usually be right, and as you say they can fix it via the UI. If not "Body", my second choice would be "Body (upgraded)". But then a valid question would be why that node type has "upgraded" when all the others do not. So just "Body" is probably best, it will lead to the fewest support questions in the forums. :-)

The patch looks good to me. Should we wait for #706842: Improve comments for the taxonomy upgrade path to remove the failure? It isn't clear to me why the test fails here but not for currently committed HEAD; does no other test call $this->performUpgrade()?

subscribe

I'm aware that this bug is awaiting #706842: Improve comments for the taxonomy upgrade path but I'm at a loose end so I thought it might be useful to test with a concrete example.

I can repeat the original bug with image.module specifically. That's not a great choice given there might be confusion over images in core, but webform no longer owns its own nodes and I don't have much experience of any other content-type-creating modules!

Here's the steps to repeat:

• D6:
  • Enable image module
  • Create image node with body
  • Disable image module
• D7:
  • Run update.php
  • Navigate to image node
  • Body has disappeared

When I apply the patch in #2 the body content is transferred successfully and visible on the new site. However, when I navigate to the node edit page, I get an error:

Notice: Undefined index: image_node_form in drupal_retrieve_form() (line 690 of /var/www/drupal-7/includes/form.inc).
Warning: call_user_func_array(): First argument is expected to be a valid callback, 'image_node_form' was given in drupal_retrieve_form() (line 724 of /var/www/drupal-7/includes/form.inc).

The image content type is not available at admin/structure/types . I guess this is related to the fact that the content type also disappears in D6 when the image node is disabled, but does that error needs catching? It does also exist on the D6 node-edit page immediately after the image.module is first disabled:

warning: call_user_func_array(): First argument is expected to be a valid callback, 'image_node_form' was given in /var/www/drupal-6-upgrade/includes/form.inc on line 376.

so maybe this is working as designed? Seems a shame to have a broken site without a clear upgrade path for end users if that's the case.

@jp.stacey: what you describe feels like as-designed. The image module is not there to handle this node in D7, so it's natural that things falls apart.

#706842: Improve comments for the taxonomy upgrade path is ready for what I think is its final review.

patch in #2 works, will retry with #706842: Improve comments for the taxonomy upgrade path patch in place.

#2: body_up.patch queued for re-testing.

IMO the label should just be "Body" rather than "Upgraded body label" - that way people upgrading may not need to bother fixing at all if they left things as default. Looks great otherwise.

+++ modules/simpletest/tests/upgrade/upgrade.node_body.test	2010-08-23 16:15:08 +0000
@@ -0,0 +1,32 @@
+ * Upgrade test for node boeis.

Typo.

Also, concerning the body label can we not simply do #13 (just "Body") and then provide a message on the finished upgrade? Something like (but more elaborate): "The labels of the body fields on your content types have all been renamed to "Body". You will need to manually edit them if you used differing labels."

Powered by Dreditor.

Can't see anything else to complain about, RTBC now.

1. Can we name the test file upgrade.node.test or something, and group all node system related tests in one file? Let's do a quick re-roll of this.

2.

+++ modules/simpletest/tests/upgrade/upgrade.node_body.test	2010-09-07 06:22:40 +0000
@@ -0,0 +1,32 @@
+    $this->drupalGet("content/1263769200");

...

+++ scripts/generate-d6-content.sh	2010-09-07 06:18:39 +0000
@@ -185,3 +185,24 @@ for ($i = 0; $i < 12; $i++) {
+node_save($node);
+path_set_alias("node/$node->nid", "content/$node->created");

It is not clear why sometimes we use a constant, and other times we compute the URL.

Looks RTBC otherwise.

Since those were Dries's only complaints, committed to HEAD. Thanks a lot for snuffing this one... how nasty.