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

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

Status as of 1.4.4 (2026-05-12): every in-scope finding below has been resolved in plugin version 1.4.4. Each finding carries an inline status marker — ✅ resolved, ⏸ deliberately deferred. See the README changelog for the corresponding code changes.

Scope: Independent read-only analytical pass over the v1.4.3 codebase. Re-read every PHP file from scratch and walked through user scenarios with deliberate scepticism toward fixes I made in the 1.4.2 / 1.4.3 cycles. The intent of this pass was specifically to look for regressions introduced by recent fixes rather than longer-standing issues.

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 studiou-wc-ord-print-statuses.php, the markdown docs, and the refreshed .po translation source. Verification limitations: Static review only. No live PHP/WP/WC available.

Severity reflects user-visible or correctness impact.

---

1. Critical

(None.)

---

2. High — newly discovered, introduced by recent fix-ups

2.1 Studiou_DB_Manager::normalise_csv_date() shifts timezone-less CSV dates by the WP timezone offset — ✅ resolved in 1.4.4

Resolution: strtotime removed entirely. Fast path uses pure-regex reformat for ISO-like inputs (no timezone shift). Fallback uses DateTimeImmutable($raw, wp_timezone())->setTimezone(wp_timezone()) — interprets timezone-less input as WP local, honours explicit timezones, throws on garbage. The structural fix recommended in the review's §9 (zero strtotime calls in includes/) is in effect.

class-db-manager.php:426-437:

private static function normalise_csv_date($raw) {
    $raw = sanitize_text_field((string) $raw);
    if ($raw === '') {
        return '';
    }
    $timestamp = strtotime($raw);
    if (!$timestamp) {
        UtilsLog::log('normalise_csv_date: could not parse "' . $raw . '"');
        return '';
    }
    return wp_date('Y-m-d H:i:s', $timestamp);
}

This was changed from date(...) to wp_date(...) in 1.4.2 with the stated intent of "matching current_time('mysql') elsewhere". The change introduced a timezone shift bug. Trace:

WordPress calls date_default_timezone_set('UTC') very early in its boot. So strtotime($raw) for a timezone-less input string interprets it as UTC. Then wp_date($fmt, $timestamp) treats its $timestamp argument as a UTC value and formats it in the WP site timezone.

Concrete walkthrough with WP site timezone Europe/Prague (UTC+2 in summer):

1. CSV externalorderdate cell: "2026-05-12 14:30" — the print shop wrote this intending Prague-local 14:30.
2. strtotime("2026-05-12 14:30") in WP's UTC-default PHP context → timestamp T (representing 14:30 UTC).
3. wp_date('Y-m-d H:i:s', T) → formatted in Prague timezone → "2026-05-12 16:30:00".
4. Stored to meta. Operator opening the order edit screen sees 16:30. CSV said 14:30. 2-hour drift.

The same direction-of-drift bug as 1.4.0 §2.2 / 1.4.1 §2.1, just in a different function. The previous fix-pass made Order_Fields_Manager::mysql_to_datetime_local and datetime_local_to_mysql use pure string reformatting (no strtotime/date/wp_date) precisely to avoid this. Custom_Columns_Manager::format_meta_date was rebuilt with DateTimeImmutable(..., wp_timezone()) for the same reason. normalise_csv_date was missed.

Suggested fix — fast-path the common "no explicit timezone" pattern with pure string reformat, fall back to DateTimeImmutable with wp_timezone() for everything else:

private static function normalise_csv_date($raw) {
    $raw = sanitize_text_field((string) $raw);
    if ($raw === '') {
        return '';
    }
    // Fast path: ISO-ish date with no explicit timezone — treat as WP local
    // time, no shift.
    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;
    }
    // Fallback: parse in WP timezone (no shift on naive input), convert
    // to WP timezone (handles explicit-TZ input by shifting to local).
    try {
        $dt = (new DateTimeImmutable($raw, wp_timezone()))->setTimezone(wp_timezone());
        return $dt->format('Y-m-d H:i:s');
    } catch (Exception $e) {
        UtilsLog::log('normalise_csv_date: could not parse "' . $raw . '"');
        return '';
    }
}

