Closed samuelclay closed 2 years ago
Specifically, new saved stories aren't appearing on mobile, so that means it's likely the same problem and those users have cached stories that can't be sorted.
@samuelclay In the /reader/starred_stories
response payload, feeds
is sent as an object and the app expects a list. Once that is fixed I can check the sorting issue.
feeds
should be an object, there's no order to it. I don't think we changed that recently either.
Looking through the web code I realize we have a bug in that we don't parse the feeds
key on starred story load, so if you save a story from a feed you later unsubscribe from, the saved story will show up without a feed attached.
@samuelclay There are a few pointers that indicate that this has changed recently for reader/starred_stories
.
The response POJO is shared across endpoints, reader/read_stories
and social_river_stories
return an array for feeds
.
Also this line hasn't changed in a long time https://github.com/samuelclay/NewsBlur/blob/952e6937b70e942108474715603741e3100d8797/clients/android/NewsBlur/src/com/newsblur/network/domain/StoriesResponse.java#L30
If moving forward, reader/starred_stories
will have its own separate response structure, that can be accommodated but keep in mind that all the old apps in the wild won't be able to load saved stories until they update.
keep in mind that all the old apps in the wild won't be able to load saved stories until they update.
Not great but right now current app users are having trouble, so I'd rather switch that around.
Coming back to this, the user said:
Yes it is definitely still happening. The stories are not reliably in chronological order, and as I scroll down everything jumps around and the order changes.
I do get nightly updates from Google Play so I'm pretty sure I have the current version.
Anything I can do to help ship this?
Only thing would be to update the response to return a list of feeds but it seems that isn't an option. Having a separate response just for starred stories is making the code messy and I'm looking to find the best approach to make the update.
In the end the gson type adapter had to be reworked and it wasn't invasive. It will handle feeds both as a list and a map.
@samuelclay could you smoke test the fix? It touches most of the network responses and it won't hurt to have new eyes on it.
LGTM
Is this ready for production? I'd be happy to submit it today.
@samuelclay I think it is but could you publish a beta version for 1-2 days before going prod?
Terrific, that's now in the review queue for Open Testing.
@samuelclay It looks like the beta build is ready to be pushed to production.
@samuelclay When is the change for the starred_timestamp
expected to go out?
I launched the change two weeks ago, but I found three more instances.
https://github.com/samuelclay/NewsBlur/commit/c469608fc6401ced4c2a0d3992ed48442e8a6c0d and 92f48a082
Deployed and the user just emailed me back to say it's fixed. Thanks for your help!
They aren't loading for me and I've now seen three reports of saved stories not loading for others. Also, it looks like some people can get them to load but can't change the story sort order.