revise the code for findOne() by _id in server/record.js

[JEFFTIMES]

When I follow the tutorial How to Use MERN Stack: A Complete Guide to build the app, I found a few issues to stop the app work properly, include missing the endpoint get(/record/:id, function (req, res) { }) for entering the edit page, misuse id for _id with mongodb driver, give the id the property without converting it to ObjectId, and an unnecessary parameter which passed to getDb().

When I fork this project, I found someone added the endpoint but the other issues are still there, so I ask this pull request for the rest of them.

[joellord]

Thank you for raising this issue @JEFFTIMES. I am experimenting with the code and trying to reproduce the errors right now. Would you be so kind as to provide the error message you were getting concerning the misuse id for _id?

[JEFFTIMES]

Hi Joel

You will not receive any error but the logic of the code is incorrect, let me explain it to you. And I am not sure if it is a bug from the findOne() API from the mongodb nodejs package. the findOne() will always return the first document if we pass it a query object with the property set to undefined.

The original code snippet for the '/record/:id' path is showing below, it passes the req.body.id into 'myquery' object { id : req.body.id } as the value for the id property and fire the query, then respond a result in JSON.

// This section will help you get a single record by id recordRoutes.route("/record/:id").get(function (req, res) { let db_connect = dbo.getDb(); let myquery = { id: req.body.id }; db_connect .collection("records") .findOne(myquery, function (err, result) { if (err) throw err; res.json(result); }); });

But when I created few records for the App and try to click the 'edit' anchor of any record, in the 'edit' page, I always get the first record showed, no matter what record I had chosen from.

[image: Screen Shot 2021-09-17 at 11.21.45 AM.png]

Base on my understanding of the 'named segments' http://expressjs.com/en/guide/routing.html#route-parameters pattern in ExpressJS , the id should be passed by the req.params object, but I am not hundred percent sure if there is some data in the req.body that could be used. So I add 4 logs for further understanding of what happened.

// This section will help you get a single record by id recordRoutes.route("/record/:id").get(function (req, res) {

let db_connect = dbo.getDb(); let myquery = { id: req.body.id };

console.log(' req.params.id in "/record/:id" path: ', req.params.id); console.log(' req.body.id in "/record/:id" path: ', req.body.id); console.log('myquery obj in "/record/:id" path: ', myquery);

db_connect .collection("records") .findOne(myquery, function (err, result) { if (err) throw err; console.log('result in "/record/:id" path: ', result);

res.json(result); }); });

The log comfirm my suspicion that the req.body is empty, and the id is passed into by the req.params object. But something intrest me is the findOne() returned the first record from the collection no matter it takes an object which id property is undefined. It behaves not as it should be.

[image: Screen Shot 2021-09-17 at 11.48.25 AM.png]

I take the further action to pass the right data from the req.params.id to myquery's id property, and try it again. That is not what I expected, the frontend is yelling me an 500 error, log shows that findOne() return me a null.

[image: Screen Shot 2021-09-17 at 12.03.18 PM.png] [image: Screen Shot 2021-09-17 at 12.05.08 PM.png]

I turned into the database to have a looking what's the problem the findOne() return a null with a well set query. Then I found there isn't any property named as "id", only "_id" property there.

{ "_id": { "$oid": "6137f0137015894a7cd4d9a0" }, "person_name": "victor", "person_position": "higher", "person_level": "Intern" }

But I stil got a null result even I changed the 'id' property of myquery object to '_id', because the value of the "_id" is not a string, it is an object - generated by ObjectId() API.

I finally got the edit initial page right after I revised the definition of myquery as below

/ This section will help you get a single record by id recordRoutes.route("/record/:id").get(function (req, res) {

let db_connect = dbo.getDb();

// take _id as the property name, and assign a value returned by ObjectId(); let myquery = { _id: ObjectId(req.params.id) };

console.log(' req.params.id in "/record/:id" path: ', req.params.id); console.log(' req.body.id in "/record/:id" path: ', req.body.id); console.log('myquery obj in "/record/:id" path: ', myquery);

db_connect .collection("records") .findOne(myquery, function (err, result) { if (err) throw err; console.log('result in "/record/:id" path: ', result);

res.json(result); }); });

