Skip to content

Implement server#1

Open
yulsmir wants to merge 37 commits intomasterfrom
implement-server
Open

Implement server#1
yulsmir wants to merge 37 commits intomasterfrom
implement-server

Conversation

@yulsmir
Copy link
Copy Markdown
Owner

@yulsmir yulsmir commented May 18, 2023

Implement server using express module

  • GET /todos ---> List all todos
  • GET /todos?11. ----> List only single todo 1 in this case is just example of id but it should work with any id stored
  • POST /todos ---> create new todo
  • PATCH /todos/1 ---> update existing todo using the id
  • DELETE /todos?1 ---> removes existing todo using the id

@yulsmir yulsmir self-assigned this May 18, 2023
@yulsmir
Copy link
Copy Markdown
Owner Author

yulsmir commented May 23, 2023

I would also live a comment what to do:

  • add unique id creation.

Done.
391dba4

},
"dependencies": {
"express": "^4.18.2",
"uuid": "^9.0.0"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It always best to rely on standard modules if possible i.e modules you get from nodejs in this case. Node has this inbuilt https://nodejs.org/api/crypto.html#cryptorandomuuidoptions

const { randomUUID } = require('crypto');;
Then use randomUUID() to generate random uuid.

}

const newId = Math.max(...todos.map((todo) => todo.id)) + 1;
const newTodo = new Todo(newId, `Title ${newId}`, false);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This restrict todo to Title and random id... what if user wants to save their own todo ?
You should capture the values for create Todo from req.body. So if call endpoint
POST /todos and send something like { title: 'I am learning express and nodejs' } ... that sould be persisted.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also since first time you create todo... its obvious its not completed and id should be auto generated coz users do not care about that,...they just provide title of todo and done.
so when create todo... using the https://github.com/yulsmir/todo-express-api/pull/1/files#r1209546591, you could have body of request to be like

{ id: randomUUID(), title: req.body.title, completed: false }

Then you won't need this part const newId = Math.max(...todos.map((todo) => todo.id)) + 1;

Copy link
Copy Markdown
Collaborator

@kenjox kenjox left a comment

Choose a reason for hiding this comment

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

Click the buttonResolve conversationfor what you have fixed.
Handle the post method as per comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants