qdr.app.woo.plugins/studiou-wc-ord-print-statuses/docs/revise-1.4.4.md

Plugin Revision — v1.4.4 (Fresh-Eye Analytical Review)

Status as of 1.4.5 (2026-05-12): all four mediums and items §4.1, §4.2 are resolved in plugin version 1.4.5. §4.3 and §4.4 are documented and deferred (architectural / edge case). §3.4 is documented in README "Known limitations" without a code change. Each finding carries an inline ✅ resolved / ⏸ deferred status marker.

Scope: Independent read-only analytical pass over the v1.4.4 codebase. Read with deliberate scepticism toward the changes introduced in 1.4.4 itself, looking for regressions and gaps the previous reviews missed.

Reviewer date: 2026-05-12 Target environment: WordPress 6.9.4, WooCommerce 10.7.0 (HPOS enabled), PHP 7.2+. Files reviewed: all PHP under includes/, the bootstrap, the markdown docs, and the .po translation source. Verification limitations: Static review only. No live PHP/WP/WC available.

Severity reflects user-visible or correctness impact.

Headline: no critical findings in this round. The plugin is in a steady state. Four medium-severity items and four low-severity items remain — most are defensive improvements rather than active defects. Two of the mediums (§3.1, §3.3) are gaps in fixes I introduced in 1.4.4 itself.

---

1. Critical

(None.)

---

2. High

(None.)

---

3. Medium — gaps and asymmetries

3.1 normalise_csv_date fast-path lacks range validation — asymmetric with datetime_local_to_mysql — ✅ resolved in 1.4.5

Resolution: the fast path now runs checkdate($month, $day, $year) plus explicit hour ≤ 23 / minute ≤ 59 / second ≤ 59 bounds. Inputs that match the regex structurally but fall outside the valid ranges (e.g., "2026-13-45T25:99") drop through to the DateTimeImmutable fallback, which throws and routes to the catch → empty string + logged.

class-db-manager.php:459-462:

if (preg_match('/^(\d{4})-(\d{2})-(\d{2})[T ](\d{2}):(\d{2})(?::(\d{2}))?$/', $raw, $m)) {
    $second = isset($m[6]) ? $m[6] : '00';
    return $m[1] . '-' . $m[2] . '-' . $m[3] . ' ' . $m[4] . ':' . $m[5] . ':' . $second;
}

The regex matches structurally — "2026-13-45T25:99" passes — but doesn't validate that the date is real or that the time components are in range. The fast-path returns "2026-13-45 25:99:00", which is then stored to postmeta as-is.

Compare with Order_Fields_Manager::datetime_local_to_mysql (class-order-fields-manager.php:118-131), which I extended in 1.4.3 to add checkdate() plus explicit hour/minute/second bounds. The same defence belongs on the import side, because:

• A CSV from the print shop's system could realistically include malformed dates (data-entry typo, locale mismatch, broken upstream tool).
• The bogus stored value round-trips through display: Order_Fields_Manager::mysql_to_datetime_local matches the regex but produces nonsense (2026-13-45T25:99), which the browser renders as an empty input. The operator can't tell anything happened until they save the order — at which point datetime_local_to_mysql finally rejects it.

Suggested fix — apply the same validation in the fast path:

if (preg_match('/^(\d{4})-(\d{2})-(\d{2})[T ](\d{2}):(\d{2})(?::(\d{2}))?$/', $raw, $m)) {
    list(, $y, $mo, $d, $h, $mi) = $m;
    $s = isset($m[6]) ? $m[6] : '00';
    if (!checkdate((int) $mo, (int) $d, (int) $y) || (int) $h > 23 || (int) $mi > 59 || (int) $s > 59) {
        // Fall through to the DateTimeImmutable path, which will throw on
        // unparseable values and reach our catch.
        return self::normalise_csv_date_fallback($raw);
    }
    return $y . '-' . $mo . '-' . $d . ' ' . $h . ':' . $mi . ':' . $s;
}

…or simpler: drop the fast path entirely and always use DateTimeImmutable($raw, wp_timezone()). The fast path was an optimisation; the fallback handles the common case correctly too.

3.2 emit_import_summary is silent when 0 successes and 0 errors — ✅ resolved in 1.4.5

Resolution: added an elseif (empty($errors)) branch that emits an info notice ("The CSV file contained no actionable rows (empty order_no column).") so the operator gets feedback even when dedup-by-order_no filters out every row.

class-import-manager.php:172-209. The 1.4.4 change gated the "%d orders updated successfully" notice on $updated > 0. That's correct when there are errors to surface, but if both counters are zero the operator sees nothing.

How can both be zero? run_import already bails earlier when $csv_data is empty (line 121). But dedupe_by_order_no (line 413) further filters out rows whose order_no is empty after trim(). A CSV with header + N rows where every row has an empty order_no column produces count($csv_data) === N but yields zero rows after dedup. No order_no → no wc_get_order lookup → no error entry → zero on both sides.

