humanmade / humanmade/hm-content-import

Bug: Post::insert() reuses canonical ID without checking post status

Open
#94 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.