[image: Screen Shot 2021-09-17 at 1.05.49 PM.png]

the same issue occured in the '/update/:id' and '/delete/:id' path. so I correct all of them as showed below to make the App work properly. Otherwise you will always update or delete the first record.

// This section will help you update a record by id. recordRoutes.route("/update/:id").post(function (req, res) { let db_connect = dbo.getDb("employees"); let myquery = { _id: ObjectId(req.params.id) }; let newvalues = { $set: { person_name: req.body.person_name, person_position: req.body.person_position, person_level: req.body.person_level, }, }; db_connect .collection("records") .updateOne(myquery, newvalues, function (err, res) { if (err) throw err; console.log("1 document updated"); }); });

// This section will help you delete a record recordRoutes.route("/:id").delete((req, res) => { let db_connect = dbo.getDb("employees"); var myquery = { _id: ObjectId(req.params.id) }; db_connect.collection("records").deleteOne(myquery, function (err, obj) { if (err) throw err; console.log("1 document deleted"); }); });

So, that's all. Hope it is helpful.

Jeff

On Fri, Sep 17, 2021 at 6:31 AM Joel Lord @.***> wrote:

Thank you for raising this issue @JEFFTIMES https://github.com/JEFFTIMES. I am experimenting with the code and trying to reproduce the errors right now. Would you be so kind as to provide the error message you were getting concerning the misuse id for _id?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mongodb-developer/mern-stack-example/pull/5#issuecomment-921799893, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMQEJG634E6VPDROUUAAZ6DUCM7MJANCNFSM5DUBIB7A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[joellord]

Wow, thank you so much for this explanation Jeff. I must admit I've only tested with one record which explains why i wasn't getting the error. I'll work on this first thing Monday morning.

On Fri, Sep 17, 2021, 16:31 Jeff Times @.***> wrote:

Hi Joel

You will not receive any error but the logic of the code is incorrect, let me explain it to you. And I am not sure if it is a bug from the findOne() API from the mongodb nodejs package. the findOne() will always return the first document if we pass it a query object with the property set to undefined.

The original code snippet for the '/record/:id' path is showing below, it passes the req.body.id into 'myquery' object { id : req.body.id } as the value for the id property and fire the query, then respond a result in JSON.

// This section will help you get a single record by id recordRoutes.route("/record/:id").get(function (req, res) { let db_connect = dbo.getDb(); let myquery = { id: req.body.id }; db_connect .collection("records") .findOne(myquery, function (err, result) { if (err) throw err; res.json(result); }); });

But when I created few records for the App and try to click the 'edit' anchor of any record, in the 'edit' page, I always get the first record showed, no matter what record I had chosen from.

[image: Screen Shot 2021-09-17 at 11.21.45 AM.png]

Base on my understanding of the 'named segments' http://expressjs.com/en/guide/routing.html#route-parameters pattern in ExpressJS , the id should be passed by the req.params object, but I am not hundred percent sure if there is some data in the req.body that could be used. So I add 4 logs for further understanding of what happened.

// This section will help you get a single record by id recordRoutes.route("/record/:id").get(function (req, res) {

let db_connect = dbo.getDb(); let myquery = { id: req.body.id };

console.log(' req.params.id in "/record/:id" path: ', req.params.id); console.log(' req.body.id in "/record/:id" path: ', req.body.id); console.log('myquery obj in "/record/:id" path: ', myquery);

db_connect .collection("records") .findOne(myquery, function (err, result) { if (err) throw err; console.log('result in "/record/:id" path: ', result);

res.json(result); }); });

The log comfirm my suspicion that the req.body is empty, and the id is passed into by the req.params object. But something intrest me is the findOne() returned the first record from the collection no matter it takes an object which id property is undefined. It behaves not as it should be.

[image: Screen Shot 2021-09-17 at 11.48.25 AM.png]

I take the further action to pass the right data from the req.params.id to myquery's id property, and try it again. That is not what I expected, the frontend is yelling me an 500 error, log shows that findOne() return me a null.

