Are you planning to provide an upgrade path for existing field data? If so, do you want to coordinate with the field upgrade at #781088: Updating CCK Fields and Data from D6 to D7? If you want to coordinate, my process is to do a query of the data in the old tables (which still exists and is unalterd), but construct the results of that query into an array that matches the new configuration, then use it to create new fields. This leaves the old data intact and unaltered. Obviously if you want to migrate information about which fields go into which groups, the fields need to be migrated too.

I haven't cracked open your code to see how fieldgroups are configured in this module and how it compares to the way they were configured in D6, so hopefully you can handle that part.

I'd be glad to let you add a tab to the field migration page for fieldgroups to handle this however you want.

CommentFileSizeAuthor
#20 1018550-20-fix-fieldgroup-D7-update.patch929 bytesdrzraf
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

KarenS’s picture

Or actually, maybe the fieldgroup update can be a straight update hook to create the new tables with all the fields and some sort of provision for the fact that some fields may not yet exist in D7. Then as the fields are created they will become available in the groups.

Stalski’s picture

Fieldgroup has little notion of fields currently for D7. I will try out to create an array, alter it to the new configuration and see how it goes. An update hook would be nice, but this is the first time i am trying to merge something between majors ;). I'll do my best.

What's concerning the fields, the only thing field_group stores is a "children" section in the configuration array. I will test what it gives if just doing this with fields that are not even there (yet) in the new d7.

regards,
Stalski

Stalski’s picture

After re-reading this, it seems odd and like i am going to do some trial and error ... but in fact that's exactly what i am going to do at first. I just have a gut feeling this just could work.

Stalski’s picture

Status: Active » Needs review

Was indeed not that hard.
Run field_group_update_7000 and the tables should be migrated. I did not remove the old tables.
I had to do is this way since field_group is not the same as fieldgroup. For drupal the field_group module is not installed so it will be enabled later on, where the migration is done.

Stalski’s picture

Status: Needs review » Fixed
Stalski’s picture

Status: Fixed » Closed (fixed)
Mark Theunissen’s picture

Never mind! :)

Stalski’s picture

It's not a update hook. It's install hook itself.

function field_group_install() {
  $groups = _field_group_install_read_groups();
...

This reads the content table for drupal6 and merges them into new format.

Mark Theunissen’s picture

Status: Closed (fixed) » Active

Hi Stalski

I think the upgrade process is broken, as after upgrading from D6 to D7 my field_group table had no data in the 'identifier' column, which is required.

I think this is because the installation of this module will create the table in it's complete state, with the 'identifier' column present with no data. In order to create this data, the function '_field_group_recreate_identifiers();' needs to run, but this function is only called in update_7001() under this code:

if (!db_field_exists('field_group', 'identifier')) { 

So it doesn't get called. By manually calling it after running the update, the 'identifier' column is populated correctly. I think this can be fixed by adding a call to '_field_group_recreate_identifiers();' in the _install() hook.

Thanks
Mark

Mark Theunissen’s picture

Status: Active » Closed (fixed)

No, wait, I see 7002 also calls this function... Hmm I will have to investigate further.

Mark Theunissen’s picture

Status: Closed (fixed) » Active

Ah, yes I see the problem.

Since this module is new, it begins with a schema of -1. Then, after it's enabled and installed, Drupal assigns it the maximum schema of 7002, which means these upgrade functions will not be run, and thus the identifier will not be generated.

To fix, I think simply putting a call to '_field_group_recreate_identifiers();' in the install hook will suffice!

yched’s picture

Stalski’s picture

Status: Active » Fixed

#12 is fixed.
the bug of empty identifiers is fixed as proposed in #11 as this seems in fact the best solution. It should be a harmless function anyway.

Status: Fixed » Closed (fixed)

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

tim.plunkett’s picture

Status: Closed (fixed) » Active

This sounds like it should still be open per #11.

swentel’s picture

Status: Active » Closed (fixed)

Actually, that function is in.

blasto333’s picture

How do you get this to run? I went to update.php and didn't see any of these functions in update.php (7001).

Should I enable multigroup after running the CCK migrate fields or before?

drzraf’s picture

Status: Closed (fixed) » Active

same issue:

select "content_group_fields", count(*) from content_group_fields UNION select "content_group", count(*) from content_group UNION select "field_group", count(*) from field_group;
+----------------------+----------+
| content_group_fields | count(*) |
+----------------------+----------+
| content_group_fields |       16 |
| content_group        |        4 |
| field_group          |        6 |
+----------------------+----------+

One of my content_group was used for 2 content-types but only 1 was migrated.

drzraf’s picture

finally, I think that update simply does not handle multiple group instances.
(A given group attached to different bundles).
The $group array iterated during update is created in _field_group_install_read_groups().
The key used is $record['group_name'] with no attention to type_name. Thus, latest wins.

Moreover old schemas should be deleted if they are not used anymore.

drzraf’s picture

Status: Active » Needs review
FileSize
929 bytes

after update using this patch:

select "content_group_fields", count(*) from content_group_fields UNION select "content_group", count(*) from content_group UNION select "field_group", count(*) from field_group;
+----------------------+----------+
| content_group_fields | count(*) |
+----------------------+----------+
| content_group_fields |       16 |
| content_group        |        4 |
| field_group          |       12 |
+----------------------+----------+

[ this does not fix the fact that old tables don't get dropped ]

jcisio’s picture

Status: Needs review » Reviewed & tested by the community

#20 is necessary if you use the same field group name for multiple node types.

drzraf’s picture

ping , maintainer ?

I would add that {content_group_fields} must be dropped after the upgrade.

jcisio’s picture

#22 not really. If it is dropped, you would not be able to rollback and redo the upgrade.

drzraf’s picture

if not dropped (even if upgrade is successful), you'll keep the table forever. It makes no sense.
most upgrade (at least in core), throw away old tables in order to keep a clean set and avoid confusion.
The upgrade of field_group seems quite simple and foolproof (if the above get pushed) in comparison to core upgrades.

mynameiscorey’s picture

I for one am grateful that the D6 tables are not dropped. I've just run through the process of enabling field_group during a site upgrade twice and both times have found that most standard field groups are successfully upgraded but a few content types are ignored. Dropping the table as part of the upgrade would make this very difficult to debug and from what I've seen so far the upgrade is not foolproof.

If I figure out what's going on I'll submit a fix

mynameiscorey’s picture

Silly me. I assumed that the above patches were already in the version of the module I was using, alas they weren't.. The problem I was having is fixed by #22.

I'm still in favour of keeping the old tables. They can be dropped by the developer when they are happy that things are working.

drzraf’s picture

tables are ... inside the initial dump, before the upgrade was fired.

nils.destoop’s picture

Status: Reviewed & tested by the community » Fixed

I don't have a d6 fieldgroup site, so i trust the reviews :)
Committed #22 to dev

Status: Fixed » Closed (fixed)

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