Skip to content

Manually check for queried taxonomy terms when building facets#151

Merged
kevinfodness merged 8 commits intomainfrom
fix/multiple-taxonomy-terms
Jun 23, 2022
Merged

Manually check for queried taxonomy terms when building facets#151
kevinfodness merged 8 commits intomainfrom
fix/multiple-taxonomy-terms

Conversation

@kevinfodness
Copy link
Copy Markdown
Member

Fixes #150

Copy link
Copy Markdown

@scottnelle scottnelle left a comment

Choose a reason for hiding this comment

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

👻 Looks good to me. Good detective work!

@mboynes
Copy link
Copy Markdown
Contributor

mboynes commented Feb 8, 2022

Would love to see a unit test added for this prior to merge. I can do it if you don't have time @kevinfodness

@kevinfodness
Copy link
Copy Markdown
Member Author

I can handle.

@kevinfodness
Copy link
Copy Markdown
Member Author

@mboynes I added a test for facet selection. In order to do that, for compatibility with phpunit in the latest version of WP, I also needed to move phpunit to composer, which I've also done with phpcs. The phpcs ruleset will fail if you run it locally, and we'll need another PR later to fix that up prior to activating the check in #145.

Copy link
Copy Markdown
Contributor

@mboynes mboynes left a comment

Choose a reason for hiding this comment

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

It's SearchPress, dangit!

function test_facet_by_taxonomy() {
// Fake a taxonomy query to WP_Query so the query vars are set properly.
global $wp_query;
$wp_query = new WP_Query(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpicky, but this will trigger a database call... maybe there's no way around that, but if there is, it would be nice to avoid.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think that might be possible - I'll give it a try.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep, turns out you can just $wp_query->parse_query( $new_query ) and it works like a charm. I added that and a few other cleanup items that I noticed while setting this up on my new m1 (phpunit polyfills path, trusting phpcs in composer.json, a few other places to use capital_p_dangit)

mboynes and others added 2 commits February 28, 2022 12:27
…es of SearchPress with incorrect capitalization, add PHPUnit Polyfills path constant, trust phpcs in composer.json
@kevinfodness kevinfodness merged commit e37f27e into main Jun 23, 2022
@kevinfodness kevinfodness deleted the fix/multiple-taxonomy-terms branch June 23, 2022 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Selected Flag Doesn't Work Properly with More Than One Selected Term

3 participants