fix($resource): still use the cancellable value if invalid timeout value

petebacondarwin · petebacondarwin · commit 0b5ecc64f0e8 · 2015-11-26T13:15:25.000Z
We log a deprecation message if `timeout` contains an invalid value.
Now we also use the `callable` value if provided.
diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js
@@ -576,18 +576,17 @@ angular.module('ngResource', ['ng']).
 
         forEach(actions, function(action, name) {
           var hasBody = /^(POST|PUT|PATCH)$/i.test(action.method);
-          var cancellable;
-
-          if (angular.isNumber(action.timeout)) {
-            cancellable = false;
-          } else if (action.timeout) {
-            $log.debug('ngResource:\n' +
-                       '  Only numeric values are allowed as `timeout`.\n' +
-                       '  Promises are not supported in $resource, because the same value has to ' +
-                       'be re-used for multiple requests. If you are looking for a way to cancel ' +
-                       'requests, you should use the `cancellable` option.');
-            delete action.timeout;
-          } else {
+          var cancellable = false;
+
+          if (!angular.isNumber(action.timeout)) {
+            if (action.timeout) {
+              $log.debug('ngResource:\n' +
+                         '  Only numeric values are allowed as `timeout`.\n' +
+                         '  Promises are not supported in $resource, because the same value has to ' +
+                         'be re-used for multiple requests. If you are looking for a way to cancel ' +
+                         'requests, you should use the `cancellable` option.');
+              delete action.timeout;
+            }
             cancellable = angular.isDefined(action.cancellable) ? action.cancellable :
                          (options && angular.isDefined(options.cancellable)) ? options.cancellable :
                          provider.defaults.cancellable;
diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js
@@ -1427,6 +1427,35 @@ describe('cancelling requests', function() {
     })
   );
 
+  it('should use `cancellable` value if passed a non-numeric `timeout` in an action',
+    inject(function($log, $q) {
+      spyOn($log, 'debug');
+      $httpBackend.whenGET('/CreditCard').respond({});
+
+      var CreditCard = $resource('/CreditCard', {}, {
+        get: {
+          method: 'GET',
+          timeout: $q.defer().promise,
+          cancellable: true
+        }
+      });
+
+      var creditCard = CreditCard.get();
+      expect(creditCard.$cancelRequest).toBeDefined();
+      expect(httpSpy.calls[0].args[0].timeout.then).toBeDefined();
+
+      // $httpBackend.flush();
+
+      // expect(httpSpy).toHaveBeenCalledOnce();
+      // expect(httpSpy.calls[0].args[0].timeout).toBe(jasmine.any());
+      // expect($log.debug).toHaveBeenCalledOnceWith('ngResource:\n' +
+      //     '  Only numeric values are allowed as `timeout`.\n' +
+      //     '  Promises are not supported in $resource, because the same value has to ' +
+      //     'be re-used for multiple requests. If you are looking for a way to cancel ' +
+      //     'requests, you should use the `cancellable` option.');
+    })
+  );
+
   it('should not create a `$cancelRequest` method for instance calls', function() {
     $httpBackend.whenPOST('/CreditCard').respond({});
 
