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

Marking feed notifications as read/seen does not return updated data #423

Open
mboraski opened this issue Feb 24, 2021 · 12 comments
Open

Marking feed notifications as read/seen does not return updated data #423

mboraski opened this issue Feb 24, 2021 · 12 comments

Comments

@mboraski
Copy link

mboraski commented Feb 24, 2021

  • Version: 5+
  • System: Node.js

What steps will reproduce the bug?

Mark individual or groups of notifications as read/seen returns unchanged data

client.feed('notification', feedId).get({ limit: 10, mark_read: notificationId })
client.feed('notification', feedId).get({ limit: 10, mark_seen: true })

or directly...

https://api.stream-io-api.com/api/v1.0/enrich/feed/notification/[userId]/?api_key=[key]&limit=10&mark_read=0fd5ag06-7677-11eb-8080-800096ace01e.like_2021-01-13

What is the expected behavior?

Endpoint returns data with specific notifications marked true
is_read: true

What do you see instead?

is_read property is still false. A second call for fresh notification data is required.

Screen Shot 2021-02-24 at 1 02 18 PM

Additional details

Seeing the same behavior for marking notifications "seen"

gz#9687

@shodgetts
Copy link

Agent comment from Anders Lund in Zendesk ticket #9687:

Hi there,

This is expected behavior. As per the docs:

The next time the feed is read, the seen and/or read status will be updated accordingly. Please keep in mind that the first call will always return the read and seen state prior to the change. This allows you to read a notification feed, mark activities as read or seen and display them according to the current state in a single API request.

I'll link you to this section here: https://getstream.io/activity-feeds/docs/javascript/flat_feeds/?language=javascript.

@mboraski
Copy link
Author

mboraski commented Feb 24, 2021

Thanks for the quick response. Question though...marking the notifications as changed means they are now different, and means I as the client know the old AND new data. I don't see a reason to need the old data again. If I'm marking something as changed, that means I have what I need on my end, but I want the data provider to send me its latest. I don't see a purpose for telling the server the data has changed (meaning I know the old AND new data) just to have the server send me the old data again. Please help me understand a use case for this design. Thank you.

@mboraski
Copy link
Author

Side note: It seems that a PUT or POST might make more sense than a GET here to make changes to activity data. If you don't mind, I'd love to understand this design choice also. Thank you, we are designing our api to work well with yours or others, so we want to be explicit when we can.

@micha3ldavid
Copy link

micha3ldavid commented Feb 24, 2021

Hi guys! Glad we're opening a discussion around this (:

@shodgetts I am also curious as to why this endpoint behaves in this manner. From an implementation perspective, if I make a request and explicitly set mark_read or mark_seen to true I would expect the response to contain the next state of the data, not the previous state.

Echoing @mboraski 's last comment, since this endpoint is modifying data, I too am curious why the choice to make it a GET was used over a PUT. It seems more scalable and less complicated to separate fetching from updating.

Lastly, based on some documentation we found, it seems this endpoint can also accept a list of ids. Is that correct? If so, we are seeing some strange responses, where we send it a list of ids to mark_read and mark_seen but the response contains the latest 25 results, not the list of objects we specified. If this is not the endpoint we should be using to update a list of notifications from mark_read or mark_seen to true please let us know which endpoints we should be using.

Thanks!

@tbarbugli
Copy link
Member

@micha3ldavid the reason a feed.get with mark_read=true returns the previous state is to avoid calling the API twice (read and mark). Facebook's notification dropdown is a good example, it shows the current count of unseen posts and as you click on it you can see the ones that are counted as unseen + dismiss their state.

@micha3ldavid
Copy link

micha3ldavid commented Feb 25, 2021

@tbarbugli Sorry, it still doesn't make sense to me.

If I make a request to fetch a list of notifications along with the unseed and unread fields, I am not going to pass the mark_read=true or mark_seen=true flag to the endpoint because the user has not seen or read those notifications yet. The user may never decide to see or read those notifications and so I do not want to make the assumption they will, but I do want to render the UI and the unseen/unread count for them. So, my first request will be without the mark_read or mark_seen flag. If/When the user decides to open the dropdown and see/read the notifications then I would like to mark them as read/seen and I would expect the most up-to-date state of the data to be returned because I explicitly asked the endpoint to update the data. Asking an endpoint to perform an action and not returning the results of that actions is not only confusing but problematic.

