Not sure if this broke due to changes in the cck content_migrate module or what, but node_reference throws errors upon visiting the admin/structure/content_migrate page. The entire instance_alter hook fails at the moment, so the upgrade path will break.

There was an assumption made that another hook would set the module name and such, and I think that is not the case, if it was at some point.

This patch fixes the issue for me.

CommentFileSizeAuthor
#5 references_migrate-1032808-5.patch3.43 KBlliss
#3 references_migrate-1032808-3.patch922 bytesyched
#4 references_migrate-1032808-4.patch1.95 KByched
references-undefined_index_module.patch871 bytesloganfsmyth

Comments

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Needs review » Postponed (maintainer needs more info)

Do you mean you migrated an existing D6 site with CCK/node_reference to D7 with References ? This has only had preliminary work yet.

loganfsmyth’s picture

loganfsmyth commented
Status: Postponed (maintainer needs more info) » Needs review

Yeah, I've been testing it out on a site of ours that we're considering upgrading. We're aware it's kind of iffy, but I just wanted to post this since it seems to fix the issue. I don't doubt there is more to be done.

yched’s picture

yched commented
StatusFileSize
new references_migrate-1032808-3.patch922 bytes

Thie code in node_reference_content_migrate_instance_alter() aims to massage the $instance prepared by content_migrate, only if its an instance of a 'node_refererence' field.

We want to be switching on the field type, not on the widget type module (I guess 99% existing D6 noderef fields out there use one of the widgets defined in noderef.module, but there are contrib modules providing alternate noderef widgets)

So I'd say something more like the attached patch ?

What is confusing is that the hook example and most implementations within content_migrate itself currently build their logic on $instance_value['widget']['module'] like the patch in the OP.

[edit: opened #1039466: What to do with custom formatters and widgets? ]

yched’s picture

yched commented
StatusFileSize
new references_migrate-1032808-4.patch1.95 KB

Attached patch should take care of updating formatters as well (the 'full' and 'teaser' formatters have been merged into a formatter setting)

lliss’s picture

lliss commented
StatusFileSize
new references_migrate-1032808-5.patch3.43 KB

So while this one does seem to solve the problem of the missing references it does not allow the CCK Migration to actually recognize the node references as fields that can be migrated. If we alter the value used in the switch statement we will actually get noticed by CCK migration but there still seems to be trouble with actually pulling in the values when we run the migration. This patch at least solves the former issue in both the node_ and user_references migration.

karens’s picture

karens commented

I just committed a simple fix that gets rid of the notices (the arguments had changed for these functions). I did not see or do anything yet with the patch to update formatters, or yched's note about switching on type instead of module.

I will see if I can tell what else needs to be done.

karens’s picture

karens commented

I got all this much committed as a start. I don't have time right now to figure out what else is needed. If someone does, please post an additional patch.

yched’s picture

yched commented

Thanks Karen - while you're in there, there seems to be an indentation issue around the '//massage the widget' line in the committed code :-)
http://drupalcode.org/viewvc/drupal/contributions/modules/references/nod...

(sorry, I'm not where I can easily commit a quick fix)

yched’s picture

yched commented

Fixed indentation, + committed the code to the 7.x-2.x branch instead of HEAD ;-).

I'm probably going to remove all the code from HEAD for now, just to avoid further confusion - unless someone beats me to it.

yched’s picture

yched commented

"I'm probably going to remove all the code from HEAD for now, just to avoid further confusion" : Done.

danielb’s picture

danielb commented

Is the variable name spelled "$dislplay" on purpose?

I found this issue because I have written a module that tries to determine whether a field is a User reference field and it does so by checking $field['display']['default']['module'] == 'user_reference' where $field is an array value of the bundle and the bundles come from field_info_instance('node'), which seems to work fine for me, but other users are reporting that 'module' is an undefined index. #1069994: Undefined index: module in nodeaccess_userreference_node_access_records() (line 138

yched’s picture

yched commented
Status: Needs review » Closed (fixed)

The "$dislplay" typo has been fixed since then, it seems.

@danielb : I'm not sure what's the status of #1069994: Undefined index: module in nodeaccess_userreference_node_access_records() (line 138, but it does seem strange to check the module implementing the formatter that happens to be used on a given view mode to determine whether a field is a User reference field. Why not just $field['type'] == 'user_reference' ?

As for this specific issue, I think we can now mark this closed.