Remove maximum number of posts that can be loaded in ReaderPostAdapter #20958
Conversation
Generated by 🚫 Danger |
|
 |
App Name | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr20958-ed269ed | |
| Commit | ed269ed | |
| Direct Download | jetpack-prototype-build-pr20958-ed269ed.apk |
|
 |
App Name | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr20958-ed269ed | |
| Commit | ed269ed | |
| Direct Download | wordpress-prototype-build-pr20958-ed269ed.apk |
|
|
I believe most of the issues from the original GH issue were previously fixed thanks to your work on using dates instead of strings as well as some other minor changes we did throughout past Reader projects, so removing this limit should make sure we have an infinite list of posts as long as the user keeps scrolling and there are posts available in that feed. 🙇🏼 That said, I still think there's some issue going on on mobile, check the rest of this comment below:
Not sure which tag you used, but could you have reached the last post from the tag you were testing? I tested with a As mentioned in the original issue (#19811) for tag/topic feed, the field used for the App Last Post for App Inspector (last post highlighted): On the Web I was able to get past it. Note that it looked stuck for a bit when I reached that same post, but after I scrolled up and down again, the posts loaded below it as expected. Also notice that the web says "Invalid Date" for the post date. |
There was a problem hiding this comment.
Look at my previous comment, I ended up sending it without the Code Review, but it has all the information about what I investigated with your changes.
I think the changes as-is have value, removing this unnecessary soft-limit that exists on the Android Client, but there is still some issue going on related to fetching by date, though it does look like an issue in the data retrieved by the backend, so I'm not sure what we can do from a client-side. Maybe we can try to figure out what the web does, since in the web they seem to have some workaround that "invalid date" issue (see #20958 (comment)).
|
Thanks for the review and investigation, @thomashorta ! I agree this PR provides some value by removing the maximum number of posts to be loaded but it doesn't fully address the problem. Since it seems like the web is doing a workaround for invalid data returned by the back-end and we'll need to understand what they're doing, I propose we merge this PR and create a new issue/update the existing issue including your investigation. WDYT? |
Fixes #19811
I wasn't able to reproduce this part. I suspect it was fixed by this PR.
By removing the
mCanRequestMorePostsTemplogic (originally implemented in this PR) the unfollowed tag post list loaded a few more items, but eventually it stops. I thought it was something in the client, but this also happens in web which makes me think it's something in the back-end that eventually stops sending us more items.To Test:
See the issue
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR Submission Checklist:
RELEASE-NOTES.txtif necessary.Testing Checklist (strike-out the not-applying and unnecessary ones):