h
h copied to clipboard
Replies showing "Message not available" in the sidebar
Steps to reproduce
- Open this direct link: https://hyp.is/AVRP4RFaH9ZO4OKSl57O/www.digitalpedagogylab.com/hybridped/teaching-in-our-right-minds/
- Open the same as a standalone link: https://hypothes.is/a/AVRP4RFaH9ZO4OKSl57O
- Expose the replies in each.
Expected behaviour
The threads in both should be the same.
Actual behaviour
Inside the sidebar, many of the replies are collapsed with "Message not available".
Additional details
If you watch the annotation load in the sidebar via the direct link you'll see the reply count pause for about 15 seconds at "2" and then it goes to "7". It's kind of like it's waiting for some of those missing replies and then gives up.
The reply count changing over time is a separate issue to do with the mad way that the client side code computes the reply counts to display.
I think the "Message not available" is because we have a hardcoded limit of 100 replies. So if a page has more than 100 replies, counting all replies to all annotations on the page (within the selected group) not just replies in a single thread, then you'll get some "Message not available"s.
I think the proper fix for this is to design a new API that has a way of paginating through the replies to a single annotation. The initial request for all annotations of this page in this group would receive the top-level annotations and the first 10 (say) replies to each, as well as an indicator of whether each annotation has more replies not loaded yet, then in the UI there'd be a "Load more replies" button that sends a new API request to get the next "page's" worth of replies (or instead of a button that would happen automatically on scroll or thread expand; or instead of that you'd get a link to the top level annotation's individual page showing the entire thread with infinite scrolling or "Load more" buttons...). So it involves some non-trivial UI and API re-design.
A quick fix would be to increase the hardcoded limit of 100 replies per API request.
Ok, separately we recently clamped the limit on the number of annotations that can be returned per search request to 200. That's for the top-level annotations, not the replies. The replies are already clamped to 100.
Presumably we clamped the top-level annotations to 200 because we felt that more than that was too hard on the server. So presumably we wouldn't want to increase the limit on the replies to higher than 200 either.
@nickstenning @chdorner ?
Increasing the 100 limit to 200 will alleviate this problem a little bit but it's far from a fix.
When there are > 200 top-levels I think the client makes multiple API requests until it has them all, so you don't get any top-level "Message not available"s. But the client does not make multiple requests for replies when there are > 100/200 replies (and it cannot: the API doesn't support that).
Basically it looks to me like we can alleviate this slightly by increasing 100 to 200 but anymore will be too hard on the server so we'll need to fix the problem properly instead.
But the client does not make multiple requests for replies when there are > 100/200 replies (and it cannot: the API doesn't support that).
Well actually there may be a way to do this with the existing search API but there isn't a particularly nice API for it.
Sure, raising the limit to 200 for replies should be alright for the time being. The real fix though is to sort out how replies are being fetched and presented in the API response. Especially pagination should be possible, so the client can load more without putting too much strain on the server.
Personally, I think this is important enough to fix properly. Have a look at the number of times we're hitting the "too many replies" warning in Sentry.
Fixing it properly would mean building the annotation threads on the server and getting rid of this ridiculous _include_replies
hack that we have that was always intended to be temporary.
By all means bump the limit to 200 for now, but we should follow that up with a real fix.
Yep, but I think the real fix requires some design for how it should work both server and client-side. I'm happy to have a think about that and send an email. In the meantime I'll increase the limit..
I'll reopen this so we don't forget about properly fixing the replies issue. The fix that is going out today will only work when there are less than 200 replies.