Fix generating file names for edited files, and use wp_unique_filename() #23440
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also some better errors and some cleanup of names.
|
Size Change: +1.62 kB (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
ellatrix
reviewed
Jun 25, 2020
ellatrix
reviewed
Jun 25, 2020
ellatrix
reviewed
Jun 25, 2020
|
|
||
| if ( is_wp_error( $saved ) ) { | ||
| return $saved; | ||
| } | ||
|
|
||
| // Update attachment details. | ||
| // Create new attachment post. |
There was a problem hiding this comment.
Should we copy the rest of the attachment data?
There was a problem hiding this comment.
Yeah, may make sense. But then we'll get two (or more) attachments with the same description and caption. In general don't think the image alt text should be carried over as alt has to be contextual. Editing an older, already used image would suggest it is being used in different context. Editing an image just after uploading it won't have alt text.
ellatrix
reviewed
Jun 25, 2020
| return new WP_Error( 'rest_cannot_find_attached_file', __( 'Unable to find original media file.', 'gutenberg' ), array( 'status' => 500 ) ); | ||
| } | ||
| if ( isset( $params['rotation'] ) ) { // TODO: use `rotate` instead? | ||
| // CW / CCW |
There was a problem hiding this comment.
Might be good to write it in full :)
ellatrix
reviewed
Jun 25, 2020
|
|
||
| $media_id = $params['media_id']; | ||
| // This also confirms the attachment is an image. | ||
| $image_file = wp_get_original_image_path( $attachment_id ); |
There was a problem hiding this comment.
Might be good to leave a comment saying that this is the original uploaded file.
ellatrix
reviewed
Jun 25, 2020
|
|
||
| $filename = rtrim( dirname( $image_path ), '/' ) . '/' . $target_file; |
There was a problem hiding this comment.
Previously we were overriding the last edited file, right?
There was a problem hiding this comment.
Yeah, kind of. Was pretty messy. Overwriting the last edited file if it is in the same uploads subdirectory, but not all sub-sizes of it. If the edited file was smaller, and not all sub-sizes were created, some of the old sub-sizes were left "orphaned".
This can be fixed but then we will either need to delete the attachment for the previously edited image, or reuse it and replace the image files and meta. Both of these are not good options if the previously edited image was used in a post.
Better workflow and UX would be to keep all edited files, perhaps let the user know they can delete unused images from the media library.
ellatrix
approved these changes
Jun 25, 2020
…at has not been edited.
- Move the check if the crop dimensions are > 0.1% to the js. - Test if the crop dimensions are whitin bounds in PHP. - Make crop values optional.
No need to look at `x` and `y`. If they have changed, the width or/and height have changed too. Also fix white space in (ugly) array alignment...
Co-authored-by: Timothy Jacobs <[email protected]>
TimothyBJacobs
approved these changes
Jun 26, 2020
github-actions
bot
added
the
First-time Contributor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow-up to #23284.
Description
Fixes few errors and inconsistencies when editing an already-edited image file, and when creating the new attachment post. Also adds some better errors and some cleanup of vars, names, etc. and still has few TBD/TODO.
How has this been tested?
When testing please monitor what files are created in the uploads dir (
wp-content/uploads/year/month/).Checklist: