From 80a7153b2f4bdaf372180447fdb1e69bb0a35ac3 Mon Sep 17 00:00:00 2001 From: Santiago Petrone Date: Mon, 23 Oct 2017 18:04:27 -0300 Subject: [PATCH] Address PR comments --- src/storage_manager/model/RemoteStorage.js | 26 ++++++++-------------- src/utils/fetch.js | 9 ++------ test/specs/storage_manager/model/Models.js | 2 +- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/src/storage_manager/model/RemoteStorage.js b/src/storage_manager/model/RemoteStorage.js index a5d9cf43e..019ae18e9 100644 --- a/src/storage_manager/model/RemoteStorage.js +++ b/src/storage_manager/model/RemoteStorage.js @@ -74,15 +74,7 @@ module.exports = require('backbone').Model.extend({ }, load(keys, clb) { - let queryString = ''; - keys.forEach(function(key, index, keys) { - queryString += key; - if (index < keys.length - 1) { - queryString += ','; - } - }, this); - - this.request(`${this.get('urlLoad')}?keys=${queryString}`, {method: 'get'}, clb); + this.request(this.get('urlLoad'), {method: 'get'}, clb); }, /** @@ -133,18 +125,18 @@ module.exports = require('backbone').Model.extend({ headers, }; - // Body should not be included on GET, HEAD, OPTIONS or DELETE methods - if (!bodilessMethods.includes(fetchOptions.method)) { + // Body should only be included on POST method + if (fetchOptions.method === 'post') { fetchOptions.body = body; } this.onStart(); - this.fetch(url, fetchOptions).then(res => (res.status/200|0) == 1 ? - res.text() : res.text().then((text) => - Promise.reject(text) - )) - .then((text) => this.onResponse(text, clb)) - .catch(err => this.onError(err)); + this.fetch(url, fetchOptions) + .then(res => (res.status/200|0) == 1 + ? res.text() + : res.text().then((text) => Promise.reject(text))) + .then((text) => this.onResponse(text, clb)) + .catch(err => this.onError(err)); }, }); diff --git a/src/utils/fetch.js b/src/utils/fetch.js index a694d7e65..33bdd5ebd 100644 --- a/src/utils/fetch.js +++ b/src/utils/fetch.js @@ -5,7 +5,6 @@ window.Promise = window.Promise || Promise; export default typeof fetch == 'function' ? fetch.bind() : (url, options) => { return new Promise((res, rej) => { const req = new XMLHttpRequest(); - const bodilessMethods = ['get', 'head', 'options', 'delete']; req.open(options.method || 'get', url); req.withCredentials = options.credentials == 'include'; @@ -25,11 +24,7 @@ export default typeof fetch == 'function' ? fetch.bind() : (url, options) => { req.upload.onprogress = options.onProgress; } - if (bodilessMethods.includes(options.method)) { - req.send(); - } else { - req.send(options.body); - } - + // Include body only if present + options.body ? req.send(options.body) : req.send(); }); } diff --git a/test/specs/storage_manager/model/Models.js b/test/specs/storage_manager/model/Models.js index 3afc26173..25094c678 100644 --- a/test/specs/storage_manager/model/Models.js +++ b/test/specs/storage_manager/model/Models.js @@ -98,7 +98,7 @@ module.exports = { obj.load(['item1', 'item2']); const callResult = obj.fetch; expect(callResult.called).toEqual(true); - expect(callResult.firstCall.args[0]).toEqual(`${endpointLoad}?keys=item1,item2`); + expect(callResult.firstCall.args[0]).toEqual(endpointLoad); }); });