FamilySearch / gedcomx-php

PHP SDK for GEDCOM X Processing. See the documentation at
http://familysearch.github.io/gedcomx-php/
Other
35 stars 15 forks source link

Add getPerson() method to PersonsState #43

Closed justincy closed 9 years ago

justincy commented 9 years ago

I don't know if the method should accept a plain personId or a ResourceReference. Perhaps both.

jimmyz commented 9 years ago

I'm thinking ResourceReference, since I believe the use case for this would likely be fetching the person object from the Gedcomx object based upon a reference found within a relationship object.

justincy commented 9 years ago

This method could be useful in a lot of other states to (basically any state with a response that could have more than one person in it). So I propose adding getPerson to Gedcomx\Rs\Client\GedcomxApplicationState.

justincy commented 9 years ago

Or we could add it to PersonsState and have any state extend it that might have multiple persons returned.

jimmyz commented 9 years ago

I like that. (PersonsState)

justincy commented 9 years ago

Turns out it's not that simple. The PersonsState is for the Persons Resource which only exists to create persons. The state has an addPerson method which wouldn't make sense for any other state.

Granted, there are some things about it that don't line up. Currently the PersonsState can be obtained in two ways:

  1. Calling readPersons on CollectionState. But that call is worthless; it's guaranteed to return a 405 because you can only POST to the resource; you can't GET.
  2. Calling readPersons or readPersonas on SourceDescriptionState. This case makes sense, and it's be used exactly how we would want to use for a list of persons.

Another thing about the PersonsState that doesn't make sense is the readCollection method. It appears to be useless. The code performs a GET on the COLLECTION rel for the given state, but as it stands it's only being used for lists of people which is not a resource and does not have any links (the people have links, but not the lists of people).

I think I'm going to proceed but there's a slight risk that we mess something up because it's in an awkward state to begin with.