Reader: Fix the url we send to the follow button in Full Post View #3418
Conversation
|
Repro'ing this is "fun". To see it, you have to have an account with at least 50 subs, preferably around 1000. Find a one of the first things you subscribed to, open the feed steam, then open a post into full post view. Reload full post view. Watch the follow button start off correct, then flop to the unfollowed state, then flip back to followed once the subscription loads in the background. |
blowery
added
[Type] Bug
|
cc @bluefuton as he may have some insight on how to best fix this |
|
|
bluefuton
force-pushed
the
fix/follow-button-in-full-post
branch
from
005e9c1 to
da28870
Compare
|
Rebased and forced. I have around 270 feeds and I'm able to repro with one of my early subscriptions, http://calypso.localhost:3000/read/post/feed/44902/935964996. Digging into it now. |
| }; | ||
| } | ||
| forEach( subscriptionsWithState, function( subscription ) { | ||
| addSubscription( subscription.toJS(), false ); |
There was a problem hiding this comment.
The toJS() isn't ideal, but addSubscription() currently expects a plain JS object.
There was a problem hiding this comment.
oof. that's a lot of mutations.
|
The fix here is to run new pages of subscriptions through |
blowery
added
[Status] Needs Review
|
Todo:
|
| forEach( subscriptionsWithState, function( subscription ) { | ||
| addSubscription( subscription.toJS(), false ); | ||
| } ); | ||
| subscriptions.list = subscriptions.list.asImmutable(); |
There was a problem hiding this comment.
I think we need to tease out what addSubscription actually does into a helper.
|
Looks good on the whole, but I found one odd quirk. After viewing a full post for an existing sub, it now pops to the top of the sub list (as seen in Manage Following): |
bluefuton
added
[Status] Needs Author Reply
and removed
[Status] Needs Review
|
Ah, nice catch. Looks like we need to sort the list on sub date client side after accepting new subs |
This still has a big problem: the following state will be correct on load, but when the first page of results come in from the subscription sync, the following state will be dropped. We need to change how we consume pages of subs in the feed-subscription-store, merging the page with current results, and possibly resorting the results by sub date.
…ubscription() to ensure existing subs are updated correctly
This still needs refactoring, but good enough for now.
blowery
force-pushed
the
fix/follow-button-in-full-post
branch
from
f6e60d9 to
cf47b98
Compare
|
Rebased and repushed. Working on fixing the bump to top. |
blowery
added
[Status] Needs Review
|
OK, ready for review again @bluefuton :) |
|
Cool! 👀 |
|
That's done the trick. 🚢 |
bluefuton
added
[Status] Ready to Merge
and removed
[Status] Needs Review
Reader: Fix the url we send to the follow button in Full Post View
This still has a big problem: the following state will be correct on load, but when the first page of results come in from the subscription sync, the following state will be dropped.
We need to change how we consume pages of subs in the feed-subscription-store, merging the page with current results, and possibly resorting the results by sub date.