The parts where the obvious implementation is wrong, and why.
The password is excluded twice, because one exclusion cannot see the other
The schema marks it select: false, so no ordinary query returns the hash even if someone forgets to exclude it. Login opts back in explicitly, at the one place that genuinely needs it — the exception is visible in the code rather than assumed.
// Models/User.js
password: { type: String, select: false }
// auth.controller.js — the one deliberate opt-in
const user = await User.findOne({ email }).select("+password -__v");That protection has a hole, and it is not obvious. select is a Mongoose feature, and the recommendation query is not a Mongoose query — it is an aggregation pipeline handed straight to MongoDB. $sample returns whole documents, hash included, and the schema never gets a say. So the pipeline strips them by hand:
// aggregates don't trigger mongoose selects
const sanitizedUsers = recommendedUsers.map((user) => {
const { password, ...safeUser } = user;
return safeUser;
});One friendship, two representations
The same relationship is stored as a FriendRequest document and as an entry in each user’s friends array. That is duplication, and it is deliberate.
They answer different questions. The document answers how did this edge come to be — who asked, who accepted, when — which is what the notifications page is built from. The array answers who are this user’s friends right now, which is asked on nearly every screen and wants to be one indexed lookup, not a scan over a request collection in two directions.
The cost is that they can drift, so every transition writes both, and both sides of every array.
Declining and cancelling are the same row
Accepting is asymmetric — only the recipient may do it, and the sender gets a 403. But deletion is not, because one document serves two features: the recipient declining, and the sender changing their mind.
// Both the sender (cancelling) and
// recipient (declining) may delete it
const isInvolved =
friendRequest.recipient.toString() === req.user.id ||
friendRequest.sender.toString() === req.user.id;
Writing the recipient-only check here by symmetry with accept would have quietly removed the ability to withdraw a request.
One socket, one owner
Online dots need the same Stream connection the chat is already using. Opening a second one works in development and is wrong everywhere else — two sockets per tab, two presence streams, and a race over which one disconnects last. The presence hook attaches to the existing client instead, and its cleanup deliberately does less than it looks like it should:
client = StreamChat.getInstance(STREAM_API_KEY);
if (!client.userID) { /* connect only if nobody has */ }
return () => {
mounted = false;
handler?.unsubscribe?.();
// Do NOT disconnect the client here — ChatPage owns the connection
};A hook that borrows a resource has to unsubscribe from it without closing it. Ownership is a decision someone has to make explicitly, and the comment is there because the correct code looks like an omission.