Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions chat-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@
"type": "module",
"main": "server.js",
"scripts": {
"start": "node server.js"
"start": "nodemon server.js"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice touch using nodemon! This can save you a lot of time and effort during development. Well done!

},
"dependencies": {
"cors": "^2.8.5"
"body-parser": "^1.20.2",
"cors": "^2.8.5",
"express": "^4.19.2",
"nodemon": "^3.1.0"
},
"repository": {
"url": "https://glitch.com/edit/#!/hello-express"
Expand All @@ -21,5 +24,8 @@
"node",
"glitch",
"express"
]
],
"devDependencies": {
"yarn": "^1.22.22"
}
}
66 changes: 64 additions & 2 deletions chat-server/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ process.env.PORT = process.env.PORT || 9090;
import express from "express";
import cors from "cors";
import path from "path";
import bodyParser from "body-parser";
import { fileURLToPath } from "url";

const app = express();

app.use(cors());
app.use(bodyParser.json());

// Get __dirname in ES module
const __dirname = path.dirname(fileURLToPath(import.meta.url));
Expand All @@ -21,8 +23,68 @@ const welcomeMessage = {
//We will start with one message in the array.
const messages = [welcomeMessage];

app.get("/", (request, response) => {
response.sendFile(__dirname + "/index.html");
app.get("/", (req, res) => {
res.sendFile(__dirname + "/index.html");
});

app.post("/messages", (req, res) => {
const newMessage = req.body;
if (
!newMessage.hasOwnProperty("text") ||
!newMessage.hasOwnProperty("from") ||
newMessage.from === "" ||
newMessage.text === "" ||
typeof newMessage.text != "string" ||
typeof newMessage.from != "string"
) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you want to go one further step, you can create your validation fun and give single responsibility to it. It looks like this:

function validateMessage(newMessage) {
     //your validation returns boolean
 }

app.post("/messages", (req, res) => {
  const newMessage = req.body;

  const isMessageValid = validateMessage(newMessage

  if (!isMessageValid)) {
    res.status(400).send("wrong input");
    return;
  }
// Rest of your code...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have modified the route function. Could you have a look please?

res.sendStatus(400).send("wrong input");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

One suggestion for improvement is that instead of using res.sendStatus(400).send("wrong input"), you can use res.status(400).send("wrong input"). res.status(400) set the status code and then send the error message using send.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ahh I see, that makes more sense. Thank you.

}
newMessage["id"] = messages[messages.length - 1].id + 1;
newMessage["timeSent"] = new Date();
messages.push(newMessage);
res.json(messages);
});

app.get("/messages", (req, res) => {
res.json(messages);
});

app.get("/messages/id/:id", (req, res) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There is no right and wrong, but you generally don't need to include the word 'id' in the route path. you can simply use /messages/:id. It makes the endpoint more concise and follows a common convention in RESTful APIs.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I really had a bad time fixing this 😆 when I tried to create another route (ex: /message/search) it was getting recognised as :id parameter. I couldn't solve the problem and found this solution. After seeing this message spent an hour but still couldn't solve 😆

const messageId = req.params.id;
const searchedMessage = messages.find((message) => message.id == messageId);
res.json(searchedMessage);
});

app.delete("/messages/id/:id", (req, res) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same as before!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Can we set up a meeting for this issue? I couldn't find how to resolve dynamic and static path blocking

const messageId = req.params.id;
const deleteMessage = messages.find((message) => message.id == messageId);
messages.splice(messages.indexOf(deleteMessage), 1);
res.json(messages);
});

app.get("/messages/search", (req, res) => {
const searchInput = req.query.text.toLowerCase();
const filteredMessages = messages.filter((message) =>
message.text.toLowerCase().includes(searchInput)
);
res.json(filteredMessages);
});

app.get("/messages/latest", (req, res) => {
const latestMessages = messages.slice(-10);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm sure you can to this more dynamic instead of using hardcoded length 💯

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes it makes more sense. I have updated the route with /messages/latest/:length route

res.json(latestMessages);
});

app.patch("/messages/id/:id", (req, res) => {
console.log(req.params.id);
const messageId = req.params.id;
const updatedInfo = req.body;
const patchMessage = messages.find((message) => message.id == messageId);
messages[messages.indexOf(patchMessage)] = {
...messages[messages.indexOf(patchMessage)],
...updatedInfo,
};
res.json(messages);
});

app.listen(process.env.PORT, () => {
Expand Down
Loading