Opened 13 years ago
Closed 13 years ago
#18433 closed task (blessed) (fixed)
Retrieve a list of posts via XML-RPC
Reported by: | nprasath002 | Owned by: | westi |
---|---|---|---|
Milestone: | 3.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | XML-RPC | Keywords: | has-patch mobile westi-likes |
Focuses: | Cc: |
Description
Attachments (6)
Change History (24)
#1
@
13 years ago
- Keywords mobile westi-likes added
- Owner set to westi
- Status changed from new to reviewing
#2
@
13 years ago
- Cc max@… added
I've taken a pass at cleaning up the patch a little. There were a number of typos that broke the code, but I believe it now works more-or-less as advertised.
Someone with better knowledge about internal WP APIs needs to audit this to make sure the logic makes sense. I left most of the logic as nprasath002 had it, assuming that he had good reason for why he did things.
Before this goes in, I think it would make sense to add additional filter options; currently only 'post_type', 'numberposts' and 'offset' are supported, but we should probably expand that to include most of the valid filters to wp_get_recent_posts (category, orderby, post_status, include, exclude, etc.).
There's also a lot of duplication between this and mw_getRecentPosts. Duplication is generally a problem in this file, so I'm not sure if that is intentional or a side-effect of incremental development.
Given the preference to not go with the approach in #16316, I think we definitely need a wp.* method to allow filterable retrieval of posts.
#3
@
13 years ago
Would be possible to add another optional parameter, that a client could use to list the fields to retrieve? I'd like to to get rid of mt.getRecentPostTitles and start using this new call.
#4
@
13 years ago
Good idea Dan. I see three options to solve that problem:
- Have an additional parameter that is an array of strings representing the fields that the caller wishes to receive back. An empty array would be interpreted as "all".
- Have an additional string parameter representing 'response type', with possible values like 'full', 'basic', 'minimal'. This would give some amount of flexibility to the caller while not making the implementation a mess by needing to check every single field.
- Have an additional array/struct parameter that is a set of filters on the response. Possible filters would be things like 'include_content', 'include_custom_fields', 'include_taxonomies', etc.
I personally am leaning towards 2 but would certainly appreciate others' thoughts.
#5
@
13 years ago
I like the third idea.
It easy to understand and makes the code more readable.
If that is ok i will work on a patch for that.
#6
@
13 years ago
i vote for the option #2. It should give us the possibility to retrieve just few fields like (ID, title, status), and also a 'full' response.
#7
@
13 years ago
I think an array of strings makes sense. You can combine this and three pretty easily: array( 'post', 'meta', 'taxonomies' ), array( 'title', 'date', 'content', 'author', 'excerpt', 'meta', 'taxonomies' ), etc.
Option 2 seems both arbitrary and inflexible. Also, the implementation for the above would be pretty easy.
#8
follow-up:
↓ 9
@
13 years ago
The new patch (3) implements a combined strategy as suggested by nacin. It also adds 'post_status' as a filter option.
For fields, there are two types of values: conceptual groups and specific fields. The conceptual groups are 'post', 'taxonomies', and 'custom_fields'. Since the latter two require extra queries, performance-conscious callers can skip them by omitting them from 'fields'.
And to give extra control, you can specify specific individual fields instead of/in addition to conceptual groups. So for daniloercoli's use-case, he could use array('title', 'dateCreated', 'post_status').
I'm still torn about whether we should keep using backcompat-names (mt_*, wp_*, camelCase) or cut the duplication and give them sensical names in the context of modern WordPress. API consumers will already have to change some code to use the new method, but I'm not sure if field name changes would be too much of an additional burden.
Also, should we add a filter to let plug-ins modifying the $post_data array after it's generated?
#9
in reply to:
↑ 8
@
13 years ago
Replying to maxcutler:
For fields, there are two types of values: conceptual groups and specific fields. The conceptual groups are 'post', 'taxonomies', and 'custom_fields'. Since the latter two require extra queries, performance-conscious callers can skip them by omitting them from 'fields'.
And to give extra control, you can specify specific individual fields instead of/in addition to conceptual groups. So for daniloercoli's use-case, he could use array('title', 'dateCreated', 'post_status').
Nice, I like this implementation because we can specify individual fields (what we really need in our mobile apps). At the end don't think it's so tedious specify every single field you need in the response, or at least this should make you think before ask for everything.
Also, should we add a filter to let plug-ins modifying the $post_data array after it's generated?
+1
#10
follow-up:
↓ 11
@
13 years ago
I made a couple of changes for the third patch
- Added validation for post statuses
- Added support for an array of post statuses. The default values are draft, publish, future, pending, private. To override this we must support array of post statuses. The new method supports draft,auto-draft, future,pending, trash, inherit, publish, private This means we can get trashed posts and post revisions.
- I separated adding offset and number of posts. We can specify an offset without specifying number of posts. Is it better to add a default value say 50?
- fixed a small bug. Added post_id in individual post validation.
WDYT?
#11
in reply to:
↑ 10
@
13 years ago
Replying to nprasath002:
- Added support for an array of post statuses. The default values are draft, publish, future, pending, private. To override this we must support array of post statuses. The new method supports draft,auto-draft, future,pending, trash, inherit, publish, private This means we can get trashed posts and post revisions.
I'm not sure we can expose the complete set of statuses because of the following reasons:
- The statuses list returned by calling wp.getPostStatusList contains only 'draft', 'pending', 'private' and 'publish'.
- Most probably you can't modify a post by setting its status to something different from draft, publish, pending, private.
#12
@
13 years ago
Ok, another rev of the patch.
I've extracted the return value preparation code into a new function ('prepare_post') so that it can be reused in wp.getPost (#18432).
I've added 'orderby' and 'order' to the filters parameter to allow sorting of the resultset.
I'm tempted to abandon the back-compat naming convention in the return value, and instead use the variable names used internally by WordPress (e.g., "comment_status" instead of "mt_allow_comments"). I know this would make it a little harder for existing apps to upgrade to the new API, but I think it buys us a lot more flexibility in the future to evolve the wp.* API namespace independent of the legacy APIs. Thoughts?
#13
@
13 years ago
Another rev that I think I'm happy with.
I cleaned up the $fields logic so that it wasn't just a crazy list of if statements. I also use prepare_term
from #18442/#18443 for formatting the terms field.
I mostly gave up on backwards compatibility in field naming, in favor of the WordPress internal names. The PHPDoc strings have been updated appropriately (see #18432 for wp.getPost's PHPDoc).
Updated patch formatting and fixed some typos.