Short version:
If a field in a multigroup has empty values, the ordering is reset on node_save().

Multigroup with two fields: field_users (required), field_nodes (not required).
It initially has the following values
UID 2, NID 2

The node is edited and (UID 3, NID 3) is added via the node edit page. As expected, the result is
UID 2, NID 2
UID 3, NID 3

Using a custom module, I execute (where # is this node's nid)
$node = node_load(#);

The values are now
UID 1, NID 2
UID 2, NID 3

Marking critical: As this occurs on node_save with NO changes to the node, this causes any manipulation of nodes by other modules to corrupt cck multigroup data

Members fund testing for the Drupal project. Drupal Association Learn more


markus_petrux’s picture

Assigned: Unassigned » markus_petrux


Too bad because I already tried to cover node_load+node_save at some point (see comments #46 and #47 in Deleting unwanted multiple values / multiple values delta issues), but it seems that was broken with the next changes to CCK core. :(

Well, aside from fixing this issue, we need to change the behavior of multigroups to store NULL values in the database for empty items (see comment #38 in Synchronized deltas Views integration: Filter on equal deltas in multigroups), and this should make things easier here.

Both issues are related, and I think we don't have an issue opened in the queue for the second one, so we may deal with both here.

Assigning to myself so I don't miss this. I'll work on it as soon as I can.

danielb’s picture

Having this problem too! Didn't discover this bug till I created hundreds of nodes by hand assuming this was working :(

Any clues how I can quickly fix this myself? I will look into it after lunch :( argh

danielb’s picture

I don't see your 'keep_empty' hack in content.module (from comment #47 in the issue you linked)

danielb’s picture

I notice you keep mentioned CCK 6.x-3.x but no such version exists on the project page. I will need this fixed in 6.x-2.x.
edit: ah found it on the 'all releases' page.
I'd consider an upgrade, but if it doesn't work in 3.x either, then perhaps it isn't worth it.

danielb’s picture

Hmm OK, I've upgraded to CCK 3.x

Via the GUI - the ordering is preserved.

I don't think that was made clear in the original post of this issue, as node_save() is indeed executed during a node form submission.

In that case - for my usage purposes, there is no apparent bug.

markus_petrux’s picture

Status: Active » Needs review
1.15 KB

Well, I've been thinking about this a bit... this is not so easy as it seems because there are things in the implementation of multigroups that may change. I mean, we are still using separate tables for each field, however we may implement tables at multigroup level some day (see #520956: Add a new storage method for fields in multigroups), and that means we'll have rows for all fields, including those that are empty (NULL in the database).

Even if that happens, during node_load processing we may need to ensure that all deltas are consecutive, so we do not generate holes that are destroyed when node_load/node_save is executed, which is what triggers this issue where we are now.

So, here's a patch that tries to address this issue in that way. What we do here is ensure deltas are consecutive during node_load(). It should be safe enough and cause no trouble with other field implementations, but it could happen, so I won't commit this until some feedback is provided. Please, test with as much fields as possible. Specially critical are fields shipped with CCK package, Date and FieldField/ImageField. Thanks in advance!

markus_petrux’s picture

Title: Multigroup fields with empty values loose their ordering when saved via node_save. » Multigroup fields with empty values loose their ordering when saved via node_save
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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