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

Conversation

Projects
None yet
3 participants
@cl-ment
Contributor

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.

Added the ability to use attributes other than 'id' in the destroy te…
…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

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Oct 10, 2013

Contributor

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.

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

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Oct 10, 2013

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

Contributor

imjoshdean commented Oct 10, 2013

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

- Fix the "Destroy Deferred" tests for Zepto, Mootools, Dojo, and YUI.
- Trying to keep Model.destroy backward compatible.
@cl-ment

This comment has been minimized.

Show comment
Hide comment
@cl-ment

cl-ment Oct 11, 2013

Contributor

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){
        ....
Contributor

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

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Oct 14, 2013

Contributor

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.

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

Merge pull request #492 from cl-ment/master
Destroy templates and attributes.

@daffl daffl merged commit b259f7c into canjs:master Oct 14, 2013

1 check passed

default The Travis CI build passed
Details

This was referenced Oct 14, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment