Complete migration to platform-client for API calls #56
Conversation
Use `tidepool.getUserId()` when needed to construct api calls.
- Migrate all device data calls and processing out of this codebase, and use platform-client - Complete change from `teamId` to get notes to `userId`
|
Closes #31 |
|
Had to add a few changes to take into account the new All good to go now. BEFORE: AFTER: |
|
Is it just me, or do all of the replies seem to come together into just a blob of text for other people too? |
|
Are you talking about the styling you see in the screenshot? If you are, I have a task which is "tweak message thread styling", so I'll get to it and make it better. Just focusing on functionality here... |
|
Ah, yes, I was. Sounds good if it's a todo item. |
|
(With pretty design stuff, I always assume it is just me, but I also vocalize my thoughts just in case.) |
| if (user && user.firstName && user.lastName) { | ||
| result = user.firstName + ' ' + user.lastName; | ||
| } | ||
| return result; |
There was a problem hiding this comment.
A nit, but why not just return the value inside the if as well as here.
There was a problem hiding this comment.
Hum yeah I guess I could've done that and do without the result var altogether...
|
And also...forgive me if this has been discussed, but months ago we spec'd that we'd be storing fullname and shortname specifically to avoid the problem of firstname and lastname. Why did we decide not to do that, again? |
|
No @kentquirk this is a good point and it hasn't been really discussed. I think this dates back from the very beginning where I started working on the Blip UI but the backend wasn't ready yet, so I just went with this user data model, and I guess it just stuck. I do have a note somewhere "align better with backend user data model", and that could become part of it. I'd rather leave it out of the PR if that's all right? For instance, I would state that most apps I've used ask for first name/last name and do just fine? But it'd be best not to have that discussion here :) @cheddar Does everything else seem good? |
|
Hum, I take that back, a lot of apps do ask for a "Full Name" it seems. Let's revisit outside of this PR then? Thanks |
|
Ok, I've created a card for it in trello. In western cultures it mostly works to do first/last, but even there you run into issues. I've seen people get upset when their first name is used without permission. The fullname / shortname model allows people to control how others address them. |
|
Also, can you figure out what's up with Travis before merging, please? |
|
Yes I understand, the more I think about it the more it makes sense. Thanks for creating the Trello card. I just checked the build locally, its fine, this is another case of Travis bugging. I could restart it but I'm a little lazy to wait for it to finish :) Merging to master will run it again. Is this good to go then? |
Fwiw, I have memories of discussing this months ago as well and agreeing to storing full name... Don't really know if it got documented or where the spec changed though, so probably just got lost in the ether. |
Complete migration to platform-client for API calls
NOTE: this is dependent on tidepool-org/platform-client#9
See
platform-clientPR linked above for more details on migration.This PR also completes data model change where messages where tied to a
teamIdbut are now tied to auserId.I've tested this locally (creating accounts, profiles, uploading data, visualizing data, etc.) and things seem to work fine.