Introduce the 'Uploaded to' onto the Full Media Editor

[garrett-eclipse]

Hello,

When editing Media you'll find the 'Uploaded by'/'Uploaded to' information on the Media Modale Popup Editor. The Uploaded to is basically the parent information. However when you're viewing the FUll Media Editor it's not available. It would be nice to add 'Uploaded to' onto the Full Media Editor's publish metabox.

Thank you

[garrett-eclipse]

Uploaded to information on Media Modal editor

[joemcgill]

Hi @garrett-eclipse,

I think this could be a nice enhancement. Thanks for the suggestion.

[Mista-Flo]

Hi, I have submitted a patch for the suggested changes.

[Mista-Flo]

Example with uploaded by

[Mista-Flo]

@joemcgill Tell me if you have any suggestion, I wanted to reuse the same string 'Uploaded By' but it was a bit different with the uppercase on the b and the semi colon.

Do you think it's displayed in the good order? The file URL field seems dominant so it's a bit weird, but I thought it was the best place giving the fact we just printed the uploaded date before.

[garrett-eclipse]

Updated patch to provide more consistency, icons and links to edit users.

[garrett-eclipse]

Full Attachment Editing Screen - Added Icons and linked 'Uploaded by'

[garrett-eclipse]

Updated Attachment Details Modal - Made consistent with ordering of meta, linked Uploaded By

[garrett-eclipse]

Thanks @Mista-Flo for getting this started, I've uploaded 46866.2.diff as another step as I found a few items we could improve on here while I tested your patch (which worked great btw).

1. I've made the ordering of meta on the Attachment Details modal consistent with the full editor, this moves the Uploaded By & Uploaded To into the top right of the modal and allowed me to make their strings consistent.
2. I've added icons to the Uploaded By & Uploaded To fields in the Full Attachment Editor to be consistent with Uploaded On.
3. I've linked the Uploaded By field to the author profile. This I thought would be a nice bonus for admins allowing them to follow that breadcrumb if they find NSFW (etc.) images and need to go and ban the user responsible.

Would love your feedback/testing and I'm sure together we can get this into 5.6.

Marked needs-design-feedback to confirm the choice of dashicons and meta ordering as seen in the images above are appropriate.

[Mista-Flo]

Hey Garrett, thanks for your feedback and your patch, really nice enhancements :)

About the use of get_edit_user_link function, what will be shown to users that have access to edit attachment page but cannot edit user profiles data? A capability check would be nice I think except if you think WordPress will throw a WP_Error anyway and tell the user he is not allowed to view that page (I might be wrong).

[garrett-eclipse]

Thanks @Mista-Flo, to my understanding get_edit_user_link does the capability check for us;