This restores the documented invariant: imported external_ref_ord_date reads back at the value the print shop sent.

The drift was hidden in 1.4.2 because Custom_Columns_Manager::format_meta_date had its own opposite drift bug — the two cancelled out in the columns view. Now that the columns view is correct (1.4.2), the import-side drift is the visible asymmetry.

---

3. Medium — pre-existing or longstanding

3.1 Plugin assumes HPOS without enforcing it — ✅ resolved in 1.4.4

Resolution: get_orders_for_printing() now runs table_exists("{$wpdb->prefix}wc_orders") parallel to the existing wc_order_product_lookup check. If HPOS is disabled, the export bails with an admin notice ("Export requires WooCommerce HPOS … Enable High-Performance Order Storage …") instead of failing with a cryptic SQL error.

class-db-manager.php:84 references {$wpdb->prefix}wc_orders (the HPOS custom table). The bootstrap declares HPOS compatibility:

FeaturesUtil::declare_compatibility('custom_order_tables', __FILE__, true);

The third argument is $compatible=true, meaning "this plugin works correctly with HPOS". It does not mean "this plugin requires HPOS". If a site has HPOS turned off (legacy CPT storage only), the wc_orders table either doesn't exist or is unsynced. The export query then either:

• Errors with "Table 'wp_wc_orders' doesn't exist" — caught by the $results === false branch, logged but user-visible as "no exportable items".
• Returns rows from an incomplete or stale lookup — the export is incorrect but emits no warning.

The plugin has a precedent for surfacing this kind of dependency: table_exists($wpdb->prefix . 'wc_order_product_lookup') already runs at the top of the same method (1.4.2 §3.7). A parallel check for wc_orders would close the gap, or the plugin could refuse to load entirely under legacy CPT and surface an admin notice à la woocommerce_missing_notice.

The Search Manager has fallback (woocommerce_shop_order_search_fields for legacy CPT alongside the HPOS filter), but the export path doesn't. Asymmetric.

3.2 bulk_set_print_status doesn't pass a note to update_status, while the import handlers do — ✅ resolved in 1.4.4

Resolution: bulk_set_print_status now passes "Marked Prepare to Printing by bulk action." / "Marked In Printing by bulk action." to update_status. The audit trail in the order activity feed is now consistent across bulk-action and import paths.

class-db-manager.php:227:

$order->update_status($status_slug);

…versus class-db-manager.php:280-283:

$order->update_status(
    'in-print',
    __('Marked In Printing by InPrint Protocol import.', 'studiou-wc-ord-print-statuses')
);

Bulk-action-triggered transitions get WC's default "Status changed from X to Y" note. Import-triggered transitions get the explanatory "Marked … by … import." note. Asymmetric audit trail. The 1.4.3 release added explanatory notes to the import paths; the bulk paths were missed.

Pass an explanatory note from bulk_set_print_status too — e.g., "Marked … by bulk action.".

3.3 normalise_csv_date fallback (after fix) — strtotime() is permissive — ✅ resolved in 1.4.4

Resolution: subsumed by §2.1's fix. DateTimeImmutable throws on inputs strtotime would silently misinterpret ('foo', '1' etc.), so the catch-Exception branch logs and returns empty rather than persisting a meaningless timestamp.

Independent of §2.1, strtotime returns a valid timestamp for many ambiguous inputs:

• strtotime('1') → current year, January 1st.
• strtotime('next thursday') → next Thursday.
• strtotime('foo bar') → false (the only one that errors).

If a print shop's CSV has a malformed date that happens to parse, the import silently stores a meaningless date. The current if (!$timestamp) guard only catches the false return value, not the "parsed but absurd" cases.

The fix-suggestion in §2.1 — going through DateTimeImmutable($raw, wp_timezone()) — is stricter; it throws on unparseable input rather than guessing.

---

4. Low — style and cosmetic

4.1 Re-entrancy of $order->save() inside handle_status_transitions — ⏸ left as-is (forward-looking comment kept in code)

The behaviour is correct under WC 10.7.0's caching semantics; the docblock in class-order-status-manager.php:83-93 already explains the contract. Refactoring would require a more invasive design change to avoid the listener saving entirely.

