Updated webhook update method to not send id in request (#165)

SparkPost · Aug 26, 2016 · 57684c8 · 57684c8
1 parent ff39f79
commit 57684c8
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 40 deletions.
diff --git a/lib/webhooks.js b/lib/webhooks.js
@@ -5,7 +5,7 @@ var api = 'webhooks'
 
 module.exports = function(client) {
   var webhooks = {
-    all: function(options, callback) {
+    all: function (options, callback) {
       var reqOpts = {
         uri: api
       };
@@ -22,10 +22,10 @@ module.exports = function(client) {
 
       client.get(reqOpts, callback);
     },
-    describe: function(options, callback) {
+    describe: function (options, callback) {
       options = options || {};
 
-      if(!options.id) {
+      if (!options.id) {
         callback(new Error('id is required'));
         return;
       }
@@ -40,13 +40,13 @@ module.exports = function(client) {
 
       client.get(reqOpts, callback);
     },
-    create: function(webhook, callback) {
-      if(typeof webhook === 'function') {
+    create: function (webhook, callback) {
+      if (typeof webhook === 'function') {
         callback = webhook;
         webhook = null;
       }
 
-      if(!webhook) {
+      if (!webhook) {
         callback(new Error('webhook object is required'));
         return;
       }
@@ -57,25 +57,52 @@ module.exports = function(client) {
 
       client.post(options, callback);
     },
-    update: function(webhook, callback) {
-      if(typeof webhook === 'function') {
+    update: function (webhook, callback) {
+      var object, options, id;
+
+      if (typeof webhook === 'function') {
         callback = webhook;
         webhook = null;
       }
 
-      if(!webhook) {
+      if (!webhook) {
         callback(new Error('webhook object is required'));
         return;
       }
 
-      var object = toApiFormat(webhook)
-        , options = {
-          uri: api + '/' + webhook.id
-          , json: object
-        };
+      if (!webhook.id) {
+        callback(new Error('webhook.id is required'));
+        return;
+      }
+
+      id = webhook.id;
+      delete webhook.id;
+
+      object = toApiFormat(webhook);
+      options = {
+        uri: api + '/' + id
+        , json: object
+      };
 
       client.put(options, callback);
     },
+    delete: function (id, callback) {
+      if (typeof id === 'function') {
+        callback = id;
+        id = null;
+      }
+
+      if (!id) {
+        callback(new Error('id is required'));
+        return;
+      }
+
+      var options = {
+        uri: api + '/' + id
+      };
+
+      client.delete(options, callback);
+    },
     validate: function(options, callback) {
       options = options || {};
 
@@ -142,23 +169,5 @@ module.exports = function(client) {
     }
   };
 
-  webhooks['delete'] = function(id, callback) {
-    if (typeof id === 'function') {
-      callback = id;
-      id = null;
-    }
-
-    if (!id) {
-      callback(new Error('id is required'));
-      return;
-    }
-
-    var options = {
-      uri: api + '/' + id
-    };
-
-    client['delete'](options, callback);
-  };
-
   return webhooks;
 };
diff --git a/package.json b/package.json
@@ -16,7 +16,7 @@
     "url": "https://github.com/SparkPost/node-sparkpost"
   },
   "author": "Message Systems, Inc.",
-  "license": "Apache 2.0",
+  "license": "Apache-2.0",
   "bugs": {
     "url": "https://github.com/SparkPost/node-sparkpost/issues"
   },

diff --git a/test/spec/webhooks.spec.js b/test/spec/webhooks.spec.js
@@ -13,7 +13,7 @@ describe('Webhooks Library', function() {
       get: sinon.stub().yields(),
       post: sinon.stub().yields(),
       put: sinon.stub().yields(),
-      'delete': sinon.stub().yields()
+      delete: sinon.stub().yields()
     };
 
     webhooks = require('../../lib/webhooks')(client);
@@ -124,28 +124,36 @@ describe('Webhooks Library', function() {
         done();
       });
     });
+
+    it('should throw an error if webhook.id is missing', function(done) {
+      webhooks.update({}, function (err) {
+        expect(err.message).to.equal('webhook.id is required');
+        expect(client.post).not.to.have.been.called;
+        done();
+      });
+    });
   });
 
   describe('delete Method', function() {
     it('should call client delete method with the appropriate uri', function(done) {
-      webhooks['delete']('test', function(err, data) {
-        expect(client['delete'].firstCall.args[0].uri).to.equal('webhooks/test');
+      webhooks.delete('test', function(err, data) {
+        expect(client.delete.firstCall.args[0].uri).to.equal('webhooks/test');
         done();
       });
     });
 
     it('should throw an error if id is null', function(done) {
-      webhooks['delete'](null, function(err) {
+      webhooks.delete(null, function(err) {
         expect(err.message).to.equal('id is required');
-        expect(client['delete']).not.to.have.been.called;
+        expect(client.delete).not.to.have.been.called;
         done();
       });
     });
 
     it('should throw an error if id is missing', function(done) {
-      webhooks['delete'](function(err) {
+      webhooks.delete(function(err) {
         expect(err.message).to.equal('id is required');
-        expect(client['delete']).not.to.have.been.called;
+        expect(client.delete).not.to.have.been.called;
         done();
       });
     });