Skip to content
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

Unable to remove document #4320

Closed
cameronus opened this issue Jul 11, 2016 · 3 comments
Closed

Unable to remove document #4320

cameronus opened this issue Jul 11, 2016 · 3 comments

Comments

@cameronus
Copy link

cameronus commented Jul 11, 2016

I am having trouble removing a document from a collection in an express-based REST API. The delete route code and relevant router.param is here:

const express = require('express'),
router = express.Router(),
Thread = require('./models')

router.param('tID', (req, res, next, id) => {
  Thread.findById(req.params.tID, (err, thread) => {
    if (err) return next(err)
    if (!thread) {
      err = new Error('Not Found')
      err.status = 404
      return next(err)
    }
    req.thread = thread
    next()
  })
})

router.param('cID', (req, res, next, id) => {
  req.comment = req.thread.comments.id(id)
  if (!req.comment) {
    err = new Error('Not Found')
    err.status = 404
    return next(err)
  }
  next()
})


...


// DELETE /threads/:tID/comments/:cID
router.delete('/:tID/comments/:cID', (req, res, next) => {
  console.log('trying to remove!');
    req.comment.remove((err) => {
    console.log('removed!');
    if (err) return next(err)
    req.thread.save((err, thread) => {
      if (err) return next(err)
      res.json(thread)
    })
  })
})

The :tID is the ID for threads and the :cID is the ID for comments on those threads. When I hit the route with a request using postman, I see 'trying to remove' logged, but never 'removed' or a HTTP response. It just hangs without even an error being triggered. This is a gist with my model.js file if it is necessary for debugging. I have tried using remove() with exec() and then() but it still isn't working. I find the docs to be confusing with removing documents. I really do hope it isn't an obvious mistake, but I have been looking through the docs and bug fixing for an hour or more. Thanks for the help, CJ.

@vkarpov15
Copy link
Collaborator

So in mongoose, calling docArray.remove() is a synchronous operation that you persist to mongodb with a doc.save() call. Remove the callback and just do req.thread.comments.remove(req.comment) and you should be good.

As an aside, IMO you're abusing express middleware. I'd recommend reading @rauchg 's (original author of mongoose, socket.io, and now) excellent comments on the subject: vercel/micro#8 (comment)

@cameronus
Copy link
Author

Thanks! The docs were just a littler unclear to me. Could you elaborate on how I am abusing middleware. I understand the article but not how it pertains to my case.

@vkarpov15
Copy link
Collaborator

Adding properties to req in middleware (req.param() is essentially middleware) as a general practice for getting docs from mongodb can get super confusing as your app grows, because once you have enough middleware it starts getting really confusing to figure out "where did req.blah come from?"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants