WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#11243 closed defect (bug) (fixed)

Don't show deleted Pages in wp.getPages results

Reported by: josephscott Owned by: josephscott
Milestone: 2.9 Priority: normal
Severity: blocker Version: 2.9
Component: XML-RPC Keywords: has-patch
Focuses: Cc:

Description

Clients will get confused by the new trash status of Pages in the results of wp.getPages. This is a patch to filter out Pages with a status of trash from the results.

There was a similar issue with Posts, but this fix is different because it's calling get_posts() to collect the Pages data.

Attachments (3)

xmlrpc.php.diff (1.2 KB) - added by josephscott 6 years ago.
11243.diff (1.9 KB) - added by Denis-de-Bernardy 6 years ago.
11243.2.diff (2.5 KB) - added by ryan 6 years ago.

Download all attachments as: .zip

Change History (14)

@josephscott6 years ago

comment:1 follow-up: @Denis-de-Bernardy6 years ago

  • Severity changed from normal to blocker

I think we need a more general patch, that fixes this bug instead:

"post_status => all" should result in "post_status <> trash"

comment:3 @Denis-de-Bernardy6 years ago

untested patch attached

@Denis-de-Bernardy6 years ago

comment:4 @iammattthomas6 years ago

  • Cc mt@… added

comment:5 in reply to: ↑ 1 ; follow-up: @westi6 years ago

  • Cc westi added

Replying to Denis-de-Bernardy:

I think we need a more general patch, that fixes this bug instead:

"post_status => all" should result in "post_status <> trash"

I disagreee

post_status = all should return posts of all statuses whatever there meaning.

comment:6 in reply to: ↑ 5 @Denis-de-Bernardy6 years ago

Replying to westi:

I disagreee

post_status = all should return posts of all statuses whatever there meaning.

It's your call, at the end of the day...

FWIW, though, testing reveals that trashed posts and comments do creep up here and there, in WP and (more perversely) in plugins. 2.9 won't be as destructive as the introduction of the post_type column in the posts table. But the new status will result in issues that can easily be avoided by ensuring that the API remains backward-compatible.

Until now, "all posts" has been short hand for public, private, pending, draft. Revisions were (conveniently) not returned since they had a different post_type. The new trash status, on the other hand, adds unexpected junk (no pun intended) to the list.

comment:7 @josephscott6 years ago

I had considered pushing this down to a lower layer (like Denis suggested), but was concerned that other parts of WP had become dependent on 'all' returning all posts/pages, including those in the trash.

I suspect the trash status of posts and pages is going to cause some confusion because these functions now do different things. It's probably worth a detailed write up in the Codex on what things trash has changed.

comment:8 @ryan6 years ago

We exclude certain post types for 'any' requests. For post status queries, perhaps we could have 'any' mean all post types except those that should be excluded from public searches and 'all' really mean all types.

@ryan6 years ago

comment:9 @azaozz6 years ago

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

(In [12303]) Don't show deleted Pages in wp.getPages results, props ryan, props Denis-de-Bernardy,fixes #11243

comment:10 @westi6 years ago

I'm not happy with this change.

I think out XMLRPC/APP apis should expose things as well as our internal apis do.

I think this is probably the best thing for 2.9 but for 3.0 we need to revisit this and make it clear that apis like this will expose everything.

Maybe for example getPages should allow you to filter on status as an extra feature.

Clients should be careful about what they do with api calls like these which are open ended they should be graceful in there consumption of things they don't expect so we don't have to limit the flexibility we provide.

I'm going to open a new ticket for this for 3.0

Note: See TracTickets for help on using tickets.