Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LogService::prune_logs removes old posts from custom post type #168

Closed
soup-bowl opened this issue May 30, 2024 · 1 comment · Fixed by #169 · May be fixed by #170
Closed

LogService::prune_logs removes old posts from custom post type #168

soup-bowl opened this issue May 30, 2024 · 1 comment · Fixed by #169 · May be fixed by #170
Assignees
Labels
bug Something isn't working

Comments

@soup-bowl
Copy link
Owner

From the plugin forums posted by @odkr:

https://wordpress.org/support/topic/logservice-prune_logs-removes-old-posts-from-custom-post-type/

There is a proven concern that the post cleanup scheduler has the possibility to delete posts that aren't from the CPT of the plugin.

@soup-bowl soup-bowl added the bug Something isn't working label May 30, 2024
@soup-bowl soup-bowl self-assigned this May 30, 2024
@odkr
Copy link

odkr commented May 31, 2024

I had some time to think about this since I opened the issue, and it would seem to me that it comes down to is that the expectation that a WP_Query returns the posts that the query was for, while reasonable, may not hold up in the real world, for three reasons:

  1. is_home(), is_blog() have counter-intuitive edge cases; in my case, that is_home() holds true for any query made using get_posts, even if it is made in a cron job. Moreover, plugins may run arbitrary queries and modify them in arbitrary ways that are not covered by conditional tags to begin with (e.g., plugins for plugins).

  2. The WP Codex offers little normative guidance what to use pre_get_posts, parse_query, query, etc. for and how to use them, so any query may be modified. In fairness, it does say that functions that hook into pre_get_posts should use is_main_query(). I forgot about that because it's counter-intuitive to me that is_home() == true does not imply is_main_query() == true.

  3. There are people, like me 😄 , whose intuitions do not fit WordPress' API.

So, I'd still recommend to check the post type before calling wp_delete_post(). The cron job can delete a lot of posts (in our case, it deleted roughly 2,000 posts that pre-dated the installation of WP Simple SMTP), and some people don't make backups (or so I've been told). What is more, because it only deletes old posts, the deletion may go unnoticed for a while (it took us some days to notice, so we couldn't just revert to the backup from two hours ago, but had to merge backups with the content added since then).

@soup-bowl soup-bowl linked a pull request Jul 14, 2024 that will close this issue
@soup-bowl soup-bowl linked a pull request Jul 21, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants