-
Notifications
You must be signed in to change notification settings - Fork 389
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
Fix delete files on delete attachment #307
Conversation
In WordPress 4.9.7, attachment file deleting changed again, which mangles the s3:// paths very badly. What's more, there's no hooks to be able to adjust / change this behaviour. More details in the code comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linting failed (1029 errors, 81 warnings).
(1100 notices occurred in your codebase, but were on files/lines not included in this PR.)
Codecov Report
@@ Coverage Diff @@
## master #307 +/- ##
============================================
+ Coverage 48.39% 49.05% +0.65%
+ Complexity 248 247 -1
============================================
Files 5 5
Lines 686 685 -1
============================================
+ Hits 332 336 +4
+ Misses 354 349 -5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a type hint on the comment. This is pretty annoying! Looks like it should have been fixed looking at this trac ticket https://core.trac.wordpress.org/ticket/39476
Looks like this is the related core changeset https://core.trac.wordpress.org/changeset/43392 shakes fist at @johnbillion
* | ||
* @param string $file_path | ||
* @return string | ||
* @param $post_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @param $post_id | |
* @param int $post_id |
@roborourke Yeah this was a fix for a core security issue. See https://codex.wordpress.org/Version_4.9.7. |
@johnbillion ah cool, thanks for the context :) |
In WordPress 4.9.7, attachment file deleting changed again, which mangles the s3:// paths very badly. What's more, there's no hooks to be able to adjust / change this behaviour. More details in the code comment.