Unlimited cardinality and changing the number of rows/columns (e.g. shrink table size not being honored as old data persists)

OK so it seems like if you modify a number of row/column settings at once, only the last modified table will be correctly updated.

Exactly! This is the same issue that we are encountering as well. I posted some notes about it at #1361702: Rebuild or csv upload action that shrinks table size not being honored (old roes/columns persist after save) which I am going to mark as a dup of this issue (as this one seems more recent/active).

I was poking around in the code a bit as well (tablefield_rebuild_form_ajax(), etc.) but did not see an obvious solution. I would like to explore this more but won't personally be able to dedicate time to it until some deadlines pass. We have some workarounds in place that allow us to avoid this issue for now, but I know its going to come back to bite us later. I may jump-back in on this at some point, but will certainly help review any patches that may be posted.

Also I don't think the "needs work" status should be used until after there is a patch available for review (as it signifies that an existing patch was reviewed but needs more work before it can be reviewed further).

Thanks for keeping this alive :) A patch for this would be most welcome!

Thanks erdos, I hope to give this a test shortly. Marking "needs review" as there is now a patch for review.

Oh man, sorry that it's taken so long to review this as promised. I got hung up on a number of deadlines.

Unfortunately, I didn't have luck with the patch in #8. Even with it applied I was still unable to get more than one re-sized/imported table to save correctly. As before only the last edited table was saved correctly.

No matter what, your analysis helps contextualize the issue, and I agree that something funky is going on in tablefield_field_widget_form() (most likely related to the initial calculation of the $default_value variable). After staring at the code a bit it seems like the default data for each tablefield is being reset (fetched freshly out of the DB) on each ajax submit except for the element that is triggering the ajax submit. Based on my understanding of the FAPI this makes sense, as I believe the entire form must be re-built (calling each fields' hook_field_widget_form()) even if only one form element will be changed via ajax. When this happens, it seems that tablefield_field_widget_form() isn't smart about maintaining the default values from the form_state of any tablefields that are not being updated via the ajax request, and instead it (re)loads their data from the DB. This is of course a problem if they happened to have been updated on a previous ajax request. This "resetting" is not reflected in the browser as the ajax only updates the triggering field visually, but it does affect the default values in the form_state data, which is ultimately what's saved.

So it seems to me that we need to bypass the part of tablefield_field_widget_form() that re-loads fresh data from the DB when a tablefield is being re-built via ajax but is not the field that triggered the ajax. I think that is here (around line 411):

  elseif (isset($items[$delta]['value'])) {
    // Default from database (saved node)
    $default_value = tablefield_rationalize_table(unserialize($items[$delta]['value']));
  }

Changing this to something like the following might ensure that these tablefields are always rebuilt from form_state data (in the final else { } ) as opposed to being refreshed from the DB:

  elseif (isset($items[$delta]['value']) && !isset($form_state['clicked_button']['#value'])) {
    // Default from database (saved node)
    $default_value = tablefield_rationalize_table(unserialize($items[$delta]['value']));
  }

I'll look at this some more and if it's working I'll make a patch.

Ok, I've played around a bit more with the change noted from my previous comment and have been seeing good results. With this change we have been able to successfully do several complex sets of edits that were not possible before (import via CSV to overwrite existing data, re-size multiple tables, mix/match imports and re-sizes on same save, etc.). So I'm rolling as a path (made with git format-patch). It's a very simple patch.

erdos, Kevin, would you be able to check this out?

Just wanted to bump this. Anyway else able to test this (very simple) patch so we can try to get a fix committed?

Thx @rjacobs. I have also played around and it looks good. There is still a problem if one sets some default values in field settings and locks them. I think this is because those values do not end up in $form_state['input'].

Here's a patch based on #12 that deals with locked values.

This still does not work in the new node creation form, although it does work in the node edit form. Debugging shows that in the case of node creation the following if clause gets triggered:

elseif ($form_state['submitted'] && isset($items[$delta]) && isset($items[$delta]['tablefield'])) {
    // A form was submitted
    $default_value = tablefield_rationalize_table($items[$delta]['tablefield']);
}

in tablefield_widget_form function.

One should probably add to it the && !isset($form_state['clicked_button']['#value']) check as in the patch above.

Same problem when having more than one tablefield field.

@uno, it might be that the problem you report got fixed by the fix of #2387001: Import CSV option does not work with Field Collection module. Could you check?

Hi vitalie,

Thanks for reply, could you point me to the fix?

Sorry, @uno, I suspect it is this one: https://www.drupal.org/node/2387001#comment-9574877, I am not sure the patch there will apply cleanly to the 7.x-2.3 version, could you test it on current dev version?

Re #20, the patch has been committed as part of #1673416: Image upload interferes with Rebuild table