Skip to content
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions addon/adapters/osf-adapter.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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'
},
Expand Down
54 changes: 49 additions & 5 deletions addon/models/osf-model.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Ember from 'ember';
import DS from 'ember-data';

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
Expand All @@ -14,7 +14,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'),

Expand Down Expand Up @@ -65,11 +65,55 @@ export default DS.Model.extend(HasManyQuery.ModelMixin, {
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}`) || {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a drive-by fix -- not sure if it actually caused misbehavior, though.

Copy link
Member

Choose a reason for hiding this comment

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

Wow, been broke for 2 years: https://github.com/aaxelb/ember-osf/blame/06d0f12f7b486e75fc97d5e8d93b829636e02d90/addon/models/osf-model.js#L68

But, like you said, this is likely not causing any misbehavior, yet...

Ember.merge(other, changes);
this.set(`_dirtyRelationships.${rel}`, other);
}
});
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 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`);
if (!url) {
reject(`Could not find a link for '${propertyName}' relationship`);
return;
}
const options = Ember.merge({
url,
data: queryParams,
headers: Ember.get(OSFAdapter, 'headers'),
}, ajaxOptions);

authenticatedAJAX(options).then(
Ember.run.bind(this, this.__queryHasManyDone, resolve),
reject
);
});

const ArrayPromiseProxy = Ember.ArrayProxy.extend(Ember.PromiseProxyMixin);
return ArrayPromiseProxy.create({ promise });
},

__queryHasManyDone(resolve, payload) {
Copy link
Member

Choose a reason for hiding this comment

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

I like the double underscore prefix for "superprivate" methods that should never be called directly (only as a callback). AFAIK, this is not a convention in JS, but I like it.

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);
},
});
2 changes: 1 addition & 1 deletion addon/services/file-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion addon/utils/load-relationship.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
81 changes: 80 additions & 1 deletion tests/unit/models/osf-model-test.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,91 @@
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) {
let model = this.subject();
// 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
},
};

const nodeIds = ['guid2', 'guid3', 'guid4']
const nodePayloads = [];
for (const id of nodeIds) {
nodePayloads.push({
'links': {
'self': `https://api.osf.io/v2/nodes/${id}/`,
'html': `https://osf.io/${id}/`
},
'attributes': {
},
'type': 'nodes',
'id': id
});
}

const queryParams = {'param': 7};

Ember.$.mockjax({
url: userNodesUrl,
responseText: {
'data': nodePayloads,
'meta': {
'total': nodePayloads.length,
},
'links': {
'self': `https://api.osf.io/v2/users/${userId}/nodes/`,
}
}
});

assert.expect(3 + nodeIds.length);

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();
});
});
});
6 changes: 0 additions & 6 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down