Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#14898 closed defect (fixed)

Grids and wrapped Memory store - cant delete row

Reported by: Matej Owned by: Evan
Priority: undecided Milestone: 1.7.3
Component: DojoX Grid Version: 1.7.2
Keywords: Cc: Kris Zyp
Blocked By: Blocking:

Description

I can not delete row inside the grid. I am using memory store. Is it bug or something wrong in my code?

Attachments (1)

ms-claro.html (4.0 KB) - added by Matej 7 years ago.
Source code

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by Matej

Attachment: ms-claro.html added

Source code

comment:1 Changed 7 years ago by bill

Cc: Kris Zyp added
Component: GeneralDojoX Grid
Owner: set to Evan

Not sure if this is supposed to work or not? Not sure how complete the adapter from dojo.store API --> dojo.data API is.

comment:2 in reply to:  1 Changed 7 years ago by Evan

Replying to bill:

Not sure if this is supposed to work or not? Not sure how complete the adapter from dojo.store API --> dojo.data API is.

The store adapter works well, seems to be related with EDG.pagination, going to take a close look.

comment:3 Changed 7 years ago by Evan

Milestone: tbd1.7.3

So it's not specific to EDG.pagination, when using Grid with new store API wrapper, store.save() will be needed, adding store.save() to the test case will make it work well, e.g

store.deleteItem(item);[[BR]]
store.save();

comment:4 Changed 7 years ago by Evan

Resolution: fixed
Status: newclosed

In [28059]:

Fixes #14898, when Grid is used with new store wrapper, removeSelectedRows() should explicitly invoke store.save(), otherwise the deleted rows won't be reflected in Grid view.

Also backport to 1.7.x branch

comment:5 Changed 7 years ago by adros

I don't think that this answer is correct. Docs to DataGrid? says:

"If you want to add (remove) data programmatically, you just have to add (remove) it from the underlying data store. Since DataGrid? is "DataStoreAware?", changes made to the store will be reflected automatically in the DataGrid?."

So why should be 'removeSelectedRows' method used? There will be absolutely the same behaviour with JsonRestStore?. And what if I do not want to save the delete action immediately but after some more actions in one transaction?

comment:6 in reply to:  5 Changed 7 years ago by Evan

Replying to adros:

I don't think that this answer is correct. Docs to DataGrid? says:

"If you want to add (remove) data programmatically, you just have to add (remove) it from the underlying data store. Since DataGrid? is "DataStoreAware?", changes made to the store will be reflected automatically in the DataGrid?."

So this is more related with a known issue of existing Grid. Especially when Grid is used with a server side store, Grid never aware the concept of store.save(), each time store is changed, e.g when a row is deleted by store.deleteItem(), the sequence will be:

  1. Grid refreshes the UI at once since it doesn't aware and couldn't be able to wait an unexpected store.save()
  2. A fetching request is sent to server getting back the wrong data - the newly deleted row is fetched back since store.save() is not invoked yet.
  3. Grid finishes rendering the UI with the deleted row still occurring in Grid view

There hasn't been an ideal fix for this, even we use store.save() explicitly, there are still cases GET(fetching request) get back earlier than POST(store.save()) which results into wrong data in Grid view port.


So why should be 'removeSelectedRows' method used? There will be absolutely the same behaviour with JsonRestStore?. And what if I do not want to save the delete action immediately but after some more actions in one transaction?

I don't have the exact answer due to the long legacy history, but I suppose it was provided mostly as a convenient way.

I didn't mean "removeSelectedRows()" must be used for delete rows, if you want to make changes in a transaction, explicitly using store.deleteItem() and store.save() is more suitable.

No matter with [28059] or not, removeSelectedRows() won't work in a transaction with server side stores(as mentioned sequence above) - Grid will refresh itself with wrong data right after removeSelectedRows() finishes, and an explicit store.save() won't help in that case neither.

Anyway I will rollback [28059] a bit and only apply that into EDG.pagination for minimized affects and any better fix patches are welcomed.

comment:7 Changed 7 years ago by Evan

In [28070]:

Refs #14898 !strict, apply [28059] to EDG.pagination for minimized affects

Note: See TracTickets for help on using tickets.