Merge pull request #691 from stripe/richardm-data-in-queryparams

GET and DELETE requests data: body->queryParams
stripe · Sep 9, 2019 · ba9e023 · ba9e023
2 parents b3ebb8a + 7b6ed2c
commit ba9e023
Show file tree

Hide file tree

Showing 16 changed files with 86 additions and 83 deletions.
diff --git a/lib/StripeMethod.js b/lib/StripeMethod.js
@@ -47,3 +47,5 @@ function stripeMethod(spec) {
 }
 
 module.exports = stripeMethod;
+
+module.exports = stripeMethod;
diff --git a/lib/makeRequest.js b/lib/makeRequest.js
@@ -46,10 +46,15 @@ function getRequestOpts(self, requestArgs, spec, overrideData) {
     spec.validator(data, {headers});
   }
 
+  const dataInQuery = spec.method === 'GET' || spec.method === 'DELETE';
+  const bodyData = dataInQuery ? {} : data;
+  const queryData = dataInQuery ? data : {};
+
   return {
     requestMethod,
     requestPath,
-    data,
+    bodyData,
+    queryData,
     auth: options.auth,
     headers,
     host,
@@ -77,11 +82,18 @@ function makeRequest(self, requestArgs, spec, overrideData) {
       }
     }
 
+    const emptyQuery = Object.keys(opts.queryData).length === 0;
+    const path = [
+      opts.requestPath,
+      emptyQuery ? '' : '?',
+      utils.stringifyRequestData(opts.queryData),
+    ].join('');
+
     self._request(
       opts.requestMethod,
       opts.host,
-      opts.requestPath,
-      opts.data,
+      path,
+      opts.bodyData,
       opts.auth,
       {headers: opts.headers},
       requestCallback

diff --git a/test/StripeResource.spec.js b/test/StripeResource.spec.js
@@ -48,21 +48,27 @@ describe('StripeResource', () => {
     });
 
     describe('_request', () => {
-      it('encodes the body in GET requests', (done) => {
+      it('encodes data for GET requests as query params', (done) => {
+        const data = {
+          customer: 'cus_123',
+          subscription_items: [
+            {plan: 'foo', quantity: 2},
+            {id: 'si_123', deleted: true},
+          ],
+        };
         const options = {
           host: stripe.getConstant('DEFAULT_HOST'),
           path: '/v1/invoices/upcoming',
-          data: {
-            customer: 'cus_123',
-            subscription_items: [
-              {plan: 'foo', quantity: 2},
-              {id: 'si_123', deleted: true},
-            ],
-          },
+          data,
         };
 
         const scope = nock(`https://${options.host}`)
-          .get(options.path, options.data)
+          .get(
+            `${
+              options.path
+            }?customer=cus_123&subscription_items[0][plan]=foo&subscription_items[0][quantity]=2&subscription_items[1][id]=si_123&subscription_items[1][deleted]=true`,
+            ''
+          )
           .reply(200, '{}');
 
         realStripe.invoices.retrieveUpcoming(options.data, (err, response) => {
@@ -71,6 +77,22 @@ describe('StripeResource', () => {
         });
       });
 
+      it('encodes data for DELETE requests as query params', (done) => {
+        const data = {
+          foo: 'bar',
+        };
+        const host = stripe.getConstant('DEFAULT_HOST');
+
+        const scope = nock(`https://${host}`)
+          .delete(/.*/)
+          .reply(200, '{}');
+
+        realStripe.invoiceItems.del('invoiceItemId1', data, (err, response) => {
+          done(err);
+          scope.done();
+        });
+      });
+
       it('encodes the body in POST requests', (done) => {
         const options = {
           host: stripe.getConstant('DEFAULT_HOST'),

diff --git a/test/mocha.opts b/test/mocha.opts
@@ -1,2 +1 @@
---bail
 --recursive
diff --git a/test/resources/BitcoinReceivers.spec.js b/test/resources/BitcoinReceivers.spec.js
@@ -35,11 +35,9 @@ describe('BitcoinReceivers Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/bitcoin/receivers/receiverId/transactions',
+        url: '/v1/bitcoin/receivers/receiverId/transactions?limit=1',
         headers: {},
-        data: {
-          limit: 1,
-        },
+        data: {},
       });
     });
   });

diff --git a/test/resources/CreditNotes.spec.js b/test/resources/CreditNotes.spec.js
@@ -38,9 +38,9 @@ describe('CreditNotes Resource', () => {
       stripe.creditNotes.list({count: 25});
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/credit_notes',
+        url: '/v1/credit_notes?count=25',
         headers: {},
-        data: {count: 25},
+        data: {},
       });
     });
   });

diff --git a/test/resources/Events.spec.js b/test/resources/Events.spec.js
@@ -21,9 +21,9 @@ describe('Events Resource', () => {
       stripe.events.list({count: 25});
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/events',
+        url: '/v1/events?count=25',
         headers: {},
-        data: {count: 25},
+        data: {},
       });
     });
   });

diff --git a/test/resources/Invoices.spec.js b/test/resources/Invoices.spec.js
@@ -33,9 +33,9 @@ describe('Invoices Resource', () => {
       stripe.invoices.list({count: 25});
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/invoices',
+        url: '/v1/invoices?count=25',
         headers: {},
-        data: {count: 25},
+        data: {},
       });
     });
   });
