Shortcode taxonomy filters (cat_name, cat_id, tag_name, tag_id) are silently ignored

[xnaveira]

Version: 1.1.11
Component: src/App/View/Calendar/CalendarShortcodeView.php

Summary

The [osec] shortcode never applies category or tag filtering. The resolved
term IDs are written to one pair of variables and read from a different pair,
so cat_ids and tag_ids are always empty strings in the request query.

The Gutenberg block is not affected — BlockController::getQuery() builds
cat_ids / tag_ids directly (lines 99–100) and never touches this code path.

Steps to reproduce

1. Create two event categories, e.g. Aktivitet and Möte, with events in each.
2. Make sure Settings → Viewing events → Preselected calendar filters is
   empty (see "Why this is easy to miss" below).
3. Put [osec cat_name="Aktivitet" view="agenda"] on a page.

Expected: only Aktivitet events.
Actual: all events, from every category.

The same applies to cat_id, tag_name and tag_id.

A quicker check: [osec cat_id="999999"] with a term ID that cannot exist
should render an empty calendar. It renders every event instead.

Cause

$mappings (lines 60–69) maps the shortcode attributes onto the prefixed
taxonomy names introduced by the 1.0.7 rename:

$mappings = [
    'cat_name'     => 'osec_events_categories',
    'cat_id'       => 'osec_events_categories',
    'tag_name'     => 'osec_events_tags',
    'tag_id'       => 'osec_events_tags',
    // ...
];

Line 92 iterates that array with $type as the value, and line 125 writes the
resolved term ID through a variable variable built from it:

foreach ($mappings as $att_name => $type) {   // line 92
    // ...
    ${'_' . $type}[] = $argument;             // line 125
}

With $type === 'osec_events_categories', that writes to
$_osec_events_categories. With $type === 'osec_events_tags', it writes to
$_osec_events_tags.

But the two accumulators declared at lines 44–45 — and read back at lines
132–133 when the query is assembled — still carry the pre-rename names:

$_events_categories = [];                                  // line 44
$_events_tags = [];                                        // line 45

// ...

$query = [
    'cat_ids' => implode(',', $_events_categories),        // line 132
    'tag_ids' => implode(',', $_events_tags),              // line 133
    // ...
];

Nothing ever writes to $_events_categories or $_events_tags, so both
implode() calls return ''.

Downstream, EventSearch::getFilterSql() instantiates FilterCatIds /
FilterTagIds with empty value arrays. FilterInt::get_where() returns ''
for those, array_filter() at line 333 removes them, and the query runs with
no taxonomy conditions at all.

This looks like leftover from the 1.0.7 taxonomy rename
(events_categories → osec_events_categories): the $mappings values were
updated, the variable declarations were not.
.

Suggested fix

Rename the two accumulators to match what line 125 writes:

@@ -41,8 +41,8 @@
         $view               = $default_view;
-        $_events_categories = [];
-        $_events_tags = [];
+        $_osec_events_categories = [];
+        $_osec_events_tags = [];
         $post_ids = [];
@@ -129,8 +129,8 @@
         $query        = [
-            'cat_ids'      => implode(',', $_events_categories),
-            'tag_ids'      => implode(',', $_events_tags),
+            'cat_ids'      => implode(',', $_osec_events_categories),
+            'tag_ids'      => implode(',', $_osec_events_tags),
             'post_ids'     => implode(',', $post_ids),

Two named arrays keyed by taxonomy would be more robust than variable
variables here, but the rename above is the minimal change.

Notes

• Present in every published release checked: 1.0.11, 1.1.0, 1.1.4, 1.1.9,
  1.1.10, 1.1.11. Not a recent regression.
• A related failure mode in the same loop: when get_term_by() fails to
  resolve a name (lines ~105–120), the value is skipped with continue and no
  warning. Once the bug above is fixed, a typo in cat_name will silently
  widen the result set rather than narrow it. Worth an admin notice or at
  least a _doing_it_wrong().

[digitaldonkey]

@xnaveira xnaveira Great fix. Thank you.
Seems dug through some non-intuitive code parts I tried to avoid :)
You are welcome to add a error handling for "get_term_by() fails to resolve a name" in #54.
Before big rewrites I'd prefer to have tests, which would be welcome too, to improve reliability the 'shortcodes' feature.