0

I have this

router.get('/documents', function(req, res, next) {
  Document.find(function(err, doc){
    res.render('documents', { doc: doc });
  });
});

router.post('/documents', function(req, res, next) {
  // Create function
  req.body.newdocs.forEach(function(newdoc) {
    Document.create({title : newdoc.title, desc: newdoc.desc}, function (err, ndoc) {
      if (err) {
        res.flash('error', "Error. Try again.");
        res.redirect('/documents');
      }
    });
  });      

  // Update function
  req.body.docs.forEach(function(doc) {
    var d = {
      title: doc.title,
      desc: doc.desc
    }
    Document.update({'_id': doc.id}, d, {overwrite: true}, function(err, raw) {
      if (err) return handleError(err);
    });
  });
  res.redirect('/documents');
});

When I create some documents, post them (documents are created in database) and the redirection works. So I get the page, but I only have the documents before the post. And when I refresh the page (GET the page one more time), I have well the new documents.

Do you have some ideas of explanation, to fix this ?

XavierB
  • 2,198
  • 1
  • 9
  • 12

2 Answers2

0

The problem in your code is that you dont wait for the update function to complete. :) You tell the database to save the documents with:

Document.update({'_id': doc.id}, d, {overwrite: true}...

But mongo does updates async, which means that this code will just query them and continue, without waiting for the actual update. For your code to be proper, you need to run res.redirect('/documents'); in the callback (which is the function that gets executed after the actual update is done). So your code should look like this:

Document.update({'_id': doc.id}, d, {overwrite: true}, function(err, raw) {
   if (err) return handleError(err);
   res.redirect('/documents');
});

Promise.all example, as requested by @XavierB

//Push all of the promises into one array
let promises = [];
promises.push(Document.update({'_id': doc.id}, d, {overwrite: true}));
//Await for all of the promises to be done
Promises.all(promises).then(function(){
    //All of the promises were resolved
    res.redirect('/documents');
}).catch(err => { 
    //Something went terribly wrong with ANY of the promises.
    console.log(err); 
});;
vicbyte
  • 3,690
  • 1
  • 11
  • 20
  • The original code performs multiple DB queries within a pair of loops. It might be better to use `Promise.all` to wait for them all rather than trying to juggle the callbacks. – skirtle Oct 29 '17 at 00:14
  • Youre right, my answer was just a simplified version, focusing on explaining why code doesnt work. :) If he wants to replace both .each loops, then yes, Promise.all is the way to go. – vicbyte Oct 29 '17 at 00:36
  • Can you be more precise. How can I use `Promise.all` ? I found this [answer](https://stackoverflow.com/questions/38362231/how-to-use-promise-in-foreach-loop-of-array-to-populate-an-object) but don't know exactly how to use. – XavierB Oct 29 '17 at 15:14
0

It is async, it takes for a while to finish. You should use async await or .then to run the res.redirect('/') in the callback, it will work as you expect

dnp1204
  • 471
  • 5
  • 14