-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Post update backend #77
Conversation
Looks good to me so far (aside from the testing). Only have one complaint which I'll link with code |
…jest test doesn't exit.
async function seedDB() { | ||
|
||
//insert users | ||
await users.deleteMany(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this deleting all users' data? Or just the ones that are being inserted in any test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ones in this test. I don't think we need it since the db is empty anyway, but i put it there just in case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if it is for the test, we need it because seedDB() is getting called before each test is being run and we want to make sure tests are independent of any data inserted/deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following lines (which are in the beforeAll
function call) will stop the MongoDB server running on the local machine and start it again, therefore clearing all data. It also appears that this is one of this things that could have been coded better, but I coded it and never looked back
if (db) await MongoDB.close();
db = await MongoDB.open();
gameTimeUTC: new Date(), | ||
duration: "1hr", | ||
location: "Pluto" | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't there be one more test where the post doesn't exist in DB, but the user is trying to update it?
Just to check the status code!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, i can do that, but that would need a postID to be passed in ...like updatePost(postID), and if the post is not found, it won't update... but will return 401 anyways.. I'll add the test
let loggedUserID = tokenDocument.userID; | ||
let postUserID = null | ||
|
||
let oldPost = await getPost(postID); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't just passing a post id in the body from the frontend be a simplified and efficient option instead of passing it in the query and then to getPost and then ultimately fetching from getPost?
that's how delete is set up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to update a post we need to fetch it first and when we ask for something from db that is a query.. body contains what we want to POST. So, it wouldn't be logical to put the postID in the body since we are not POSTing it or changing it.. instead we are GETing it from the db. So that's a query. I had it in the body first, but while writing tests I realized it wasn't logical. So i changed it.
Also, changing account stuff is also set the same way i think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to add to that, you need to be passing hex form of the id.. whether you add it to the body or the query.
LGTM |
Closes #22
Closes #23
Development:
Test: incomplete