|
|
@@ -0,0 +1,310 @@
|
|
|
+# Plugin Revision — v1.4.0 (Analytical Review)
|
|
|
+
|
|
|
+**Scope:** Read-only analytical pass over the v1.4.0 codebase. No code changes were made. This document is the counterpart of [`revise-1.3.2.md`](revise-1.3.2.md) — it catalogues residual gaps, possible syntax pitfalls, regressions newly introduced by the 1.4.0 fix-up, and items from the prior review that remain open.
|
|
|
+
|
|
|
+**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/`, plus `studiou-wc-ord-print-statuses.php`, `README.md`, `CLAUDE.md`, `docs/translations.txt`.
|
|
|
+**Verification limitations:** No PHP interpreter and no WordPress / WooCommerce install were available in the working environment, so syntax errors visible only at compile time are not ruled out and WC-internal filter names / hook semantics could not be cross-checked against a running site. The findings below reflect WC 10.7.0 / WP 6.9.4 as the *target* but are derived from source reading, not live behaviour.
|
|
|
+
|
|
|
+Findings are grouped by severity. The severity reflects user-visible or correctness impact, not implementation effort to fix.
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+## 1. Critical — newly introduced regressions
|
|
|
+
|
|
|
+### 1.1 Export SQL `GROUP BY` may fail under default MySQL strict mode
|
|
|
+
|
|
|
+`class-db-manager.php:35-76` — the new export query introduces `GROUP BY orders.id, order_products.product_id, order_products.variation_id` so that products with multiple categories no longer duplicate rows. The `SELECT` list, however, contains several non-aggregated columns that are **not** in the `GROUP BY` clause:
|
|
|
+
|
|
|
+- `products.post_title`
|
|
|
+- `product_variations.post_title`
|
|
|
+- `product_variations_name.name`
|
|
|
+- `images.guid`
|
|
|
+- `order_products.product_qty`
|
|
|
+- `orders.billing_email`
|
|
|
+
|
|
|
+Under MySQL 5.7+ and MariaDB 10.3+ the default `sql_mode` includes `ONLY_FULL_GROUP_BY`. MySQL does recognise *functional dependency* on a primary key column, so `products.post_title` is technically dependent on `order_products.product_id` (which references `products.ID`). In practice the recognition is brittle: it depends on engine version, whether the FK is declared, the optimiser's understanding of the join, and the exact sql_mode.
|
|
|
+
|
|
|
+**Risk:** the bulk export may fail with `ERROR 1055 (42000): Expression #N of SELECT list is not in GROUP BY clause and contains nonaggregated column ...` on installations that follow the MySQL default. The previous v1.3.2 query had no `GROUP BY` and was immune.
|
|
|
+
|
|
|
+**Mitigation paths** (informational only):
|
|
|
+- Wrap every non-aggregated column in `ANY_VALUE(...)` (MySQL 5.7+) or `MIN(...)`.
|
|
|
+- Add every non-aggregated column to `GROUP BY` explicitly.
|
|
|
+- Reduce the join with a sub-query that pre-aggregates `product_cat`, then join in the rest without `GROUP BY`.
|
|
|
+
|
|
|
+### 1.2 CSV exports now write a UTF-8 BOM; CSV imports do not strip it
|
|
|
+
|
|
|
+`class-bulk-actions-manager.php:136` writes a leading UTF-8 BOM (`\xEF\xBB\xBF`) so Excel on Windows interprets the file as UTF-8. The matching read side `Studiou_DB_Manager::parse_csv()` (`class-db-manager.php:224-248`) does **not** strip a BOM from the first byte of the first header.
|
|
|
+
|
|
|
+**Symptom:** if a user exports `prepare_to_printing_export.csv`, edits it, and re-imports it as a protocol, the first header is read as `\xEF\xBB\xBForder_no` instead of `order_no`. The header normaliser (`normalise_header`, line 250) lowercases and collapses whitespace but does not touch the BOM. Subsequent `isset($row['order_no'])` checks fail for every row, the import emits "Invalid row format in CSV." for the entire file, and no orders are updated.
|
|
|
+
|
|
|
+This combination is realistic — exports often serve as templates for the next stage. Stripping a BOM is a single-line fix in `parse_csv`.
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+## 2. High — correctness, HPOS, time-zone
|
|
|
+
|
|
|
+### 2.1 HPOS search filter name is unverified against WC 10.7.0
|
|
|
+
|
|
|
+`class-order-search-manager.php:20` hooks `woocommerce_order_table_search_query_meta_keys`. The legacy CPT filter `woocommerce_shop_order_search_fields` (line 17) is well-established. The HPOS filter's exact name has varied across WC releases and could not be confirmed against WC 10.7.0's source without access to a running install.
|
|
|
+
|
|
|
+**Risk:** if the filter name is wrong (or has been renamed in the active WC 10.7.0 build), search by external order number works only under legacy CPT, not under HPOS — which is exactly the failure mode the v1.3.2 review flagged as critical.
|
|
|
+
|
|
|
+**Verification ask (WC 10.7.0):** confirm the filter still exists, e.g. by `grep -RIn 'woocommerce_order_table_search_query_meta_keys' wp-content/plugins/woocommerce/`, or by adding a temporary `error_log` inside the callback and observing whether it fires on the orders page with HPOS enabled. If the filter has been renamed, alternatives to consider in WC 10.x are `woocommerce_order_query_args` / `woocommerce_orders_table_query_clauses` for injecting `meta_query` at a higher level.
|
|
|
+
|
|
|
+### 2.2 Datetime fields use server timezone, not WP timezone
|
|
|
+
|
|
|
+`class-order-fields-manager.php:90` and `:99` use `date('Y-m-d\TH:i', $timestamp)` and `date('Y-m-d H:i:s', $timestamp)` for the `datetime-local` round-trip. PHP's `date()` formats with the server's PHP timezone, not WordPress's site timezone.
|
|
|
+
|
|
|
+Elsewhere the plugin stamps timestamps with `current_time('mysql')` — which honours `wp_timezone()` / `get_option('timezone_string')`. As a result:
|
|
|
+
|
|
|
+- Bulk action and import paths stamp `to_print_date` etc. in WP local time.
|
|
|
+- Manual edits via the Print Information panel display and save in server local time.
|
|
|
+- If the server is UTC and WP timezone is `Europe/Prague` (UTC+1 or +2 with DST), the panel shows values shifted by one or two hours.
|
|
|
+
|
|
|
+Replacing `date(...)` with `wp_date(...)` or constructing a `DateTimeImmutable` with `wp_timezone()` would unify behaviour.
|
|
|
+
|
|
|
+The orders-list column (`class-custom-columns-manager.php:58`) already does this correctly via `date_i18n(...)` — so the discrepancy is between the column display and the editable panel.
|
|
|
+
|
|
|
+### 2.3 Status-change listener and bulk-set both stamp the same meta
|
|
|
+
|
|
|
+`Order_Status_Manager::handle_status_transitions()` (lines 71-86) is triggered by every `woocommerce_order_status_changed` event. `Studiou_DB_Manager::bulk_set_print_status()` (lines 108-146) and `Studiou_DB_Manager::import_inprint_protocol()` both call `$order->update_status(...)` which fires that event, and *also* explicitly call `$order->update_meta_data($meta_key, current_time('mysql'))`.
|
|
|
+
|
|
|
+End result per status change via the bulk/import path:
|
|
|
+
|
|
|
+1. `update_status` → fires `woocommerce_order_status_changed` → listener stamps meta (only if currently empty).
|
|
|
+2. Caller then overwrites meta with another `current_time('mysql')` (potentially a millisecond later).
|
|
|
+3. Single `save()` persists the second value.
|
|
|
+
|
|
|
+The values are nearly identical, so this is **not a correctness bug**, but it is wasted work and a maintenance trap: future contributors changing one path may miss the other. The two paths should agree on a single source of truth — either remove the explicit stamping in the bulk/import path (let the listener handle it) or have the listener defer to the bulk/import path when the operation is bulk.
|
|
|
+
|
|
|
+A subtler asymmetry: the listener stamps **only if empty** (`if (!$order->get_meta($meta_key))`), the bulk path **always** stamps. On a second-pass bulk action, the bulk path overwrites; the listener's "only if empty" guard is bypassed for the bulk path but enforced for manual transitions. That divergence is intentional in one direction (bulk should re-stamp on each run) but should be commented in the code.
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+## 3. Medium — robustness, maintainability
|
|
|
+
|
|
|
+### 3.1 `GROUP_CONCAT` is silently bounded by `group_concat_max_len`
|
|
|
+
|
|
|
+`class-db-manager.php:38` uses `GROUP_CONCAT(DISTINCT t.name ORDER BY t.name SEPARATOR ', ')`. The MySQL default for `group_concat_max_len` is 1024 bytes; once exceeded the result is **silently truncated** with no error returned. Products belonging to many or long-named categories may lose the tail of `prod_cat` without warning.
|
|
|
+
|
|
|
+Either raise the limit with `SET SESSION group_concat_max_len = 65535` before the query, or accept the truncation and document the limit.
|
|
|
+
|
|
|
+### 3.2 `parse_csv` does not skip the UTF-8 BOM
|
|
|
+
|
|
|
+Related to §1.2. Even outside of the export-and-reimport workflow, any CSV authored in Notepad with "UTF-8 with BOM" encoding will hit the same issue. A single-line guard:
|
|
|
+
|
|
|
+```php
|
|
|
+$first = fread($handle, 3);
|
|
|
+if ($first !== "\xEF\xBB\xBF") {
|
|
|
+ rewind($handle);
|
|
|
+}
|
|
|
+```
|
|
|
+
|
|
|
+inside `parse_csv` would close this gap.
|
|
|
+
|
|
|
+### 3.3 `output_csv` fallback path mixes CSV into HTML on header-sent edge
|
|
|
+
|
|
|
+`class-bulk-actions-manager.php:130-138`:
|
|
|
+
|
|
|
+```php
|
|
|
+private function output_csv($csv_data, $filename) {
|
|
|
+ if (!headers_sent()) {
|
|
|
+ nocache_headers();
|
|
|
+ header('Content-Type: text/csv; charset=utf-8');
|
|
|
+ header('Content-Disposition: attachment; filename="' . $filename . '"');
|
|
|
+ }
|
|
|
+ echo "\xEF\xBB\xBF" . $csv_data;
|
|
|
+ exit;
|
|
|
+}
|
|
|
+```
|
|
|
+
|
|
|
+If headers have already been emitted (an upstream plugin printed something, a `print_r` left over from debugging, BOM in an included file…) the CSV is echoed without the right Content-Type. The browser will render it as part of the HTML page. Better: when `headers_sent()` returns true, log the event, queue an error notice via the new transient mechanism (problematic since we're about to exit), or `wp_die()` with a clear message instead of streaming garbage.
|
|
|
+
|
|
|
+### 3.4 `if ($file === null) { $this->redirect_back(); }` is statically unreachable-friendly
|
|
|
+
|
|
|
+`class-import-manager.php:88-91`:
|
|
|
+
|
|
|
+```php
|
|
|
+$file = $this->validated_upload($file_field);
|
|
|
+if ($file === null) {
|
|
|
+ $this->redirect_back();
|
|
|
+}
|
|
|
+$csv_data = Studiou_DB_Manager::parse_csv($file['tmp_name']);
|
|
|
+```
|
|
|
+
|
|
|
+`redirect_back()` ends with `exit;` so the code that follows is correctly unreachable, but static-analysis tools (Psalm, PHPStan) will flag `$file['tmp_name']` as a possible null-index access because the function signature on `redirect_back()` does not advertise it as a `noreturn`. Re-shaping as `if ($file === null) { return $this->redirect_back(); }` (or annotating `@return never`) makes intent explicit and silences the warning.
|
|
|
+
|
|
|
+### 3.5 `normalise_csv_date` returns the raw string on parse failure
|
|
|
+
|
|
|
+`class-db-manager.php:271-278`:
|
|
|
+
|
|
|
+```php
|
|
|
+private static function normalise_csv_date($raw) {
|
|
|
+ $raw = sanitize_text_field((string) $raw);
|
|
|
+ if ($raw === '') { return ''; }
|
|
|
+ $timestamp = strtotime($raw);
|
|
|
+ return $timestamp ? date('Y-m-d H:i:s', $timestamp) : $raw;
|
|
|
+}
|
|
|
+```
|
|
|
+
|
|
|
+On parse failure the raw string is stored. Downstream, `Order_Fields_Manager::mysql_to_datetime_local()` runs `strtotime` again, fails, and displays empty — so the operator silently loses the value. Either log the failure, surface a per-row import warning, or store an explicit sentinel (`''`) so the field is consistently blank rather than "looks blank, but has bogus data underneath".
|
|
|
+
|
|
|
+### 3.6 Unused constants and dead label maps
|
|
|
+
|
|
|
+- `STUDIOU_WC_OPS_VERSION` and `STUDIOU_WC_OPS_FILE` are defined in the bootstrap (`studiou-wc-ord-print-statuses.php:22-23`) but never referenced inside the plugin. They are useful for code outside the plugin (test code, migrations), but if nothing reads them they are effectively documentation.
|
|
|
+- `Order_Fields_Manager::$datetime_fields` and `$text_fields` map keys to English labels, but the labels are never read — the rendered labels are hardcoded inline in `display_custom_order_fields()`. Either drop the labels (just keep the key arrays) or drive `display_custom_order_fields` from the maps to eliminate the duplication.
|
|
|
+
|
|
|
+### 3.7 `set_order_processing_status` produces two order notes per payment-complete event
|
|
|
+
|
|
|
+`class-order-status-manager.php:103-109`:
|
|
|
+
|
|
|
+```php
|
|
|
+$order->update_status('processing', __('Payment received, order is now processing.', ...));
|
|
|
+$order->add_order_note(__('Order automatically set to processing by Studiou WC Order Print Statuses plugin.', ...));
|
|
|
+```
|
|
|
+
|
|
|
+The second argument to `update_status` is already a note. Then a second note is added explicitly. The order activity log will show two consecutive lines for the same event. Choose one: pass the longer text as the `update_status` note, or drop the second-argument note and keep `add_order_note`.
|
|
|
+
|
|
|
+### 3.8 Bulk action result UX when zero orders are eligible
|
|
|
+
|
|
|
+`class-bulk-actions-manager.php:52-78` — if every selected order is in a terminal state, `set_orders_to_prepare_printing` returns `0` and emits a "0 orders moved" notice. The DB manager additionally emits an "X orders skipped" warning. The "0 orders moved" notice is technically truthful but uninformative noise on top of the more useful "skipped" warning. Suppress the success notice when count is zero.
|
|
|
+
|
|
|
+### 3.9 PHP 8.4 deprecation of implicit `fputcsv` / `fgetcsv` escape character
|
|
|
+
|
|
|
+`class-bulk-actions-manager.php:117, 119` and `class-db-manager.php:232, 238` call `fputcsv` / `fgetcsv` with only the delimiter argument. From PHP 8.4 onward the implicit `escape='\\'` parameter is deprecated and emits warnings; passing an explicit empty string is the preferred forward-compatible form. The plugin currently requires PHP 7.2+ and does not pin a maximum, so installations on PHP 8.4 (released late 2024) will fill `debug.log` with deprecation warnings.
|
|
|
+
|
|
|
+### 3.10 `Order_Fields_Manager::save_custom_order_fields` lacks a capability check
|
|
|
+
|
|
|
+The handler hooks `woocommerce_process_shop_order_meta`, which WC fires after its own form-handling pipeline that does include capability validation. So this is not exploitable in normal flow. Still, the pattern most third-party plugins follow includes a `current_user_can('edit_shop_order', $order_id)` (or the legacy `edit_post`) inside the callback for defense in depth, and to handle programmatic invocation from custom code paths.
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+## 4. Low — cosmetic, style, longer-term
|
|
|
+
|
|
|
+### 4.1 `dedupe_by_order_no` is case-sensitive
|
|
|
+
|
|
|
+`class-db-manager.php:257-269` — keys are `trim((string) $row['order_no'])`. Order IDs in WooCommerce are numeric so this is irrelevant in practice, but if any deployment uses an order-number plugin that produces alphabetic prefixes (e.g. `ORD-12345`), `ord-12345` and `ORD-12345` would not dedupe. Normalising to a single case in the dedup key is cheap insurance.
|
|
|
+
|
|
|
+### 4.2 `run_import` action-name dispatch is brittle
|
|
|
+
|
|
|
+`class-import-manager.php:102-106` selects the DB handler by string equality on `$action`. Refactoring to a callable map keyed by action would prevent silent fall-through if a future protocol is added.
|
|
|
+
|
|
|
+### 4.3 `wp_kses_post` may be heavier than necessary for plain notices
|
|
|
+
|
|
|
+`utils-log.php:66` renders notice messages through `wp_kses_post()`. Notices are mostly plain text with the occasional `<code>` tag; `wp_kses_post` allows a broad set of post-content tags. If notice messages will only ever contain simple inline markup, a narrower allowlist via `wp_kses` would be safer (and faster). Low priority since current usage is already escaped at call sites.
|
|
|
+
|
|
|
+### 4.4 Notice transient TTL is 60 seconds
|
|
|
+
|
|
|
+`utils-log.php:8` — if the redirect that should display the notice is delayed (slow network, reverse-proxy hiccup, user closes the browser and re-opens), the notice is dropped. 5 minutes is a more typical TTL for "post-action feedback" patterns. Drop is silent — there is no way for the user to recover the message.
|
|
|
+
|
|
|
+### 4.5 Per-tab race in `render_notices`
|
|
|
+
|
|
|
+`utils-log.php:49-69` deletes the transient on first render. If a user has two admin tabs open and both load simultaneously after a bulk action, only one tab shows the notices. Not a correctness issue, but a slight UX surprise.
|
|
|
+
|
|
|
+### 4.6 `STUDIOU_WC_OPS_FILE` and `STUDIOU_WC_OPS_VERSION` are unused
|
|
|
+
|
|
|
+Already listed in §3.6. Repeated here only to note the cosmetic angle: define-time effort without a reader is noise.
|
|
|
+
|
|
|
+### 4.7 No `Network:` header in the plugin file
|
|
|
+
|
|
|
+Carried over from §4.6 of the previous review. Only relevant for multisite installs.
|
|
|
+
|
|
|
+### 4.8 No `uninstall.php` / cleanup hook
|
|
|
+
|
|
|
+Carried over from §3.11 of the previous review. Order meta keys remain in the database after plugin removal. Acceptable but undocumented.
|
|
|
+
|
|
|
+### 4.9 `register_post_status` is invoked even under HPOS
|
|
|
+
|
|
|
+`Order_Status_Manager::register_custom_order_statuses()` registers the statuses via `register_post_status`. Under HPOS this is harmless and still needed for WC admin chrome that calls `get_post_statuses()`, but the contract is slightly mixed-paradigm. No action required.
|
|
|
+
|
|
|
+### 4.10 `update_status` calls do not pass `manual = true`
|
|
|
+
|
|
|
+`bulk_set_print_status` and the import handlers call `$order->update_status($slug)` without the optional `$manual` parameter. WC distinguishes manual (user-driven) from automated transitions in its hook payloads. Currently these transitions appear as "automated" in WC reports — which is arguably correct (the bulk action *is* automated) but is a UX nuance worth conscious choice.
|
|
|
+
|
|
|
+### 4.11 WooCommerce 10.7.0 block-based order edit screen
|
|
|
+
|
|
|
+WC 10.x continues to ship the block-based / React order edit screen alongside the classic one (feature-flagged per install). `Order_Fields_Manager` hooks `woocommerce_admin_order_data_after_order_details` and `woocommerce_process_shop_order_meta`, both of which target the **classic** order edit screen. Under the block-based screen the Print Information panel does not render and edits are not saved through that pipeline. Currently this is acceptable (most production installs of WC 10.7.0 still default to the classic screen), but if Studiou opts into the block-based edit experience, a parallel data-source / slot-fill integration will be needed.
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+## 5. Verification gaps (against WC 10.7.0 / WP 6.9.4)
|
|
|
+
|
|
|
+The following claims in v1.4.0 rest on assumptions that could not be confirmed without an interactive test on the target stack:
|
|
|
+
|
|
|
+1. **HPOS search filter** — see §2.1. Needs confirmation that `woocommerce_order_table_search_query_meta_keys` is still emitted by WC 10.7.0.
|
|
|
+2. **MySQL functional-dependency recognition** for the new `GROUP BY` — see §1.1. Production MySQL/MariaDB version and `sql_mode` should be checked; WP 6.9.4 itself imposes no constraint here, the risk is the database server config.
|
|
|
+3. **`woocommerce_order_table_search_query_meta_keys` payload shape** — the callback assumes WC passes an indexed array of meta-key strings. If WC 10.7.0 has changed it to an associative array (key → label) or a different shape, `in_array($meta, $keys, true)` would silently no-op.
|
|
|
+4. **Transient delivery timing under WC 10.7.0** — `UtilsLog::message()` queues notices via a transient keyed by user ID. If a request is served while WC is invalidating user-scoped object caches (e.g. session bootstrap), the notice could be lost between queue and render. Difficult to reproduce; worth a smoke test.
|
|
|
+5. **Header ordering on the export response** — `nocache_headers()` precedes our `Content-Type: text/csv`. WC may emit its own headers earlier in the request via WP filters; the resulting Content-Type and Cache-Control order should be inspected with a browser inspector or `curl -I` on a WC 10.7.0 install.
|
|
|
+6. **Block-based order edit screen** — §4.11. WC 10.7.0 ships both classic and block-based order edit screens. The Print Information panel is wired to the classic screen only; behaviour on the block-based screen has not been verified.
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+## 6. Status of 1.3.2 findings
|
|
|
+
|
|
|
+| 1.3.2 finding | Status in 1.4.0 |
|
|
|
+|---------------|------------------|
|
|
|
+| §1.1 Search by External Order Number is broken | Resolved in code, **pending verification** (see §2.1 of this review). |
|
|
|
+| §1.2 `handle_status_transitions` writes to wrong table under HPOS | Resolved (`$order->update_meta_data() + save()`). |
|
|
|
+| §2.1 No CSV row deduplication | Resolved (`Studiou_DB_Manager::dedupe_by_order_no`). |
|
|
|
+| §2.2 CSV header casing drift | Resolved (`normalise_header`). |
|
|
|
+| §2.3 Bulk export non-atomic | Resolved (CSV built before status flip). |
|
|
|
+| §2.4 Multi-category row multiplication | Resolved via `GROUP BY` — **new risk introduced**, see §1.1. |
|
|
|
+| §2.5 CSV UTF-8 BOM | Resolved on export — **new asymmetry with import**, see §1.2. |
|
|
|
+| §2.6 Payment-complete demotion | Resolved. |
|
|
|
+| §2.7 Import file validation | Resolved (`validated_upload`). |
|
|
|
+| §2.8 Bulk handlers cap check | Resolved. |
|
|
|
+| §3.1 `UtilsLog::message()` dead code | Resolved (transient + `admin_notices`). |
|
|
|
+| §3.2 `WP_DEBUG` guard too strict | Resolved. |
|
|
|
+| §3.3 Asymmetric logging | Resolved (shared `bulk_set_print_status`). |
|
|
|
+| §3.4 No state-machine guard | Resolved (terminal-status skip). |
|
|
|
+| §3.5 Plugin-init order risk | Resolved (priority pinning). |
|
|
|
+| §3.6 SQL prepare via intval | Resolved (`$wpdb->prepare()` + `%d`). |
|
|
|
+| §3.7 Init-time logging volume | Resolved (init logs removed). |
|
|
|
+| §3.8 `parse_csv` line-length cap | Resolved (`fgetcsv(... 0 ...)`). |
|
|
|
+| §3.9 `external_ref_ord_date` validation | Partially resolved — see §3.5 of this review. |
|
|
|
+| §3.10 `external_ref_ord_date` UI exposure | Resolved. |
|
|
|
+| §3.11 No uninstall cleanup | Not addressed (conscious carry-over). |
|
|
|
+| §3.12 No version constant | Resolved (`STUDIOU_WC_OPS_VERSION`), albeit unused. |
|
|
|
+| §3.13 HPOS column callback parameter naming | Resolved (`$order_or_id`). |
|
|
|
+| §4.1 Translations incomplete | Resolved (`docs/translations.txt` extended). The `.mo` file still needs a rebuild. |
|
|
|
+| §4.2 README/CLAUDE.md overclaim HPOS | Resolved in wording — but contingent on §2.1 verification. |
|
|
|
+| §4.3 CLAUDE.md SQL prepare claim | Resolved. |
|
|
|
+| §4.4 `// visualized` comments | Resolved. |
|
|
|
+| §4.5 Bracket formatting | Resolved as a side-effect of rewriting the class. |
|
|
|
+| §4.6 No `Network:` header | Not addressed (carried over). |
|
|
|
+| §4.7 README architecture note | Cosmetic — left as-is. |
|
|
|
+
|
|
|
+Net: 26 of 30 findings closed in code; 3 carried over; 1 (search) closed but flagged as verification-pending.
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+## 7. Risk-prioritised summary
|
|
|
+
|
|
|
+Ordered by expected user-visible impact:
|
|
|
+
|
|
|
+1. **§1.1 Export SQL `GROUP BY` may fail on default MySQL strict mode.** This is the most likely cause of a fresh production incident on 1.4.0.
|
|
|
+2. **§1.2 BOM-in-imported-CSV breakage.** Subtle, easy to reproduce by exporting then re-importing.
|
|
|
+3. **§2.1 HPOS search filter unverified.** If wrong, the headline 1.4.0 fix is incomplete.
|
|
|
+4. **§2.2 Server-vs-WP timezone drift in Print Information panel.** Will produce off-by-one-hour confusion in CZ timezone with DST.
|
|
|
+5. **§3.3 `output_csv` fallback echoes raw CSV into HTML.** Edge case but visible when it happens.
|
|
|
+6. **§3.5 `normalise_csv_date` silently keeps bogus dates.** Operator-visible inconsistency.
|
|
|
+7. **§3.1 `GROUP_CONCAT` truncation.** Unlikely but silent.
|
|
|
+8. **§3.9 PHP 8.4 deprecation noise.** Cosmetic but log-spammy.
|
|
|
+9. **§3.7 Two order notes per payment-complete event.** Cosmetic noise in order history.
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+## 8. Out of scope for this review
|
|
|
+
|
|
|
+- Live behaviour testing against a running WordPress 6.9.4 / WooCommerce 10.7.0 install — every finding above is derived from source reading.
|
|
|
+- Performance profiling of the rewritten export query (`GROUP BY` plus four `LEFT JOIN`s) under large order volumes.
|
|
|
+- Behaviour of the WP admin notices renderer when WC's own admin notices are also queued in the same request.
|
|
|
+- Behavioural verification on the WooCommerce 10.7.0 block-based order edit screen (covered analytically in §4.11).
|
|
|
+- `.po` / `.mo` correctness — translations were not exercised; this review covers only their source-side coverage in `docs/translations.txt`.
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+*End of review.*
|