The operator gets no feedback that the import "succeeded" (it ran without error) but did nothing.

Suggested fix — add an else branch:

} elseif (empty($errors)) {
    UtilsLog::message(
        __('The CSV file contained no actionable rows (empty order_no column).', 'studiou-wc-ord-print-statuses'),
        'info'
    );
}

3.3 csv_escape checks only the literal first character — leading-whitespace bypass — ✅ resolved in 1.4.5

Resolution: csv_escape now calls ltrim and checks the first non-whitespace character. Cells like " =CMD" are now prefixed with '.

class-db-manager.php:174-186:

$first = $value[0];
if (in_array($first, array('=', '+', '-', '@', "\t", "\r"), true)) {
    return "'" . $value;
}

Cells beginning with a space followed by a danger character (e.g., " =CMD") slip past — $value[0] is the space. Some spreadsheet apps trim leading whitespace before evaluating, others don't; behaviour varies by version and config.

Vector likelihood: low in this codebase — the danger-prone fields are prod_name, prod_var, prod_cat, email. Product / category names are admin-controlled. email is customer-controlled but WC's email validation rejects strings starting with space-equals.

Still, the OWASP recommendation is to check the value after ltrim, not just $value[0]. One-line fix:

$trimmed = ltrim($value);
if ($trimmed !== '' && in_array($trimmed[0], array('=', '+', '-', '@', "\t", "\r"), true)) {
    return "'" . $value;
}

3.4 Possible collation mismatch on terms.slug = postmeta.meta_value — ⏸ documented in README "Known limitations"

Resolution path: the issue is theoretical and hasn't been observed in production. Adding an explicit COLLATE clause to the join condition risks breaking installs whose actual collation differs from whatever we'd hardcode. The pragmatic choice is to document the failure mode (which manifests as ERROR 1267 (Illegal mix of collations) from the bulk export) so future operators can add the fix if it surfaces. README "Known limitations" now mentions this.

class-db-manager.php:104-105:

LEFT JOIN `{$wpdb->terms}` `product_variations_name`
    ON `product_variations_name`.`slug` = `product_variations_attr`.`meta_value`

terms.slug is VARCHAR(200); postmeta.meta_value is LONGTEXT. Both columns should use utf8mb4_unicode_ci on a default WP install, but installs that have been around long enough to have been migrated across MySQL versions (or had per-table collation changes applied) can end up with mismatched collations.

When MySQL encounters a join condition between columns with incompatible collations, it can either:

• Throw ERROR 1267 (HY000): Illegal mix of collations and abort the query, or
• Implicitly coerce via temporary string conversion, which can silently change case sensitivity / accent handling for that join.

The export query has no COLLATE clause, so it inherits the DB-default collation. Most production sites won't be affected — Studiou's install presumably wasn't, given that earlier 1.3.2 / 1.4.0 / 1.4.1 reviews didn't surface this. Worth noting as an unbounded edge case rather than a confirmed bug.

Mitigation (only if it becomes a problem): add COLLATE utf8mb4_unicode_ci to the JOIN condition.

---

4. Low

4.1 bulk_set_print_status note selection is a binary ternary — implicit else — ✅ resolved in 1.4.5

Resolution: lifted to an explicit array('to-print' => …, 'in-print' => …) map with isset fallback to empty string. Future contributors adding a third slug get a no-op note rather than the wrong note.

class-db-manager.php:229-231:

$note = ($status_slug === 'to-print')
    ? __('Marked Prepare to Printing by bulk action.', 'studiou-wc-ord-print-statuses')
    : __('Marked In Printing by bulk action.', 'studiou-wc-ord-print-statuses');

The function is only ever called with 'to-print' or 'in-print', so the else branch is correct today. If a future contributor extends the helper to a third status, the else-implicit selection of "In Printing" would silently produce a misleading note. Defensive option: a small dispatch map.

4.2 parse_csv doesn't detect duplicate headers — ✅ resolved in 1.4.5

Resolution: after array_map(normalise_header, $headers), parse_csv compares count($headers) against count(array_unique($headers)) and bails (with an UtilsLog::log entry naming the headers) on mismatch. The downstream import then emits the existing "could not be parsed" notice.

class-db-manager.php:391-397:

$headers = array_map(array(__CLASS__, 'normalise_header'), $headers);
while (($data = fgetcsv($handle, 0, ',', '"', '')) !== false) {
    if (count($data) !== count($headers)) {
        continue;
    }
    $csv_data[] = array_combine($headers, $data);
}

If the CSV has two columns sharing a normalised name (e.g., Order No and order_no both normalising to order_no), array_combine silently overwrites — the second column's data wins for every row, the first column's data is lost. The user gets no warning.

Realistic? Unlikely with a well-formed protocol CSV. Defensive — add count($headers) === count(array_unique($headers)) check and abort with a clear notice if not.

