near / near-discovery

The homebase for Near Builders
https://dev.near.org
The Unlicense
49 stars 71 forks source link

Push Notifications do not contain value.item.BlockHeight #658

Closed gagdiez closed 1 month ago

gagdiez commented 1 year ago

When a user is notified on near.social, the notification includes an item that contains:

Here is a complete notification, as retrieved by near.social:

{
 "accountId":"dev-support.near",
"blockHeight":102599246,
 "value": {
    "type":"like",
    "item": {"type": "social", "path": "gagdiez.near/post/main", "blockHeight":91513063 }
}

Currently, our push notifications are missing the value.item.blockHeight, which is important to know which element is being "liked" (or acted upon)

{"id": 44014, "blockHeight": 102599246, "initiatedBy": "dev-support.near", "itemType": "social", "message": null, "path": "gagdiez.near/post/main", "receiver": "gagdiez.near", "valueType": "like"}

One would expect to either get all the item information, or the notification blockheight. It seams that currently we have a mixture of both.

charleslavon commented 1 year ago

@gagdiez our queryAPI indexer for notifications does store the blockheight. Are you suggesting the actual push event which is sent to a client should contain the blockheight as well?

gagdiez commented 1 year ago

@charleslavon indeed, without the value.item.blockheight we are saying "somebody liked one of your posts", if we include the value.item.blockheight then we say "somebody liked this specific post"

amirsaran3 commented 8 months ago

The blockHeight property is only being added to the item property, so there is no mixture and confusion of blockHeight being in two places.

"Somebody liked the specific post" also is working fine since in the notification we can see who liked our post.

The "Post not found" issue is still happening when we try to view the post after clicking specific like action in notification. It seems to be a problem with the indexer.

gagdiez commented 8 months ago

There is no mixture and confusion, it is clear that they are two different things:

value.blockheight

Is the blockheight in which the notification was indexed. Knowing this blockheight we still know nothing about the item we are being notified about, we need to call the indexer to retrieve the whole notification: Social.index(action, key, blockheight).

value.item.blockheight

Is the blockheight in which the item - e.g. post, like, etc - we are being notified about was created. Knowing the path and this blockheight allows us to link directly to the item. Notice that the urls to a post only need an username and a blockheight:

In conclusion

In the first case, we need to contact the indexer. In the second one, we don't. In my personal opinion, not having the indexer as a dependency is a huge win.

amirsaran3 commented 8 months ago

The current implementation is correct. There is a value.item.blockHeight property when liking.

gagdiez commented 8 months ago

You can see in line 92 that it is not:

In my service worker, when somebody likes one of my posts, I get this info:

[Service Worker] Push had this data: "{"id":137892,"blockHeight":111288603,"initiatedBy":"dev-support.near","itemType":"social","message":null,"path":"gagdiez.near/post/main","receiver":"gagdiez.near","valueType":"like"}"

The blockHeight represents the block in which the notification was created. Therefore, I need to query the indexer in my service worker to know what the notification is about.

The current notification tells me that somebody liked one of my posts, but it does not tell me which post was liked (in this case, the post uniquely identified by the tuple: {accountId: gagdiez.near, blockHeight: 102378870})

charleslavon commented 7 months ago

@gagdiez let's consider this for a future sprint

shelegdmitriy commented 7 months ago

@gagdiez you're totally right. I checked that on my fork of notifications indexer and added a new field to that. I'll update the original indexer and ping you

shelegdmitriy commented 7 months ago

Hey @gagdiez! The notifications indexer updated! (updated on Tuesday) You can check the indexer logs or try to run some query in hasura. I called that field actionAtBlockHeight. I don't think we want to re-collect all the data again so only the fresh notifications will have a value. All the previous records will have actionAtBlockHeight: null as a default value. Please let me know if you have any questions.

gagdiez commented 7 months ago

@shelegdmitriy perfect, will test it after EthDenver, thank you so much!