Add the fields that triggered update notifications

[Yoann-Abbes]

What does this PR do ?

See the PR title

https://jira.kaliop.net/browse/KZL-1127

How should this be manually tested?

Execute this script and see the notification:

const {
    Kuzzle,
    WebSocket
} = require('kuzzle-sdk');

const kuzzle = new Kuzzle(
    new WebSocket('localhost')
);

function callback (notification) {
    console.log(notification);
}

kuzzle.on('networkError', error => {
    console.error('Network Error:', error);
});

const run = async () => {
    try {
        await kuzzle.connect();
        await kuzzle.document.createOrReplace(
            'nyc-open-data',
            'yellow-taxi',
            'some-id',
            { capacity: 4, members: 5 },
        );
        const filters = { exists: 'capacity' };
        await kuzzle.realtime.subscribe('nyc-open-data', 'yellow-taxi', filters, callback);
        const response = await kuzzle.document.update(
            'nyc-open-data',
            'yellow-taxi',
            'some-id',
            { capacity: 100, members: 50 }
        );
        console.log(response);
    } catch (error) {
        console.error(error.message);
    } finally {
        kuzzle.disconnect();
    }
};
run();

[codecov]

Codecov Report

Merging #1364 into 1-dev will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##            1-dev    #1364      +/-   ##
==========================================
+ Coverage   93.89%   93.89%   +<.01%     
==========================================
  Files         106      106              
  Lines        7304     7306       +2     
==========================================
+ Hits         6858     6860       +2     
  Misses        446      446

Impacted Files	Coverage Δ
lib/api/core/notifier.js	73.33% <100%> (+0.4%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c37de8c...d4579ac. Read the comment docs.

[Aschen]

I didn't understand the test well I think, for me the field foo should be present in the _updatedField array no?
And here _updatedField is not an array

[Yoann-Abbes]

Here is the request sent in this test :

request = new Request({
      controller: 'document',
      action: 'update',
      index: 'foo',
      collection: 'bar',
      _id: 'Sir Isaac Newton is the deadliest son-of-a-bitch in space',
      body: {
        _kuzzle_info: {
          'canIhas': 'cheezburgers?'
        },
        foo: 'bar'
      }
    });

The updated field is _kuzzle_info as you can see

In deed it's not an array, why should it be ?

[Yoann-Abbes]

_source is the document, in the request it's updated by adding the field _kuzzle_info

[Aschen]

You cannot update the field _kuzzle_info, it's ignored by the Elasticsearch service.
I mean an array because an update request may modify several fields

[Yoann-Abbes]

Right for the array
for the _kuzzle_info field :
Since Kuzzle update _kuzzle_info automatically, do I remove it from the resulted array or not ?
['foo', '_kuzzle_info'] or
['foo']

(for now, i removed it, let me know if its more relevant to keep it)

[scottinet]

The updatedFields gives the following information: "the list of changed fields that triggered that real-time notification`.

_kuzzle_info is merely a side-effect, always updated alongside any update request, so it cannot be the cause of a real-time notification.

Ergo, _kuzzle_info should never appear in the updatedFIelds list.

[alexandrebouthinon]

You should try with a document containing more than one field. I'm not sure but I think your code just return all document fields except _kuzzle_info 😅 And you should also update the functional tests with a updated fields check 😉

EDIT: I tested with a multiple fields document and it works. But you really should update functional tests to avoid regression.

[scottinet]

(nitpicking) use string litterals