WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 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 5 years ago.
11243.diff (1.9 KB) - added by Denis-de-Bernardy 5 years ago.
11243.2.diff (2.5 KB) - added by ryan 5 years ago.

Download all attachments as: .zip

Change History (14)

@josephscott5 years ago

comment:1 follow-up: @Denis-de-Bernardy5 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-Bernardy5 years ago

untested patch attached

@Denis-de-Bernardy5 years ago

comment:4 @iammattthomas5 years ago

  • Cc mt@… added

comment:5 in reply to: ↑ 1 ; follow-up: @westi5 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-Bernardy5 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 @josephscott5 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 @ryan5 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.

@ryan5 years ago

comment:9 @azaozz5 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 @westi5 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.