Make WordPress Core

Opened 13 years ago

Closed 12 years ago

#18433 closed task (blessed) (fixed)

Retrieve a list of posts via XML-RPC

Reported by: nprasath002's profile nprasath002 Owned by: westi's profile westi
Milestone: 3.4 Priority: normal
Severity: normal Version:
Component: XML-RPC Keywords: has-patch mobile westi-likes
Focuses: Cc:

Description


Attachments (6)

wp.getPosts.patch (8.4 KB) - added by nprasath002 13 years ago.
wp.getPosts.2.patch (5.4 KB) - added by maxcutler 12 years ago.
Updated patch formatting and fixed some typos.
wp.getPosts.3.patch (7.7 KB) - added by maxcutler 12 years ago.
Added 'fields' parameter using nacin's suggested strategy.
wp.getPosts.4.patch (9.0 KB) - added by nprasath002 12 years ago.
wp.getPosts.5.patch (8.3 KB) - added by maxcutler 12 years ago.
wp.getPosts.6.patch (7.0 KB) - added by maxcutler 12 years ago.

Download all attachments as: .zip

Change History (24)

#1 @westi
12 years ago

  • Keywords mobile westi-likes added
  • Owner set to westi
  • Status changed from new to reviewing

@maxcutler
12 years ago

Updated patch formatting and fixed some typos.

#2 @maxcutler
12 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 @daniloercoli
12 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 @maxcutler
12 years ago

Good idea Dan. I see three options to solve that problem:

  1. 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".
  1. 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.
  1. 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 @nprasath002
12 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 @daniloercoli
12 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 @nacin
12 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.

@maxcutler
12 years ago

Added 'fields' parameter using nacin's suggested strategy.

#8 follow-up: @maxcutler
12 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?

Last edited 12 years ago by maxcutler (previous) (diff)

#9 in reply to: ↑ 8 @daniloercoli
12 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: @nprasath002
12 years ago

I made a couple of changes for the third patch

  1. Added validation for post statuses
  1. 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.
  1. 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?
  1. fixed a small bug. Added post_id in individual post validation.

WDYT?

Last edited 12 years ago by nprasath002 (previous) (diff)

#11 in reply to: ↑ 10 @daniloercoli
12 years ago

Replying to nprasath002:

  1. 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.
Last edited 12 years ago by daniloercoli (previous) (diff)

#12 @maxcutler
12 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 @maxcutler
12 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).

#14 @westi
12 years ago

  • Milestone changed from Awaiting Review to 3.4
  • Type changed from feature request to task (blessed)

Marking this as a Blessed Task for 3.4 - This is part of the first iteration of XML-RPC new features we will be working on.

#15 @westi
12 years ago

Close #6850 as a dupe of this

#16 @westi
12 years ago

In [19848]:

XMLRPC: Introduce new create,read,update and delete XMLRPC apis for Posts, Pages and all Custom Post Types.
Introduces: wp.newPost, wp.editPost, wp.deletePost, wp.getPost, and wp.getPosts
See #18429, #18430, #18431, #18432, and #18433 props maxcutler and markoheijnen.

#17 @koke
12 years ago

  • Cc jbernal@… added

#18 @maxcutler
12 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

New bugs or enhancements on new tickets.

Note: See TracTickets for help on using tickets.