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

$.Model.delete URL cannot be templated beyond {id} #130

Open
josephtate opened this issue May 23, 2012 · 2 comments
Open

$.Model.delete URL cannot be templated beyond {id} #130

josephtate opened this issue May 23, 2012 · 2 comments

Comments

@josephtate
Copy link
Contributor

// The $.Model.destroy method

            return function( id, success, error ) {
                var attrs = {};
                attrs[this.id] = id;
                return ajax( str || this._shortName+"/{"+this.id+"}", attrs, success, error, fixture(this, "Destroy", "-restDestroy"), "delete")
            }

As you can see the attrs is stripped down, so the rest of the attrs are unavailable for URL substitution.

My REST server uses nested urls to denote hierarchy; /document/3/property/7 (represented as "{docUrl}/property/{id}/"), etc. I need to be able to operate on different urls based on the document context, but I have to write a custom delete so that I can substitute the URL properly.

@josephtate
Copy link
Contributor Author

A custom destroy method won't work because makeRequest strips out all the useful bits. I thought about refactoring the url substitution so that it happened in makeRequest instead of the anonymous ajax/$.Model._ajax method, but would that break all the custom ajax calls out there?

@josephtate
Copy link
Contributor Author

I've given this a lot more thought. I think this patch fixes the issue without breaking everybody. It'd be nice if the url substitution only happened once though. This manages to not break any tests, and it doesn't change the order of parameters.


diff --git a/model/model.js b/model/model.js                                                                                                                                                                  
index 9e0e589..c591b25 100644                                                                                                                                                                                 
--- a/model/model.js                                                                                                                                                                                          
+++ b/model/model.js                                                                                                                                                                                          
@@ -138,11 +138,11 @@ steal('jquery/class', 'jquery/lang/string', function() {                                                                                                                                
                                jqXHR,                                                                                                                                                                        
                                promise = deferred.promise();                                                                                                                                                 

-                       // destroy does not need data                                                                                                                                                         
-                       if ( type == 'destroy' ) {                                                                                                                                                            
-                               args.shift();                                                                                                                                                                 
+                       // pass the old data to delete                                                                                                                                                        
+                       if (type !== 'delete') {                                                                                                                                                              
+                               //Put the attrs at the end                                                                                                                                                    
+                               args.push(args.shift());                                                                                                                                                      
                        }                                                                                                                                                                                      
-
                        // update and destroy need the id
                        if ( type !== 'create' ) {
                                args.unshift(getId(self))
@@ -642,11 +642,14 @@ steal('jquery/class', 'jquery/lang/string', function() {
                         * @param {Function} success the callback function, it must be called with an object 
                         * that has the id of the new instance and any other attributes the service needs to add.
                         * @param {Function} error a function to callback if something goes wrong.  
+                        * @param {Object} origAttrs the original attributes for the item being deleted
                         */
-                       return function( id, success, error ) {
-                               var attrs = {};
+                       return function( id, success, error, origAttrs ) {
+                               var attrs = {}, ajaxOb = {};
                                attrs[this.id] = id;
-                               return ajax( str || this._shortName+"/{"+this.id+"}", attrs, success, error, fixture(this, "Destroy", "-restDestroy"), "delete")
+                               ajaxOb.url = str || this._shortName+"/{"+this.id+"}"
+                               ajaxOb.url = $.String.sub(ajaxOb.url, origAttrs, true);
+                               return ajax(ajaxOb, attrs, success, error, fixture(this, "Destroy", "-restDestroy"), "delete")
                        }
                },

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

1 participant