Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper Functions #468

Open
shazahm1 opened this issue Dec 9, 2016 · 8 comments
Open

Helper Functions #468

shazahm1 opened this issue Dec 9, 2016 · 8 comments
Assignees

Comments

@shazahm1
Copy link
Member

shazahm1 commented Dec 9, 2016

Create a set of helper functions to make it easier to programmatically insert and edit entries.

ref: https://wordpress.org/support/topic/future-of-json-api/#post-8531507

tag @ranchhand6

@shazahm1 shazahm1 self-assigned this Dec 9, 2016
@ranchhand6
Copy link

We had a little discussion around the shop here, and the prevailing opinion is that we're not fussed with writing deep nested JSON objects to do adds or edits.

The reason is that you typically write API client code in a structured way, with a function for each sub-class or type. When you need to insert that object into the JSON as it's being built, you call the function or method that is responsible for building that part of the structure and move on. You almost never build these JSON structures by hand, except maybe occasionally for testing, and you never build them in a "flat" piece of code.

Additionally, all of the languages that are going to use this API (php, javascript, java, Objective C, rails, etc.) have good facilities for dealing with nested JSON.

Now, if there is a use case where updating something like a single address is more efficient than updating the whole structure, then I'm all for it. But from what I've seen so far in your code, you retrieve the old record state first, then overwrite with the updated data, then write the record back to the database. Thus, if I only wanted to update one address, I just have to make sure that I have the correct address selected during the update to make an efficient call.

Put another way, I don't necessarily want to do multiple API POST or PATCH operations to update one record. It complicates the client code, and I think you want REST API operations to be as atomic as you can get them, and doing multiple operations runs the risk of having something be out of alignment in the data if a GET call gets interleaved between the two PATCH calls.

I hope I've understood your question correctly. Let me know what you think.

@ranchhand6
Copy link

BTW, I went a little way down the path of building out the entry GET return, including addresses, emails, and photos. It's in my json-api branch if you would like to look at it.

https://github.com/ranchhand6/Connections/blob/feature/json-api/includes/api/endpoints/class.cn-rest-entry-controller.php

I wasn't ready to do a pull request quite yet, but you can have it if it's useful to you now.

@ranchhand6
Copy link

Thinking about this a bit further, I see the WP-CLI case as a separate requirement. In that case, perhaps it is more convenient to break the structure up into pieces because you potentially have a person doing the typing. However, I can see this getting unwieldy in a hurry. Thinking about how to structure the arguments, it would go something like this, perhaps:

wp connections address add <slug> ['address1 => '101 1st Street]

That would require you to take it in small bites, but I think you can do the same thing by injecting the argument into your normal update at the correct place.

There are thousands of ways to skin the cat. Which one is best?

@shazahm1
Copy link
Member Author

@ranchhand6

As far as the client side GET/POST/DELETE requests, I agree. My goal will be as few requests as possible. Actually, you should be to GET change the object and POST it back to commit a change.

My comments about the code were purely from the server side PHP logic. Any way, I think I know how I am going to proceed. My goal is to be able to share as much logic as possible between the current "entry actions", the REST API and wp-cli. I'm am going move abstract the code, such as addresses, into individual "collection" objects. I'm sure there is likely a proper name for the pattern. I just do not know it. Hopefully it will make more sense later this week as I push the code changes.

@shazahm1
Copy link
Member Author

@ranchhand6

I looked at you current changes to the REST API. The phone seems fine. The number field might need to be an array with a raw and rendered key, like the address fields. I have to take a look to see which filters I run on the field as part of escaping it.

fyi, the raw key will only be sent in the "edit" context to match the WP core REST endpoints. I think this is that WP core does, I have to double check.

The photo/logo REST response... I'd have to review what WP core returns for image attachments on posts and pages. I might want my response to match.

fyi, for your client app, There is a TimThumb-like endpoint available with nearly all the TimThumb query parameters being supported.

Usage:

http://domain.com/cn-image/?src=full-uri-to-image&cn-entry-slug=entry-slug&w=int&h=int&zc=int

See: https://github.com/ranchhand6/Connections/blob/feature/json-api/includes/image/class.image.php#L188

I wrote this awhile back but have not really put it to use. Seems like it would be handy in client apps to fetch the image already processed to the proper width x height. I would suggest caching the result locally in the client app.

@ranchhand6
Copy link

Ok, I'm understanding better now. You were following a similar functional structure inside the class.cn-rest-entry-controller.php, I tried to follow that when I was doing my additions.

I will take a look at the TimThumb endpoint, I hadn't seen that in the code. My thought was to put all of the processed image sizes into the reply, but I hadn't figured out how to do that. The thought for the client application was to do as you propose, cache both the photos and the images locally in the application so that the application provides good local performance.

The other thing that I'm thinking about is how to detect changes in an entry so that the application can request a refresh. I think there are a few strategies:

  1. Keep an "updated at" date on the "entries" collection reply, perhaps in a separate version request that doesn't return the entire data set. The application checks the date of it's internal data against the date of the data available through the API and requests a full update if the dates don't match.

  2. Keep an "updated at" on each individual record. I've seen this implemented when the data records are large, although this tends to produce a "chatty" exchange. I wouldn't think it would be necessary in this case.

  3. Simply periodically (or manually) poll for the entire data set. This is the simplest and most brute force approach. It assures data is up-to-date, but is not kind to data plans.

I like option 1 the best. In my experience, the connections directory doesn't change very often, maybe a few times a quarter in our business case. Checking that the local data is up-do-date once a day is probably adequate. When changes are made, downloading the entire data set a few times a quarter should not cause any problems.

Not sure how WP is addressing this, I suspect they don't have an "offline" business case so they probably didn't put that versioning facility in.

What do you think?

@shazahm1
Copy link
Member Author

@ranchhand6

The response will include an "added by", "data added" and "last modified date". That should work for you, right?

Doing the first would require a new endpoint. I'm not against it. You could extend the CN_REST_Entry_Controller class and override the register_routes() and prepare_item_for_response() methods and just return a collections of last updated dates.

Just to clarify, it is not TimThumb, that has been abandoned by its author. My endpoint is just similar, mostly matching it parameters.

@shazahm1
Copy link
Member Author

@ranchhand6

I know I've been fairly quiet on this but work has been progressing. Well, not directly on the REST API itself. I realized it would be best to take the time to refactor some of my core code which will make implementing the full REST API easier, I hope.

I just merge this in my develop branch. It takes addresses and makes them into Collections of Addresses which itself is managed by a new class. Making it easier to work with addresses. I was able to even leverage my template code to allow the address rendering to be templated which opened up endless design possibilities with templating.

I'm going to release this to shake out any bugs and then abstract the address code and refactor it and implement it for the other repeatable fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants