Samira w2 node.js

[samira313]

Hello Samir,

In this pull request, I have made the following improvements:
• Refactored the project to use ES Modules by adding "type": "module" in package.json.
• Implemented a weather API using Express and node-fetch.
• Added error handling for invalid or missing city names.
• Created automated tests with Jest and Supertest to validate API functionality.

Looking forward to your feedback.

[eidosam]

You did very well in this week's assignment, Samira!

• You used environment variables for storing the credentials instead of writing it in the code, that's a great security practice.
• The test covers the main cases.
• You used early-return pattern which improves code readability.

One thing is missing; the test configuration files, please add them, and fix the response structure, so we can mark this PR as approved.

[eidosam]

Great that you are using gitignore file, but it should be named .gitignore without extension; see https://git-scm.com/docs/gitignore

[eidosam]

I would put the comment in its own line above the code.

    // Extract city name from request body
    const { cityName } = req.body;

[eidosam]

Same here, the comment in its own line. Also try to use chaining to improve code readability

    // Validate the request
    if (!cityName) {
        return res.status(400)
            .json({ message: "City name is required" });
    }

[eidosam]

Since you have the BASE_URL, you use it here.

[eidosam]

So great to see you use the early-return pattern; it makes the code much cleaner and readable.

[eidosam]

In the instructions in week2/MAKEME.md, the return object should be

{
    weatherText: <message>
}

[eidosam]

In the instructions in week2/MAKEME.md, the return object should be

{
    weatherText: "City is not found!"
}

[eidosam]

There is a redundant space after PORT.

[eidosam]

Like a pro!

[eidosam]

That's great test coverage!

[samira313]

Hello Samir, Thank you so much for your feedback. I have made the necessary changes, and all tests have passed successfully. You can see it below as proof .Please review the updates and let me know if anything else is needed. Best regards, Samira

…

