Opened 4 years ago
Last modified 3 years ago
#49490 new enhancement
Proposal: New wrapper function and hook for creating attachments
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Media | Keywords: | dev-feedback 2nd-opinion needs-patch |
Focuses: | rest-api, performance, coding-standards | Cc: |
Description
Everywhere you look for the code related to inserting a new attachment you find 3 steps are needed:
- wp_insert_attachment
- wp_generate_attachment_metadata
- wp_update_attachment_metadata
I see no reason not to have a wrapper function instead of having these 3 functions being repeated all over core files + 3rd party themes and plugins. Plus, as people commit changes in core mistakes are occurring 49449.
The most important reason to create this new wrapper function is to improve interoperability by adding a real hook to know when an attachment has been totally processed by core. This is needed by many plugins that work with images or caching. Right now the hook most used to know of a new attachment is add_attachment
found in wp_insert_post()
, but this is run before step 2 and 3, therefore attachment metadata has not been created yet.
Developers are having to create a custom wrapper function to add this hook (my company included) in their projects and that means attachments inserted via WP Dashboard wont be taken into action.
media_handle_upload()
is a good example of the wrapper function I am talking about, but sadly it goes out of scope by assuming the file will be located in $_FILES
variable instead of receiving the file by a parameter, therefore is not friendly or future proof. When that function was created I guess it wasn't taken into consideration that files can be uploaded to the server in many ways. Plus is lacking the appropriate hooks too.
Even the WP_REST_Attachments_Controller
had to create custom methods to fulfill a scenario where attachment data is being sent in the body of the request.
A fast search in core for wp_insert_attachment
will illustrate how this function is followed by wp_generate_attachment_metadata()
and wp_update_attachment_metadata()
in most cases:
media_handle_sideload()
media_handle_upload()
wp_ajax_crop_image()
Custom_Background::handle_upload()
WP_Site_Icon::insert_attachment()
wp_xmlrpc_server::mw_newMediaObject()
WP_REST_Attachments_Controller
I wont even mention third party implementations I’ve found.
A quick boilerplate...
<?php /** * Creates a single attachment. * * @param string|array $args Required. Arguments for inserting a post of type attachment. * @param array $data Required. An array of data for a single file * Like the one supplied by wp_handle_upload() or wp_handle_sideload() * @param string $context Optional. Provide context for hooks (eg. AJAX, REST) * @param bool $wp_error Optional. Whether to return a WP_Error on failure. Default false. * * @return int|WP_Error The attachment ID on success. The value 0 or WP_Error on failure. * * @author Luis Rivera */ function wp_process_attachment ( string|array $args, string $data = false, string $context, bool $wp_error = false ) { /** * Just let us play * Maybe I need to remove a filter base on X condition */ do_action( 'pre_filter_process_attachment', $args, $data, $context ); /** * Allow modifications to the file * First filter because post args may depend on file data */ $data = apply_filters( 'wp_process_attachment_file', $data, $args, $context ); /** * Allow modifications to the post args */ $args = apply_filters( 'wp_process_attachment_args', $args, $data, $context ); /** * Filters whether to short-circuit the process * At last to evaluate values after filters has been applied * * If a non-null value is passed to the filter, all the process will be cancel * * @param mixed $process If null (default) continue process * If is_wp_error() && $wp_error the error will be returned */ $process = apply_filters( 'wp_process_attachment', null, $args, $data, $context ); if ( $process !== null ) { if ( $wp_error && is_wp_error($process) ) { return $process; } return false; } do_action( 'before_process_attachment', $args, $data, $context ); if ( empty( $args['post_title'] ) ) { $args['post_title'] = preg_replace( '/\.[^.]+$/', '', wp_basename( $data['file'] ) ); } if ( empty( $args['post_mime_type'] ) ) { $args['post_mime_type'] = $data['type']; } if ( empty( $args['post_status'] ) ) { $args['post_status'] = 'inherit'; } $attachment_id = wp_insert_attachment( $args, $data['file']); /** Include image functions to get access to wp_generate_attachment_metadata(). */ require_once ABSPATH . 'wp-admin/includes/image.php'; $metadata = wp_generate_attachment_metadata( $attachment_id, $data['file'] ); wp_update_attachment_metadata( $attachment_id, $metadata ); do_action( 'after_process_attachment', $attachment_id); return $attachment_id; }
Change History (13)
This ticket was mentioned in Slack in #core-media by pbiron. View the logs.
4 years ago
This ticket was mentioned in Slack in #core-restapi by pbiron. View the logs.
4 years ago
#4
@
4 years ago
Sorry for the spam but...
Wow! Just noticed this was added to Milestone
Feeling so proud about being able to get involved in such a big project and giving back to this amazing community!
As I mentioned in Slack, I'm pretty sure that attachments are the most used post-type in 99% of WP sites. It has always seemed very strange to me that the absence of proper hooks in the attachment creation process could have been ignored all this time.
My boilerplate should work as is but maybe someone more involved in core-media should double check that I'm not missing anything important. I tried to cover most scenarios I've come across while developing projects where I use WP as a headless rest-api provider.
I think this little improvement will open a world of opportunities for developers, specially the ones building media plugins or the ones trying to integrate WP REST Api. I believe having a fully hookable core is the key for removing any compromises that may exist in the use of our REST api. This will keep WP ahead for the years to come.
This ticket was mentioned in Slack in #core-media by luisrivera. View the logs.
4 years ago
This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-restapi by luisrivera. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-media by luisrivera. View the logs.
3 years ago
#9
@
3 years ago
- Summary changed from Missing wrapper function and action hook for new attachments to Proposal: New wrapper function and hook for creating attachments
#10
@
3 years ago
- Milestone changed from 5.5 to 5.6
We scrubbed this in #core-restapi today. Beta 1 is this Tuesday so I'm going to punt this to 5.6. I think this is too large of a change to get in at this point.
This ticket was mentioned in Slack in #design by karmatosed. View the logs.
3 years ago
#12
@
3 years ago
- Keywords needs-design removed
Hey @luisrivera,
Do you want to take a crack at turning this into a Pull Request? That'd make it easier to review and see the impacts.
I'm removing the needs-design
keyword. For reference, we use that for when a ticket needs visual design, not code design :)
This was discussed in the Media component meeting and it was agreed that better consistency across image handling in Core would be a great goal for 5.5. Moving it into the milestone so we can keep an eye on it and discuss further. :)