Media: Improve plugin compatibility of taxonomy query

[chirag2208]

Context

We are using folders plugin to structure media files in folders, web stories plugin creates invalid SQL query which does not provide any results

Summary

When eb Stories plugin is active and there is no any $tax_query in WP_Query object it adds 0 = 1 in SQL query which is always wrong so not returning any results in WP_Query, Please check this video for more details: https://tinyurl.com/2anx49r9

Relevant Technical Choices

We have fixed issue in this file:
/wp-content/plugins/web-stories/includes/Media/Media_Source_Taxonomy.php added this code after line number: 382

if(empty($tax_query)) {
    return [
        [
            'taxonomy' => $this->taxonomy_slug,
            'field'    => 'slug',
            'terms'    => [
                self::TERM_POSTER_GENERATION,
                self::TERM_SOURCE_VIDEO,
                self::TERM_SOURCE_IMAGE,
                self::TERM_PAGE_TEMPLATE,
            ],
            'operator' => 'NOT IN',
        ],
    ];
}

To-do

Any filter functionality on WP media page

User-facing changes

Before: https://tinyurl.com/25j68dyu
After: https://tinyurl.com/23c7k7kd

Testing Instructions

Install Folders plugin from wordpress.org (https://wordpress.org/plugins/folders/)
After Installing folders on your wordpress website, go to media and create folders and assign some media items to that folders (check this video for how to create folders and assign media items to it) - https://tinyurl.com/2anx49r9 then click on any Folders it is not showing any results

This PR can be tested by following these steps:

1. Create folders and assign some media items into it and then click on folders or follow the steps as shown in video (https://tinyurl.com/2anx49r9)

Reviews

Does this PR have a security-related impact?

No

Does this PR change what data or activity we track or use?

No

Does this PR have a legal-related impact?

No

Checklist

• This PR addresses an existing issue and I have linked this PR to it in ZenHub
• I have tested this code to the best of my abilities
• I have verified accessibility to the best of my abilities (docs)
• I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
• This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
• I have added documentation where necessary
• I have added a matching Type: XYZ label to the PR

---

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[swissspidy]

@chirag2208 Could you please sign the Contributor License Agreement (CLA) over at https://cla.developers.google.com/? We can't accept your contribution otherwise.

[chirag2208]

I have signed CLA

[swissspidy]

Thanks a lot!

So I had a quick look and it indeed looks like the tax query nesting is not really correct. The array_unshift there doesn’t really do what the comment says it should.

It would be more accurate to do something like this:

/**
 * Merge with existing tax query if needed,
 * in a nested way so WordPress will run them
 * with an 'AND' relation. Example:
 *
 * [
 *   'relation' => 'AND', // implicit.
 *   [ this query ],
 *   [ [ any ], [ existing ], [ tax queries] ]
 * ]
 */
$new_tax_query = [
	'relation' => 'AND',
	[
		'taxonomy' => $this->taxonomy_slug,
		'field'    => 'slug',
		'terms'    => [
			self::TERM_POSTER_GENERATION,
			self::TERM_SOURCE_VIDEO,
			self::TERM_SOURCE_IMAGE,
			self::TERM_PAGE_TEMPLATE,
		],
		'operator' => 'NOT IN',
	],
];

if ( ! empty( $tax_query ) ) {
	$new_tax_query[] = [ $tax_query ];
}

return $new_tax_query;

Second, to make this work more accurately, we should increase the filter priorities so that it truly runs after all existing tax queries:

// Hide video posters from Media grid view.
add_filter( 'ajax_query_attachments_args', [ $this, 'filter_ajax_query_attachments_args' ], PHP_INT_MAX );
// Hide video posters from Media list view.
add_action( 'pre_get_posts', [ $this, 'filter_generated_media_attachments' ], PHP_INT_MAX );
// Hide video posters from web-stories/v1/media REST API requests.
add_filter( 'web_stories_rest_attachment_query', [ $this, 'filter_rest_generated_media_attachments' ], PHP_INT_MAX );

cc @ayushnirwal

[deshanki12]

Hi @swissspidy

This is great.

What is the next step now in order for this fix to go live?

[swissspidy]

This still needs testing and tests to verify this fixes bugs and doesn‘t cause regressions.

[AnuragVasanwala]

@swissspidy LGTM ✅

Tested the PR in every possible way to make sure it doesn't have any conflicts.