Forum Replies Created

Viewing 1 replies (of 1 total)
  • Thread Starter emanueleemolo

    (@emanueleemolo)

    Thanks for the quick turnaround. I pulled 1.13.2 and confirmed line 45 is fixed here.

    Since you mentioned the Importer sits on a path your own testing never goes near, here is the next thing on that same path. I hit it about ten minutes after the import completed.

    File: plugin/Models/WPBannersQuery.php
    Lines: ~129-157 in 1.13.2

    When campaigns is passed as slugs, each one goes through get_term_by(‘slug’, …, ‘wp_bannerize_tax’) and only the ones that resolve get collected. If none resolve, $categories is an empty array, but the tax_query is built regardless:

    $query['tax_query'] = [
      [
        'taxonomy' => 'wp_bannerize_tax',
        'terms'    => $categories,   // can be []
      ],
    ];

    WP_Tax_Query returns its $no_results clause (WHERE 0 = 1) for an IN comparison with empty terms, so the shortcode renders nothing at all. No banners, no wrapper markup, no notice, no log entry. From the site owner’s side it is indistinguishable from “this campaign is empty”.

    Why it bites specifically on migration: the legacy plugin used [wp_bannerize group=”…”], and after importing, the old group names do not necessarily survive as wp_bannerize_tax slugs. So every legacy shortcode in the content renders empty, on every page, and nothing anywhere points at the cause. I only found it by reading the query builder.

    Two suggestions, in the order I would want them:

    1. If $categories is still empty after the resolution loop, skip the tax_query entirely or short-circuit and return an empty string. Building a clause that is guaranteed to match nothing is the worst of both options.
    2. When a supplied slug does not resolve, surface it somewhere. A _doing_it_wrong, an admin notice, or at minimum an HTML comment in the output. The silence is what makes this expensive to debug, much more than the behaviour itself.

    Separately, while I was in the tree: 15 implicitly nullable parameter declarations (string $x = null rather than ?string $x = null), all under vendor/. Twelve in mobiledetect/mobiledetectlib, one in wpbones Str.php:429, one in View.php:73. PHP 8.4 deprecated that form, so on 8.4 and 8.5 they emit a deprecation on every request that touches the user-agent code. It is a composer bump rather than anything in your own source, but it is noise in the log for anyone on a current PHP.

    On the text domain point you raised: in resources/views/importer/index.php it is 18 of 23 i18n calls missing it, if that helps scope the next minor.

    Environment as before: WordPress 7.1, PHP 8.5.9, MySQL 8, migrating from legacy WP Bannerize with 134 banners.

    One last thing. If it is useful to you, I would be glad to help as a contributor rather than just filing tickets.

    What I can actually offer is the thing you said you do not have: a real legacy migration environment. A production site with 134 banners coming from the old plugin, on WordPress 7.1, PHP 8.5 and MySQL 8, with real group names and real shortcodes scattered across seven thousand posts. That is the exact path both of these bugs were hiding on. I am happy to run release candidates through it before you ship and report back, which beats finding these two weeks later.

    I am also a native Italian speaker, so when you get to the text domain pass I can take the it_IT catalogue off your hands.

    And if there is a repo I can open pull requests against, point me at it and I will send patches instead of prose. I am an IT engineer, I work with WordPress regularly, and I would rather contribute upstream than keep patching things locally.

    Either way, thanks for maintaining this. Plugins that answer bug reports in an afternoon are rarer than they should be.

Viewing 1 replies (of 1 total)