if ( empty( $user_id ) || ! current_user_can( 'edit_user', $user_id ) ) {

Source - ​https://github.com/WordPress/wordpress-develop/blob/81a12a7bec6f82bb681176555308335822bd4010/src/wp-includes/link-template.php#L1625

The author was available already so the above check in get_edit_user_link means the link will only be made available if they have the edit_user capability.

That being said I do encourage testing on the capabilities to make sure we've not overlooked something.

And now looking at get_edit_post_link we can maybe save on the conditional in your original code, before grabbing the link from that function, as it already does a check on edit_post capability;
​https://github.com/WordPress/wordpress-develop/blob/81a12a7bec6f82bb681176555308335822bd4010/src/wp-includes/link-template.php#L1378

[Mista-Flo]

Hi there, just uploaded a new refreshed patch, just to avoid some PHP notice and extra unnecessary cap checks.

[Mista-Flo]

Clean the code

[hellofromTonya]

This ticket needs design feedback to move it forward. @karmatosed Is this one on your radar?

[karmatosed]

@hellofromTonya thanks for raising this up, I'll make sure it gets a review this week.

[hellofromTonya]

Thank you @karmatosed.

Capturing notes from today's Media scrub on the status and next steps for this ticket:

Per @antpb:

We just need a thumbs up there, not really a design from the team so this doesnt seem like too big a lift to happen before Beta

I think this one is a crucial one for Media 5.6, it's such a nice visible enhancement

Once we get the thumbs up from design, this ticket can move forward to commit.

[garrett-eclipse]

Full Media Editor missing Uploaded to information. Would be nice to display this just below the Uploaded on in the Save metabox [Note: this is a replacement image scrubbing the PII of the original.]

[garrett-eclipse]

Uploaded to information on Media Modal editor [Note: this is a replacement image scrubbing the PII of the original.]

[garrett-eclipse]

You can ignore the two 'new' images, I noticed there was some PII on the original versions so scrubbed them. They were the original two from the ticket so wanted to preserve them just remove the PII.

[garrett-eclipse]

To avoid confusing design, below is the current iteration (not the two most recent images);

[SergeyBiryukov]

Just a quick note that the latest patch appears to remove some checks from wp_prepare_attachment_for_js():

• $parent_type && $parent_type->show_ui
• current_user_can( 'edit_post', $attachment->post_parent )

At a glance, these are still relevant to avoid displaying a link that should not be displayed if the parent post type doesn't have a UI or the current user cannot edit the parent post.

Would be good to test and verify that removing these does not lead to any unexpected results, e.g. displaying a link leading to nowhere, or PHP notices when the post type no longer exists.

[Mista-Flo]

Replying to SergeyBiryukov:

Just a quick note that the latest patch appears to remove some checks from wp_prepare_attachment_for_js():

• $parent_type && $parent_type->show_ui
• current_user_can( 'edit_post', $attachment->post_parent )

At a glance, these are still relevant to avoid displaying a link that should not be displayed if the parent post type doesn't have a UI or the current user cannot edit the parent post.

Would be good to test and verify that removing these does not lead to any unexpected results, e.g. displaying a link leading to nowhere, or PHP notices when the post type no longer exists.

Hi Sergey,

get_edit_post_link( $attachment->post_parent, 'raw' ); is actually doing capability check, returns an empty string if the user does not have the cap. Then in the template, we have the <# if ( data.uploadedToLink ) { #>.

So I think it was extra capability checks that are not useful anymore, we can clean the code, what do you think?

[garrett-eclipse]

Updated Patch to leave permission handling to the get_link functions. As well update author to provide no author when missing.

[garrett-eclipse]

Hello, I've updated the patch in 46866.5.diff and feel we're nearing the final iteration so please test and review away @Mista-Flo @antpb @SergeyBiryukov.

Replying to SergeyBiryukov:

Just a quick note that the latest patch appears to remove some checks from wp_prepare_attachment_for_js():

• $parent_type && $parent_type->show_ui
• current_user_can( 'edit_post', $attachment->post_parent )

At a glance, these are still relevant to avoid displaying a link that should not be displayed if the parent post type doesn't have a UI or the current user cannot edit the parent post.

Would be good to test and verify that removing these does not lead to any unexpected results, e.g. displaying a link leading to nowhere, or PHP notices when the post type no longer exists.

Yes as @Mista-Flo mentioned this was on purposed, after investigation all the checks for capabilities and show_ui is handled from within the get_link functions for us so we can remove them here and just rely on the empty link check to tell if we can insert the link or not.

In get_edit_user_link the following check is done for us;
current_user_can_edit_user - ​https://github.com/WordPress/wordpress-develop/blob/79703088c4fea9655b5f071cadee4a7949dd1cca/src/wp-includes/link-template.php#L1627

In get_edit_post_link the following checks are done for us;
current_user_can_edit_post - ​https://github.com/WordPress/wordpress-develop/blob/79703088c4fea9655b5f071cadee4a7949dd1cca/src/wp-includes/link-template.php#L1380
post_type_object_has_edit_link - ​https://github.com/WordPress/wordpress-develop/blob/79703088c4fea9655b5f071cadee4a7949dd1cca/src/wp-includes/link-template.php#L1380
In this function there's an _edit_link check, and in the constructor for post_types that will be empty if show_ui or has_edit_link are empty/false - ​https://github.com/WordPress/wordpress-develop/blob/f36c42c70034348c4007157c1fc8a8d69e50c90c/src/wp-includes/class-wp-post-type.php#L469

So to summarize, all checks are done for us. There was two 'read' checks but from our discussion on slack here they were removed to match with the author setup and avoid admins seeing a attachment as an orphan when it isn't.

Also in this update I use the author exist check to provide no author verbiage if they don't similar to what was found for the media template.

I do encourage thorough testing of the capabilities and review.

[garrett-eclipse]

I'm going to own the ticket so it appears in the links I've been sharing requesting testing.

[karmatosed]

This is quite a long ticket so let me try and work through this with some feedback.

My initial thought is this section is getting informationally dense, but that's something to maybe look to iterate and shouldn't be something we judge this usefulness on.

I do however wonder if we need the icon. I guess if we have a president that is ok, but I'd be tempted to explore even removing those. I just would lean to no icon, particularly as that one means you would click to upload.

Beyond that, I think working within the boundaries you have, this goes with that. I truly would love to iterate this section though, but that's beyond the scope of this addition.

[hellofromTonya]

Moving this ticket to commit after reviewing with @antpb and @garrett-eclipse.

[garrett-eclipse]

Posts/Pages Publish modal are primarily icon based to illustrate the precedent.

[garrett-eclipse]

Update the Uploaded To icon to a folder to indicate it's not the action but the location.

[garrett-eclipse]

Updated screen with folder icon.

[garrett-eclipse]

Thank you for the feedback @karmatosed :

This is quite a long ticket so let me try and work through this with some feedback.

My initial thought is this section is getting informationally dense, but that's something to maybe look to iterate and shouldn't be something we judge this usefulness on.

Agreed it's starting to get heavy, I'm hoping that's the extent we'll ever get to in core otherwise would start moving the file specific meta somewhere less prominent.

I do however wonder if we need the icon. I guess if we have a president that is ok, but I'd be tempted to explore even removing those. I just would lean to no icon, particularly as that one means you would click to upload.

I think we should look into the removal of icons in future as the precedent comes from the conformity to the other editor screens (Posts, Pages) which I'll attach. We can revisit for all of these screens if you feel there's merit.
As to the upload icon, that makes sense, I've uploaded 46866.6.diff to switch it out for the folder icon which represents storage location more. We could also go with the post/page icon instead or am open to your thoughts on options. We should us dashicons, the one I'm using now - ​https://developer.wordpress.org/resource/dashicons/#category

Beyond that, I think working within the boundaries you have, this goes with that. I truly would love to iterate this section though, but that's beyond the scope of this addition.

Sounds good, let's try to connect in a design meeting once 5.7 has started up.

[karmatosed]

@garrett-eclipse much prefer this icon. I do think there is merit in the future exploring no icons, but that's not for today. Thanks for taking the feedback and iterating.

[antpb]

In 49207:

Media: Add 'Uploaded to' for individual media items in the media editor.
Adds a link in the media editor showing which post a media item was uploaded to.
Props karmatosed, garrett-eclipse, Mista-Flo, SergeyBiryukov, joemcgill, hellofromTonya.
Fixes #46866.

[SergeyBiryukov]

In 49209:

Coding Standards: Fix WPCS issues in [49207].

See #46866.