Closed Snaver closed 7 years ago
Hi @Snaver, thanks for the PR!
I think the addition of a Pages endpoint would be quite handy for the small community who like this project. This could be it's own, quite simple, separate PR.
Regarding the change to the json_decode
behavior, the change in the constructor, by removing the optional parameter for the website URL, will break implementations where someone creates the client, and then sets the URL some place later in code, using the setWordpressUrl
function.
I think it'd also be a good change to remove the public property of json_assoc
from the client. Even if we switched to a getter and a setter, it'd allow people to do the same as setting the WP URL. Perhaps this decision needs to be made later in code. In fact, it probably doesn't belong in the client at all. Since this decoding happens in the endpoint itself, perhaps this should just be an optional argument to the get
method. You'd also need to update the documentation to indicate that the result is now mixed - it could be an object or an array.
Oh, and yes, SchoolUsers should not be part of this project. 👍
I think a few people in our org forgot that this component was open source. We'll need to do a little refactoring and then will pull that out at some point.
Hey boboudreau
This makes sense, I've put in a separate pull request just for the WP pages.
Agreed the json decode option implementation could be done better, I'll have a think.
Was playing around, not sure if this is suitable or the best method 🤔
https://github.com/Snaver/wordpress-rest-api-client/commit/8050773fdf93ac065a77f730eaccb5a8ebeca1b7
Added Pages support (https://developer.wordpress.org/rest-api/reference/pages/) & json decode assoc option.
Also should SchoolUsers be part of this project? xD