Added User Registration #23
Conversation
|
@sakshamsaxena @sr6033 Please review. 😄 |
|
@sakshamsaxena @sr6033 Please review the PR. |
|
User Schema already exists, I'd recommend using that. I'll review it soon |
|
@sakshamsaxena Updated the code accordingly. Please review and suggest changes, if any. |
schema/UserSchema.js
Outdated
| _id: { | ||
| type: ObjectId, | ||
| required: true, | ||
| auto: true, |
There was a problem hiding this comment.
Could you explain the purpose of these two attributes ? @IamRaviTejaG
There was a problem hiding this comment.
@sakshamsaxena The required: true makes the _id much similar to a SQL NOT NULL column, and auto: true generates a new ObjectId automatically for every new UserSchema entry.
There was a problem hiding this comment.
Okay, then would you suggest adding these options to all the schema, or does it make sense here only? Also, link to docs for these ?
There was a problem hiding this comment.
Yes, I think we need to add these to all the schemas containing an _id parameter. This answer on StackOverflow explains the purpose well:
https://stackoverflow.com/questions/39871236/what-is-the-meaning-required-in-mongoose-schema
There was a problem hiding this comment.
I'm more curious about auto option though. required can be, and ideally should be, handled at the application level itself via validations. But why is auto necessary, is my question
There was a problem hiding this comment.
If we do not have the auto: true, it throws an error to specify an _id everytime we create a new Schema object. auto: true takes care of it.
There was a problem hiding this comment.
Can you share how you tested it out ?
There was a problem hiding this comment.
I simply made the appropriate routes and when I sent a GET request, it simply logged an error to the console saying it requires an _id parameter; and didn't modify the database.
There was a problem hiding this comment.
Alright. Seems to be so. Let's rather remove the _id field from the schema instead of keeping it with options, as it was originally meant for indicative purpose only. I've tested it without the _id field, works fine, and is adding new records perfectly.
There was a problem hiding this comment.
I got the error everytime I tried to do it. I'll remove it and make a new commit then. 😄
|
@sakshamsaxena Updated the UserSchema. Please review. |
|
Thanks! |
Fixes #10.