equalizedigital / accessibility-checker

GNU General Public License v2.0
14 stars 8 forks source link

Post edit move to trash button causes error and prevents issue purge #535

Closed SteveJonesDev closed 5 months ago

SteveJonesDev commented 6 months ago

Please give us a description of what happened.

The Move to trash button on the post-edit screen causes the plugin to trigger a new scan which fails and also fails to run purge methods.

https://github.com/equalizedigital/accessibility-checker/assets/2895788/076e3c4e-7d0d-4ccf-a539-01b61b4caf57

Please describe what you expected to happen and why.

Clicking the Move to trash button shouldn't initial a new scan and should successfully run the purge actions.

How can we reproduce this behavior?

  1. Install EDAC
  2. Create a post
  3. Click the Move to trash button in the post edit screen

Technical info

pattonwebz commented 6 months ago

I could reproduce this on a local test site as well. When posts are trashed from the block editor, they hit the save_post action (for some reason, unsure why), triggering our save flow.

It reaches edac_validate(), which tries to build a simple_html_dom object from the URL - a 404 by that point - resulting in the 'password protected' notice being thrown up.

Looking at the $post object status, by the time it reaches edac_save_post(), is already set to trash, so we can check for that in the function and return early before attempting to scan the no-longer-accessible post.

Early return prevents the error from being thrown and doesn't halt the rest of the flow, which deletes the rows from the table.

https://github.com/equalizedigital/accessibility-checker/assets/3902039/3c941730-5989-4b6b-9e9b-f8ef4d61c30c

SteveJonesDev commented 6 months ago

That check is potentially a good failsafe. We should still identify why edac_save_post is being called when that button is clicked.

pattonwebz commented 5 months ago

@SteveJonesDev I tested some more with this and with older versions of WP, and it seems like the block editor has always - or at least for the last year - used the wp/v2/posts/ endpoint as a single, consistent way to manage all post changes (including status updates like trashing posts).

Checking the classic editor for how it works differently, it calls functions directly to update post values in the database when trashing - bypassing any post insert or update.

I looked for alternative ways to prevent the error that occurs when trashing from inside the block editor, and so far, I haven't found anything as simple as short-circuiting edac_save_post() early. I'll open a PR with that change, but will think about it over the weekend and see if any other ideas come to mind