This endpoint also falls short when I want to set a single notification to read or seen. If I pass mark_read=<notification_id> to the endpoint it returns me an entire list of recent notifications again (along with the incorrect previous state mentioned above). I don't actually want an entire list of notifications I just want the one notification I requested to be marked as read.

Personally, I think this endpoint is trying to do way too much and makes too many assumptions. It forces frontend applications to conform to its very restrictive design pattern rather than allowing frontend applications the flexibility to make their own design and state management decisions. This is one of the major reasons the separation of GET from PUT was created. The fact that this endpoint merges these two concepts together and attempts to help manage a frontend application's state is not only confusing but concerning for me.

Due to the implementation of this endpoint, I would rather not use it for anything other than fetching the most recent list of notifications. Do you have alternative endpoints we can use? I see little documented about notifications. I know that notifications are just a type of feed but they have special behavior associated with them and their data is shape differently. Ideally, we are looking for a way to:

  1. Fetch the most recent state for a list of notifications
  2. Mark a single notification as read/seen and return the most up-to-date state for that notification
  3. Mark an array of notifications as read/seen and return the most up-to-date state of that array of notifications

Alternatively an endpoint that returned the number of unseen/unread notifications would better enable us to use your design pattern. Our UI renders a bell icon with the number of unread recent notifications so we need that data before the user ever interacts with the dropdown, which is why we make the initial request for the notifications without the mark_seen mark_read flags. If we had a way to fetch the number of unread/unseen notifications we could delay the request for recent notifications until the user opened the dropdown, allowing us to set the mark_seen and mark_read flags and leverage your endpoints current behavior.

@shodgetts
Copy link

shodgetts commented Mar 2, 2021

Hello - Stephen here from the Stream Customer Success department. Thank you for all of the feedback - we've been keeping an eye on this issue. Are you currently a Stream customer? If so, do you mind please sending your organization id? You're also welcome to email support@getstream.io too.

@mboraski
Copy link
Author

mboraski commented Mar 9, 2021

Hi @shodgetts, thanks for reaching out. I'm a little disappointed you don't remember Michael and I. We all had a video zoom meeting a little over a month ago. Yes we are a Stream customer. I don't feel comfortable sending the organization id over a public forum, but we can chat through support. I already opened a support ticket through the email you mentioned on February 24. I had a chat with Anders Lund. Please check your support history for details.

@mboraski
Copy link
Author

mboraski commented Mar 9, 2021

Also, do you have a client that is consuming/using this endpoint in the way you have it implemented? We would love to ask them about it and learn if there is some benefit we just are not seeing. Thank you.

@shodgetts
Copy link

Agent comment from Stephen Hodgetts in Zendesk ticket #9687:

Hello! Sorry, I did not recognise you from your Github handle! Thanks for sending over the support request and I'm glad Anders has been of help. I believe your account manager is following up on your more recent question, that's a little out of the hands of the engineering team.

@tkrevh
Copy link

tkrevh commented Sep 1, 2021

We are experiencing the same issue. Our current workaround is to call the feed.get(mark_seen=True) API twice.
If we don't want to change the state of seen flag (just to get the unseen count), we call it with mark_seen=False anyway.

@shiivan
Copy link

shiivan commented Oct 15, 2021

We're having similar integration issues, I wonder if this could also be mitigated by adding a subscription event that pushed the lasted unseen/unread counts of the feed. (This would also allows apps to update their internal state for things like bell icons/badges indicating a user has unread/unseen notifications.)

Then the caller would get a push event of the result of the PUT masquerading as a GET. So something like:

// Realtime notification data format 
  { 
      "data": { 
          "deleted": "array activities", 
          "new": "array activities", 
          "unseen": <count integer>,
          "unread": <count integer>,
          "feed": "the feed id", 
          "app_id": "the app id", 
          "published_at":"time in iso format", 
      }, 
      "channel":"", 
  }

Though I guess this would be a breaking API change since it's possible developers would expect at least one of "deleted" or "unseen" to be a truthy array and now both could be empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants