Closed rgant closed 8 years ago
I think there is a typo here:
let relationList = this.resourceDef['relationlist'];
I think the "L" in list should be capitalized. Also it might be better written:
let relationList = this.resourceDef.relationList;
However I am not sure.
@BlairAllegroTech I'd be interested in your comments on this changeset Where I added a relationList definition to the typings. It uncovered what I think is an issue with getChildRelations
that I have attempted to resolve.
Sorry for the late reply. Yes adding relations list to typing makes it cleaner. I just wasn't sure if it had been purposely omitted?
With regard to getChildRelations.The code looks good. I'll try to review first thing in the morning. I've been away for a few days.
Thanks @rgant those changes look good! Do you want to create a pull request ?
As for the first part of this issue. Yes i think the relationship handling for empty relationships can be improved now. I guess the use case for this one is.
NOTE : There is a convenience method that i have added to `findRelated
that may do some of what you require see JsonApiSerializer.ts L1352. It checks to see if we have the relationship loaded and
if any of the related data are reference placeholders and if so loads the related link from the server.
Thanks going through this has helped me understand better what needs to be done here. I will make the changes
So I am a bit confused by the "EXPERIMENTAL MANY TO MANY RELATIONSHIPS SPECIAL HANDLING" part of the code. So I started experimenting. I created this changeset which nominally creates the relationships. But it's ugly and I still didn't get what I wanted.
I am trying load the account relation for a user that I find:
DS.find('users', 'me')
.then(function (myUser) {
console.log('Users', myUser);
return DS.loadRelations('users', myUser, ['account']);
}).then(function (myUser) {
console.log('Users.account', myUser.account);
vm.user = myUser;
});
However even with my changes this doesn't work. In the js-data code I get to the point where it is executing loadRelations and I see the params variable has the value {undefined: "2322"}
because the foreignKey isn't defined.
I assumed that I was doing something wrong so I went play with js-data directly and see if I could get it to work. Playing around it appears that it could/should work so there must be something missing either in js-data-jsonapi or my application.
Yes to make this work we need to solve this issue which is to store relationship meta data without the requirement for there to be data in the relationship. What you have in your change set looks to be on the right track and is similar to what i was going to to.....
As mentioned above there is a findRelated
method that i attach that is currently the preferred way of loading related data, though the down sides to this is that it is a custom method.
My reasoning for this that there is no loadRelations
method on the adapter. It ends up calling findAll
on the adapter and so i was not sure if i could reliably detect that it is a relationship being requested. But i think this should be possible. It would be best to have this working i think it is much cleaner as it follows the standard js-data api.
Hey @rgant
Ok looking at the current implementation of findAll. this will only work if there is a belongsTo
relationship on the child with the parent property set to true!.
see : Karma.start.js. This is because we need to get hold of the relationship metadata from the parent. At the moment unless there is a belongsTo
with parent : true
i don't know of any other way to get hold of the parent.
This is why i have created the findRelated
method, which is not ideal but achieves what i would like loadRelations
to do.
Once again i really appreciate all your feedback, it's really helping to push this along!! Keep it up
I think what you are telling me is that because the belongsTo
relation supports a parent
flag it allows you to identify which of the relations on the "child" is to a "parent" one. However if I use belongsTo
then #26 issue crops up.
I think you have a more "big picture" view of the relations that I do so I don't quite understand why you need to know the parent at all. Assuming that I setup a Resource with this structure: Users.relations.hasOne.accounts.localField.account
then I think I know everything I need to know to identify the relationships.
hasOne
vs. hasMany
the data key in JSONAPI response to the related or self links will be a single document or an array of documents respectively.accounts
this is the js-data Resource.name (type in JSONAPI) that will be returned in the related link.
RelationDefinition
this is the relation
field, and we can use getResource
to get the class to create the Resources.localField.account
the second half of this (account) is the name of the relation field on the User Resource where we store the accounts Resource from 2.What I am not thinking about here is the JSONAPI relationship self links. In my backend I hack this a bit and just have one Model called ResourceIdentifier where I just change the type
field dynamically to allow me to use one Model for all of the relationship self links. Don't know if that would work in js-data.
What it comes down to is, if you inspect the parameters that are passed to the adapters findAll
method as a result of a call to loadRelations
there is not the information we require to retrieve the parent resource, from which we would then retrieve the metadata and then the relationships related link.
If we could just get the original parameters that were passed to loadRelations
we would be able to do this easily!
I think i may have found a solution to this:
Looking at the code in js-data for loadRelations
, notice there is a load
method that can be defined on a relationship to perform custome relationship loading.
This method is passed the parameters we need! Looking at the js-data docs for relations this documents custom relationship getters.
This solution would require attaching custom relationship getters to all the resources hasOne
and hasMany
relationships we retrieve via our jsonapi adapter this can be done in our beforeInject
hook that we already have.
We also need to take into consideration the multi adapter scenario so that when not using our adapter we should not alter the default load relations behaviour and fall back to the default behaviour.
Actually all we need to do here is attach our custom jsonApiPath
value to the options
.
It will be passed to the adapter, if it is passed to our adapter we will use it to retrieve the relationship.
Other adapters will ignore this parameter
Hi @rgant , i have submitted a change set that i think should resolve this issue. It is not very pretty code but at the moment i can't see any other way of doing this.
I haven't done another release yet, can you try out these changes and see if it resolves your issues pls
Thanks! I shall!
-_Rob
On Thu, Jul 21, 2016 at 5:43 AM BlairJ notifications@github.com wrote:
Hi @rgant https://github.com/rgant , i have submitted a change set that i think should resolve this issue. It is not very pretty code but at the moment i can't see any other way of doing this.
I haven't done another release yet, can you try out these changes and see if it resolves your issues pls
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/BlairAllegroTech/js-data-jsonapi/issues/25#issuecomment-234206358, or mute the thread https://github.com/notifications/unsubscribe-auth/AArXmXr9XWS14WJroVmTYU31mmt5wwxtks5qXz8cgaJpZM4JO9E1 .
Hi @rgant I was just wondering how you were going after these changes were made ? I'd like to know if this is working for you or not!
Thanks
Blair, sorry for my delays in replying. I still cannot get loadRelations
to function correctly in version 15 of js-data-jsonapi.
These are the relations that I have setup for Accounts and Users:
DS.defineResource({
name: 'accounts',
relations: {
hasMany: {
files: {
localField: 'all_files',
// One Account for many Files; use foreignKey.
foreignKey: 'accountid'
},
account_tags: {
localField: 'all_tags',
// One Account for many Tags; use foreignKey.
foreignKey: 'accountid'
},
users: {
localField: 'users',
// One Account for many Users; use foreignKey.
foreignKey: 'accountid'
},
// Funder Accounts Only
portfolios: {
localField: 'portfolios',
// One Account for many Portfolios; use foreignKey.
foreignKey: 'accountid'
},
// Grantee Accounts Only
investments: {
localField: 'investments',
// One Account for many Investments; use foreignKey.
foreignKey: 'granteeaccountid'
},
reports: {
localField: 'reports',
// One Account for many Reports; use foreignKey.
foreignKey: 'accountid'
}
},
hasOne: {
files: {
localField: 'logo_file',
localKey: 'logofileid'
}
}
}
});
DS.defineResource({
name: 'users',
relations: {
hasOne: {
accounts: {
localField: 'account',
localKey: 'accountid'
}
}
}
});
And this is the code I am running to try and test loadRelations
:
DS.find('users', 'me')
.then(function (myUser) {
console.log('Users', myUser);
return DS.loadRelations('users', myUser, ['account']);
}).then(function (myUser) {
console.log('Users.account', myUser.account);
vm.user = myUser;
});
This is my console log output showing that the loadRelations
call doesn't actually make a network request for the accounts related link.
GET http://localhost:8080/v3/users/me 200 OK 175ms js-data....2.2.js (line 1169)
Mon, 08 Aug 2016 09:33:25 GMT - GET /v3/users/me - 200 227ms Object { data={...}, status=200, statusText="OK", more...} js-data....2.2.js (line 144)
Users Users { type="admin", last_name="Test", email="test4@example.com", more...} user-directive.js (line 20)
Users.account undefined user-directive.js (line 23)
The API is running on localhost:8080 and the angular app is running on localhost:8000.
Inspecting the myUser object $_JSONAPIMETA_.relationships.account.related
shows it has the value: {type: "accounts", url: "/v3/users/me/accounts"}
. This is the correct URL I think, but maybe there is an issue because it isn't fully qualified?
Also I have a shortcut id for getting the currently logged in user by requesting the id me
. But the actual id for the user is 1130
, maybe that is also causing some confusion?
@rgant Have a look at #30, I thought i could use JsonApi urls as is but i think it is a better approach to prefix with basePath when configured. I hope this helps.
Configure basePath
either on your adapter or resource or pass in options to loadRelations
and see if this helps.
Nice, thanks Blair. I'll give it a look
Still not working. This is how I have configured basePath & baseURL:
angular.module('js-data').provider('DSJsonApiAdapter', function () {
var _this = this;
_this.defaults = {};
_this.$get = function () {
return new DSJsonApiAdapter.JsonApiAdapter(_this.defaults);
};
});
angular.module('app.models', ['app', 'js-data'])
.config(function (DSJsonApiAdapterProvider, $windowProvider) {
var $window = $windowProvider.$get();
var apiUrl = 'http://localhost:8080';
if ($window.location.hostname === 'example.com') {
apiUrl.replace('//stage-api.', '//api.');
}
angular.extend(DSJsonApiAdapterProvider.defaults, {
basePath: '/v3',
httpConfig: {baseURL: apiUrl}
});
})
I also tried this:
angular.extend(DSJsonApiAdapterProvider.defaults, {
basePath: apiUrl + '/v3'
});
My console still shows the same result; no 2nd request for /v3/users/me/account
when loadRelations run.
I'm sorry this is probably my lack o experience with js data. I used base basePadth I did not use baseUrl can you try setting basePath to your full base url. As long as your other non jsonapi paths are still correct. Have a look at my overloaded version of get path in the adapter. This needs to work out the path base in a similar way that the HTTPS adapter does. Otherwise I will take a closer look and fix this.
I did try that, see my second code block; "I also tried this". Same result. The issue with using baseURL is that isn't a feature of $http, which we have now switched to for the DSJsonApiAdapter:
angular.module('js-data').provider('DSJsonApiAdapter', function () {
var _this = this;
_this.defaults = {};
_this.$get = function (DSHttpAdapter, $http) {
_this.defaults.adapter = _this.defaults.adapter || DSHttpAdapter;
_this.defaults.adapter.http = function (config) {
return $http(config).then(function (data) {
return Object.assign({}, data, {
headers: data.headers()
});
});
};
return new DSJsonApiAdapter.JsonApiAdapter(_this.defaults);
};
});
angular.module('app', ['js-data'])
.config(function config($windowProvider, $httpProvider) {
$httpProvider.interceptors.push(function () {
var $window = $windowProvider.$get();
var apiUrl = 'https://stage-api.example.com';
var apiPath = '/v3';
if ($window.location.hostname === 'www.example.com') {
apiUrl.replace('//stage-api.', '//api.');
}
return {
/**
* Append the API URL and path to requests that look like API requests.
* @param {object} config request configuration object containing config.url
* @return {object} config with url modified if needed.
*/
request: function (config) {
var ext = config.url.substr(-5);
if (ext !== '.html' && ext !== '.json' && config.url !== 'theForm') {
// URL doesn't begin with a protocol so assume this is a path.
if (!config.url.match(/^(?:[a-z]+:)?\/\//)) {
// Prefix the URL with a / if missing.
if (config.url[0] !== '/') {
config.url = '/' + config.url;
}
// Check if the path already starts with the API version
if (config.url.substr(0, apiPath.length) !== apiPath) {
config.url = apiPath + config.url;
}
config.url = apiUrl + config.url;
}
}
return config;
}
};
});
});
Now I am probably in a whole set of problems. However this setup also doesn't seem to work for me.
Can we move this discussion to #30. I know what needs to be done here. I'll add some notes to #30 and try to get this resolved. It would be good to get your relationships working. I'm sure we are very close. Hopefully we just need to sort out these Urls ! Sorry its taking sooo long.
Sorry after checking my code and HttpAdapter and your code, can you not just do this?
Assuming this (https://stage-api.example.com/v3) is the correct basePath
to your api.
see here
See also commit/bug fix against #30 that may have been preventing you from doing whats suggested below
angular.module('js-data').provider('DSJsonApiAdapter', function () {
var _this = this;
_this.defaults = {basePath: 'https://stage-api.example.com/v3'};
_this.$get = function (DSHttpAdapter, $http) {
_this.defaults.adapter = _this.defaults.adapter || DSHttpAdapter;
_this.defaults.adapter.http = function (config) {
return $http(config).then(function (data) {
return Object.assign({}, data, {
headers: data.headers()
});
});
};
return new DSJsonApiAdapter.JsonApiAdapter(_this.defaults);
};
});
Parent Id's are not being set correctly on the data returned by loadRelations
.
Existing related data is updated / loaded correctly. Other related items are loaded but their relationship to the parent is not configured!
e.g you can do a get and find them in the data store, but they are not tied back to the parents relationship getter function
In my application we don't always include the relationship data with the primary data object. This causes a problem when I later try to load the related data using DS.loadRelations as it doesn't know what the links are.
The issue is around the test for hasData in JsonApiSerializer.ts. I believe there are two issues here:
data
key, but we should still add the links to theJSONAPI_META.relationships
.I've created a branch on my fork to support my work trying to address this issue. If you have any suggestions then I would be happy to discuss. I'll submit a pull request when I have something.