Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Tracked by #621
gagdiez opened this issue Oct 4, 2023 · 10 comments
Open
Tracked by #621

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

gagdiez opened this issue Oct 4, 2023 · 10 comments
Assignees
Labels
bug Something isn't working. Highlights a PR's description in the 'Fixes' changelog section

Comments

@gagdiez
Copy link
Contributor

gagdiez commented Oct 4, 2023

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

  • the type of the notification (e.g. like)
  • the path to the element being liked
  • the blockHeight that represents such element

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.

@gagdiez gagdiez added the bug Something isn't working. Highlights a PR's description in the 'Fixes' changelog section label Oct 4, 2023
@gagdiez gagdiez changed the title Push Notifications do not contain value.BlockHeight Push Notifications do not contain value.item.BlockHeight Oct 4, 2023
@charleslavon
Copy link
Contributor

@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
Copy link
Contributor Author

gagdiez commented Oct 4, 2023

@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
Copy link
Contributor

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
Copy link
Contributor Author

gagdiez commented Jan 24, 2024

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:

  • near.org/s/p?a=gagdiez.near&b=91513063
  • near.social/mob.near/widget/MainPage.N.Post.Page?accountId=gagdiez.near&blockHeight=91513063

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
Copy link
Contributor

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

@gagdiez
Copy link
Contributor Author

gagdiez commented Jan 25, 2024

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
Copy link
Contributor

@gagdiez let's consider this for a future sprint

@shelegdmitriy
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor Author

gagdiez commented Feb 22, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. Highlights a PR's description in the 'Fixes' changelog section
Projects
Status: Done
Development

No branches or pull requests

7 participants