From 97193d0dcdc1d3b721a79f8b11ac0d2764bc5be0 Mon Sep 17 00:00:00 2001 From: Eric Kelly Date: Mon, 7 Dec 2015 23:21:41 -0500 Subject: [PATCH] Allow `include` query parameter with store.findRecord & store.findAll Related RFC: https://github.com/emberjs/rfcs/pull/99 These changes allow an `include` paremeter to be specified when using `store.findRecord` and `store.findAll`: ```javascript store.findRecord('post', 123, { include: 'comments' }); store.findAll('post', { include: 'comments' }); ``` The value for `include` is copied into the `adapterOptions` that are passed to the corresponding adapter function as part of the `snapshot` or `snapshotRecordArray`. This value is then pulled from the snapshot and used as a query parameter (`include`) when making an AJAX request via `adapter.ajax`. --- addon/adapters/rest-adapter.js | 30 ++++++++++--- addon/system/store.js | 21 ++++++--- .../integration/adapter/rest-adapter-test.js | 45 ++++++++++++++++--- .../integration/adapter/store-adapter-test.js | 25 ++++++++++- tests/unit/adapters/rest-adapter/ajax-test.js | 6 +-- 5 files changed, 105 insertions(+), 22 deletions(-) diff --git a/addon/adapters/rest-adapter.js b/addon/adapters/rest-adapter.js index 26098e3eb58..22caba5c7f8 100644 --- a/addon/adapters/rest-adapter.js +++ b/addon/adapters/rest-adapter.js @@ -10,8 +10,12 @@ import { AbortError } from 'ember-data/adapters/errors'; import EmptyObject from "ember-data/system/empty-object"; -var get = Ember.get; -var MapWithDefault = Ember.MapWithDefault; + +const { + MapWithDefault, + get, + merge +} = Ember; import BuildURLMixin from "ember-data/adapters/build-url-mixin"; @@ -371,7 +375,17 @@ export default Adapter.extend(BuildURLMixin, { @return {Promise} promise */ findRecord(store, type, id, snapshot) { - return this.ajax(this.buildURL(type.modelName, id, snapshot, 'findRecord'), 'GET'); + const url = this.buildURL(type.modelName, id, snapshot, 'findRecord'); + const adapterOptions = snapshot.adapterOptions || {}; + const { include } = adapterOptions; + + let query; + + if (include) { + query = { include }; + } + + return this.ajax(url, 'GET', { data: query }); }, /** @@ -389,13 +403,19 @@ export default Adapter.extend(BuildURLMixin, { @return {Promise} promise */ findAll(store, type, sinceToken, snapshotRecordArray) { - var query, url; + const url = this.buildURL(type.modelName, null, null, 'findAll'); + const adapterOptions = snapshotRecordArray.adapterOptions || {}; + const { include } = adapterOptions; + + let query; if (sinceToken) { query = { since: sinceToken }; } - url = this.buildURL(type.modelName, null, null, 'findAll'); + if (include) { + query = merge(query || {}, { include }); + } return this.ajax(url, 'GET', { data: query }); }, diff --git a/addon/system/store.js b/addon/system/store.js index 8d1e2b52ec8..41006c0dff4 100644 --- a/addon/system/store.js +++ b/addon/system/store.js @@ -59,6 +59,7 @@ export let badIdFormatAssertion = '`id` has to be non-empty string or number'; var Backburner = Ember._Backburner || Ember.Backburner || Ember.__loader.require('backburner')['default'] || Ember.__loader.require('backburner')['Backburner']; var Map = Ember.Map; var isArray = Array.isArray || Ember.isArray; +var merge = Ember.merge; //Shim Backburner.join if (!Backburner.prototype.join) { @@ -499,12 +500,16 @@ Store = Service.extend({ @param {Object} options @return {Promise} promise */ - findRecord(modelName, id, options) { + findRecord(modelName, id, options = {}) { assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string'); assert(badIdFormatAssertion, (typeof id === 'string' && id.length > 0) || (typeof id === 'number' && !isNaN(id))); - var internalModel = this._internalModelForId(modelName, id); - options = options || {}; + const internalModel = this._internalModelForId(modelName, id); + const { include } = options; + + if (include) { + options.adapterOptions = merge(options.adapterOptions || {}, { include }); + } if (!this.hasRecordForId(modelName, id)) { return this._findByInternalModel(internalModel, options); @@ -1001,9 +1006,15 @@ Store = Service.extend({ @param {Object} options @return {DS.AdapterPopulatedRecordArray} */ - findAll(modelName, options) { + findAll(modelName, options = {}) { assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string'); - var typeClass = this.modelFor(modelName); + + const typeClass = this.modelFor(modelName); + const { include } = options; + + if (include) { + options.adapterOptions = merge(options.adapterOptions || {}, { include }); + } return this._fetchAll(typeClass, this.peekAll(modelName), options); }, diff --git a/tests/integration/adapter/rest-adapter-test.js b/tests/integration/adapter/rest-adapter-test.js index 4d99db2543c..d891515e435 100644 --- a/tests/integration/adapter/rest-adapter-test.js +++ b/tests/integration/adapter/rest-adapter-test.js @@ -56,7 +56,7 @@ test("find - basic payload", function(assert) { run(store, 'find', 'post', 1).then(assert.wait(function(post) { assert.equal(passedUrl, "/posts/1"); assert.equal(passedVerb, "GET"); - assert.equal(passedHash, undefined); + assert.equal(passedHash.data, undefined); assert.equal(post.get('id'), "1"); assert.equal(post.get('name'), "Rails is omakase"); @@ -83,7 +83,7 @@ test("find - basic payload (with legacy singular name)", function(assert) { run(store, 'find', 'post', 1).then(assert.wait(function(post) { assert.equal(passedUrl, "/posts/1"); assert.equal(passedVerb, "GET"); - assert.equal(passedHash, undefined); + assert.equal(passedHash.data, undefined); assert.equal(post.get('id'), "1"); assert.equal(post.get('name'), "Rails is omakase"); @@ -101,7 +101,7 @@ test("find - payload with sideloaded records of the same type", function(assert) run(store, 'find', 'post', 1).then(assert.wait(function(post) { assert.equal(passedUrl, "/posts/1"); assert.equal(passedVerb, "GET"); - assert.equal(passedHash, undefined); + assert.equal(passedHash.data, undefined); assert.equal(post.get('id'), "1"); assert.equal(post.get('name'), "Rails is omakase"); @@ -121,7 +121,7 @@ test("find - payload with sideloaded records of a different type", function(asse run(store, 'find', 'post', 1).then(assert.wait(function(post) { assert.equal(passedUrl, "/posts/1"); assert.equal(passedVerb, "GET"); - assert.equal(passedHash, undefined); + assert.equal(passedHash.data, undefined); assert.equal(post.get('id'), "1"); assert.equal(post.get('name'), "Rails is omakase"); @@ -143,7 +143,7 @@ test("find - payload with an serializer-specified primary key", function(assert) run(store, 'find', 'post', 1).then(assert.wait(function(post) { assert.equal(passedUrl, "/posts/1"); assert.equal(passedVerb, "GET"); - assert.equal(passedHash, undefined); + assert.equal(passedHash.data, undefined); assert.equal(post.get('id'), "1"); assert.equal(post.get('name'), "Rails is omakase"); @@ -167,7 +167,7 @@ test("find - payload with a serializer-specified attribute mapping", function(as run(store, 'find', 'post', 1).then(assert.wait(function(post) { assert.equal(passedUrl, "/posts/1"); assert.equal(passedVerb, "GET"); - assert.equal(passedHash, undefined); + assert.equal(passedHash.data, undefined); assert.equal(post.get('id'), "1"); assert.equal(post.get('name'), "Rails is omakase"); @@ -175,6 +175,21 @@ test("find - payload with a serializer-specified attribute mapping", function(as })); }); +test("findRecord - passes `include` as a query parameter to ajax", function(assert) { + adapter.ajax = function(url, verb, hash) { + assert.deepEqual(hash.data, { include: 'comments' }, + '`include` parameter sent to adapter.ajax'); + + return run(Ember.RSVP, 'resolve', { + post: { id: 1, name: 'Rails is very expensive sushi' } + }); + }; + + run(store, 'findRecord', 'post', 1, { include: 'comments' }).then(assert.wait(function() { + // Noop + })); +}); + test("create - an empty payload is a basic success if an id was specified", function(assert) { ajaxResponse(); var post; @@ -1028,6 +1043,22 @@ test("findAll - passes buildURL the requestType", function(assert) { })); }); +test("findAll - passed `include` as a query parameter to ajax", function(assert) { + adapter.ajax = function(url, verb, hash) { + assert.deepEqual(hash.data, { include: 'comments' }, + '`include` params sent to adapter.ajax'); + + return run(Ember.RSVP, 'resolve', { + posts: [{ id: 1, name: 'Rails is very expensive sushi' }] + }); + }; + + run(store, 'findAll', 'post', { include: 'comments' }).then(assert.wait(function() { + // Noop + })); +}); + + test("findAll - returning sideloaded data loads the data", function(assert) { ajaxResponse({ posts: [ @@ -1461,7 +1492,7 @@ test("findMany - findMany does not coalesce by default", function(assert) { }); run(post, 'get', 'comments').then(assert.wait(function(comments) { assert.equal(passedUrl, "/comments/3"); - assert.equal(passedHash, null); + assert.equal(passedHash.data, null); })); }); diff --git a/tests/integration/adapter/store-adapter-test.js b/tests/integration/adapter/store-adapter-test.js index 1ccc6c3c418..2bcef0dd414 100644 --- a/tests/integration/adapter/store-adapter-test.js +++ b/tests/integration/adapter/store-adapter-test.js @@ -1303,7 +1303,7 @@ test("record.save should pass adapterOptions to the deleteRecord method", functi }); -test("findRecord should pass adapterOptions to the find method", function(assert) { +test("store.findRecord should pass adapterOptions to adapter.findRecord", function(assert) { assert.expect(1); env.adapter.findRecord = assert.wait(function(store, type, id, snapshot) { @@ -1316,8 +1316,18 @@ test("findRecord should pass adapterOptions to the find method", function(assert }); }); +test("store.findRecord should pass 'include' inside adapterOptions passed to adapter.findRecord", function(assert) { + assert.expect(1); + + env.adapter.findRecord = assert.wait(function(store, type, id, snapshot) { + assert.deepEqual(snapshot.adapterOptions, { include: 'books' }); + return Ember.RSVP.resolve({ id: 1 }); + }); + + run(() => store.findRecord('person', 1, { include: 'books' })); +}); -test("findAll should pass adapterOptions to the findAll method", function(assert) { +test("store.findAll should pass adapterOptions to the adapter.findAll method", function(assert) { assert.expect(1); env.adapter.findAll = assert.wait(function(store, type, sinceToken, arraySnapshot) { @@ -1331,6 +1341,17 @@ test("findAll should pass adapterOptions to the findAll method", function(assert }); }); +test("store.findAll should pass 'include' inside adapterOptions passed to adapter.findAll", function(assert) { + assert.expect(1); + + env.adapter.findAll = assert.wait(function(store, type, sinceToken, arraySnapshot) { + const { adapterOptions } = arraySnapshot; + assert.deepEqual(adapterOptions, { include: 'books' }); + return Ember.RSVP.resolve([{ id: 1 }]); + }); + + run(() => store.findAll('person', { include: 'books' })); +}); test("An async hasMany relationship with links should not trigger shouldBackgroundReloadRecord", function(assert) { var Post = DS.Model.extend({ diff --git a/tests/unit/adapters/rest-adapter/ajax-test.js b/tests/unit/adapters/rest-adapter/ajax-test.js index ade40bc15d3..f699282cf67 100644 --- a/tests/unit/adapters/rest-adapter/ajax-test.js +++ b/tests/unit/adapters/rest-adapter/ajax-test.js @@ -35,8 +35,8 @@ test("When an id is searched, the correct url should be generated", function(ass return Ember.RSVP.resolve(); }; run(function() { - adapter.findRecord(store, Person, 1); - adapter.findRecord(store, Place, 1); + adapter.findRecord(store, Person, 1, {}); + adapter.findRecord(store, Place, 1, {}); }); }); @@ -47,7 +47,7 @@ test("id's should be sanatized", function(assert) { return Ember.RSVP.resolve(); }; run(function() { - adapter.findRecord(store, Person, '../place/1'); + adapter.findRecord(store, Person, '../place/1', {}); }); });