Follow Method and database updates #2
Conversation
| @@ -99,6 +99,19 @@ export const getActions = ({ user, directActions, hideAdminControls }) => { | |||
| } | |||
| }; | |||
|
|
|||
|
|
|||
| const hasAlreadyFollowed = (username) => { | |||
There was a problem hiding this comment.
How about we name it / make a generic "canFollow" method? Which can be later extended to support other use cases (if any)?
| let userObject = Users.findOneByUsername(username); | ||
| if ('followers' in userObject) { | ||
| const followersObject = userObject.followers; | ||
| const { _id } = Users.findOneByUsername(Meteor.user().username); |
There was a problem hiding this comment.
You can directly use Meteor.userId()
There was a problem hiding this comment.
Meteor.userId() is the best approach here, but just for your knowledge you could have used Meteor.user()._id since Meteor.user() will return the entire user's object, so on that case you are requesting the entire object twice.
And, if you want just one field from a query, please always pass that information to the query to not transfer unnecessary data from db to rocket.chat:
Users.findOneByUsername(Meteor.user().username, {fields: {_id: 1}});| followersObject[_id] = ''; | ||
| Users.update(userObject._id, { $set: { followers: followersObject } }); | ||
| } else { | ||
| const followersObject = new Object(); |
There was a problem hiding this comment.
Why not validate at server startup and create this object there for every user to maintain consistency across all?
| // Update following keys | ||
| userObject = Users.findOneByUsername(Meteor.user().username); | ||
| if ('following' in userObject) { | ||
| const followingObject = userObject.following; |
There was a problem hiding this comment.
As the number of Following/Followers increase, it would increase in size of each user object considerably. As the User object is perhaps the most commonly fetched object for number of other purposes in all other areas of the application, we might want to structure our schema as described here. Thoughts? @bizzbyster @dtoth
Check this
There was a problem hiding this comment.
@kb0304 @fliptrail I am not an expert on the server side architecture but I tend to agree with Karan based on the link he sent. If you are still unsure we can try to get some inputs from others on the RC team. But in my opinion this seems like a good approach.
|
|
||
| Meteor.methods({ | ||
| followUser(username) { | ||
| if (settings.get('Newsfeed_enabled')) { |
There was a problem hiding this comment.
Please, never put all your code inside an if, use the if first to stop the process:
if (!settings.get('Newsfeed_enabled')) {
return false;
}
It will make the code cleaner and more understandable.
| const followersObject = userObject.followers; | ||
| const { _id } = Users.findOneByUsername(Meteor.user().username); | ||
| followersObject[_id] = ''; | ||
| Users.update(userObject._id, { $set: { followers: followersObject } }); |
There was a problem hiding this comment.
Why not use a direct update?
Users.update(userObject._id, { $set: { `followers.${ _id }`: '' } });| let userObject = Users.findOneByUsername(username); | ||
| if ('followers' in userObject) { | ||
| const followersObject = userObject.followers; | ||
| const { _id } = Users.findOneByUsername(Meteor.user().username); |
There was a problem hiding this comment.
Meteor.userId() is the best approach here, but just for your knowledge you could have used Meteor.user()._id since Meteor.user() will return the entire user's object, so on that case you are requesting the entire object twice.
And, if you want just one field from a query, please always pass that information to the query to not transfer unnecessary data from db to rocket.chat:
Users.findOneByUsername(Meteor.user().username, {fields: {_id: 1}});| // Update followers keys | ||
| let userObject = Users.findOneByUsername(username); | ||
| if ('followers' in userObject) { | ||
| const followersObject = userObject.followers; |
There was a problem hiding this comment.
Here you could prevent the code duplication by deciding like:
const followersObject = userObject.followers || {};
No description provided.