@ngrx/data optimisticUpdate does not work with custom selectId

[marcinmajkowski]

Minimal reproduction of the bug/regression with instructions:

https://stackblitz.com/edit/angular-4lxmjk

Use @ngrx/data and define entity with following properties:

• entityDispatcherOptions.optimisticUpdate set to true
• selectId selecting property other than id (it is _id in my case).

Extend EntityCollectionServiceBase and call its update method (click "Update Foo name" button in Stackblitz application).

Update fails with following error:

Error: Hero EntityAction guard for "[Hero] @ngrx/data/save/update-one": payload has a missing or invalid entity key (id)

Expected behavior:

Optimistic update works same way as for entities with id named id.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

Any browser. @ngrx/[email protected].

Other information:

My quick guess is that this mustBeEntity call should be replaced with call to mustBeUpdate.

I would be willing to submit a PR to fix this issue

[X] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

[JiaLiPassion]

@marcinmajkowski, yeah, I think you are right, should use mustBeUpdate.

[brandonroberts]

Thanks @marcinmajkowski, it's yours to fix if you want it.

[krisjobs]

I'm heaving a similar issue with a call to mustBeUpdates()

Error: Hero EntityAction guard for "[Hero] @ngrx/data/entity-cache/save-entities": payload , item 1, has a missing or invalid entity key (id)

Even though the type checker yields no errors.

There is a stackblitz to reproduce (it's based on @marcinmajkowski's stackblitz)
https://stackblitz.com/edit/angular-4lxmjk-gbpd1g (EDITED)

[marcinmajkowski]

@krisjobs you most likely shared wrong stackblitz since url is same as mine.

[krisjobs]

Sorry -> https://stackblitz.com/edit/angular-4lxmjk-gbpd1g

I could also reproduce the error when the metadata's selectId is active.

I also find it strange, that entityCollectionService update() uses Partial< Hero >, while the changeSetItemFactory/entityCacheDispatcher update() uses Update< Hero >

[krisjobs]

@marcinmajkowski may I have a look on your branch?

[marcinmajkowski]

@krisjobs I am not sure which branch do you mean. This is where I have implemented a fix (one line changed): https://github.com/marcinmajkowski/platform/tree/2059 .

[marcinmajkowski]

@krisjobs I was able to update multiple entities at once using following code:

    const entities = [
      { _id: 123, name: 'XXX' },
      { _id: 456, name: 'YYY' },
    ];

    this.heroService.updateManyInCache(entities);

There was no need for accessing dispatcher directly and no error occured.

I was also able to dispatch your changeSet by adding id property to changes object.

[JiaLiPassion]

@marcinmajkowski , you can call updateManyInCache if the data is only updated in frontend and will not be send to backend, but in the normal case, we need to use dispatcher to dispatch action for update both backend and frontend.
And also add id property is not recommended in this case because the model doesn't have id property, to be consistent, app and ngrx/data should all use selectId function to get identifier.

[krisjobs]

Please refer to my updated stackblitz @ https://stackblitz.com/edit/angular-4lxmjk-gbpd1g -> example.component.ts

@JiaLiPassion exactly, I would like to simultaneously make multiple CRUD saves on the back end side. I haven't tested the upsert operation (maybe I can use it as a workaround) but update does not work. Add/Delete do work.

[JiaLiPassion]

@krisjobs, yeah, maybe upsert has the same issue.

[krisjobs]

@JiaLiPassion I've just tested it and it turns out dispatcher->upsert works properly, as it expects T[] and not Update[] as argument.

It's just dispatcher->update that isn't working

[JiaLiPassion]

@krisjobs , thanks for the clarification!

[krisjobs]

I just found my problem was related to the fact, that update.changes should contain the id as well (even though it is already available as update.id.

as explained here:

Update is an object with a strict subset of the entity properties. It must include the properties that participate in the primary key (e.g., id). The update property values are the properties-to-update; unmentioned properties should retain their current values.

This is due to the fact update objects get flattened: updates.map(u => u.changes) in the DefaultDataService