Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

Follow refactor #676

Merged
merged 14 commits into from
Aug 18, 2017
Merged

Follow refactor #676

merged 14 commits into from
Aug 18, 2017

Conversation

rmisio
Copy link
Contributor

@rmisio rmisio commented Aug 17, 2017

This PR refactors the following / follower functionality as follows (see what I did there?):

  • puts in faux pagination, so profiles aren't fetch for an entire list of followers / followings
  • fetches the profile using async and usecache flags so the request aren't jamming up the xhr request queue
  • since other nodes following / followers list are cached, the follow data of our own node will be used to adjust the list as appropriate (at least as it pertains to our own node showing up or not on a list)

closes #200

} else {
profile = new Profile({ peerID: guid });
this.profileFetch = profile.fetch();
this.profileFetch = getCachedProfiles([guid])[0];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to use the cached profiles after all. The latency improvement is substantial. And... even having the freshest moderator info doesn't really solve the issue. After a mod is added, whether it's 1 minute, 1 hour or 6 months later, that mod can change their terms and the users using that mod would have no idea. Perhaps it's as simple as sending a notification to users of a mod if the mod changes their terms? Seems like a bigger UX discussion we could circle back to.

@jjeffryes
Copy link
Contributor

The connected peers page looks a little weird now.

image

@jjeffryes
Copy link
Contributor

I made a quick patch to apply the same changes to the connected peers page. #688

@jjeffryes
Copy link
Contributor

Edit: deleted my comment about the following, I got tripped up by Mike following me back within seconds of me following him.

Apply the same changes to the connected peers page.
Copy link
Contributor

@jjeffryes jjeffryes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. A few minor things.

@@ -1,10 +1,10 @@
/* Used for as a list item of both follower and following lists */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"used for as a list" sounds like a typo?

}
}

className() {
return 'userPageFollow flexRow';
renderUsers(models = [], insertionType = 'append') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does models need a default if we throw an error if the models aren't passed in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't hurt. If they passed in null, it would default it.

this.miniProfile.setState({ followsYou: data.followsMe });
}

if (this.followerCount === 0) this.followerCount += 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the followerCount being changed when checking if they follow you? That should affect the following count instead.

Copy link
Contributor Author

@rmisio rmisio Aug 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix that. It should be if you're on someone else's user page and they are following you, but their cached following count is 0, it should be 1 (because we know they're following at least one person).

@jjeffryes jjeffryes merged commit 42108a2 into master Aug 18, 2017
@jjeffryes jjeffryes deleted the follow-refactor branch August 18, 2017 22:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Home: Follower count incorrect
2 participants