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

Destroy templates and attributes. #492

Merged
merged 2 commits into from
Oct 14, 2013
Merged

Destroy templates and attributes. #492

merged 2 commits into from
Oct 14, 2013

Conversation

cl-ment
Copy link
Contributor

@cl-ment cl-ment commented Oct 10, 2013

Added the ability to use attributes other than 'id' in the destroy templates:

var Person = can.Model.extend({
                findAll: "GET /groups/{groupid}/persons",
                findOne: "GET /groups/{groupid}/persons/{id}",
                create:  "POST /groups/{groupid}/persons",
                update:  "PUT  /groups/{groupid}/persons/{id}",
                destroy: "DELETE /groups/{groupid}/persons/{id}",
                …
             },{});

new Person({ id: 2, groupid: 4}),destroy();

I would like to be able to delete with the same way I use to update.

…mplates:

	var Person = can.Model.extend({
					findAll: "GET /groups/{groupid}/persons",
					findOne: "GET /groups/{groupid}/persons/{id}",
					create:  "POST /groups/{groupid}/persons",
					update:  "PUT  /groups/{groupid}/persons/{id}",
					destroy: "DELETE /groups/{groupid}/persons/{id}",
					…
				 },{});

	new Person({ id: 2, groupid: 4}),destroy();
@daffl
Copy link
Contributor

daffl commented Oct 10, 2013

This looks good, thanks! I actually ran into this a little while ago, too. Looks like the build got stuck. I'll re-run it and it should pass.

@imjoshdean
Copy link
Contributor

This worked for jQuery but stalled on the Destroy Deferred tests in can.model for Zepto, Mootools, Dojo, and YUI. Make sure to give them a test on latest master for each of the different libraries

- Trying to keep Model.destroy backward compatible.
@cl-ment
Copy link
Contributor Author

cl-ment commented Oct 11, 2013

I am not really happy with this fix.
IMHO it would be valuable to change the model.destroy signature from:

 can.Model("Person",{
        destroy : function(id, success, error){
        ....

to:

 can.Model("Person",{
        destroy : function(id, attr, success, error){
        ....

@daffl
Copy link
Contributor

daffl commented Oct 14, 2013

I think I will bring it in, as it solves #428 as well. I think using the model attributes is the most common case. I would argue that you could just implement your own .destroy to make it possible to pass additional parameters.

daffl added a commit that referenced this pull request Oct 14, 2013
Destroy templates and attributes.
@daffl daffl merged commit b259f7c into canjs:master Oct 14, 2013
This was referenced Oct 14, 2013
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.

3 participants