umputun / remark42

comment engine
https://remark42.com
MIT License
4.83k stars 375 forks source link

As a user, I want to be notified about actions on my comments #111

Closed misterioss closed 5 years ago

misterioss commented 6 years ago

I really miss features from Disqus where I can see:

Guria commented 6 years ago

Hmm, remark doesn't have any ways to sent you notifications as far as I can see.

umputun commented 6 years ago

@Guria the only notification you can get now are RSS based, i.e. pull, for site updates and post updates. You can feed them into IFTTT and make tweets, telegram, slack messages or anything else you like.

@misterioss probably I can add "user RSS" to report replies, but votes summary is more complicated, and I don't think anyone actually doing it. I don't recall any notification on votes from disqus, reddit or hackernews.

also, see https://github.com/umputun/remark/issues/15

akosourov commented 6 years ago

@umputun I have a few questions, it relates what we talked at #125

  1. For example a user posts a comment. Then somebody replies him with comment A. And user wants to be notified, that is ok. But if another one replies to comment A (not to initial comment) what should happen? Should remark notify user or not? This is reply to user's reply
  2. I think remark should notify user from replies to all his comments, not for last N. I mean it will better :) what do you think?
umputun commented 6 years ago

For example a user posts a comment. Then somebody replies him with comment A. And user wants to be notified, that is ok. But if another one replies to comment A (not to initial comment) what should happen? Should remark notify user or not? This is reply to user's reply

We can simplify this thing logically and technically if we consider direct replies only. In this case, we could match by parent-id (pid) which should be relatively straightforward.

I think remark should notify user from replies to all his comments, not for last N. I mean it will better :) what do you think?

RSS will be limited anyway. If the user got 1000 replies during his rss-update window we won't be able to serve such RSS with 1000 items. I don't think this will be any real issue if we set max. items for such rss to 100 and hopefully client-side (rss reader, ifttt, etc..) will pull it every 5 minutes.

akosourov commented 6 years ago

@umputun First one is ok, I got it.

I think remark should notify user from replies to all his comments, not for last N. I mean it will better :) what do you think?

I mean about search implementation details. I can get last K user's comments from users bucket and then find replies for them from posts bucket. Then sort and limit up to maxRssItems. But if a last reply was added to K+1 user's comment this query doesn't search it. And a user won't be notified. So to find last replies for all user's comments K limit should be increased to length of all user's comments. And I think it is not good, because this search will full scan all posts bucket. So if this point makes sense I can offer to implement replies bucket to store replies to a user. Like a users bucket where userID will be a (sub)bucket with ts -> replyID to this user

umputun commented 6 years ago

Ok, I see what you mean. K+1 will be missed in the case you have described, agree.

Technically speaking, implementing replies bucket (probably it should be replies_to) with userID as a first level key holding bucket as a value, and inside of this bucket something unique (comment time?) as a key and comment meta as a value, can work. The question I have - what will be a good key and what should be the struct for value? Probably you can make this value similar to the PostInfo with Locator, User, ParentID, and Timestamp to make this new bucket flexible enough for any reply-related extraction.

From the other hand, adding such replies_to "index" and extending engine interface for this particular use case feels a little bit too much work/change for such enhancement. I think the better way is to keep engine and buckets untouched and figure a way to retrieve replies by leveraging the exiting interface. If you change the logic described in #125 and start with K last comment for the site instead of user comments, it can be done just on the service level without touching the engine. I mean:

I think this way you can get a sorted stream of replies with linear performance. In addition, you can break the loop the moment you got N results (max. number of RSS items) or reached time-stamp outside of allowed duration (like 30 mins or so).

akosourov commented 6 years ago

That's great! I had thoughts about Last bucket, but I was confused because I was thinking about this implementation as a "static" endpoint, that must return last replies always and not depending on time. I mean the case when first pull retrieves last N replies and after amount of time when newer comments/replies adds (not for that user) and window with height K moves upper so the second pull can retrieve less replies. And it should mean for rss reader that newer replies were not added (not that replies were deleted), yes?

umputun commented 6 years ago

with #145 merged we have GET /api/v1/rss/reply?site=site-id&user=user-id in place. Now we need a third RSS option in "Subscribe to this ..." on UI side.

umputun commented 6 years ago

@Guria @igoradamenko - I'm not sure if we need another link or dropdown list or something else.

akosourov commented 6 years ago

@umputun I'm not sure that it makes sense. But if a reply is editted cache won't be flushed. So the next pull from reader (to get replies to this user) may retrive old version of reply.

umputun commented 6 years ago

@akosourov update invalidates several caching scopes - locator.URL, "last" and user.ID

In order to register reply cache with proper scopes, I have added "last" to its cache's scope keys and this should do it. s.Cache.Get(cache.Key(cache.URLKey(r), siteID, "last")

jackburg commented 6 years ago

@umputun, @akosourov maybe Chrome Push Notifications could be helpful in this scenario? (https://developers.google.com/web/fundamentals/codelabs/push-notifications/).

umputun commented 6 years ago

pinging @Guria

We want one more option for RSS subscription. I think we better put them under dropdown like other elements with multiple choices

s5jfs_20180725_112414 jpfxc