Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow include query parameter with store.findRecord & store.findAll #3976

Merged
merged 1 commit into from
Dec 16, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ entry in `config/features.json`.

## Feature Flags

- `ds-find-include`

Allows an `include` query parameter to be specified with using
`store.findRecord()` and `store.findAll()` as described in [RFC
99](https://github.com/emberjs/rfcs/pull/99)

- `ds-references`

Adds references as described in [RFC 57](https://github.com/emberjs/rfcs/pull/57)
35 changes: 27 additions & 8 deletions addon/-private/adapters/rest-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@ import {
AbortError
} from 'ember-data/-private/adapters/errors';
import EmptyObject from "ember-data/-private/system/empty-object";
var get = Ember.get;
var MapWithDefault = Ember.MapWithDefault;

import BuildURLMixin from "ember-data/-private/adapters/build-url-mixin";
import isEnabled from 'ember-data/-private/features';

const {
MapWithDefault,
get
} = Ember;

/**
The REST adapter allows your store to communicate with an HTTP server by
Expand Down Expand Up @@ -372,7 +375,10 @@ 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 query = this.buildQuery(snapshot);

return this.ajax(url, 'GET', { data: query });
},

/**
Expand All @@ -390,14 +396,13 @@ 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 query = this.buildQuery(snapshotRecordArray);

if (sinceToken) {
query = { since: sinceToken };
query.since = sinceToken;
}

url = this.buildURL(type.modelName, null, null, 'findAll');

return this.ajax(url, 'GET', { data: query });
},

Expand Down Expand Up @@ -959,6 +964,20 @@ export default Adapter.extend(BuildURLMixin, {
return ['Ember Data Request ' + requestDescription + ' returned a ' + status,
payloadDescription,
shortenedPayload].join('\n');
},

buildQuery(snapshot) {
const { include } = snapshot;

let query = {};

if (isEnabled('ds-finder-include')) {
if (include) {
query.include = include;
}
}

return query;
}
});

Expand Down
5 changes: 1 addition & 4 deletions addon/-private/system/model/internal-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,7 @@ InternalModel.prototype = {
@private
*/
createSnapshot(options) {
var adapterOptions = options && options.adapterOptions;
var snapshot = new Snapshot(this);
snapshot.adapterOptions = adapterOptions;
return snapshot;
return new Snapshot(this, options);
},

/**
Expand Down
5 changes: 2 additions & 3 deletions addon/-private/system/record-arrays/record-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
},

createSnapshot(options) {
var adapterOptions = options && options.adapterOptions;
var meta = this.get('meta');
return new SnapshotRecordArray(this, meta, adapterOptions);
const meta = this.get('meta');
return new SnapshotRecordArray(this, meta, options);
}
});
10 changes: 8 additions & 2 deletions addon/-private/system/snapshot-record-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
@module ember-data
*/

import isEnabled from 'ember-data/-private/features';

/**
@class SnapshotRecordArray
@namespace DS
Expand All @@ -10,7 +12,7 @@
@param {Array} snapshots An array of snapshots
@param {Object} meta
*/
export default function SnapshotRecordArray(recordArray, meta, adapterOptions) {
export default function SnapshotRecordArray(recordArray, meta, options = {}) {
/**
An array of snapshots
@private
Expand Down Expand Up @@ -48,7 +50,11 @@ export default function SnapshotRecordArray(recordArray, meta, adapterOptions) {
@property adapterOptions
@type {Object}
*/
this.adapterOptions = adapterOptions;
this.adapterOptions = options.adapterOptions;

if (isEnabled('ds-finder-include')) {
this.include = options.include;
}
}

/**
Expand Down
15 changes: 14 additions & 1 deletion addon/-private/system/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import Ember from 'ember';
import EmptyObject from "ember-data/-private/system/empty-object";
import isEnabled from 'ember-data/-private/features';

var get = Ember.get;

/**
Expand All @@ -13,7 +15,7 @@ var get = Ember.get;
@constructor
@param {DS.Model} internalModel The model to create a snapshot from
*/
export default function Snapshot(internalModel) {
export default function Snapshot(internalModel, options = {}) {
this._attributes = new EmptyObject();
this._belongsToRelationships = new EmptyObject();
this._belongsToIds = new EmptyObject();
Expand All @@ -29,6 +31,17 @@ export default function Snapshot(internalModel) {
this.type = internalModel.type;
this.modelName = internalModel.type.modelName;

/**
A hash of adapter options
@property adapterOptions
@type {Object}
*/
this.adapterOptions = options.adapterOptions;

if (isEnabled('ds-finder-include')) {
this.include = options.include;
}

this._changedAttributes = record.changedAttributes();
}

Expand Down
1 change: 1 addition & 0 deletions config/features.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"ds-finder-include": null,
"ds-references": null
}
51 changes: 43 additions & 8 deletions tests/integration/adapter/rest-adapter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Ember from 'ember';
import {module, test} from 'qunit';

import DS from 'ember-data';
import isEnabled from 'ember-data/-private/features';

var env, store, adapter, Post, Comment, SuperUser;
var passedUrl, passedVerb, passedHash;
Expand Down Expand Up @@ -56,7 +57,7 @@ test("findRecord - basic payload", function(assert) {
run(store, 'findRecord', 'post', 1).then(assert.wait(function(post) {
assert.equal(passedUrl, "/posts/1");
assert.equal(passedVerb, "GET");
assert.equal(passedHash, undefined);
assert.deepEqual(passedHash.data, {});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be preferred to continue returning undefined rather than {} in these cases? adapter.ajax seems to act the same with { data: {} } and {}.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returning {} seems ok to me.


assert.equal(post.get('id'), "1");
assert.equal(post.get('name'), "Rails is omakase");
Expand All @@ -83,7 +84,7 @@ test("find - basic payload (with legacy singular name)", function(assert) {
run(store, 'findRecord', 'post', 1).then(assert.wait(function(post) {
assert.equal(passedUrl, "/posts/1");
assert.equal(passedVerb, "GET");
assert.equal(passedHash, undefined);
assert.deepEqual(passedHash.data, {});

assert.equal(post.get('id'), "1");
assert.equal(post.get('name'), "Rails is omakase");
Expand All @@ -101,7 +102,7 @@ test("findRecord - payload with sideloaded records of the same type", function(a
run(store, 'findRecord', 'post', 1).then(assert.wait(function(post) {
assert.equal(passedUrl, "/posts/1");
assert.equal(passedVerb, "GET");
assert.equal(passedHash, undefined);
assert.deepEqual(passedHash.data, {});

assert.equal(post.get('id'), "1");
assert.equal(post.get('name'), "Rails is omakase");
Expand All @@ -121,7 +122,7 @@ test("findRecord - payload with sideloaded records of a different type", functio
run(store, 'findRecord', 'post', 1).then(assert.wait(function(post) {
assert.equal(passedUrl, "/posts/1");
assert.equal(passedVerb, "GET");
assert.equal(passedHash, undefined);
assert.deepEqual(passedHash.data, {});

assert.equal(post.get('id'), "1");
assert.equal(post.get('name'), "Rails is omakase");
Expand All @@ -143,7 +144,7 @@ test("findRecord - payload with an serializer-specified primary key", function(a
run(store, 'findRecord', 'post', 1).then(assert.wait(function(post) {
assert.equal(passedUrl, "/posts/1");
assert.equal(passedVerb, "GET");
assert.equal(passedHash, undefined);
assert.deepEqual(passedHash.data, {});

assert.equal(post.get('id'), "1");
assert.equal(post.get('name'), "Rails is omakase");
Expand All @@ -167,14 +168,31 @@ test("findRecord - payload with a serializer-specified attribute mapping", funct
run(store, 'findRecord', 'post', 1).then(assert.wait(function(post) {
assert.equal(passedUrl, "/posts/1");
assert.equal(passedVerb, "GET");
assert.equal(passedHash, undefined);
assert.deepEqual(passedHash.data, {});

assert.equal(post.get('id'), "1");
assert.equal(post.get('name'), "Rails is omakase");
assert.equal(post.get('createdAt'), 2013);
}));
});

if (isEnabled('ds-finder-include')) {
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("createRecord - an empty payload is a basic success if an id was specified", function(assert) {
ajaxResponse();
var post;
Expand Down Expand Up @@ -983,7 +1001,7 @@ test("findAll - returning an array populates the array", function(assert) {
store.findAll('post').then(assert.wait(function(posts) {
assert.equal(passedUrl, "/posts");
assert.equal(passedVerb, "GET");
assert.equal(passedHash.data, undefined);
assert.deepEqual(passedHash.data, {});

var post1 = store.peekRecord('post', 1);
var post2 = store.peekRecord('post', 2);
Expand Down Expand Up @@ -1028,6 +1046,23 @@ test("findAll - passes buildURL the requestType", function(assert) {
}));
});

if (isEnabled('ds-finder-include')) {
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: [
Expand Down Expand Up @@ -1473,7 +1508,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.deepEqual(passedHash.data, {});
}));
});

Expand Down
29 changes: 27 additions & 2 deletions tests/integration/adapter/store-adapter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Ember from 'ember';
import {module, test} from 'qunit';

import DS from 'ember-data';
import isEnabled from 'ember-data/-private/features';

/*
This is an integration test that tests the communication between a store
Expand Down Expand Up @@ -1303,7 +1304,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) {
Expand All @@ -1316,8 +1317,20 @@ test("findRecord should pass adapterOptions to the find method", function(assert
});
});

if (isEnabled('ds-finder-include')) {
test("store.findRecord should pass 'include' to adapter.findRecord", function(assert) {
assert.expect(1);

test("findAll should pass adapterOptions to the findAll method", function(assert) {
env.adapter.findRecord = assert.wait((store, type, id, snapshot) => {
assert.equal(snapshot.include, 'books', 'include passed to adapter.findRecord');
return Ember.RSVP.resolve({ id: 1 });
});

run(() => store.findRecord('person', 1, { include: 'books' }));
});
}

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) {
Expand All @@ -1331,6 +1344,18 @@ test("findAll should pass adapterOptions to the findAll method", function(assert
});
});

if (isEnabled('ds-finder-include')) {
test("store.findAll should pass 'include' to adapter.findAll", function(assert) {
assert.expect(1);

env.adapter.findAll = assert.wait((store, type, sinceToken, arraySnapshot) => {
assert.equal(arraySnapshot.include, 'books', 'include passed to adapter.findAll');
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({
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/adapters/rest-adapter/ajax-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, {});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the 3rd argument needed here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I see now that this is the adapter method.

});
});

Expand All @@ -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', {});
});
});

Expand Down
Loading