Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Block Library - Query Loop]: Fix some missing term sanitizations #39970

Merged
merged 3 commits into from
Apr 4, 2022

Conversation

ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Apr 1, 2022

What?

Fixes a bug found by @hellofromtonya here.

$term_ids isn't being used. Shouldn't it be used to set the IDs for $query['tax_query'][]['terms']?

It should use the sanitized value($term_ids).

Notes

The changes of this PR when merged, need to be added to the core PR here: WordPress/wordpress-develop#2488.

Testing instructions

  1. Everything should work as before.

@ntsekouras ntsekouras added [Type] Bug An existing feature does not function as intended [Package] Block library /packages/block-library [Block] Query Loop Affects the Query Loop Block labels Apr 1, 2022
@ntsekouras ntsekouras self-assigned this Apr 1, 2022
@ntsekouras ntsekouras requested a review from spacedmonkey as a code owner April 1, 2022 08:11
$term_ids = array_map( 'intval', $terms );
$term_ids = array_filter( $term_ids );

if ( is_taxonomy_viewable( $taxonomy ) && ! empty( $terms ) ) {
Copy link
Contributor Author

@ntsekouras ntsekouras Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_taxonomy_viewable checks if the taxonomy is publicly_queryable. --cc @peterwilsoncc

@ntsekouras ntsekouras requested a review from peterwilsoncc April 1, 2022 13:00
@ntsekouras ntsekouras changed the title [Block Library - Query Loop]: Fix use the sanitized term ids [Block Library - Query Loop]: Fix some missing term sanitizations Apr 1, 2022
Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Nik,

I added a suggestion but it's a nitpick so you could merge as is.

There are some reports of tests failing that appear unrelated so I'm presuming they're not blockers.

lib/compat/wordpress-6.0/blocks.php Outdated Show resolved Hide resolved
lib/compat/wordpress-6.0/blocks.php Outdated Show resolved Hide resolved
@ntsekouras ntsekouras force-pushed the fix/query-use-sanitized-termids branch from 2ae0f0e to b200750 Compare April 4, 2022 07:34
@ntsekouras ntsekouras merged commit dd377a5 into trunk Apr 4, 2022
@ntsekouras ntsekouras deleted the fix/query-use-sanitized-termids branch April 4, 2022 08:24
@github-actions github-actions bot added this to the Gutenberg 13.0 milestone Apr 4, 2022
hellofromtonya added a commit to hellofromtonya/wordpress-develop that referenced this pull request Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block [Package] Block library /packages/block-library [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants