From 56ce7bf7804799d30075c41da2f173302c0e515e Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Thu, 11 Jan 2018 16:46:27 -0500 Subject: [PATCH 1/7] Add osf-model.queryHasMany --- addon/models/osf-model.js | 53 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/addon/models/osf-model.js b/addon/models/osf-model.js index 4a5b36a80..bcfae6d38 100644 --- a/addon/models/osf-model.js +++ b/addon/models/osf-model.js @@ -1,7 +1,12 @@ import Ember from 'ember'; import DS from 'ember-data'; +import ArrayProxy from '@ember/array/proxy'; +import PromiseProxyMixin from '@ember/object/promise-proxy-mixin'; +import { bind } from '@ember/runloop'; +import { Promise as EmberPromise } from 'rsvp'; -import HasManyQuery from 'ember-data-has-many-query'; +import { OSFAdapter } from 'ember-osf/adapters/osf-adapter'; +import { authenticatedAJAX } from 'ember-osf/utils/ajax-helpers'; /** * @module ember-osf @@ -14,7 +19,7 @@ import HasManyQuery from 'ember-data-has-many-query'; * @class OsfModel * @public */ -export default DS.Model.extend(HasManyQuery.ModelMixin, { +export default DS.Model.extend({ links: DS.attr('links'), embeds: DS.attr('embed'), @@ -71,5 +76,47 @@ export default DS.Model.extend(HasManyQuery.ModelMixin, { } }); return this._super(...arguments); - } + }, + + /* + * Query a hasMany relationship with query params + * + * @method queryHasMany + * @param {String} propertyName Name of a hasMany relationship on the model + * @param {Object} queryParams A hash to be serialized into the query string of the request + * @param {Object} [ajaxOptions] A hash of options to be passed to jQuery.ajax + * @returns {ArrayPromiseProxy} Promise-like array proxy, resolves to the records fetched + */ + queryHasMany(propertyName, queryParams, ajaxOptions) { + const reference = this.hasMany(propertyName); + const promise = new EmberPromise((resolve, reject) => { + // HACK: ember-data discards/ignores the link if an object on the belongsTo side + // came first. In that case, grab the link where we expect it from OSF's API + const url = reference.link() || this.get(`links.relationships.${propertyName}.links.related.href`); + if (!url) { + reject(`Could not find a link for '${propertyName}' relationship`); + return; + } + const options = Ember.merge({ + url, + data: queryParams, + headers: OSFAdapter.get('headers'), + }, ajaxOptions); + + authenticatedAJAX(options).catch(reject) + .then(bind(this, this.__queryHasManyDone, resolve)); + }); + + const ArrayPromiseProxy = ArrayProxy.extend(PromiseProxyMixin); + return ArrayPromiseProxy.create({ promise }); + }, + + __queryHasManyDone(resolve, payload) { + const store = this.get('store'); + store.pushPayload(payload); + const records = payload.data.map(datum => store.peekRecord(datum.type, datum.id)); + records.meta = payload.meta; + records.links = payload.links; + resolve(records); + }, }); From 8f790953057aba06bbcbf3a98ea4f9174b37f7eb Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Thu, 11 Jan 2018 16:46:43 -0500 Subject: [PATCH 2/7] Add queryHasMany test --- tests/unit/models/osf-model-test.js | 81 ++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/tests/unit/models/osf-model-test.js b/tests/unit/models/osf-model-test.js index a8894df36..4a404d9db 100644 --- a/tests/unit/models/osf-model-test.js +++ b/tests/unit/models/osf-model-test.js @@ -1,8 +1,9 @@ +import Ember from 'ember'; import { moduleForModel, test } from 'ember-qunit'; moduleForModel('osf-model', 'Unit | Model | osf model', { // Specify the other units that are required for this test. - needs: [] + needs: ['model:user', 'model:node'] }); test('it exists', function(assert) { @@ -10,3 +11,81 @@ test('it exists', function(assert) { // let store = this.store(); assert.ok(!!model); }); + +test('queryHasMany works', function(assert) { + const store = this.store(); + const userId = 'guid1'; + const userNodesUrl = `https://api.osf.io/v2/users/${userId}/nodes/`; + const userPayload = `{ + "data": { + "relationships": { + "nodes": { + "links": { + "related": { + "href": "${userNodesUrl}", + "meta": {} + } + } + }, + }, + "links": { + "self": "https://api.osf.io/v2/users/${userId}/", + "html": "https://osf.io/${userId}/", + }, + "attributes": { + "family_name": "Guid", + "given_name": "A", + "full_name": "A Guid", + }, + "type": "users", + "id": "${userId}" + }, + }`; + store.pushPayload(userPayload); + + const nodeIds = ['guid2', 'guid3', 'guid4'] + const nodeJson = []; + for (const id in nodeIds) { + nodeJson.pushObject(`{ + "links": { + "self": "https://api.osf.io/v2/nodes/${id}/", + "html": "https://osf.io/${id}/" + }, + "attributes": { + "category": "project", + "title": "Project with guid ${id}", + }, + "type": "nodes", + "id": "${id}" + }`); + } + + const queryParams = {'param': 7}; + + Ember.$.mockjax({ + url: userNodesUrl, + responseText: `{ + "data": [ + ${nodeJson.join()} + ], + "meta": { + "total": ${nodeIds.length}, + }, + "links": { + "self": "https://api.osf.io/v2/users/${userId}/nodes/", + } + }` + }); + + assert.expect(3 + nodeIds.length); + + const user = store.peekRecord('user', userId); + assert.ok(!!user); + assert.equal(user.get('id'), userId); + return user.queryHasMany('nodes', queryParams).then(function(nodes) { + assert.equal(nodes.length, nodeIds.length); + for (let i = 0; i < nodes.length; i++) { + assert.equal(nodes[i].get('id'), nodeIds[i]); + } + }); +}); From 1e55e2377b539bfa59b2972cd9820b2b9eff16bb Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Thu, 11 Jan 2018 16:56:42 -0500 Subject: [PATCH 3/7] Fully remove ember-data-has-many-query --- addon/adapters/osf-adapter.js | 4 +--- package.json | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/addon/adapters/osf-adapter.js b/addon/adapters/osf-adapter.js index 7675025b5..5614c4f46 100644 --- a/addon/adapters/osf-adapter.js +++ b/addon/adapters/osf-adapter.js @@ -1,7 +1,6 @@ import Ember from 'ember'; import DS from 'ember-data'; -import HasManyQuery from 'ember-data-has-many-query'; import config from 'ember-get-config'; import GenericDataAdapterMixin from 'ember-osf/mixins/generic-data-adapter'; @@ -19,10 +18,9 @@ import { * * @class OsfAdapter * @extends DS.JSONAPIAdapter - * @uses HasManyQuery.RESTAdapterMixin * @uses GenericDataAdapterMixin */ -export default DS.JSONAPIAdapter.extend(HasManyQuery.RESTAdapterMixin, GenericDataAdapterMixin, { +export default DS.JSONAPIAdapter.extend(GenericDataAdapterMixin, { headers: { ACCEPT: 'application/vnd.api+json; version=2.4' }, diff --git a/package.json b/package.json index b25cde60e..d992a8019 100644 --- a/package.json +++ b/package.json @@ -92,7 +92,6 @@ "ember-cli-node-assets": "0.1.6", "ember-cli-shims": "1.0.2", "ember-cp-validations": "^3.5.0", - "ember-data-has-many-query": "^0.2.0", "ember-diff-attrs": "^0.2.0", "ember-get-config": "0.2.1", "ember-i18n": "5.0.1", From 06d0f12f7b486e75fc97d5e8d93b829636e02d90 Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Fri, 12 Jan 2018 09:26:02 -0500 Subject: [PATCH 4/7] Downgrade imports --- addon/models/osf-model.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/addon/models/osf-model.js b/addon/models/osf-model.js index bcfae6d38..4194a63bb 100644 --- a/addon/models/osf-model.js +++ b/addon/models/osf-model.js @@ -1,10 +1,5 @@ import Ember from 'ember'; import DS from 'ember-data'; -import ArrayProxy from '@ember/array/proxy'; -import PromiseProxyMixin from '@ember/object/promise-proxy-mixin'; -import { bind } from '@ember/runloop'; -import { Promise as EmberPromise } from 'rsvp'; - import { OSFAdapter } from 'ember-osf/adapters/osf-adapter'; import { authenticatedAJAX } from 'ember-osf/utils/ajax-helpers'; @@ -89,7 +84,7 @@ export default DS.Model.extend({ */ queryHasMany(propertyName, queryParams, ajaxOptions) { const reference = this.hasMany(propertyName); - const promise = new EmberPromise((resolve, reject) => { + const promise = new Ember.RSVP.Promise((resolve, reject) => { // HACK: ember-data discards/ignores the link if an object on the belongsTo side // came first. In that case, grab the link where we expect it from OSF's API const url = reference.link() || this.get(`links.relationships.${propertyName}.links.related.href`); @@ -104,10 +99,10 @@ export default DS.Model.extend({ }, ajaxOptions); authenticatedAJAX(options).catch(reject) - .then(bind(this, this.__queryHasManyDone, resolve)); + .then(Ember.bind(this, this.__queryHasManyDone, resolve)); }); - const ArrayPromiseProxy = ArrayProxy.extend(PromiseProxyMixin); + const ArrayPromiseProxy = Ember.ArrayProxy.extend(Ember.PromiseProxyMixin); return ArrayPromiseProxy.create({ promise }); }, From 1b769dc7dde75e99f969f747e4d177c0cbbe1b22 Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Fri, 12 Jan 2018 10:14:29 -0500 Subject: [PATCH 5/7] Fix test --- addon/models/osf-model.js | 14 ++-- tests/unit/models/osf-model-test.js | 110 ++++++++++++++-------------- 2 files changed, 63 insertions(+), 61 deletions(-) diff --git a/addon/models/osf-model.js b/addon/models/osf-model.js index 4194a63bb..6067e173d 100644 --- a/addon/models/osf-model.js +++ b/addon/models/osf-model.js @@ -1,7 +1,7 @@ import Ember from 'ember'; import DS from 'ember-data'; -import { OSFAdapter } from 'ember-osf/adapters/osf-adapter'; -import { authenticatedAJAX } from 'ember-osf/utils/ajax-helpers'; +import OSFAdapter from '../adapters/osf-adapter'; +import { authenticatedAJAX } from '../utils/ajax-helpers'; /** * @module ember-osf @@ -65,7 +65,7 @@ export default DS.Model.extend({ remove: relation.canonicalMembers.list.filter(m => currentIds.indexOf(m.getRecord().get('id')) === -1) }; - var other = this.get('_dirtyRelationships.${rel}') || {}; + var other = this.get(`_dirtyRelationships.${rel}`) || {}; Ember.merge(other, changes); this.set(`_dirtyRelationships.${rel}`, other); } @@ -95,11 +95,13 @@ export default DS.Model.extend({ const options = Ember.merge({ url, data: queryParams, - headers: OSFAdapter.get('headers'), + headers: Ember.get(OSFAdapter, 'headers'), }, ajaxOptions); - authenticatedAJAX(options).catch(reject) - .then(Ember.bind(this, this.__queryHasManyDone, resolve)); + authenticatedAJAX(options).then( + Ember.run.bind(this, this.__queryHasManyDone, resolve), + reject + ); }); const ArrayPromiseProxy = Ember.ArrayProxy.extend(Ember.PromiseProxyMixin); diff --git a/tests/unit/models/osf-model-test.js b/tests/unit/models/osf-model-test.js index 4a404d9db..6fc8723bf 100644 --- a/tests/unit/models/osf-model-test.js +++ b/tests/unit/models/osf-model-test.js @@ -16,76 +16,76 @@ test('queryHasMany works', function(assert) { const store = this.store(); const userId = 'guid1'; const userNodesUrl = `https://api.osf.io/v2/users/${userId}/nodes/`; - const userPayload = `{ - "data": { - "relationships": { - "nodes": { - "links": { - "related": { - "href": "${userNodesUrl}", - "meta": {} - } - } - }, - }, - "links": { - "self": "https://api.osf.io/v2/users/${userId}/", - "html": "https://osf.io/${userId}/", - }, - "attributes": { - "family_name": "Guid", - "given_name": "A", - "full_name": "A Guid", - }, - "type": "users", - "id": "${userId}" - }, - }`; - store.pushPayload(userPayload); + const userPayload = { + 'data': { + 'relationships': { + 'nodes': { + 'links': { + 'related': { + 'href': userNodesUrl, + 'meta': {} + } + } + }, + }, + 'links': { + 'self': `https://api.osf.io/v2/users/${userId}/`, + 'html': `https://osf.io/${userId}/`, + }, + 'attributes': { + 'family_name': 'Guid', + 'given_name': 'A', + 'full_name': 'A Guid', + }, + 'type': 'users', + 'id': userId + }, + }; const nodeIds = ['guid2', 'guid3', 'guid4'] - const nodeJson = []; - for (const id in nodeIds) { - nodeJson.pushObject(`{ - "links": { - "self": "https://api.osf.io/v2/nodes/${id}/", - "html": "https://osf.io/${id}/" + const nodePayloads = []; + for (const id of nodeIds) { + nodePayloads.push({ + 'links': { + 'self': `https://api.osf.io/v2/nodes/${id}/`, + 'html': `https://osf.io/${id}/` }, - "attributes": { - "category": "project", - "title": "Project with guid ${id}", + 'attributes': { }, - "type": "nodes", - "id": "${id}" - }`); + 'type': 'nodes', + 'id': id + }); } const queryParams = {'param': 7}; Ember.$.mockjax({ url: userNodesUrl, - responseText: `{ - "data": [ - ${nodeJson.join()} - ], - "meta": { - "total": ${nodeIds.length}, + responseText: { + 'data': nodePayloads, + 'meta': { + 'total': nodePayloads.length, }, - "links": { - "self": "https://api.osf.io/v2/users/${userId}/nodes/", + 'links': { + 'self': `https://api.osf.io/v2/users/${userId}/nodes/`, } - }` + } }); assert.expect(3 + nodeIds.length); - const user = store.peekRecord('user', userId); - assert.ok(!!user); - assert.equal(user.get('id'), userId); - return user.queryHasMany('nodes', queryParams).then(function(nodes) { - assert.equal(nodes.length, nodeIds.length); - for (let i = 0; i < nodes.length; i++) { - assert.equal(nodes[i].get('id'), nodeIds[i]); - } + const done = assert.async(); + Ember.run(function() { + store.pushPayload(userPayload); + const user = store.peekRecord('user', userId); + assert.ok(!!user); + assert.equal(user.get('id'), userId); + user.queryHasMany('nodes', queryParams).then(function(nodes) { + assert.equal(nodes.length, nodeIds.length); + for (let i = 0; i < nodes.length; i++) { + assert.equal(nodes[i].get('id'), nodeIds[i]); + } + done(); + }); }); }); From c716b56f0036117ba29d13f8d359fee547de81e3 Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Fri, 12 Jan 2018 11:01:14 -0500 Subject: [PATCH 6/7] Update changelog, query uses --- CHANGELOG.md | 6 ++++++ addon/models/osf-model.js | 4 ++-- addon/services/file-manager.js | 2 +- addon/utils/load-relationship.js | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ed2392f9..ba036792b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Removed +- ember-data-has-many-query + - Code that uses `model.query()` should update to `model.queryHasMany()` + +### Added +- `osf-model.queryHasMany`, for reliable querying of hasMany relations ## [0.13.1] - 2018-01-11 ### Changed diff --git a/addon/models/osf-model.js b/addon/models/osf-model.js index 6067e173d..d301f8cc2 100644 --- a/addon/models/osf-model.js +++ b/addon/models/osf-model.js @@ -1,7 +1,7 @@ import Ember from 'ember'; import DS from 'ember-data'; -import OSFAdapter from '../adapters/osf-adapter'; -import { authenticatedAJAX } from '../utils/ajax-helpers'; +import OSFAdapter from 'ember-osf/adapters/osf-adapter'; +import { authenticatedAJAX } from 'ember-osf/utils/ajax-helpers'; /** * @module ember-osf diff --git a/addon/services/file-manager.js b/addon/services/file-manager.js index ef0c15759..8e0fc68ab 100644 --- a/addon/services/file-manager.js +++ b/addon/services/file-manager.js @@ -417,7 +417,7 @@ export default Ember.Service.extend({ * rejects with an error message. */ _getNewFileInfo(parentFolder, name) { - let p = parentFolder.query('files', { + let p = parentFolder.queryHasMany('files', { 'filter[name]': name }); return p.then((files) => { diff --git a/addon/utils/load-relationship.js b/addon/utils/load-relationship.js index 9cebfb406..a8ab83b11 100644 --- a/addon/utils/load-relationship.js +++ b/addon/utils/load-relationship.js @@ -25,7 +25,7 @@ export default function loadAll(model, relationship, dest, options = {}) { query = Ember.merge(query, options || {}); Ember.set(model, 'query-params', query); - return model.query(relationship, query).then(results => { + return model.queryHasMany(relationship, query).then(results => { dest.pushObjects(results.toArray()); if (results.meta) { var total = results.meta.pagination.total; From e1dfdd61fef49ddf9583910b84678124abbd9aef Mon Sep 17 00:00:00 2001 From: Abram Booth Date: Fri, 12 Jan 2018 11:13:23 -0500 Subject: [PATCH 7/7] Update yarn.lock --- yarn.lock | 6 ------ 1 file changed, 6 deletions(-) diff --git a/yarn.lock b/yarn.lock index 5e571f31a..c28f4655f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3412,12 +3412,6 @@ ember-data-factory-guy@2.11.7: broccoli-funnel "^1.0.1" broccoli-merge-trees "^1.1.1" -ember-data-has-many-query@^0.2.0: - version "0.2.0" - resolved "https://registry.yarnpkg.com/ember-data-has-many-query/-/ember-data-has-many-query-0.2.0.tgz#97ae2c29d6d7f77d3837cca9f165c26bf6e6fb0a" - dependencies: - ember-cli-babel "^5.1.6" - ember-data@2.11.3: version "2.11.3" resolved "https://registry.yarnpkg.com/ember-data/-/ember-data-2.11.3.tgz#46ba0e8411dce6dbb52cc02a29194f265b747ef9"