humanmade / humanmade/hm-content-import
Bug: Post::insert() reuses canonical ID without checking post status
- Dominant language
- PHP
- Stars
- 30
- Forks
- 7
- Avg merge
- 6h 39m
- Merged PRs (30d)
- 2
Description
`get_id_from_canonical_id()` queries `postmeta` directly and returns an ID even if the post is trashed or has been permanently deleted (leaving orphaned meta behind). This causes `Post::insert()` to call `wp_update_post()` against a trashed or missing post instead of inserting a fresh one.
---
### Context
`Post::insert()` in `class-post.php` looks up an existing post by canonical ID before deciding whether to insert or update:
```php
if ( empty( $post_data['ID'] ) && $canonical_id ) {
$current_id = static::get_id_from_canonical_id( $canonical_id );
if ( $current_id ) {
$post_data['ID'] = $current_id;
}
}
```
get_id_from_canonical_id() queries postmeta directly via SQL. It has no awareness of post status — it will return an ID even if the post is trashed or has been permanently deleted (leaving orphaned meta behind, in which case get_post_status() returns false).
### What goes wrong
- Trashed post: wp_update_post() runs against a trashed post, resurrecting it instead of inserting a fresh one.
- Deleted post with orphaned meta: same path — $current_id is non-zero but the post no longer exists. wp_update_post() either fails silently or creates unexpected behaviour.
This was caught in a real migration where a stub ancestor had been trashed in favour of a manually-created page. The importer kept republishing the trashed stub instead of adopting the live page.
### Suggested fix
Check the post status after the lookup and only reuse the ID when the post is in an actionable (non-trash, non-missing) state:
```
if ( empty( $post_data['ID'] ) && $canonical_id ) {
$current_id = static::get_id_from_canonical_id( $canonical_id );
$current_status = $current_id ? get_post_status( $current_id ) : false;
if ( $current_id && $current_status && $current_status !== 'trash' ) {
$post_data['ID'] = $current_id;
}
}
```
The same guard ($status && $status !== 'trash') should be applied anywhere get_id_from_canonical_id() result is used to decide whether to update vs. insert, including exists(), which currently returns true for trashed and deleted posts.
### References
Discovered and fixed in wpcomvip/gates-foundation-gfo#386 at the existing_post_id_at_path() level. The same root cause applies to the framework layer.
Contributor guide
No contributing guide indexed for this repository
Research direction
Read class-post.php, starting with Post::insert() and exists(), and trace how get_id_from_canonical_id() feeds their update-versus-insert decisions. Confirm how get_post_status() reports trashed and deleted posts, then ensure only existing non-trash posts are reused in each path. Done means trashed or missing posts are not treated as existing actionable posts.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php, wordpress
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100