Skip to content

michael wagner express mongo eval#157

Open
Michael-Wagner459 wants to merge 3 commits intoprojectshft:masterfrom
Michael-Wagner459:master
Open

michael wagner express mongo eval#157
Michael-Wagner459 wants to merge 3 commits intoprojectshft:masterfrom
Michael-Wagner459:master

Conversation

@Michael-Wagner459
Copy link

No description provided.

mongoose.connect('mongodb://localhost/products', {
useNewUrlParser: true,
useUnifiedTopology: true,
});

Choose a reason for hiding this comment

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

next time add some connection helpers to see in the console when connections was established or not, example:

mongoose.connection.on("connected", () => {
	console.log("Connected to MongoDB successfully");
  });
  
  mongoose.connection.on("error", (err) => {
	console.error(`Failed to connect to MongoDB: ${err.message}`);
  });

const Product = require('../models/product');
const Review = require('../models/review');

router.get('/generate-fake-data', (req, res, next) => {

Choose a reason for hiding this comment

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

check your code lint settings and alignment. Too much padding per line.

product: '66917821a673574ec36a3a1d',
});

// review.save();

Choose a reason for hiding this comment

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

remove commented code

try {
const totalCount = await Product.countDocuments(query);
if (totalCount === 0) {
return res.status(404).send('No products found');

Choose a reason for hiding this comment

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

is it really and error if no products were found?
404 is not the right error as it refers to a resource in the url not found, if you make a request for: '/prodcts' i would expect to see 404

})
.catch((err) => {
if (err.name === 'CastError') {
return res.status(404).send('Product not found');

Choose a reason for hiding this comment

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

same, the request is successful, but the product was not found

const { category, name, price, image } = req.body;

if (!category || !name || !price || !image) {
return res.status(400).send('All fields are required');

Choose a reason for hiding this comment

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

great use of 400


res.status(200).send(savedProduct);
} catch (err) {
res.status(500).send('Server Error');

Choose a reason for hiding this comment

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

in general we want to do something with the server error, if you are not sending it to the client at least log it into the console.

Comment on lines +205 to +206
if (deletedReview) {
return res.status(404).send('Review not found');

Choose a reason for hiding this comment

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

this is a bug, if you find a deleted review, you are sending an error instead of deleting it

Suggested change
if (deletedReview) {
return res.status(404).send('Review not found');
if (!deletedReview) {
return res.status(404).send('Review not found');

import { useDispatch, useSelector } from 'react-redux';
import { fetchProducts, setPage } from '../slices/productsSlice';

const Pagination = () => {

Choose a reason for hiding this comment

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

what is this component? I don't think its in use

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