class-order-status-manager.php:107-109. The listener fires from within woocommerce_order_status_changed, which is itself inside WC_Order::update_status(). The listener calls $order->save() to persist the meta change. The parent path (bulk action, import handler, or manual edit) then calls its own $order->save() afterwards.

Today this works because WC's data store returns the same object instance from wc_get_order($id) while the request is hot — both saves operate on the same object, the second save is a no-op or just re-saves the same state. If WC ever changes the cache semantics (e.g., to return fresh objects after a save), the parent and listener would operate on different instances and the meta change could be clobbered by the parent's save.

Brittle but not broken. Worth a forward-looking comment.

4.2 bulk_set_print_status and the listener both stamp the same meta key — ⏸ left as-is (intentional, documented)

Acknowledged in class-order-status-manager.php:71-77's docblock as intentional. The redundancy costs one extra save() per bulk-changed order, but the behaviour is correct. The 1.4.0 review §2.3 and the 1.4.1 review covered this.

4.3 Custom_Columns_Manager calls wc_get_order per cell — ✅ resolved in 1.4.4

Resolution: under HPOS the column-content callback receives the WC_Order object directly — wc_get_order is no longer called. Under legacy CPT a per-request $order_cache indexed by ID memoises the lookup so the three custom columns share one resolved instance per row.

class-custom-columns-manager.php:31. On a 50-orders-per-page list with three custom columns, that's 150 calls per page load. WC's own object cache mitigates the cost, but a per-page-load static cache keyed by ID inside the manager would be tighter.

4.4 emit_import_summary always shows "%d order(s) updated successfully" even when the count is 0 — ✅ resolved in 1.4.4

Resolution: gated on $updated > 0, matching the pattern Bulk_Actions_Manager::notify_moved_count introduced in 1.4.3.

class-import-manager.php:175-186. If 100% of rows errored, the operator sees "0 orders updated successfully" + the more useful error notice. The notify_moved_count helper in Bulk_Actions_Manager (1.4.3) already does the suppression-on-zero pattern; the import summary could adopt it.

4.5 add_action('before_woocommerce_init', function () {...}) uses an anonymous callback — ✅ resolved in 1.4.4

Resolution: converted to a named function studiou_wc_ord_print_statuses_declare_hpos_compat. Other plugins / test harnesses can now remove_action('before_woocommerce_init', 'studiou_wc_ord_print_statuses_declare_hpos_compat') if they need to override.

studiou-wc-ord-print-statuses.php:26-30. Anonymous functions cannot be unhooked from outside the file. If another plugin or test setup ever wants to disable the HPOS compatibility declaration, they can't. Cosmetic — converting to a named function is one line.

4.6 Studiou_DB_Manager::csv_escape would prefix legitimate negative quantities with ' — ⏸ left as-is

Acknowledged trade-off — OWASP's recommendation to defuse formula injection is stronger than the cosmetic concern of a leading single quote on negative numbers (which Excel hides anyway). If negative quantities ever become a real workflow signal, csv_escape can be adjusted to whitelist numeric leading-minus.

