Update order and filters in lists after a document is updated

[eric-burel]

Apollo now updates the cache directly when updating a value, so values in lists are immediately updated after a mutation.

However, we still need to filter the values again and rerun the sort, because the update can have modified them.

The code already exists since it was used in the previous implementation with watched:mutations, we simply need to write a test and reenable it through a multiQueryUpdater (see the create version for inspiration).

[yannick-mayeur]

When I make an update to an item in a list for example the movie list in the Starter, first the item stays at the same place and then after a few seconds it moves to the correct position without me doing anything. Is the expected behavior that the item instantly moves to the correct position?

When I add a new item the behavior more or less the same. First the item spawns at the top of the list and then after a few seconds it gets moved to the correct position. Does this that the create is buggy too?

[eric-burel]

Beware of versions, the Datatable uses the old resolvers so they are not good for testing this. the multi.js has indeed a small bug (collection.getParameters overrides the order wrongly I think) but it's not a top priority because this code is deprecated. We use multi2.js currently, the multi.js will be deleted in a future release.

You may need to write a unit test instead or use directly "multi2" resolver in a sample package.

[eric-burel]

@yannick-mayeur See the first schema in this article to understand what happens with the Datatable: https://blog.vulcanjs.org/vulcan-js-1-14-filtering-new-apis-fc8efc0027dd

We plan to update it of course but did not have enough time during the latest release.

[yannick-mayeur]

Beware of versions, the Datatable uses the old resolvers so they are not good for testing this. the multi.js has indeed a small bug (collection.getParameters overrides the order wrongly I think) but it's not a top priority because this code is deprecated. We use multi2.js currently, the multi.js will be deleted in a future release.

multi2.js seems to be used by the Datatable in the devel branch. So if I use a 2 repo install it should use multi2.js right?

[eric-burel]

Not sure, but you may easily test that with a console.log in the multiQuery updater function

[eric-burel]

There are unit tests for the create version too that you can run, I remember unit testing the sort specifically

[yannick-mayeur]

I think that the tests related to the create version are those in: vulcan-core/test/containers2/mutations.test.js but they fail with the following error message:

I20191217-11:35:14.788(1)?   2) vulcan:core/container/mutations2
I20191217-11:35:14.788(1)?        withCreate
I20191217-11:35:14.788(1)?          multiQuery update after create mutation for optimistic UI
I20191217-11:35:14.788(1)?            add document to multi query after a creation:
I20191217-11:35:14.788(1)?      TypeError: getApolloClient is not a function
I20191217-11:35:14.788(1)?       at Promise.asyncApply (packages/vulcan:core/lib/modules/containers/create2.js:60:18)
I20191217-11:35:14.789(1)?       at /home/yannick/.meteor/packages/promise/.0.11.2.1nfm0ms.xv6b++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/meteor-promise/fiber_pool.js:43:40
I20191217-11:35:14.789(1)?    => awaited here:
I20191217-11:35:14.789(1)?       at Function.Promise.await (/home/yannick/.meteor/packages/promise/.0.11.2.1nfm0ms.xv6b++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/meteor-promise/promise_server.js:56:12)
I20191217-11:35:14.789(1)?       at Promise.asyncApply (packages/local-test:vulcan:core/test/containers2/mutations.test.js:172:17)
I20191217-11:35:14.789(1)?       at /home/yannick/.meteor/packages/promise/.0.11.2.1nfm0ms.xv6b++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/meteor-promise/fiber_pool.js:43:40

I am currently trying to find out why getApolloClient is not recognised as a function.

[eric-burel]

It does not exist server side, that's why.

This is related to a recent PR, client-side it's important to update the client and not just the cache, so that it correctly update watched queries too (basically it propagates your update correctly).

But server-side it does not make sense, because we don't have a full client with a cache and all.

2 solutions:

• We may improve create2 like this: const client = getApolloClient ? getApolloClient() || cache;
• We just don't run this test server side, you can move the code to a new test file in the client folder (you have examples of client only tests/server only test in many part of the code)

[eric-burel]

@yannick-mayeur Basically this part of the code is never supposed to run on the server, so it's my mistake to write a server side test, I think the second solution is more appropriate

[yannick-mayeur]

I managed to get the tests working one the client side with draft PR #2484 but, the tests are still failing and I am not sure what the expected behavior of this code should be.

[eric-burel]

@yannick-mayeur what are the failing tests precisely?

[yannick-mayeur]

All of those under withCreate are failling.

[eric-burel]

Fixed by #2497