Plans In Plugin: Add plans endpoint#8834
Conversation
3337826 to
8fe8fb8
Compare
oskosk
left a comment
There was a problem hiding this comment.
Tests well, although as dicussed on DM, I got a 404 cached in the transient, so when I had all set up correctly, I still saw that the route didn't exist. Good to merge after that is solved if it needs to be solved.
jeherve
left a comment
There was a problem hiding this comment.
It would be nice to add more details about the filter for the parser to process it.
There was a problem hiding this comment.
Could you add a docblock here?
I'm also not quite sure __return_true is the best option here; it is usually meant to be used when adding filters. Just true may be easier to understand imo.
8fe8fb8 to
8594ccd
Compare
|
@jeherve Documented the filter and fixed the value being passed. While I was writing it, for some reason my brain confused |
| } | ||
|
|
||
| public static function get_plans( $request ) { | ||
| $data = get_transient( 'jetpack_plans' ); |
There was a problem hiding this comment.
Since the lines are now separated from each other, maybe we don't need the extra spaces before the = anymore? What do you think?
| * | ||
| * @param bool true Whether to cache Jetpack plans locally | ||
| */ | ||
| $use_cache = apply_filters( 'cache_jetpack_plans', true ); |
There was a problem hiding this comment.
I wonder if it would not better to change the filter to start with jetpack_ instead of cache_? It seems more in line with our other filters.
| */ | ||
| $use_cache = apply_filters( 'cache_jetpack_plans', true ); | ||
|
|
||
| if ( $data === false ) { |
There was a problem hiding this comment.
You'll want to use a Yoda condition here, as per the WP coding standards.
| } | ||
| $data = Jetpack_Client::wpcom_json_api_request_as_blog( $path, '2', array( 'headers' => array( 'X-Forwarded-For' => $ip ) ), null, 'wpcom' ); | ||
|
|
||
| if ( $use_cache ) { |
There was a problem hiding this comment.
Should we make this more specific, just in case someone decides to use the filter incorrectly and uses the false string instead of a boolean for example?
* update changelog.txt * Update readme.txt with scaffolding for 5.9 changelog and release draft shortlink * Add changelog entry for #8243 * Add changelog entry for #8296 * Add changelog entry for #8367 * Add changelog entry for #8686 * Add changelog entry for #8707 * Add changelog entry for #8709 and #8714 * Add changelog entry for #8729 * Add changelog entry for #8777 * Add changelog entry for #8780 * Add changelog entry for #8786 * Add changelog entry for #8787 * Add changelog entry for #8801 #8805 #8832 #8865 and #8804 * Add changelog entry for #8817 * Add changelog entry for #8822 * Add changelog entry for #8823 * Add changelog entry for #8829 * Add changelog entry for #8834 * move some items to major enhancements * Add changelog entry for #8836 * Add changelog entry for #8839 * Add changelog entry for #8861 * Add changelog entry for #8862 * Add changelog entry for #8863 * Add changelog entry for #8866 * Add changelog entry for #8870 * Add changelog entry for #8874 * Add changelog entry for #8875 * Add changelog entry for #8881 * Add changelog entry for #8890 * Add changelog entry for #8911 * Add changelog entry for #8927 * Add changelog entry for #8931 * Add changelog entry for #8933 * Add changelog entry for #8930 * fix wording * typo * minor fixes * replace partner scripts for Jetpack Start in changelog entry * Update to-test.md * Update to-test.md * minor style fixes to to-test.md * minor style fixes to to-test.md * minor fixes on to-test.md * Add changelog entry for #8868 * Add changelog entry for #8844 * Add changelog entry for #8664 * Add changelog entry for #8935 * Add changelog entry for #8425 * Add changelog entry for #8625
Changes proposed in this Pull Request:
Testing instructions:
/jetpack/v4/plansfrom the api explorer while logged in (using the plugin found at https://wordpress.org/plugins/rest-api-console/)You will get back an api response that contains a localized list of plans in the correct currency for your location.
Proposed changelog entry for your changes: