Dataview: update an item changing the id

Adioud commented 8 years ago

Hi everybody,

I want to submit a very little suggestion, and get your feedback. As many of you I use the grid to display data and synchronise it with a server side.

Doing this I want to proceed this way when adding a new item:

the user create the item: I add it immediatly with a temporary client side unique Id
I query the server to get the definitive server side unique id
Then I want to update the client with the definitive server side id.

So I have to use: dataView.updateItem(tempCliendId, itemWithNew)

To avoid an error with updateItem I have done this change in updateItem:

replace this : if (idxById[id] === undefined || id !== item[idProperty])
by just this: idxById[id] === undefined , to avoid an error
and add this: idxById[item[idProperty]] = idxById[id]; , in order to map the new Id to the same previous item.

Everything seems to work correctly. So my questions are:

do you think that there could be some regression anywhere else?
is there another way to do this without changing dataview plugin?

If there is no danger and no other way to do, I let you see if this change must be or not merge to the method.

Thank you all, and (one more time) thank you M. Leibman for your work, Adrien

GerHobbelt commented 8 years ago

This is what I got:

    function updateItem(id, item) {
      // see also https://github.com/mleibman/SlickGrid/issues/1082
      if (idxById[id] === undefined) {
        throw new Error("Invalid id");
      }
      // What if the specified item also has an updated idProperty?
      // Then we'll have to update the index as well, and possibly
      // the `updated` cache too.
      if (id !== item[idProperty]) {
        // make sure the new id is unique:
        var newId = item[idProperty];
        if (newId == null) {
          throw new Error("Cannot update item to associate with a null id");
        }
        if (idxById[newId] !== undefined) {
          throw new Error("Cannot update item to associate with a non-unique id");
        }
        idxById[newId] = idxById[id];
        delete idxById[id];
        if (updated && updated[id]) {
          delete updated[id];
        }

        // Also update the row indexes? Nah, `refresh()` blows away the `rowsById[]` cache!

        id = newId;
      }
      items[idxById[id]] = item;

      // Also update the rows? Nah, `refresh()` blows away the `rows[]` cache and recalculates it via `recalc()`!

      if (!updated) {
        updated = {};
      }
      updated[id] = true;
      refresh();
    }

IOW: your approach is fine; only the updated flag set should be updated too as it's id-based. rows[] and rowIdxById[] don't need updating as those are recreated in refresh() at the end.

Cave canem: my copy here is from a significantly modified slickgrid clone so you may find one or more of these caches not to be present in yours.

AFAIK there's no risk of regression outside dataview this way. I did find trouble long time ago with replacing (updating) item objects when one instance was replaced by another, but that's far back and besides: if it's a problem it should quickly surface in your own tests anyway.

Adioud commented 8 years ago

Hello,

Fine! Thank you very much for your feedback and advice, I will check in your code what should be in mine, at least the updated flag as you said,

Thank you and have a nice day, Adrien

mleibman / SlickGrid

Dataview: update an item changing the id #1082