class-db-manager.php:164-176. A qty of "-1" (which shouldn't occur but might in refund-handling scenarios) starts with -, so csv_escape adds a leading single quote: "'-1". Excel/LibreOffice display strips the quote so the user sees -1 — no UX impact — but the underlying CSV bytes look unusual to programmatic consumers. The OWASP recommendation is consistent with this trade-off (defending against formula injection is more important than clean negative-number display), but worth noting.

4.7 call_user_func($cfg['handler'], $csv_data) in Import_Manager::run_import — ✅ resolved in 1.4.4

Resolution: replaced with $handler = $cfg['handler']; $result = $handler($csv_data); — direct callable invocation. Slightly faster, less verbose.

class-import-manager.php:129. PHP 7+ supports invoking array callables directly: $cfg['handler']($csv_data). The call_user_func form is slightly slower and slightly more verbose. Style only.

4.8 add_custom_shop_order_column_content's external_order_number cell relies on $value !== '' rather than $value !== '' && $value !== '—' — ⏸ left as-is (vanishingly rare edge case)

If anyone ever stores the literal string "—" to external_ref_ord_no (operator typing it in for an unknown number), the column displays the literal "—". Confusing but a vanishingly rare edge case.

4.9 parse_csv doesn't handle \r-only line endings — ⏸ left as-is (legacy macOS, extremely rare)

class-db-manager.php:351-384. PHP's auto_detect_line_endings was deprecated in 8.1 and removed in 9.0. macOS-Classic CSVs (\r-only) wouldn't tokenise correctly. Extremely unlikely in practice — modern macOS uses \n, Excel-Mac uses \r\n. Not worth fixing.

4.10 set_orders_to_prepare_printing and set_orders_to_in_printing are thin wrappers — ⏸ left as-is (stable public API surface)

class-db-manager.php:197-209 — two two-line methods that delegate to bulk_set_print_status. The public API would be cleaner as a single bulk_set_status($order_ids, $slug) that the bulk action handler calls with the right slug. Refactor-only; behaviour unchanged.

---

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.
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	Status of in-scope findings	Carried-over deferrals
revise-1.3.2.md	All resolved in 1.4.0	3 (Network: header, uninstall, cosmetic README).
revise-1.4.0.md	All resolved in 1.4.1	1 (HPOS filter verification).
revise-1.4.1.md	All resolved in 1.4.2	2 (custom-order-number plugins, register_post_status).
revise-1.4.2.md	All resolved in 1.4.3	1 (Excel CS-locale CSV separator).

One newly introduced regression identified in this pass: §2.1 above, the side-effect of the 1.4.2 date() → wp_date() change in normalise_csv_date. Worth noting as a process observation: the same class of bug has now appeared in three separate functions across three releases (1.4.0 Order_Fields_Manager, 1.4.1 Custom_Columns_Manager, 1.4.2 normalise_csv_date). A future fix should audit every call to strtotime + date_i18n/wp_date/date in the codebase for the same pattern.

---

7. Risk-prioritised summary

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

1. §2.1 normalise_csv_date shifts timezone-less CSV dates by the WP offset. ✅ resolved.
2. §3.1 No hard guard against HPOS-disabled installs. ✅ resolved.
3. §3.2 bulk_set_print_status doesn't pass an explanatory note. ✅ resolved.
4. §3.3 normalise_csv_date fallback accepts permissive strtotime interpretations. ✅ resolved (subsumed by §2.1).
5. §4.1 Listener save() re-entrancy. ⏸ left as-is (docblock kept).
6. §4.3 Per-cell wc_get_order in column rendering. ✅ resolved.
7. §4.4 Zero-count import "success" notice. ✅ resolved.

---

8. Out of scope

• Live behaviour testing against a running WP 6.9.4 / WC 10.7.0 install.
• Performance profiling of the export query on large order volumes.
• Behavioural verification on the WC 10.x block-based order edit screen.
• Audit of the refreshed .po against the running plugin (the file was rewritten in this cycle; cross-verifying every msgstr against the source msgid is out of scope for this review).
• Multisite, uninstall, custom-order-number plugin integration (carried-over deferrals).

---

9. Process observation

The two prior fresh-eye reviews (revise-1.4.1, revise-1.4.2) each surfaced a single new high-severity bug that had survived earlier passes. This review continues the pattern with §2.1 — and the cause is a regression I introduced in the 1.4.2 fix for a different timezone bug. The same code shape (strtotime + format-in-WP-timezone) appears in three different functions across three releases, fixed each time only in the function specifically called out by the review.

The reliable fix for this class of bug is to drop strtotime for the "stored as WP-local MySQL datetime ↔ display in WP-local" pipeline entirely. Either pure string reformatting (as Order_Fields_Manager now does) or DateTimeImmutable(..., wp_timezone()) (as Custom_Columns_Manager now does) sidesteps the WP UTC-default trap. A grep for strtotime across includes/ would catch any future regression.

Today the only remaining strtotime call is in normalise_csv_date. After §2.1 is fixed, there should be none.

Confirmation in 1.4.4: a grep for strtotime across includes/ returns zero hits in executable code. Only the docblock comments in class-custom-columns-manager.php, class-db-manager.php, and class-order-fields-manager.php mention it — explaining why each function avoids the pattern.

---

End of review.