Grid DojoData add/remove row broken

[haysmark]

If you add a row to a Grid by calling grid.addRow, the underlying DojoData? model just says "that's nice" and doesn't call newItem on the store; effectively, it doesn't add the row. Maybe the DojoData? model is missing add and delete support?

[benschell]

I'm taking a stab at a whole bunch of Grid bugs. Re-assigning.

[benschell]

Fixes add/remove rows. Note, this patch includes fixes/enhancements from patch 5030.

[Jared Jurkiewicz]

Reviewed. There are some minor stylistic things to fix (I'll go ahead and fix them). The logic seems reasonable for the actions.

Tested on:

FireFox? 2.0.0.11: PASSED

IE 6: PASSED

Opera 9.2: PASSED

Safari B3: PASSED

Seamonkey 1.1.2: PASSED

[Jared Jurkiewicz]

Updated patch for style issues.

[Jared Jurkiewicz]

(In [12322]) Updated to the DojoData? model. fixes #5724 !strict

[guest]

Remove function passes arguments, not inRowIndexes to _removeItems on line 683. Should be:

this._removeItems(inRowIndexes);

CLA Manninen

[guest]

To reproduce the abovementioned issue see: ​test_dojo_data_edit. "Remove (Store)" is broken. The first item gets removed instead of the selected ones.

[benschell]

Replying to guest:

To reproduce the abovementioned issue see: ​test_dojo_data_edit. "Remove (Store)" is broken. The first item gets removed instead of the selected ones.

Actually, this is as intended. That button removes the first item in the model directly from the store. This is in contrast to the other Remove button, which removes the selected item using the Grid's removeSelectedRows function. More specifically, the Remove (Store) button calls this:

removeItem = function() {
	// Removes the first item in the model from the store
	// Grid should reflect removal of the first item and items should be re-indexed
	jsonStore.deleteItem(dataModel.data[0].__dojo_data_item);
}

Line 683 in model.js does indeed look odd as indicated above, however this is working properly. Items removed from the store (by using the "Remove (Store)" button) are removed from both Grids on the indicated test page, which is the expected behavior. I'm not seeing where the bug is.

[guest]

I still argue that there's a bug here. "Remove (Store)" was maybe my mistake, but see "Remove". On the the page I mentioned the grid rows are in the beginning:

0 Afrika
1 Egypt
2 Kenya
3 Nairobi
4 Mombasa

Select 1 & 3 and click Remove - what you have now is:

0 Egypt
1 Kenya
2 Nairobi
3 Mombasa

Only Africa is gone.

Try also 1) refreshing the page, 2) selecting 1 & 3 and 3) clicking "Remove (Store)". You get "Removal [ [ 0]]" twice on FireBug? console and once again only the first item is gone.

[benschell]

The second item you listed is still as expected. The "Remove (Store)" button has nothing to do with the selected rows in the table, as it is interacting directly with the data store (which has no knowledge of the rows selected in the Grid). "Removal 0?" is printed twice, as each Grid receives the notification from the data store and removes the appropriate row.

Hoewever, I'll agree that the first behavior (selecting two rows and selecting 'Remove' only results in the first row being removed) is a bug, and I'll look into it. I think my folly here was not testing deleting multiple rows at once, as you pointed out.

[benschell]

Oh my, I broke Trac. I meant "Removal [ [ 0]]"

[benschell]

As suggested, this patch fixes the problem. Now, selecting multiple rows to be deleted will result in the correct rows being deleted from the current Grid.

[Jared Jurkiewicz]

(In [12739]) Minor tweak to #5724 fix. Provided by Ben Schell. fixes #5724 !strict