WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 5 months ago

#23148 closed enhancement (wontfix)

Add hook in wp_ajax_save_attachment for additional attachment fields

Reported by: griffinjt Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: has-patch
Focuses: Cc:

Description

As of now, wp_ajax_save_attachment only looks for the attachment title, caption, description and alt text fields. Without a hook, there is no way to extend the new media system properly and take advantage of the save functionality.

There should be a hook provided at the end of this function (before the JSON response is sent back) that allows devs to update any additional attachment fields that have been rendered by extending the media.view.Attachment.Details subview.

Attachments (1)

23148.diff (365 bytes) - added by griffinjt 2 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 @SergeyBiryukov2 years ago

  • Type changed from defect (bug) to enhancement

@griffinjt2 years ago

comment:2 @griffinjt2 years ago

I've added a patch that adds a hook right after WordPress internally updates the post. Passing two params, $post and $changes, is sufficient for a user to have the information needed to make a successful update_post_meta/wp_update_post call.

comment:3 @SergeyBiryukov2 years ago

  • Keywords has-patch added; needs-patch removed

comment:4 @griffinjt2 years ago

As an update, I have tested out this patch and it works as expected. Here is my code as a proof of concept for both rendering the custom field template and saving the data: https://gist.github.com/f3d93e78ed118b283938. Note that the call to "wp_ajax_save_attachment" is the hook I have added in the patch.

It would be great if the patch could be committed to 3.5.1 so that I can actively start using the new media manager in both my plugins and client work. ;-)

Last edited 2 years ago by griffinjt (previous) (diff)

comment:5 @SergeyBiryukov2 years ago

  • Milestone changed from Awaiting Review to 3.6

comment:6 @wpsmith2 years ago

  • Cc t@… added

comment:7 @goto102 years ago

  • Cc dromsey@… added

comment:8 @mordauk2 years ago

  • Cc pippin@… added

comment:9 @sc0ttkclark2 years ago

  • Cc lol@… added

comment:10 @chrisvanpatten2 years ago

  • Cc chris@… added

comment:11 @mordauk2 years ago

  • Cc pippin@… removed

comment:12 follow-up: @bradyvercher2 years ago

  • Cc brady@… added

Any fields added using the backward compatible filter attachment_fields_to_edit are already rendered in the new media modal and can be saved by hooking into attachment_field_to_save, which is fired by wp_ajax_save_attachment_compat(). These fields can also be registered to work on the new "Edit Media" screens, so I'm not sure a hook is really necessary here.

Here's how something similar would be done:

function attachment_url_field_23148( $form_fields, $post ) {
	$form_fields['attachment_url_23148'] = array(
		'label'        => 'Link',
		'input'        => 'text',
		'value'        => get_post_meta( $post->ID, 'attachment_url_23148', true ),
		'show_in_edit' => false,
	);

	return $form_fields;
}
add_filter( 'attachment_fields_to_edit', 'attachment_url_field_23148', 10, 2 );

function save_attachment_url_23148( $post, $data ) {
	if ( isset( $data['attachment_url_23148'] ) ) {
		update_post_meta( $post['ID'], 'attachment_url_23148', esc_url_raw( $data['attachment_url_23148'] ) ); 
	}

	return $post;
}
add_filter( 'attachment_fields_to_save', 'save_attachment_url_23148', 10, 2 );

comment:13 in reply to: ↑ 12 @griffinjt2 years ago

Replying to bradyvercher:

Any fields added using the backward compatible filter attachment_fields_to_edit ... These fields can also be registered to work on the new "Edit Media" screens, so I'm not sure a hook is really necessary here.

Red flags should be thrown up when you are required to use a backwards compatible filter with a new API. I disagree and believe this hook is necessary to promote good and future-minded code with the new media APIs. Beyond this, the new API is JavaScript, and I don't see promoting an entirely PHP solution as a good thing moving forward. This hook simply opens up the "model" portion of Backbone so that it can be utilized by other people properly extending Backbone's other APIs.

comment:14 follow-up: @bradyvercher2 years ago

No need for red flags; those filters are the API and have been since before the new media modal was implemented, so why not use them? Just trying to help.

comment:15 in reply to: ↑ 14 @griffinjt2 years ago

Replying to bradyvercher:

No need for red flags; those filters are the API and have been since before the new media modal was implemented, so why not use them? Just trying to help.

If I came off accusatory, I apologize - I promise I didn't mean it! Maybe I should have just said that it seems odd to use code that is now not the primary way to interact with the new API. Either way, I still think a hook here would be beneficial because it would allow you to take advantage of the new JS media code without having to write completely new JS glue to save your custom fields added with JS. :-)

comment:16 @griffinjt2 years ago

One other thing to note about the old code is that the new JS media manager does not recognize anything set by WP_Screen (at least from my testing), so unless there is another way to test, there isn't a way to limit where the attachment fields appear.

comment:17 @bradyvercher2 years ago

No worries and good point on context-sensitive fields. In that case, data in the $_REQUEST global is available to every action fired during the request, so edit_attachment should eventually get called and work fine for your purposes:

function soliloquy_save_attachment_fields( $attachment_id ) {
	if ( defined( 'DOING_AJAX' ) && DOING_AJAX && ! empty( $_REQUEST['changes']['soliloquy_link'] ) )
		update_post_meta( $attachment_id, '_soliloquy_image_link', esc_url_raw( $_REQUEST['changes']['soliloquy_link'] ) );
}
add_action( 'edit_attachment', 'soliloquy_save_attachment_fields' );

Would something like that work?

comment:18 @ryan20 months ago

  • Milestone changed from 3.6 to Future Release

comment:19 @wonderboymusic5 months ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

No one has chimed in to re-ask for this filter in 15 months, I think there are sufficient ways to accomplish what you want

Note: See TracTickets for help on using tickets.