Open lindapaiste opened 7 months ago
I like to work on this issue @lindapaiste can I get assigned?
Well I feel the correct implementation can be this bellow snnipid:
export function getProject(req, res) {
const { project_id: projectId, username } = req.params;
User.findByUsername(username, (err, user) => {
if (!user) {
return res
.status(404)
.send({ message: 'User with that username does not exist' });
}
Project.findOne({
user: user._id,
$or: [{ _id: projectId }, { slug: projectId }]
})
.populate('user', 'username')
.exec((err, project) => {
if (err) {
console.log(err);
return res.status(500).send({ message: 'Internal Server Error' });
}
if (!project) {
return res
.status(404)
.send({ message: 'Project with that id does not exist' });
}
return res.json(project);
});
});
}
@mathanraj0601 I've assigned you the issue. I would love for you to start with item 3 on your list and write some tests for the project.controller.js
file. TBH we're kind of in the process of refactoring a lot of the API code so whatever issues you find in item 2 might already be addressed elsewhere.
Thanks, @lindapaiste. Could you please confirm if it's necessary to make the changes mentioned in item 1 and write test cases for that, or should I focus on writing test cases for the existing controller method which have test cases already? Let me know your preference! : )
Increasing Access
We want our APIs to return accurate and descriptive errors, even when provided with invalid arguments.
Feature enhancement details
Problem
When our project API is called with an invalid project id it will return a 200 success error code with the body
null
instead of the appropriate 404 not found error code.To Reproduce
You can open the dev tools console on https://editor.p5js.org/ and paste the following snippet. You must be in a window with the editor due to cross-origin policies.
I'm not flagging this is a "bug" since it has no repercussions on the editor web app. You can't make this API call by navigating to an incorrect URL, since we will not load the page for a project which doesn't exist. The bug is only seen when querying the API directly.
Root Cause
The problem is in the following code in the
project.controller
: https://github.com/processing/p5.js-web-editor/blob/6cac275429c3911d87afdcdbbdbf185092aeac29/server/controllers/project.controller.js#L94-L107 The mongoosefindOne
function with not return anerr
when there is no document. This is considered normal behavior and not an error. It will return anull
project, so we need to check for that. In other places we handle this withif (err || !project) {
.err
here would be some unforeseen problem in mongoose, and should probably be a 500 internal server error instead of a 404 not found, in my opinion. I think we can let the error be thrown and caught by default express error-handling middleware?Tasks:
project.controller
file to verify the current incorrect behavior and future correct behavior. We want to know that an API request with a valid username but an invalid projectId would return a 404 not found error.