@@ -97,12 +97,10 @@ describe('Invoices Resource', () => {
 
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/invoices/upcoming',
+        url:
+          '/v1/invoices/upcoming?customer=cus_abc&subscription_items[0][plan]=potato&subscription_items[1][plan]=rutabaga',
         headers: {},
-        data: {
-          customer: 'cus_abc',
-          subscription_items: [{plan: 'potato'}, {plan: 'rutabaga'}],
-        },
+        data: {},
       });
     });
   });
@@ -117,13 +115,10 @@ describe('Invoices Resource', () => {
 
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/invoices/upcoming/lines',
+        url:
+          '/v1/invoices/upcoming/lines?customer=cus_abc&subscription_items[0][plan]=potato&subscription_items[1][plan]=rutabaga&limit=5',
         headers: {},
-        data: {
-          customer: 'cus_abc',
-          subscription_items: [{plan: 'potato'}, {plan: 'rutabaga'}],
-          limit: 5,
-        },
+        data: {},
       });
     });
   });

diff --git a/test/resources/OrderReturns.spec.js b/test/resources/OrderReturns.spec.js
@@ -23,10 +23,8 @@ describe('OrderReturn Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/order_returns',
-        data: {
-          limit: 3,
-        },
+        url: '/v1/order_returns?limit=3',
+        data: {},
         headers: {},
       });
     });
@@ -37,11 +35,9 @@ describe('OrderReturn Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/order_returns',
-        data: {
-          order: 'orderIdFoo123',
-        },
+        url: '/v1/order_returns?order=orderIdFoo123',
         headers: {},
+        data: {},
       });
     });
   });

diff --git a/test/resources/Orders.spec.js b/test/resources/Orders.spec.js
@@ -65,10 +65,8 @@ describe('Order Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/orders',
-        data: {
-          limit: 3,
-        },
+        url: '/v1/orders?limit=3',
+        data: {},
         headers: {},
       });
     });
@@ -79,10 +77,8 @@ describe('Order Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/orders',
-        data: {
-          status: 'active',
-        },
+        url: '/v1/orders?status=active',
+        data: {},
         headers: {},
       });
     });

diff --git a/test/resources/PaymentMethods.spec.js b/test/resources/PaymentMethods.spec.js
@@ -40,9 +40,9 @@ describe('PaymentMethods Resource', () => {
       stripe.paymentMethods.list(data);
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/payment_methods',
+        url: '/v1/payment_methods?customer=cus_123&type=card',
         headers: {},
-        data,
+        data: {},
       });
     });
   });

diff --git a/test/resources/Products.spec.js b/test/resources/Products.spec.js
@@ -43,10 +43,8 @@ describe('Product Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/products',
-        data: {
-          limit: 3,
-        },
+        url: '/v1/products?limit=3',
+        data: {},
         headers: {},
       });
     });
@@ -57,10 +55,8 @@ describe('Product Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/products',
-        data: {
-          shippable: true,
-        },
+        url: '/v1/products?shippable=true',
+        data: {},
         headers: {},
       });
     });

diff --git a/test/resources/Radar/ValueListItems.spec.js b/test/resources/Radar/ValueListItems.spec.js
@@ -44,11 +44,9 @@ describe('Radar', () => {
         });
         expect(stripe.LAST_REQUEST).to.deep.equal({
           method: 'GET',
-          url: '/v1/radar/value_list_items',
+          url: '/v1/radar/value_list_items?value_list=rsl_123',
           headers: {},
-          data: {
-            value_list: 'rsl_123',
-          },
+          data: {},
         });
       });
     });

diff --git a/test/resources/SKUs.spec.js b/test/resources/SKUs.spec.js
@@ -47,10 +47,8 @@ describe('SKU Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/skus',
-        data: {
-          limit: 3,
-        },
+        url: '/v1/skus?limit=3',
+        data: {},
         headers: {},
       });
     });
@@ -61,10 +59,8 @@ describe('SKU Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/skus',
-        data: {
-          product: 'prodId123',
-        },
+        url: '/v1/skus?product=prodId123',
+        data: {},
         headers: {},
       });
     });

diff --git a/test/resources/SubscriptionItems.spec.js b/test/resources/SubscriptionItems.spec.js
@@ -71,12 +71,9 @@ describe('SubscriptionItems Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/subscription_items',
+        url: '/v1/subscription_items?limit=3&subscription=test_sub',
         headers: {},
-        data: {
-          limit: 3,
-          subscription: 'test_sub',
-        },
+        data: {},
       });
     });
   });

diff --git a/test/resources/Subscriptions.spec.js b/test/resources/Subscriptions.spec.js
@@ -178,13 +178,9 @@ describe('subscriptions Resource', () => {
       });
       expect(stripe.LAST_REQUEST).to.deep.equal({
         method: 'GET',
-        url: '/v1/subscriptions',
+        url: '/v1/subscriptions?limit=3&customer=test_cus&plan=gold',
         headers: {},
-        data: {
-          limit: 3,
-          customer: 'test_cus',
-          plan: 'gold',
-        },
+        data: {},
       });
     });
   });
Original file line number	Diff line number	Diff line change
Expand Up		@@ -47,3 +47,5 @@ function stripeMethod(spec) {
		}

		module.exports = stripeMethod;

		module.exports = stripeMethod;