Remove the redundancy of adding row actions in `class-wp-media-list-table.php`

[kebbet]

This is a ticket related to #57890 which dealt with issues of row actions not displayed while a user has filtered the media library list.

The reason is _get_row_actions which duplicates code for multiple row actions in the if ( $this->detached ) and else logic.

The goal of this ticket is to remove the redundancy in that function to avoid furter issues like the ones in #57890.

Props to @pbiron for suggesting creation of this ticket.

Version set to 3.1, introduced in [16229].

https://core.trac.wordpress.org/ticket/57893

[kebbet]

My attached PR (​#PR 4364) also adresses #57890. So there is a bit of a overlap.

[kebbet]

PR ready for review, if you have the time @SergeyBiryukov @pbiron

To test:

• test all filters in the media list view
• test setting and unsetting media constants EMPTY_TRASH_DAYS and MEDIA_TRASH
• make sure all row actions actually perform their action
• extra: make sure media_row_actions works as expected.

What has changed from current implementation in trunk

• Download is shown in all filter views (even trash)
• Copy is shown in all views except trash

[kebbet]

If this ticket gets merged into core before #57890, then that ticket can be closed.

[oglekler]

I tested the patch, Copy URL and Download file are in place and working, but Delete permanently link went missing (screenshot is above).

[kebbet]

Thanks so much for testing @oglekler. I can confirm your findings.

[kebbet]

Linked PR (​PR#4364) is updated to fix the issue. Please test again.

[kebbet]

I've now tested with both setting and unsetting EMPTY_TRASH_DAYS and MEDIA_TRASH in wp-config.php, with kept behavior (except the changes intended with this ticket, see comment 6).

[kebbet]

Hi @desrosj (as you are a maintainer of media component). Can you please review this ticket and the linked PR? Is it possible to get this into 6.3?

[kebbet]

Attached PR has been updated after a nice review by @costdev, thanks!

It now skips my suggested introduction of a foreach loop, and adds the action directly to the $actions array after the conditional check for each action.

It also now removes the $delete_ays variable which was only used once.

[SergeyBiryukov]

In 55949:

Media: Display the “Copy URL” and “Download file” row actions when the “Unattached” filter is applied.

Due to partially duplicated logic for displaying row actions in the Media Library with and without the “Unattached” filter, the “Copy URL” and “Download file” row actions were unintentionally missing with the filter applied.

This commit aims to simplify the logic and bring more consistency to the code.

Includes displaying the “Download file” row action even when the “Trash” filter is applied, giving the user one more chance to download the media file before they delete it.

Follow-up to [8901], [13100], [16227], [16229], [52842], [55198], [55221].

Props kebbet, costdev, pbiron, oglekler, SergeyBiryukov.
Fixes #57890, #57893.

[costdev]

@SergeyBiryukov ​PR 4364 included a check that $attachment_url was truthy before adding the copy and download actions. This wasn't included in [55949], so the copy and download actions may added with an empty string URL if wp_get_attachment_url() returns false.

Applying add_filter( 'wp_get_attachment_url', '__return_false' ); shows that when clicking Copy URL or Download. Copy won't do anything, and Download will download the wp-admin/upload.php as an HTML file.

I'm preparing a PR with the conditions added as well as PHPUnit tests to protect the behaviour touched in [55949]. I'll have this ready later this evening.

[costdev]

​PR 4651 is ready for review

The PR looks good, looking at the media table part. I'm a novice at tests, so can not really say anything about them.

[costdev]

@SergeyBiryukov Just checking if you saw comment 18 and ​PR 4651 yet? No rush, just in case the noise of pre-Beta meant this one slipped by 🙂

[kebbet]

I have tested the patch, but can not say anything on tests. What I have tested looks and works correctly, as in removal of Download link and Copy-action when adding add_filter( 'wp_get_attachment_url', '__return_false' );. When patch is not applied, those links are still present.

[kebbet]

In what way does this ticket needs testing @oglekler ?

[oglekler]

@kebbet from my point of view PR looks good, I suggest pinging this ticket owner @SergeyBiryukov 😅

[SergeyBiryukov]

In 56072:

Media: Only show “Copy” and “Download” actions when an attachment URL is available.

Includes unit tests to verify the logic for displaying row actions in the Media Library in certain scenarios, e.g. with and without the “Trash” or “Unattached” filter.

Follow-up to [55949].

Props costdev, kebbet, mukesh27, oglekler.
Fixes #57893.

Thanks for the PR! Merged in r56072.