-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Complete all the basic end points and add collaborator feature. #3
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@try-catch-stack
Some general changes which I would suggest.
- Try formatting the code with prettier or any appropriate formatter.
- Remove the comments which were added for explanation to you or when work gets done. If something needs explanation, then only comment should remain there.
- The commit messages made were not good. Please see on how to write good commits. They are also essential.
I didn't check for the collaborator feature yet as it is dependent on userId
which I am unsure of how to get(can use mongodb compass but think of me as someone who doesn't know about that).
If you can, please change the logic, otherwise, I will see to it later.
todosOfUser.push({"id":todo._id,"title":todo.title}) | ||
}else{ | ||
todosCollaboratedTo.push({"id":todo._id,"title":todo.title}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@try-catch-stack
Try using spread
operator. It is faster. See here for more details.
const {title} = req.body | ||
const user = req.user | ||
console.log | ||
if(title.trim()==""){ //Checking if title is empty string(Only white space) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
""
is treated as false
value in javascript. So, there is no need for equal sign.
Better would be
if(!title.trim()){
//logic here
}
if(savedTodo){ | ||
res.status(200).json({ | ||
"id":todo._id, | ||
"title":todo.title})}else{ | ||
res.status(400).send("Title field cannot be empty.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what will happen if an error occurs here? Nothing. Handle the case for that.
const user=req.user | ||
|
||
ToDo.findOne({$or: [{_id:id,createdBy:user._id},{_id:id,collaborators:user._id}]},function(err,foundTodo){ | ||
if(err) { console.log(err)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of consoling the error, return the error with proper codes.
Here it is an internal error, so a 500
code can be sent.
if(todo.collaborators.includes(userid)){return res.status(200).send("This user is already a collaborator for the TODO.")} | ||
todo.collaborators.push(userid) | ||
todo.save() | ||
res.status(200).json({"Message":"User with the given id successfully added as collaborator.", | ||
"TodoId":todo._id, | ||
"TodoTitle":todo.title}) | ||
}else{ | ||
res.status(400).send("Error in adding collaborator.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are adding a collaborator by userID. How am I supposed to get that user ID?
Ids are generally not meant for disclosure. Since username
or email
is kept unique, try using them for addition.
Basically, what I mean is one will provide a field userName: <value>
. Find user, get that ID and use the above logic. But don't expect ID from the body.
if(!err && todo){ | ||
|
||
if(!todo.collaborators.includes(userid)){return res.status(200).send("The user you are tying to remove is already not a collaborator.")} | ||
ToDo.findOneAndUpdate({_id:id,createdBy:user._id},{$pull:{collaborators:userid}},function(err,updatedTodo){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any specific reason as to why using $pull
here but .push
in the addition of collaborators?
title: { type: String, required: true }, | ||
createdBy: { type: Schema.Types.ObjectId, ref: "User" }, | ||
collaborators: [{ type: Schema.Types.ObjectId}] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A ref
is not created here. I don't think it will work.
const router = Router(); | ||
|
||
router.get("/", ToDoController.getAllToDo); | ||
router.post("/:id/", ToDoController.createToDo); | ||
router.get("/",middleware.checkAuth, ToDoController.getAllToDo); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use router.use()
for middlewares.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked for todos and they were working nice. Good job
"TodoId":todo._id, | ||
"TodoTitle":todo.title}) | ||
}else{ | ||
res.status(400).send("Error in adding collaborator.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I add a collab via a different token, I get an error in adding collab. Explain the error. A better way would be to explain whether I am unauthorized or there is internal error.
Marks: |
CSOC Task 5 Submission
I have completed the following tasks
Task 1: Completed all the basic endpoints- Auth - Login, Register, Get Profile and Todo - Get Detail, Get List, Create, Edit(PUT and PATCH) and Delete.
Task 2: Implemented collaborator feature- The creator of a todo can add or remove the collaborators to a todo.Collaborators have the permissions to delete or edit the todo they have been added to as a collaborator. Any todo can have multiple collaborators but a collaborator cannot add or remove a collaborator from the todo.
Note: Collaborators can edit a todo only by sending PATCH requests and not PUT requests as PUT will replace the existing document with the updated one which will also remove the existing collaborators from the todo.
End points for collaborator feature:
Both the end points take user token as the authorization header with the prefix
Token
andUser id
in the request body.Add collaborator :
POST /todo/:id/add-collaborators/
Remove collaborator :
POST /todo/:id/remove-collaborators/
Request Body (Sample):
Response Body (Sample):
Response Code:
200
API Documentation
Deployed API URL: https://rest-api-todoapp.herokuapp.com/api/