Skip to content

Commit 92517f7

Browse files
committed
Merge pull request meanjs#349 from lirantal/bugfix-348-article-not-found
fixing issue meanjs#348 - instead of returning a server error 500 on article loading which isnt found we'll throw a 404 with json message
2 parents b2e9b3d + 9952138 commit 92517f7

File tree

2 files changed

+23
-1
lines changed

2 files changed

+23
-1
lines changed

app/controllers/articles.server.controller.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,20 @@ exports.list = function(req, res) {
8888
* Article middleware
8989
*/
9090
exports.articleByID = function(req, res, next, id) {
91+
92+
if (!mongoose.Types.ObjectId.isValid(id)) {
93+
return res.status(400).send({
94+
message: 'Article is invalid'
95+
});
96+
}
97+
9198
Article.findById(id).populate('user', 'displayName').exec(function(err, article) {
9299
if (err) return next(err);
93-
if (!article) return next(new Error('Failed to load article ' + id));
100+
if (!article) {
101+
return res.status(404).send({
102+
message: 'Article not found'
103+
});
104+
}
94105
req.article = article;
95106
next();
96107
});

app/tests/article.server.routes.test.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,17 @@ describe('Article CRUD tests', function() {
201201
});
202202
});
203203

204+
it('should return proper error for single article which doesnt exist, if not signed in', function(done) {
205+
request(app).get('/articles/test')
206+
.end(function(req, res) {
207+
// Set assertion
208+
res.body.should.be.an.Object.with.property('message', 'Article is invalid');
209+
210+
// Call the assertion callback
211+
done();
212+
});
213+
});
214+
204215
it('should be able to delete an article if signed in', function(done) {
205216
agent.post('/auth/signin')
206217
.send(credentials)

0 commit comments

Comments
 (0)