On Mon, 3 Mar 2025 at 21:50, Samir Aleido ***@***.***> wrote: ***@***.**** requested changes on this pull request. You did very well in this week's assignment, Samira! - You used environment variables for storing the credentials instead of writing it in the code, that's a great security practice. - The test covers the main cases. - You used early-return pattern which improves code readability. One thing is missing; the test configuration files, please add them, and fix the response structure, so we can mark this PR as approved. ------------------------------ On assignments/hackyourtemperature/.gitignore.js <#12 (comment)> : Great that you are using gitignore file, but it should be named .gitignore without extension; see https://git-scm.com/docs/gitignore ------------------------------ In assignments/hackyourtemperature/app.js <#12 (comment)> : > +import keys from './sources/keys.js'; + +// Initialize Express app +const app = express(); + +// Middleware to parse JSON request bodies +app.use(express.json()); + +// Define a simple GET route +app.get('/', async (req, res) => { + res.send("Hello from backend to frontend") +}); + +// Post route to fetch weather data +app.post('/weather', async (req, res) => { + const { cityName } = req.body; // Extract city name from request body I would put the comment in its own line above the code. // Extract city name from request body const { cityName } = req.body; ------------------------------ In assignments/hackyourtemperature/app.js <#12 (comment)> : > + +// Middleware to parse JSON request bodies +app.use(express.json()); + +// Define a simple GET route +app.get('/', async (req, res) => { + res.send("Hello from backend to frontend") +}); + +// Post route to fetch weather data +app.post('/weather', async (req, res) => { + const { cityName } = req.body; // Extract city name from request body + + if (!cityName) { + return res.status(400).json({ message: "City name is required" }); // Validate the request + } Same here, the comment in its own line. Also try to use chaining to improve code readability // Validate the request if (!cityName) { return res.status(400) .json({ message: "City name is required" }); } ------------------------------ In assignments/hackyourtemperature/app.js <#12 (comment)> : > +// Define a simple GET route +app.get('/', async (req, res) => { + res.send("Hello from backend to frontend") +}); + +// Post route to fetch weather data +app.post('/weather', async (req, res) => { + const { cityName } = req.body; // Extract city name from request body + + if (!cityName) { + return res.status(400).json({ message: "City name is required" }); // Validate the request + } + try { + // Fetch weather data from OpenWeatherMap API using provided city name and API key + const response = await fetch( + `https://api.openweathermap.org/data/2.5/weather?q=${cityName}&appid=${keys.API_KEY}&units=metric` <https://api.openweathermap.org/data/2.5/weather?q=$%7BcityName%7D&appid=$%7Bkeys.API_KEY%7D&units=metric> Since you have the BASE_URL, you use it here. ------------------------------ In assignments/hackyourtemperature/app.js <#12 (comment)> : > +// Post route to fetch weather data +app.post('/weather', async (req, res) => { + const { cityName } = req.body; // Extract city name from request body + + if (!cityName) { + return res.status(400).json({ message: "City name is required" }); // Validate the request + } + try { + // Fetch weather data from OpenWeatherMap API using provided city name and API key + const response = await fetch( + `https://api.openweathermap.org/data/2.5/weather?q=${cityName}&appid=${keys.API_KEY}&units=metric` <https://api.openweathermap.org/data/2.5/weather?q=$%7BcityName%7D&appid=$%7Bkeys.API_KEY%7D&units=metric> + ); + const data = await response.json(); + + if (data.cod !== 200) { + return res.status(404).json({ message: "City is not found" }); So great to see you use the early-return pattern; it makes the code much cleaner and readable. ------------------------------ In assignments/hackyourtemperature/app.js <#12 (comment)> : > + try { + // Fetch weather data from OpenWeatherMap API using provided city name and API key + const response = await fetch( + `https://api.openweathermap.org/data/2.5/weather?q=${cityName}&appid=${keys.API_KEY}&units=metric` <https://api.openweathermap.org/data/2.5/weather?q=$%7BcityName%7D&appid=$%7Bkeys.API_KEY%7D&units=metric> + ); + const data = await response.json(); + + if (data.cod !== 200) { + return res.status(404).json({ message: "City is not found" }); + } + + // return fetched weather data + res.json({ + cityName : data.name, + temprature : data.main.temp + }); In the instructions in week2/MAKEME.md <https://github.com/HackYourAssignment/Node.js-Cohort51/blob/main/week2/MAKEME.md#312-fetch-it-from-our-api>, the return object should be { weatherText: <message> } ------------------------------ In assignments/hackyourtemperature/app.js <#12 (comment)> : > +const app = express(); + +// Middleware to parse JSON request bodies +app.use(express.json()); + +// Define a simple GET route +app.get('/', async (req, res) => { + res.send("Hello from backend to frontend") +}); + +// Post route to fetch weather data +app.post('/weather', async (req, res) => { + const { cityName } = req.body; // Extract city name from request body + + if (!cityName) { + return res.status(400).json({ message: "City name is required" }); // Validate the request In the instructions in week2/MAKEME.md <https://github.com/HackYourAssignment/Node.js-Cohort51/blob/main/week2/MAKEME.md#312-fetch-it-from-our-api>, the return object should be { weatherText: "City is not found!" } ------------------------------ In assignments/hackyourtemperature/server.js <#12 (comment)> : > @@ -0,0 +1,8 @@ +import app from './app.js'; + +const PORT = 3000; + +// Start the server on the specified port +app.listen(PORT , () => { There is a redundant space after PORT. ------------------------------ In assignments/hackyourtemperature/sources/keys.js <#12 (comment)> : > @@ -0,0 +1,7 @@ +import dotenv from "dotenv"; Like a pro! ------------------------------ In assignments/hackyourtemperature/tests/app.test.js <#12 (comment)> : > + .send({ cityName: "InvalidCity" }); + + expect(response.status).toBe(404); + expect(response.body.message).toContain("City"); + }); + + it("should return an error for an empty city", async () => { + const response = await request(app) + .post('/weather') + .send({ cityName: "" }); + + expect(response.status).toBe(400); + expect(response.body).toHaveProperty("message", "City name is required"); + }); +}); + That's great test coverage! — Reply to this email directly, view it on GitHub <#12 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BKWT54MP2FFFFMXXDCSLTML2SS6BVAVCNFSM6AAAAABXYTXM6OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMNJVGIYDENBZHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[eidosam]

Thank you, Samira, for addressing the feedback!
Could you please keep only one version in this PR? So you need to move the files from assignments/hackyourtemperature2 to assignments/hackyourtemperature and delete assignments/hackyourtemperature2.

[eidosam]

If you move the test configuration files babel.config.cjs and jest.config.js next to the package.json, you won't need this dependency.

[eidosam]

Thanks for addressing all the feedback, Samira! Great work!