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

Add support for extra parameters to Model.save() & RecordArray.save() #2310

Closed
wants to merge 2 commits into from
Closed

Add support for extra parameters to Model.save() & RecordArray.save() #2310

wants to merge 2 commits into from

Conversation

MikeAski
Copy link

This is especially useful when having to pass extra context data to the server, along with a standard resource action.

@MikeAski
Copy link
Author

The Travis build is failing for some other reason... 😿

@igorT
Copy link
Member

igorT commented Sep 23, 2014

I restarted the build, it now seems to fail due to the tests you added.

@MikeAski
Copy link
Author

Oops... Indeed, this time it was me: was running in the browser, but not on command-line. Fixed 😅

@tomdale
Copy link
Member

tomdale commented Sep 24, 2014

@MikeAski: Can you explain what kind of extra context data/metadata you need to include with the save? In general, we try to avoid APIs like this that serve as sidechannels to allow disparate parts of the system to communicate.

@sandstrom
Copy link
Contributor

Perhaps something like an operation, defined on the adapter, could be used to support non-CRUD actions. @MikeAski, would something like this be helpful in your case?

Here is an example from the Stripe API[1]. We've got similar ones in ours.

High-level example of how a solution could work.

// in router
record.operation('refund');

// in adapter
var ChargeAdapter = RESTAdapter.extend({
  operations: {
    'refund' : { path: '/charges/:id/refund', method: 'put' },
  }
})

For things like this we currently do an out-of-band plain xhr, and then use pushPayload on the response, to update the model state. This works, but some form of support within ED would be useful.

Naming is tentative, I'm not sure 'operation' is the ideal word here.

[1] The Strips API has a bunch of these, https://stripe.com/docs/api/curl#refund_charge

@MikeAski
Copy link
Author

@tomdale My need is to address a legacy API, which has extra behaviour flags passed along with resource attributes. The exact case is user profile update, which takes two flags indicating if the admin wishes to have the updated user's password randomly generated & another indicating if anyway some notification should be sent to the given user.

@sandstrom In my case, I am needing the extra parameters on CRUD actions. So I think your suggestion is interesting but is another topic 😉

@sandstrom
Copy link
Contributor

@MikeAski Perhaps you can use readonly attributes myFlag: DS.attr('boolean', {serialize: false}) for these two flags. Set like normal properties. Then, in your model-specific serializer, you'd read these of the record and make the necessary adjustments to the payload. Would that work for you?

@MikeAski
Copy link
Author

@sandstrom It worked, indeed. I had to override serializeIntoHash and got the same result. Thanks for the tip.

@MikeAski MikeAski closed this Sep 29, 2014
@sly7-7 sly7-7 mentioned this pull request Dec 22, 2014
2 tasks
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.

None yet

4 participants