Proof of concept for account profiles. #158
Conversation
| roles: findCustomRoles(roles) | ||
| } | ||
|
|
||
| if (options.includeProfile) { | ||
| account.profile = doc.profile | ||
| } |
There was a problem hiding this comment.
I would leave that in, but replace account.profile = doc.profile with account.profile = doc.profile || {}
|
besides updating the Let’s get this ready first, then move on to hoodiehq/hoodie-account-client#98 |
| type: 'profile', | ||
| attributes: { | ||
| id: 'userid123-profile', |
There was a problem hiding this comment.
why did you move the id? It should be next to type where it was, it’s part of the JSON API spec
There was a problem hiding this comment.
One of those "it made it pass the test" scenarios... I've instead fixed it properly since!
|
Hey @gr2m - keen on your thoughts on latest commits. We've now got tests passing, and updates via PATCH are working. One thing I noticed is that the original code assumed the profile id (i.e. {
"id": "account123-profile",
"fullname": "Dr Pat Hook",
"address": {
"city": "Berlin",
"street": "Adalberststraße 4a"
}
}Is this correct? The tests that were built (although weren't being used due to not having any profile data in tests) assumed that the id would be one level up in the request - looking something like this: {
"id": "account123-profile",
"attributes": {
"fullname": "Dr Pat Hook",
"address": {
"city": "Berlin",
"street": "Adalberststraße 4a"
}
}
}If the latter is intended, I'm guessing it's something we need to solve over in |
…reviously caused errors.
|
@gr2m in regards to my last comment, I think I've tidied this up. The latter (in my mind) was correct, and was how the client was actually operating - we just had some inconsistent code/docs which I added into the mix. Seems to be correct now? |
… Also added test for user add with profile
| var id = request.payload.data.id | ||
| var query = request.query | ||
| accounts.add({ | ||
| username: username, | ||
| password: password, | ||
| include: query.include, |
There was a problem hiding this comment.
@gr2m is this correct? Or the below? My understanding is that this should be an option, not a property?
| } | ||
| } | ||
| } | ||
| } |
Some notes on this PR:
profilerequests have been dealt to.To do:
PATCH /session/account/profilehoodie-account-serverhoodie-accountMore than happy for someone to add to this PR ;)