From c83485ecec6782426b77e5d982a196373c603b50 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 6 Sep 2019 16:10:07 -0400 Subject: [PATCH 1/4] GET and DELETE requests data: body->queryParams --- lib/StripeMethod.js | 2 + lib/makeRequest.js | 18 +++++++-- test/StripeResource.spec.js | 41 ++++++++++++++++----- test/mocha.opts | 1 - test/resources/BitcoinReceivers.spec.js | 6 +-- test/resources/CreditNotes.spec.js | 4 +- test/resources/Events.spec.js | 4 +- test/resources/Invoices.spec.js | 21 ++++------- test/resources/OrderReturns.spec.js | 12 ++---- test/resources/Orders.spec.js | 12 ++---- test/resources/PaymentMethods.spec.js | 4 +- test/resources/Products.spec.js | 12 ++---- test/resources/Radar/ValueListItems.spec.js | 6 +-- test/resources/SKUs.spec.js | 12 ++---- test/resources/SubscriptionItems.spec.js | 7 +--- test/resources/Subscriptions.spec.js | 8 +--- 16 files changed, 87 insertions(+), 83 deletions(-) diff --git a/lib/StripeMethod.js b/lib/StripeMethod.js index ace8febfe5..22f75d4527 100644 --- 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 index c6b105b980..39d0b4b4d3 100644 --- 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 index 6aea57b87d..da0c02a406 100644 --- 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,23 @@ describe('StripeResource', () => { }); }); + it('encodes data for DELETE requests as query params', (done) => { + const data = { + foo: 'bar', + }; + const host = stripe.getConstant('DEFAULT_HOST'); + const path = '/v1/invoiceitems/invoiceItemId1?foo=bar'; + + 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 index 6ea60f6426..4a52320178 100644 --- 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 index 494840c17a..9093595570 100644 --- 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 index 025e01a176..7cb01ce331 100644 --- 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 index 28fbe98ed4..455478a3c4 100644 --- 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 index 4a714c40fd..fe84e835d7 100644 --- 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 index f39052edb2..7f7fe99d8c 100644 --- 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 index c367271577..8b2c352e21 100644 --- 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 index 0860c1cb45..a9540498da 100644 --- 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 index 21c6193521..78cba35b57 100644 --- 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 index 286ef956ba..5d25722a34 100644 --- 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 index 5155d8fec8..c397c5297c 100644 --- 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 index 56ba399e65..4a6d1cde46 100644 --- 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 index d1f7631295..5575c8480b 100644 --- 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: {}, }); }); }); From a8b372e9d39c39a4c974c7eacc33ae7afddbf9f4 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 6 Sep 2019 18:00:28 -0400 Subject: [PATCH 2/4] Bail in CI --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f801f5d9a4..c95f556692 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "license": "MIT", "scripts": { "clean": "rm -rf ./.nyc_output ./node_modules/.cache ./coverage", - "mocha": "nyc mocha", + "mocha": "nyc mocha --bail", "mocha-only": "mocha", "test": "npm run lint && npm run mocha", "lint": "eslint --ext .js,.jsx .", From a5bfe08a083a0b9b40a4ce975eb860b204777dfe Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 6 Sep 2019 18:03:21 -0400 Subject: [PATCH 3/4] Fix lint --- test/StripeResource.spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/StripeResource.spec.js b/test/StripeResource.spec.js index da0c02a406..6c60c2fdbd 100644 --- a/test/StripeResource.spec.js +++ b/test/StripeResource.spec.js @@ -82,7 +82,6 @@ describe('StripeResource', () => { foo: 'bar', }; const host = stripe.getConstant('DEFAULT_HOST'); - const path = '/v1/invoiceitems/invoiceItemId1?foo=bar'; const scope = nock(`https://${host}`) .delete(/.*/) From 7b6ed2cf2bed70b1a6ace5c8a4b9ab56cb4dbc13 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Mon, 9 Sep 2019 08:30:16 -0400 Subject: [PATCH 4/4] Bail on bail --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c95f556692..f801f5d9a4 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "license": "MIT", "scripts": { "clean": "rm -rf ./.nyc_output ./node_modules/.cache ./coverage", - "mocha": "nyc mocha --bail", + "mocha": "nyc mocha", "mocha-only": "mocha", "test": "npm run lint && npm run mocha", "lint": "eslint --ext .js,.jsx .",