Skip to content

Conversation

@sylvainbx
Copy link

This changes add the following features:

  • ability to use multi-level indicators, eg.
    var opts = {
      modelArray: persons,
      storeWhere: "books.author",
      arrayPop: true,
      mongooseModel: Book,
      idField: 'contacts.author'
    };
  // information will be retrieved from Book.contacts.author and will be store in Person.books.author
  // previously it was not possible to use more than a 1-level indicator (ie. 'books' not 'books.author')
  • ability to convert directly the results to JSON, usefull in Express.js, eg.
  // this in an Express.js controller for a JSON API
  const reversePopulateForJson = require('mongoose-reverse-populate').reversePopulateForJson;

  module.exports.index = function(req, res, next) {
    User.find({}, 'firstName lastName').exec(function(err, users) {
      if (err) return next(err);

      var opts = { ... };
      // here if we had used the normal reversePopulate(), the populated fields won't had appeared
      // in the resulting JSON
      reversePopulateForJson(opts, function(err, populatedUsers) {
        if (err) return next(err);
        res.json(populatedUsers);
      });
    });
  };

This also introduce a breaking change:
var reversePopulate = require('../services/mongoose-reverse-populate');
MUST now be written like this:
var reversePopulate = require('../services/mongoose-reverse-populate').reversePopulate;

@davidbanham davidbanham assigned s-taylor and davidbanham and unassigned s-taylor Mar 20, 2016
@davidbanham
Copy link

Thanks for the PR, Sylvain! Apologies it's taken us a little while to get back to you. We need to set up a better notification system here. We'll be much more responsive in future, I promise.

I'd like to see some tests for the new reversePopulateForJson function before we look at merging it. Seeing a test would also help me better understand the intended purpose of the function, I think.

Are you willing to add a test for this new function to the suite? If there's any part of the existing test code that is unclear or you're uncertain of please let me know and I'll be happy to talk you through it.

@sylvainbx
Copy link
Author

Hi, no problem, thanks for the answer!
I'll try to add a test and I keep you updated.

@davidbanham
Copy link

Great, thanks! Let me know if you need anything.

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

Successfully merging this pull request may close these issues.

4 participants