# 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`:

```php
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:

```php
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:

```php
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`:

```php
$order->update_status($status_slug);
```

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

```php
$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.*