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

Overriding Backbone.sync breaks Rails' strong parameters #188

Open
jarrett opened this issue Feb 27, 2015 · 4 comments
Open

Overriding Backbone.sync breaks Rails' strong parameters #188

jarrett opened this issue Feb 27, 2015 · 4 comments

Comments

@jarrett
Copy link

jarrett commented Feb 27, 2015

Line 19 in backbone_rails_sync.js reads:

data = JSON.stringify(options.attrs || model.toJSON(options));

It looks like this variable is always overwritten in the next few lines:

if (model.paramRoot) {
  data = {};
  data[model.paramRoot] = model.toJSON(options);
} else {
  data = model.toJSON();
}

So line 19 seems to do nothing. If I'm correct, It's a few CPU cycles gone to waste.

More importantly though, options.attrs is ignored. That's bad because it breaks calls like the following:

myModelInstance.save({name: "example"}, {patch: true});

The above should send only {name: "example"} rather than the whole attributes hash in the HTTP request. This is very important in Rails, where strong parameters is the officially supported way to whitelist attributes. In Rails, you want to be able to set config.action_controller.action_on_unpermitted_parameters = :raise to catch your own mistakes, but it breaks when backbone-rails forces you to send every attribute with every request.

@jarrett jarrett changed the title Wasted call to JSON.stringify in Backbone.sync? Backbone.sync: Wasted call to JSON.stringify, attributes list ignored Feb 27, 2015
@jarrett jarrett changed the title Backbone.sync: Wasted call to JSON.stringify, attributes list ignored Overriding Backbone.sync breaks Rails' strong parameters Feb 27, 2015
jarrett added a commit to jarrett/backbone-rails that referenced this issue Feb 27, 2015
@jarrett
Copy link
Author

jarrett commented Feb 27, 2015

Proposed fix in pull request #189.

@jarrett
Copy link
Author

jarrett commented Mar 2, 2015

I should add that overriding Backbone.Model#toJSON is another approach to working with Strong Parameters. Nonetheless I think it's worthwhile to ensure backbone-rails preserves the documented functionality of Backbone.Model#save.

@kamaljoshi
Copy link

This behaviour by the JS patch is very annoying and it seems like that it can be easily fixed by considering options.attrs here:

if (model.paramRoot) {
  data = {};
  data[model.paramRoot] = model.toJSON(options);
} else {
  data = model.toJSON();
}

Why hasn't this been fixed yet?

@reidcooper
Copy link

+1 to bump

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

3 participants