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

Added support for JSON arrays #3228

Merged
merged 3 commits into from
Dec 23, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions lib/hooks/blueprints/actionUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,18 @@ var actionUtil = {
throw new Error('Invalid `req.options.values.blacklist`. Should be an array of strings (parameter names.)');
}

//Special handling if the body is an array, does what it would do usally but on each array object.
if(_.isArray(req.body)){
sails.log.silly("Reqest body is an array");
var values = [];
_.each(req.body, function(element, key){
values[key] = mergeDefaults(element, _.omit(req.options.values, 'blacklist'));
values[key] = _.omit(values[key], blacklist || []);
values[key] = _.omit(values[key], function (p){ if (_.isUndefined(p)) return true; });
});
return values;
}

// Merge params into req.options.values, omitting the blacklist.
var values = mergeDefaults(req.params.all(), _.omit(req.options.values, 'blacklist'));

Expand Down
2 changes: 1 addition & 1 deletion lib/hooks/blueprints/actions/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ module.exports = function createRecord (req, res) {
Model.subscribe(req, newInstance);
Model.introduce(newInstance);
}
Model.publishCreate(newInstance.toJSON(), !req.options.mirror && req);
Model.publishCreate(newInstance, !req.options.mirror && req);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hanifb What was the thinking behind this? Is it somehow necessary to get arrays to work? By pushing this logic into publishCreate, we force users of that method to pass in model instances instead of being able to send in plain objects. Sorry I didn't catch this earlier.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgress454 Wow, i dont even remember. Actually it is kinda unneccessary, because the publishCreateSingle//former publishCreate already does that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgress454 Well now i remember, the reason is because how pluralize method works, it needs an instance of a model. Do you think we should remove the call to pluralize and instead use a simple if(!_.isArray(instance)){ [instance] }?


// Send JSONP-friendly response if it's supported
Expand Down
21 changes: 20 additions & 1 deletion lib/hooks/pubsub/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1077,7 +1077,26 @@ module.exports = function(sails) {

},

/**
* Publish the creation of model or an array of models
*
* @param {[Object]|Object} models
* - the data to publish
*
* @param {Request|Socket} req - Optional request for broadcast.
* @api private
*/
publishCreate: function(models, req, options){
var self = this;

// Pluralize so we can use this method regardless of it is an array or not
models = self.pluralize(models);

//Publish all models
_.each(models, function(values){
self.publishCreateSingle(values.toJSON(), req, options);
})
},
/**
* Publish the creation of a model
*
Expand All @@ -1088,7 +1107,7 @@ module.exports = function(sails) {
* @api private
*/

publishCreate: function(values, req, options) {