churchthemes / church-theme-framework

A library of code useful for developing church WordPress themes that use the Church Content plugin.
https://churchthemes.com/guides/developer/framework/
GNU General Public License v2.0
31 stars 13 forks source link

get_term() vs get_term_by() in "/includes/events.php" #14

Closed wkoehn closed 5 years ago

wkoehn commented 5 years ago

In the ctfw_get_events() function in events.php file, is there a reason you are using get_term() instead of get_term_by() in this line?

$category_term = get_term( $args['category'], 'ctc_event_category' );

In this case, $args['category'] is stored as a 'slug', not as an 'id' which is required by get_term()?

Set in defaults section like this: $args['category'] = ! empty( $args['category'] ) ? $args['category'] : 'all';

https://codex.wordpress.org/Function_Reference/get_term "Get all Term data from database by Term ID. To retrieve term data by name, slug or ID, use get_term_by() instead"

stevengliebe commented 5 years ago

ctfw_get_events() expects $args['category'] to be an ID or 'all'. When 'all' (the default), get_term() doesn't execute because there's no need to modify the query.

// Filter by category.
if ( 'all' !== $args['category'] ) {

    $category_term = get_term( $args['category'], 'ctc_event_category' );

...
wkoehn commented 5 years ago

Ah, ok that makes sense. I saw you were comparing by 'slug' which is what threw me off track there, just figured it would be expecting 'slug'. Thanks for clarifying.

stevengliebe commented 5 years ago

I had to test it myself to understand it. The 'all' value is essentially just to avoid get_term() when no term ID is passed to the function.