FamilySearch / familysearch-javascript-sdk

See the FamilySearch API Javascript SDK documentation at
http://familysearch.github.io/familysearch-javascript-sdk
MIT License
42 stars 25 forks source link

Add getCurrentUserPerson #51

Closed justincy closed 10 years ago

justincy commented 10 years ago

I could use getCurrentUserPersonId then pass the ID into getPerson but it would be inefficient because getCurrentUserPersonId fetches the entire person object anyways.

DallanQ commented 10 years ago

I could add this, but the person object would still need to be re-fetched. Here's what's going on under the hood:

I kind of wish that FamilySearch wouldn't return 30x response codes that cause the browser to automatically redirect, but I haven't brought this up with them. Maybe I should?

justincy commented 10 years ago

Interesting. You could try bringing it up with FamilySearch, but it would have to be done in a way that doesn't break other clients. A 3xx is the proper response in these scenarios.

Perhaps we could ask for a special parameter or header that changes the response code to a 2xx status.

justincy commented 10 years ago

They've done something similar before. They used to have an exception for RootsMagic where the HTTP status code for throttling matched the code from the old API.

DallanQ commented 10 years ago

I'll bring it up

stoicflame commented 10 years ago

If you want two explicit reads instead of the automatic redirect, then why not a read to the current user and then use the personId to read the person?

There really should be a hypermedia link from the user to the person so you don't have to construct the URI to the person. Tracking that here.

justincy commented 10 years ago

Great idea. I forgot about that.

DallanQ commented 10 years ago

I didn't realize that the current user response already contained the personId. BTW, personId is not one of the fields mentioned in the example response:

https://familysearch.org/developers/docs/api/tree/Read_Current_User_usecase

How about if we drop the getCurrentUserPersonId wrapper, since personId is already available as an attribute from the current user response? Any objections to this?

I will add personId, givenName, familyName, gender, preferredLanguage, and displayName (none of these are in the example response) and remove fullName (which is part of the example response) to the SDK docs here:

http://rootsdev.org/familysearch-javascript-sdk/#/api/user.types:constructor.User

justincy commented 10 years ago

+1

On Fri, May 2, 2014 at 12:47 PM, Dallan Quass notifications@github.comwrote:

I didn't realize that the current user response already contained the personId. BTW, personId is not one of the fields mentioned in the example response:

https://familysearch.org/developers/docs/api/tree/Read_Current_User_usecase

How about if we drop the getCurrentUserPersonId wrapper, since personId is already available as an attribute from the current user response? Any objections to this?

I will add personId, givenName, familyName, gender, preferredLanguage, and displayName (none of these are in the example response) and remove fullName (which is part of the example response) to the SDK docs here:

http://rootsdev.org/familysearch-javascript-sdk/#/api/user.types:constructor.User

— Reply to this email directly or view it on GitHubhttps://github.com/rootsdev/familysearch-javascript-sdk/issues/51#issuecomment-42064755 .

DallanQ commented 10 years ago

done

justincy commented 10 years ago

We have the same 3xx redirect issue with Preferred relationships. Getting the preferred relationship returns a 303.