MongoDB mongoose subdocuments created twice

Question

I am using a simple form that can be used to register an article to a website.

the back-end looks like this:

// Post new article
app.post("/articles", function(req, res){
   var newArticle = {};
   newArticle.title         = req.body.title;
   newArticle.description   = req.body.description;
   var date                 = req.body.date;
   var split                = date.split("/");
   newArticle.date          = split[1]+'/'+split[0]+'/'+split[2];
   newArticle.link          = req.body.link;
   newArticle.body          = req.body.body;
   var platforms = req.body.platforms;
   console.log(platforms);
  Article.create(newArticle, function(err, createdArticle){
      if(err){
          console.log(err.message);
      } else {
           var counter=0;
            platforms.forEach(function(platform){

               var platformed=mongoose.mongo.ObjectID(platform);
               Platform.findById(platformed, function(err, foundPlatform){
                  if(err){
                      console.log(err);
                  } else {
                      counter++;
                        foundPlatform.articles.push(createdArticle);
                        foundPlatform.save();
                        createdArticle.platforms.push(foundPlatform);
                        createdArticle.save();
                        if(counter==platforms.length){
                            res.redirect('articles/' + createdArticle._id);
                        }
                    }
                });
            });

      }


  });

});

The platforms field is passed to the back-end as an array of strings, one string being one objectID. When platforms only contains 1 string i.e. 1 platform to be linked to, everything works fine. When platforms contains multiple string. the created article has duplicates of each platform. Or sometimes only duplicates of some platforms

Any ideas?

UPDATE 1: Article Schema: var mongoose = require("mongoose");

var articleSchema = new mongoose.Schema({
    title        :   String,
    description  :   String, 
    link         :   String,
    date         :   String,
    body         :   String,
    platforms    :   [
      {
         type: mongoose.Schema.Types.ObjectId,
         ref: "Platform"
      }
   ] 
})

module.exports = mongoose.model("Article", articleSchema);

Platform Schema:

var mongoose = require("mongoose");

var platformSchema = new mongoose.Schema({
    name        :   String,
    category            :   String,
    contacts          :   [
      {
         type: mongoose.Schema.Types.ObjectId,
         ref: "Contact"
      }
   ],
   website              :   String,
   country              :   String,
   contactInformation   :   String,
   businessModelNotes   :   String,
   source               :   String,
   generalNotes         :   String,
   projects             :   [
      {
         type: mongoose.Schema.Types.ObjectId,
         ref: "Project"
      }
   ],
   articles             :   [
      {
         type: mongoose.Schema.Types.ObjectId,
         ref: "Article"
      }
   ],
   privacy              :   String,
   comments             :   [
      {
         type: mongoose.Schema.Types.ObjectId,
         ref: "Comment"
      }
   ]


});



module.exports = mongoose.model("Platform", platformSchema);

Answer 1

The forEach loop in your attempt does not recognise the callback completion of the findById() async method before the next iteration. You need to use any of async library methods async.each, async.whilst, or async.until which are equivalent to a for loop, and will wait until async's callback is invoked before moving on to the next iteration (in other words, a for loop that will yield).

For example:

var platform_docs = [];
async.each(platforms, function(id, callback) {
    Platform.findById(id, function(err, platform) {
        if (platform) 
            platform_docs.push(platform);
        callback(err);
    });
}, function(err) {
   // code to run on completion or err
   console.log(platform_docs);
});

---

For the whole operation, you could use the async.waterfall() method which allows each function to pass its results on to the next function.

The first function in the method creates the new article.

The second function uses the async.each() utility function to iterate over the platforms list, perform an asynchronous task for each id to update the platform using findByIdAndUpdate(), and when they're all done return the results of the update query in an object variable to the next function.

The final function will update the newly created article with the platform ids from the previous pipeline.

Something like the following example:

var newArticle = {},
    platforms            = req.body.platforms,
    date                 = req.body.date,
    split                = date.split("/");

newArticle.title         = req.body.title;
newArticle.description   = req.body.description;
newArticle.date          = split[2]+'/'+split[0]+'/'+split[2];
newArticle.link          = req.body.link;
newArticle.body          = req.body.body;
console.log(platforms);

async.waterfall([

    // Create the article
    function(callback) {
        var article = new Article(newArticle);
        article.save(function(err, article){
            if (err) return callback(err);                  
            callback(null, article);
        });
    },

    // Query and update the platforms 
    function(articleData, callback) {
        var platform_ids = [];
        async.each(platforms, function(id, callback) {
            Platform.findByIdAndUpdate(id, 
                { "$push": { "articles": articleData._id } },
                { "new": true },
                function(err, platform) {
                    if (platform) 
                        platform_ids.push(platform._id);
                    callback(err);
                }
            );
        }, function(err) {
            // code to run on completion or err
            if (err) return callback(err);                  
            console.log(platform_ids);
            callback(null, {
                "article": articleData,
                "platform_ids": platform_ids
            });
        });         
    },

    // Update the article
    function(obj, callback) {
        var article = obj.article;
        obj.platform_ids.forEach(function(id){ article.platforms.push(id); });
        article.save(function(err, article){
            if (err) return callback(err);                  
            callback(null, article);
        });
    }   

], function(err, result) { 
/*
    This function gets called after the above tasks 
    have called their "task callbacks"
*/
    if (err) return next(err);
    console.log(result);
    res.redirect('articles/' + result._id);
});

Answer 2

Move your save function

if(counter==platforms.length){
     createdArticle.save(function(err, savedObject){
        if(err || !savedObject) console.log(err || "not saved");
        else {
          res.redirect('articles/' + savedObject._id.toString());
        }
     });
}

============= EDIT

Its because you have to call article.save only one time, and not at each loop. In addition you use save() as a sync function but it's async.

I thinks you should use directly update function :

} else {
  var counter=0;
  // map plateform array id with ObjectID
  var idarray = platforms.map(function(e){return mongoose.mongo.ObjectID(e);});

  // update all plateform with article id
  Platform.update({_id:{$in: idarray}}, {$push:{articles: createdArticle}}, {multi:true, upsert:false}, function(err, raw){
    if(err)
    {
       // error case
       return res.status(403).json({});
    }
    // retrieve plateform
    Platform.find({_id:{$in: idarray}}, function(err, results){
      if(err || !results)
      {
          // error case 
          return res.status(403).json({});
      }
      Article.update({_id: createdArticle._id.toString()}, {$push:{platforms:{$each: results}}}, {multi:false, upsert:false}, function(err, saved){
          if(err || !saved)
          {
             // error
              return res.status(403).json({});
          }
          res.redirect('articles/' + savedObject._id.toString());
     });
   });
 });

But it's a bad idea to store full objects, why not storing only id ??