Align notification responses format

[benoitvidis]

fixes #146
⚠️ taken from #42

Notifications

Document notifications

• match on write controller

The result entry is replaced with a document one, which is a KuzzleDocument object:

{
  "error": null,
  "status": 200,
  "roomId": "91088e3125a9794073bdcf7b882bb39c",
  "requestId": "e548ab40-1d99-4c86-8afc-36fd5638bb5b",
  "index": "index",
  "collection": "col",
  "controller": "write",
  "action": "create",
  "protocol": "websocket",
  "timestamp": 1479899117280,
  "metadata": {},
  "scope": "in",
  "state": "done",
  "room": "91088e3125a9794073bdcf7b882bb39c-12213aafe59a4f4a238d00903d03bfb8",
  "document": {
    "collection": "col",
    "id": "AViQ3Hbm_UlxC0CPmdJ2",
    "content": {
      "foo": "bar",
      "_kuzzle_info": {
        "author": "-1",
        "createdAt": 1479899117284,
        "updatedAt": null,
        "updater": null,
        "active": true
      }
    },
    "headers": {}
  }
}

Users notifications

The result field is removed, replaced by a count property. Previously, the roomId was part of the result element, which is now discarded as this information is already available to the user.

{
  "error": null,
  "status": 200,
  "roomId": "91088e3125a9794073bdcf7b882bb39c",
  "requestId": "52ea3832-9e0a-443c-85a5-c1330c24f111",
  "index": "index",
  "collection": "col",
  "controller": "subscribe",
  "action": "off",
  "protocol": "websocket",
  "timestamp": 1479899263293,
  "metadata": {},
  "room": "91088e3125a9794073bdcf7b882bb39c-12213aafe59a4f4a238d00903d03bfb8",
  "count": 1
}

Other (token expired) notifications

The only other notification is the token expired one, which does not include any result element.

Other change

KuzzleDocument

• kuzzle and dataCollection properties are not enumerable anymore.

⚠️ For the time being, the changed property is not implemented as it requires some extra development from Kuzzle side.

[codecov-io]

Current coverage is 99.63% (diff: 100%)

Merging #147 into develop will decrease coverage by 0.05%

@@            develop       #147   diff @@
==========================================
  Files            16         16          
  Lines          1644       1652     +8   
  Methods         265        265          
  Messages          0          0          
  Branches        434        436     +2   
==========================================
+ Hits           1639       1646     +7   
- Misses            5          6     +1   
  Partials          0          0          

Powered by Codecov. Last update 8f655b6...f0f1841

[scottinet]

I'm not at ease using a KuzzleDocument for notifications.

It works well for document notifications, but I see 2 problems:

• we'll soon add an "updated" field containing only the document changes, used by observers frameworks to quickly update user interfaces. This does not fit well with KuzzleDocument objects
• I think it's misleading to use a KuzzleDocument object for message notifications, as these do not involve documents at all

[xbill82]

I agree with @scottinet 's remark. It's handy to have a KuzzleDocument instance right in the Notification instance, but only when it's semantically consistent. Didn't we have a workshop on this topic? Do we need another one?

[benoitvidis]

I dont think we had a workshop and I believe we need one.

[benoitvidis]

Still need to discuss it, but some side thoughts included on the update case kuzzleio/kuzzle#485

[benoitvidis]

Following our discussion:

Notification objects will be extended this way:

• A new type property will be added to the notification object, containing a string (user or document)
• A new property named as the type value, containing either a KuzzleDocument or a User status pojo
• In case of document update notification, the KuzzleDocument object will be extended with a pojo changed object that lists only the actually updated fields.

i.e.

{
  "type": "document",
  "error": null,
  "status": 200,
  "roomId": "91088e3125a9794073bdcf7b882bb39c",
  "requestId": "e548ab40-1d99-4c86-8afc-36fd5638bb5b",
  "index": "index",
  "collection": "col",
  "controller": "write",
  "action": "update",
  "protocol": "websocket",
  "timestamp": 1479899117280,
  "metadata": {},
  "scope": "in",
  "state": "done",
  "room": "91088e3125a9794073bdcf7b882bb39c-12213aafe59a4f4a238d00903d03bfb8",
  "document": {
    "collection": "col",
    "id": "AViQ3Hbm_UlxC0CPmdJ2",
    "content": {
      "foo": "bar",
      "_kuzzle_info": {
        "author": "-1",
        "createdAt": 1479899117284,
        "updatedAt": null,
        "updater": null,
        "active": true
      }
    },
    "changed": {
      "foo": "bar"
    },
    "headers": {}
  }
}

Other discussions

• document delete notifications should also include the deleted document (@tobe done on Kuzzle side)

[xbill82]

🤔

[xbill82]

Does not seem to match the PR description.

The result field is removed, replaced by a count property.

Here, the result field is replaced by a user property, seemingly containing a count property.
Am I next to the plaque?

[ballinette]

Same question for me, but I did not follow the discussion about that, so I may be wrong...
Can you either fix that, or fix the PR description, depending what is really wanted ?

[benoitvidis]

The initial PR description has been kept for history but the actual implementation matches the one we have aggreed on.

So, no result anymore, but a new field named as the new type property. In this case, it is normal we have a user.

[benoitvidis]

For the ref: here is the updated one: #147 (comment)

[xbill82]

@benoitvidis I let you resolve the conflicts before merging to develop.

[xbill82]

This should be merged, no?