4.3 Main plugin instance is short-lived; managers persist via the WP filter array — ⏸ left as-is (architectural)

Resolution path: refactoring would require either making the main instance a singleton (with getInstance()) or moving manager initialisation to a static method. Both are larger refactors than the symptom warrants — there is no functional impact, only a documentation concern. Worth a CLAUDE.md note if the lifecycle pattern surprises a future contributor.

studiou-wc-ord-print-statuses.php:47-50:

public static function initPlugin() {
    $class = __CLASS__;
    new $class;
}

The new instance has no global / static reference. After __construct() calls init() and init() returns, the local $class variable goes out of scope and PHP's reference counter for the main instance hits zero. The instance is GC'd.

The managers stored in $this->order_status_manager etc. would normally be GC'd along with the main instance — except each manager's constructor registers WP hooks with array($this, '...'), which adds references to the manager instances inside the global $wp_filter array. Those references keep the managers alive for the rest of the request.

So the private $order_status_manager / $order_fields_manager / etc. fields on Studiou_WC_Ord_Print_Statuses are effectively decorative — they only matter during the few microseconds of initialize_managers(). After init() returns they're inaccessible.

Implications: none in normal flow. But code that tries to look up "the plugin's order status manager" via Studiou_WC_Ord_Print_Statuses (e.g., a third-party hook integrating with this plugin) will find no instance. The pattern is unusual and worth documenting.

4.4 woocommerce_missing_notice registered conditionally but never unregistered — ⏸ left as-is (edge case)

studiou-wc-ord-print-statuses.php:54-60. If WC is not yet active when our init() runs, the notice hook is registered. If WC becomes active later in the same request (extremely unlikely — WC would have to be activated mid-request by another plugin's hook), the notice would still display once on the next admin page load. Edge case.

---

5. Verification gaps (carried over from earlier reviews)

1. HPOS search filter name woocommerce_order_table_search_query_meta_keys against WC 10.7.0 source. The legacy filter woocommerce_shop_order_search_fields may not fire under HPOS at all — making it not a fallback. Smoke test by adding a temporary error_log inside the callback and observing whether the HPOS path actually fires.
2. woocommerce_process_shop_order_meta behaviour under the WC 10.7.0 block-based order edit screen.
3. Object-cache scenarios for UtilsLog::message (Redis/Memcached).

---

6. Status of prior reviews

Review	Resolved	Carried-over deferrals
revise-1.3.2.md	1.4.0	3 (Network: header, uninstall, README phrasing).
revise-1.4.0.md	1.4.1	1 (HPOS filter verification).
revise-1.4.1.md	1.4.2	2 (custom-order-number plugins, register_post_status).
revise-1.4.2.md	1.4.3	1 (Excel CS-locale CSV separator).
revise-1.4.3.md	1.4.4	(listener save() re-entrancy, bulk/listener double-stamp — both intentional, documented; csv_escape on negatives — OWASP trade-off; "—" literal column, \r-only line endings, set_orders_to_* API surface — vanishingly rare).

No new regressions introduced by 1.4.4 in the strict sense. §3.1 and §3.3 are under-applied defences — patterns that should have extended to the import path / been stricter in csv_escape — rather than active bugs.

---

7. Risk-prioritised summary

Ordered by user-visible severity. Status as of 1.4.5:

1. §3.1 normalise_csv_date fast-path missing range validation. ✅ resolved.
2. §3.2 Silent import when all rows have empty order_no. ✅ resolved.
3. §3.3 csv_escape leading-whitespace bypass. ✅ resolved.
4. §3.4 Collation mismatch on slug join. ⏸ documented in README "Known limitations".
5. §4.1 Binary note selection in bulk_set_print_status. ✅ resolved.
6. §4.2 Duplicate-header detection in parse_csv. ✅ resolved.
7. §4.3 Main plugin instance lifecycle. ⏸ left as-is (architectural, no functional impact).
8. §4.4 woocommerce_missing_notice re-fire. ⏸ left as-is (edge case).

---

8. Out of scope

• Live behaviour testing against a running WP 6.9.4 / WC 10.7.0 install.
• Performance profiling under realistic order volumes.
• Behavioural verification on the WC 10.x block-based order edit screen.
• Multisite, uninstall, custom-order-number plugin integration (carried-over deferrals).
• Verifying every msgstr in the refreshed .po against the running plugin.

---

9. Process observation

The "single new HIGH per fresh-eye pass" pattern from 1.4.0 → 1.4.3 has tailed off. This pass surfaces only mediums and lows. Either:

• The plugin has genuinely reached a stable state (likely), or
• I'm developing scope blindness on this codebase after seven reviews and four fix-up releases (possible — fresh-eye reviews work best with rotating reviewers).

If a second reviewer is available, this is a good moment for an outside pass. Otherwise, the diminishing-returns curve suggests the next iteration of fixes should focus on the carried-over verification gaps (live testing against WC 10.7.0) rather than another static review cycle.

---

End of review.