anuditverma / org.civicrm.osdi

OSDI API Implementation for CiviCRM - Google Summer of Code (GSoC) 2015
Other
1 stars 3 forks source link

/person endpoint : Adding PUT & DELETE support #14

Closed anuditverma closed 9 years ago

anuditverma commented 9 years ago

Currently the PUT & DELETE actions are supported on the person-sign up helper, so they will be moved from there and will be added to the person's endpoint.

anuditverma commented 9 years ago

Hey @joemcl, @j-ro, @eileenmcnaughton I have added the support for PUT & DELETE actions to the person's endpoint. Here is the fix : https://github.com/anuditverma/org.civicrm.osdi/commit/ce5fe185faccc0646977f841f2da241129544d79

But right now, I am also allowing to pass the actions in JSON, is there any other way to perform these actions without any switcher ? like I have been thinking to add dedicated links for PuTing and DELETE-ing on the person's endpoint page. Also I was curious how can I use the 'METHOD' form on the NON-GET request pop up to pass the chosen action ?

j-ro commented 9 years ago

Per the spec, these should be JSON at the same endpoint. Using the NON-GET, when that window opens up, it says POST in the method, just change that to PUT or DELETE.

anuditverma commented 9 years ago

"just change that to PUT or DELETE." - so while PUT-ing or DELETE-ing the record from the person's endpoint I just need to change that field to either PUT or DELETE or I have to identify that Method field's input and perform the action accordingly using the switcher ?

j-ro commented 9 years ago

This is in the browser interface. When you click the non-get button, the method there is filled in as POST by default. Type something different to perform the other methods. On the receiving API end, these will come as the HTTP headers.

anuditverma commented 9 years ago

Thanks Jason, I have fixed this one here https://github.com/anuditverma/org.civicrm.osdi/commit/3460f0799e990bae295dbccd26ed043485a6fc6c Now if we pass PUT or DELETE in the 'Method' it will act accordingly to the chosen action.

j-ro commented 9 years ago

I'm not seeing this work on the browser, does it work for you?

anuditverma commented 9 years ago

Yes, it does work. I just presented a demo about this to @joemcl and @eileenmcnaughton today on the daily check-in meeting. Could you please specify where exactly it is not working.

FYI, You could try updating a record by using this acceptable JSON:

{ "id":,"given_name":"John","additional_name":"Payne","family_name":"Smith","gender":"Male","postal_addresses":"Testfieldspacing","email":"jsmith@mail.com","phone":12345 }

Similarly for deleting a record,

{ "id": }

and Yes specifying the 'Method' too, changing it to put or delete.

j-ro commented 9 years ago

Hm...

From here:

http://camus.fuzion.co.nz/hal-browser/browser.html#/sites/default/ext/osdi/api/v3/People/person.php?id=2

I put in this, using the PUT command in non-get:

{
"given_name":"Jason"
}

which should change the given name to Jason, but it seems to do nothing. Similarly, I send the DELETE command (with empty JSON) and nothing happens, but the record should be deleted.

anuditverma commented 9 years ago

OK, so I have been testing this out on my local machine I observed these: -If we try to update an existing record (sample data) it doesn't show any changes rather old fields' values are returned, same is happening on the camus. -It does work when we POST a new record and try updating changes on it.

May be the sample data is denying over writing permissions or it is a security feature of Civi, I am not sure about this behavior.

And DELETE is working fine on all records (with empty JSON)

j-ro commented 9 years ago

DELETE does seem to work, though I get an internal server error as a response, which I guess is the API trying to load a person who's ID is now deleted. Rather, the response should be a success message I think.

anuditverma commented 9 years ago

Yes, agreed I was about to add that to make it more user friendly, just wanted to make sure it works first. Will add the response messages wherever needed.

j-ro commented 9 years ago

Cool, check out how ours does it. I believe it's just a 200 status message and something like:

{
"The person was successfully deleted."
}
anuditverma commented 9 years ago

I have added the response message which will appear in the body though it will return 404 as HTTP status because it tries to lookup that deleted contact and I think logically it does make sense to get a 404. I hope getting correct HTTP status (like 200 OK or something similar as you suggested) is not an issue to be concerned about although it will return an apt response message when the person's record is deleted. Fix : https://github.com/anuditverma/org.civicrm.osdi/commit/2babc19428d59f6c87801bb21f9941246188bf15

j-ro commented 9 years ago

yeah, I think that's fine

j-ro commented 9 years ago

are PUTs working? Can't get them to work on Camus.

anuditverma commented 9 years ago

Currently PUT may not update some of the fields which are used in API chain calls. And actually PUT is only working on new records that we POST not the ones (samples) already present on the camus. So fields like given_name and family_name gets updated on the newly added records. But I have observed that email wasn't updating. So I will fix this too. On 28-Aug-2015 1:39 am, "Jason Rosenbaum" notifications@github.com wrote:

are PUTs working? Can't get them to work on Camus.

— Reply to this email directly or view it on GitHub https://github.com/anuditverma/org.civicrm.osdi/issues/14#issuecomment-135538704 .