[image: Screen Shot 2021-09-17 at 12.03.18 PM.png] [image: Screen Shot 2021-09-17 at 12.05.08 PM.png]

I turned into the database to have a looking what's the problem the findOne() return a null with a well set query. Then I found there isn't any property named as "id", only "_id" property there.

{ "_id": { "$oid": "6137f0137015894a7cd4d9a0" }, "person_name": "victor", "person_position": "higher", "person_level": "Intern" }

But I stil got a null result even I changed the 'id' property of myquery object to '_id', because the value of the "_id" is not a string, it is an object - generated by ObjectId() API.

I finally got the edit initial page right after I revised the definition of myquery as below

/ This section will help you get a single record by id recordRoutes.route("/record/:id").get(function (req, res) {

let db_connect = dbo.getDb();

// take _id as the property name, and assign a value returned by ObjectId(); let myquery = { _id: ObjectId(req.params.id) };

console.log(' req.params.id in "/record/:id" path: ', req.params.id); console.log(' req.body.id in "/record/:id" path: ', req.body.id); console.log('myquery obj in "/record/:id" path: ', myquery);

db_connect .collection("records") .findOne(myquery, function (err, result) { if (err) throw err; console.log('result in "/record/:id" path: ', result);

res.json(result); }); });

[image: Screen Shot 2021-09-17 at 1.05.49 PM.png]

the same issue occured in the '/update/:id' and '/delete/:id' path. so I correct all of them as showed below to make the App work properly. Otherwise you will always update or delete the first record.

// This section will help you update a record by id. recordRoutes.route("/update/:id").post(function (req, res) { let db_connect = dbo.getDb("employees"); let myquery = { _id: ObjectId(req.params.id) }; let newvalues = { $set: { person_name: req.body.person_name, person_position: req.body.person_position, person_level: req.body.person_level, }, }; db_connect .collection("records") .updateOne(myquery, newvalues, function (err, res) { if (err) throw err; console.log("1 document updated"); }); });

// This section will help you delete a record recordRoutes.route("/:id").delete((req, res) => { let db_connect = dbo.getDb("employees"); var myquery = { _id: ObjectId(req.params.id) }; db_connect.collection("records").deleteOne(myquery, function (err, obj) { if (err) throw err; console.log("1 document deleted"); }); });

So, that's all. Hope it is helpful.

Jeff

On Fri, Sep 17, 2021 at 6:31 AM Joel Lord @.***> wrote:

Thank you for raising this issue @JEFFTIMES < https://github.com/JEFFTIMES>. I am experimenting with the code and trying to reproduce the errors right now. Would you be so kind as to provide the error message you were getting concerning the misuse id for _id?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/mongodb-developer/mern-stack-example/pull/5#issuecomment-921799893 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AMQEJG634E6VPDROUUAAZ6DUCM7MJANCNFSM5DUBIB7A

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mongodb-developer/mern-stack-example/pull/5#issuecomment-922064204, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMKMSL7IKKT6UWUCKLAEO3UCOQRTANCNFSM5DUBIB7A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[joellord]

Great catch! Thank you so much for this deep investigation.

[JEFFTIMES]

Hi Joel One more question, do you think it is fine that the findOne() returns the first record if it takes an undefined as the value of the query object?

[joellord]

Well, according to the documentation (https://docs.mongodb.com/manual/reference/method/db.collection.findOne/), this is the expected behaviour.

Returns one document that satisfies the specified query criteria on the collection or view. If multiple documents satisfy the query, this method returns the first document according to the natural order which reflects the order of documents on the disk.

Regarding the fact that findOne(undefined) === findOne({}), this is the expected behaviour, according to the Node.js driver documentation.

If you don't provide a query document or if you provide an empty document, MongoDB matches all documents in the collection.

With that in mind, I guess that makes sense. findOne(undefined) returns the first document matching the filter passed as a parameters. With undefined as a parameter, it doesn't use a filter and matches all the documents, returning the first one in natural order.

I hope that clears it up for you a little bit more!

Joel