Hello,

Found a bug with multivalue delete checkbox. On form submit $form_state['values']['multicrud']['delete'] can only detect the latest checked value and not the preceeding checked deleted values boxes.

Comments

chriscalip’s picture

Attached is a proposed fix.

This provides the $form_state values an array of deleted keys
on hook_form_submit:

$form_state['values']['multicrud']['delete'][$key]

chriscalip’s picture

There was a bug in the first bug fix patch :)

Here is a better patch.

donquixote’s picture

I think this does not apply to 2.x, does it?

chriscalip’s picture

While I don't use the 2.x branch -- heheh the unstable tags had me avoiding that branch; Just now I briefly installed the 2.x branch and saw that the field structure of the delete checkboxes have enough contextual information that the patch is not needed.

chriscalip’s picture

By the way in terms of universal availability .. i think 1.x is still needed because the ajax feature of 2.x makes it hard to implement on a large dataset.

donquixote’s picture

i might take the time to apply this simple fix to 1.x

> the ajax feature of 2.x makes it hard to implement on a large dataset.

where did you find ajax in 2.x?

chriscalip’s picture

Your right; ajax is not an accurate description. What I mean is the js stuff rearrange; pretty much crashed my browser @ 1600 records err field rows.

donquixote’s picture

Interesting..
you are aware that the tabledrag is an optional feature that is available both in 1.x (afaik) and 2.x? Just maybe in 2.x you tried an example which has tabledrag?

Or is the tabledrag in 2.x slower than in 1.x? Then it would be a problem I need to fix.

chriscalip’s picture

Well i think by default 1.x does not have table drag on; while 2.x has it on by default. While I am not sure that tabledrag 2.x is slower than 1.x what I can say is that 1.x without tabledrag on -- did not crash on a 1600 record fields while the 2.x with tabledrag on; crashed with a similar amount of records.

donquixote’s picture

ok so i should simply make it disabled by default in 2.x too.

chriscalip’s picture

I dont know about that; I am not sure if the majority of the use case would be using a large dataset and which ones will not use a large dataset. The contrib module I shared to the public http://drupal.org/project/sfweb2lead_webform has a normal use case of about 20 fields or less. While some of the site specific custom module I made that are dependent on multicrud are about 1000+ records. I pretty much designed an alternative for site users to either make use of a csv parser to update data or a multicrud form.

Well now that i started sharing my use case for multicrud .. I think the majority so far are about 20 record fields or less. Which should be fine; but on the other hand .. i think it might be a code smell that something in the tabledrag operation whether on page load or by trigger is not fully performant and is in fact crashing on large data sets.

Yawn its 12 am in the morning and I am still doing drupal work LOL. I am getting some sleep. Good morning :)

donquixote’s picture

There is no point in enabling tabledrag if you don't have items with a weight field. The developer needs to make an intentional decision about this.

I think the tabledrag is just as clunky on the blocks configuration or menu form, which do not use multicrud. It is just not made for large numbers of items.

I already considered to introduce some kind of pagination for large numbers..
In case of tree tabledrag, instead of pagination, I would do something where only some sub-trees are expanded at a time.
For a flat tabledrag, I have no idea how I would do the pagination.
But this is for another issue.