azmenezi/capstone-back

capstone-back's Issues

CreatePost Controller

When creating a post using the createPost controller in posts/controllers, dont forget to include the newly created post in the Place instance as a relation, since you have a list of of relations within the place instance under the property of posts
Also refer to the other issues titled Post Schema and Duplicated Create Post Logic

Mood Endpoints

router.param in mood/routes.js not implemented, as there are not current routes that utilize it.

Only include router middleware such as the router.param if they are needed an utilized.

GeoLocation Format

I would suggest using the following format for storing and using locations in areas of your code like PlaceSchema and UserSchema GeoJSON as it is a format commonly used for map services as well as supported by mongoose and mongodb.

Accepting Friend requests

Since we are deleting the instance of the FriendRequest when the other user accepts, do we need the pending property on the FriendRequest schema?

API Endpoints

Best practice for apis is to include /api prior to all api endpoints. /auth --> /api/auth

Additionally, endpoints like mood and place should be pluralized.

Router Params For Chats

In the chat/routers and chat/controllers, there are several locations that use both the chatId and the placeId. We can utilize router.param middleware here reduce repetition

Could not add Labels On Issues for some reason

No sure why but for the review I could not assign labels

Delete Post

When deleting a post, remove the ids from the users relations and the place
Also, since we have the params as a middleware, we can perform the delete query on the req.post and return a response without a conditional being required in the deletePost controller. If there is no need to include a json response, we can simply use end()

Post Schema

Given what I understand from what was stated about the project, there will be a page that will display posts users generate during a check-in.

In order for this to be saleable while keeping document size in mind, it might also be worth it to have the post schema include the id of the place, since this is a social app, one would assume that eventually that 16md limit could easily be reached, since place instances has many relations with other schemas like ratings

Public Chat Create on Place Create

I would suggest moving some of the logic from the sendPublicChat in the chat/controllers logic to the createPlace controller in place/controllers. Since every created location has a public chat, we can create it when a new place is added so that it appears that its there by default. This will help reduce the number of queries and check performed when using the chat for any given location

Checking Username

The put method for the checkUsername in the user/routes.js could either be a get or post request as you are not modifying data in the database; simply returning available options and suggestions

Remove Dead Code in Chat Controller

Remove lines 7-20

Duplicated Create Post Logic

In the posts/controllers createPost controller, there is very similar logic in the place/controllers checkin controller, is this intentional?

UserSchema

Does the location need to be stored into the UserSchema or should it be in the History or Post schemas? Since there this property will always change giving the nature of the app and what its attempting to do.

Adding moods to place

There might be a need to validate if a moodDd is duplicated in the place relations and vise versa for the mood instance

Sockets IDs and Users

It would be best to maintain a map of each users id and socket id when they are connected.
Since we have private chats, we need to send data from the originating socket to the target socket of the other user with that matches the users id.

Currently, the implementation of socket.broadcast.emit would notify everyone that there is a new chat, in the case of a private chat between two people, we should be able to join to sockets together using the socket.to(otherSocketUser).emit()
Refer to the socket cheatsheet

It is also not uncommon for sockets to perform write operations when data is received by the server, most commonly saving a message to the database.

Utility Functions in the Controller

Helper functions such as getRandomNumber and vowelToNumber that are located in the user/controllers should be placed in the utils/auth directory. This will help maintain reusability and readable/clean code.

Additionally, I would also recommend putting the suggestions logic between lines 95-134 into its own function to and placed in the utilities directory, keeping it abstracted and maintainable

Auth Routes

Using best practice approaches, the endpoints for authentication or /auth should only include things pertaining to authenticating, creating auth credentials and validating user credentials.

Currently, under the auth routes, there are additional paths and logic that handle friend-requests and getting profiles. I would suggest moving these routes and controllers into another routes and controllers folder that focuses exclusively on the social aspect of the app.

Things like adding friends being its own feature separate to the auth logic.

Posts

In the posts/controllers, there is duplicated logic when attempting to fetch a post. getPostById, upadatePost and deletePost and all using req.params and performing a search query. Since we have a router.params middleware in the posts/routes, we can simply use the req.post instead of having to deconstruct the req.params and then perform a second search.

Also, upadatePost should be updatePost. If there is no need to include a json response, we can simply use end()

Creating Friend Requests

I see what you are trying to do in the commented code block on lines 153-157, where you are trying to find if a friend request exists in the createFriendRequest controller.

Since we already have the authenticated user making the request and the target user which the friend request is going to, we can perform this search directly on the FriendRequest model, with the aim to find a record of either to or from that matches bother the current user and the target user. If this exists, then the request is duplicated.

friendRequestId Route Params

Since there are routes that have a friendRequestId as a param in the user/routes and user/controllers, we can reduce code replication by having another router.param middleware that searches for the friendRequestId.

Creating Amenities

There is currently as of this commit of a3d1597c5808e05a854a6481663280c9a438eb67 no implementation of creating amenities, and I would suggest having this soon to prevent it being a blocker for other features like rating amenities.

Getting Place Details

The getPlaceById controller in the place/controllers is performing an additional find query for the req.param. The router.param middleware is already performing a search and then passing it through the request object to the controller if the id exists.

We can simply return req.place.

Getting Post By ID

Currently there is the following error

{
    "error": {
        "message": "Cannot populate path `username` because it is not in your schema. Set the `strictPopulate` option to false to override."
    }
}

In the Post schema, you have the user property, not username.

Once fixed, be sure to select only the data that is required, as you may reveal additional data about the user such as their password

Unused Conditionals

In the checkin controller located in the place/controllers, be aware that there is logic here that is either duplicated or unimplemented, starting from line 85-99

Also, if there is no need to include a json response, we can simply use end()

Deleting friend requests

In the user/routes line 69, since we are performing a delete action, the request method should also be delete

Getting users chat

Since the objective of the getUserChat controller is to find a chat with the current user (user1) and the other user (user2), we can perform a search directly on the PrivateChat model to find a chat thats members includes both user1 and user2,

This would help reduce the conditional nesting between lines 49 and 62 and negate the need for the user.chat.find iteration method.

based on the query performed on the Private chat, we only need one if statement and can still maintain a similar logic that validates if a chat exists or to create a new one