revise-1.3.2.md 20 KB

Plugin Revision — v1.3.2 (Analytical Review)

Scope: Read-only analytical pass over the entire codebase of studiou-wc-ord-print-statuses at version 1.3.2. No code changes were made. This document catalogs gaps, inconsistencies, and risks discovered by reviewing each manager class against the plugin header, README, CLAUDE.md, and the original feature specification in docs/instructions.txt.

Reviewer date: 2026-05-12 Files reviewed:

  • studiou-wc-ord-print-statuses.php
  • includes/class-db-manager.php
  • includes/class-order-status-manager.php
  • includes/class-order-fields-manager.php
  • includes/class-order-search-manager.php
  • includes/class-bulk-actions-manager.php
  • includes/class-custom-columns-manager.php
  • includes/class-import-manager.php
  • includes/utils-log.php
  • README.md, CLAUDE.md, docs/instructions.txt, docs/translations.txt

Findings are grouped by severity. The severity reflects user-visible or correctness impact, not implementation effort to fix.


1. Critical — features that are advertised but do not work

1.1 Search by External Order Number is broken (three independent defects)

The feature is documented in README §5 and CLAUDE.md §"Order_Search_Manager", and matches spec item 3 in docs/instructions.txt. The current implementation cannot work for the following reasons:

Defect A — Meta key mismatch (would fail even under legacy CPT)

  • The field is saved as external_ref_ord_no (no leading underscore):
    • class-order-fields-manager.php:58$order->update_meta_data('external_ref_ord_no', …)
    • class-db-manager.php:184$order->update_meta_data('external_ref_ord_no', $row['externalorder'])
  • The field is searched as _external_ref_ord_no (with leading underscore):
    • class-order-search-manager.php:18
    • class-order-search-manager.php:53 ('key' => '_external_ref_ord_no')

Even if every other hook below were correct, the meta_query LIKE match would always return zero rows.

Defect B — Wrong hook for HPOS environment

class-order-search-manager.php:8 hooks parse_query and gates with:

if ('edit.php' !== $pagenow || 'shop_order' !== $typenow || !$query->is_main_query())
    return $query;

Under HPOS, the orders screen is served from admin.php?page=wc-orders (not edit.php) and $typenow is not 'shop_order'. The guard always returns early; meta_query is never injected. The plugin declares HPOS compatibility, so this code path is dead in the supported configuration.

Defect C — Search field is mounted as a column renderer, not a toolbar field

  • class-order-search-manager.php:6 hooks add_external_ref_ord_no_to_search to manage_woocommerce_page_wc-orders_columns — that filter expects the orders-list columns array, not a "searchable fields" array. The callback appends _external_ref_ord_no as if it were a column key, creating a phantom column.
  • class-order-search-manager.php:7 hooks add_external_ref_ord_no_search_field to manage_woocommerce_page_wc-orders_custom_column — that hook fires per table cell. The callback renders an <input type="text">, which (if the typenow guard ever passed) would draw a text input inside every row's cell rather than beside the "Search Order" button as the spec requires.

Net effect: The search box does not render in the expected place, the search hook does not fire under HPOS, and even on a legacy CPT setup the meta key mismatch would zero out results. This feature is non-functional.

1.2 Order_Status_Manager::handle_status_transitions() writes to the wrong table under HPOS

class-order-status-manager.php:59-62:

update_post_meta($order_id, 'to_print_date', current_time('mysql'));
…
update_post_meta($order_id, 'in_print_date', current_time('mysql'));

update_post_meta() writes to wp_postmeta. Under HPOS, WooCommerce reads order meta from wp_wc_orders_meta. The result:

  • Whenever a status transitions to to-print / in-print outside of the bulk-action or import paths (e.g., manual change from the order edit screen, programmatic transitions from third-party code), the timestamp is written only to wp_postmeta and is invisible to $order->get_meta(). The custom column and the "Print Information" panel will show empty values.
  • The bulk and import paths use $order->update_meta_data() + $order->save() (correct under both storage modes), so they happen to work — masking the bug for the common user flow.

This is a latent HPOS gap directly contradicting CLAUDE.md §"Important Considerations" point 1.


2. High — correctness, HPOS gaps, spec divergence

2.1 Original spec requires CSV row deduplication; implementation does none

docs/instructions.txt lines 28 and 35 both state: "Distinct values from import file by column 'Order No'".

Studiou_DB_Manager::import_inprint_protocol() and import_delivered_protocol() iterate every row and call wc_get_order() + update_status() + save() per occurrence. If the same order_no appears twice in a CSV (a realistic provider-side glitch), the order is mutated twice, the success counter increments twice, and the user gets a misleading "X orders updated" total.

2.2 CSV header casing — spec vs. implementation discrepancy

Spec (docs/instructions.txt lines 27, 33) specifies headers "Order No", "ExternalOrder", "ExternalOrderDate". The code reads $row['order_no'], $row['externalorder'], $row['externalorderdate'] (lowercased, no spaces). README documents the lowercase form, matching the code.

This is a documentation/spec drift rather than a code bug, but it means CSV files prepared from the original instructions will silently fail header lookup (!isset(...) branch returns the generic "Invalid row format in CSV.").

2.3 Bulk export status change is non-atomic with the export

class-bulk-actions-manager.php:34-50:

$orders_data = Studiou_DB_Manager::get_orders_for_printing($post_ids);
$this->set_status_to_prepare_to_printing($post_ids, $redirect_to); // ← changes status first
…
$this->output_csv($csv_data, 'prepare_to_printing_export.csv');     // ← then output (exits)

If the CSV stream fails mid-output (proxy timeout, client disconnect), orders are already marked to-print. The user has no recovery signal and the file may be incomplete. If output_csv were ever to throw before exit, the bulk-action redirect would still claim success.

Additionally, the $redirect_to returned from the inner set_status_to_prepare_to_printing() call is discarded — the side effect of adding bulk_action=marked_prepare_to_printing to the query is wasted.

2.4 Export SQL multiplies rows for products in multiple categories

Studiou_DB_Manager::get_orders_for_printing() joins term_relationships/term_taxonomy/terms unconstrained. A product mapped to N product_cat terms produces N rows in the CSV for the same order line. If the print provider expects one row per line item, the file will contain duplicate quantities. Whether this is intentional is unclear from the spec; the spec says "custom sql 'SELECT * FROM vsu_orders_car_prod_qty_img'" without defining cardinality.

2.5 CSV export has no UTF-8 charset signal and no Excel BOM

Bulk_Actions_Manager::output_csv() (lines 84-89) sends:

Content-Type: text/csv
Content-Disposition: attachment; filename="..."

No charset=utf-8, no UTF-8 BOM. Czech product/category names containing diacritics will render as mojibake when opened in Excel on Windows (the most likely consumer for a print-shop export). Field separator is comma; Excel in Czech locale prefers semicolon.

2.6 set_order_processing_status() can demote print-state orders

class-order-status-manager.php:65-74 forces any payment-complete order into processing unless it is already completed. If a payment settles late on an order already moved to to-print or in-print (e.g., a delayed bank transfer), the order is silently rolled back to processing and the to_print_date / in_print_date timestamps remain, leaving the order in an inconsistent state vs. its meta.

The guard should exclude all print statuses, not just completed.

2.7 Import handlers reject malformed rows but do not validate file structure

Studiou_DB_Manager::parse_csv():

  • Returns an empty array (not false) if the file is missing or empty, but the callers check if (!$csv_data) — empty array is falsy in PHP, so the "Error parsing CSV file." path is reached. OK in practice, but reliance on falsy-vs-false is fragile.
  • Reads each row with fgetcsv($handle, 1000, ',') — the 1000-byte line cap will truncate any row longer than 1 KB. Long product names or notes in delivered protocols could be silently chopped.
  • Does not check $_FILES[...]['error'] (handles UPLOAD_ERR_INI_SIZE, etc.).
  • Does not verify the MIME type server-side; the accept=".csv" attribute is client-side only.
  • Does not verify the first line is a header (if a CSV without headers is uploaded, array_combine may emit a warning and produce nonsense rows).

2.8 Bulk handlers lack a capability check

Bulk_Actions_Manager::handle_bulk_actions() trusts WordPress's per-screen capability enforcement. WP does check edit_shop_orders to display the bulk action menu, but defense in depth would call current_user_can('edit_shop_orders') (or manage_woocommerce) inside the handler. Compare with Import_Manager, which does call current_user_can('manage_woocommerce') — the inconsistency itself is worth noting.


3. Medium — robustness, maintainability, internal consistency

3.1 UtilsLog::message() is dead code

includes/utils-log.php:15-27 — the entire body is commented out. The method is called in at least three places (class-db-manager.php:128, 153; class-bulk-actions-manager.php:42). The user-facing notice intent (type: info, dismissible: true) is never delivered. Bulk operations therefore have no visible feedback in the admin UI for non-debug users.

3.2 UtilsLog::log() guard is overly strict

if ( true === WP_DEBUG ) requires WP_DEBUG to be the boolean literal true. If the user defines define('WP_DEBUG', 1) (common in legacy wp-config.php examples), nothing logs. The conventional guard is if ( defined('WP_DEBUG') && WP_DEBUG ).

3.3 set_orders_to_prepare_printing logs per iteration; set_orders_to_in_printing does not

class-db-manager.php:120-131 logs entry, success, and not-found for each order. class-db-manager.php:143-157 logs neither entry, success, nor not-found. Symmetry would aid debugging.

3.4 No transaction or state-machine guard on status transitions

Studiou_DB_Manager::set_orders_to_*() calls update_status() regardless of current state. An order in cancelled or refunded would be reactivated into to-print — almost certainly unintended. The bulk action UI itself does not filter eligible orders.

3.5 Plugin-init order risk for translations

studiou-wc-ord-print-statuses.php:101-107 registers two plugins_loaded callbacks. WordPress runs them in registration order. Order_Status_Manager::__construct() calls _x() / _n_noop() inside the constructor (line 9-15). If Studiou_WC_Ord_Print_Statuses::initPlugin registers before studiou_wc_ord_print_statuses_load_textdomain, those translation calls fire before the text domain is loaded, and untranslated strings are cached. Order is currently: initPlugin registered after load_textdomain, so OK today, but the dependency is implicit and fragile.

3.6 SQL injection defense relies on intval(), not $wpdb->prepare()

class-db-manager.php:93:

WHERE `orders`.`id` IN (" . implode(',', array_map('intval', $order_ids)) . ")

Using intval() on each ID is functionally safe — it coerces to integer and emits zero on garbage input. However, CLAUDE.md §"Important Considerations" point 4 misleadingly claims this "uses $wpdb->prepare() implicitly through intval()" — that is not accurate; $wpdb->prepare() is never called. The documentation should match what the code actually does.

3.7 Logging volume on every request

Every page-load triggers UtilsLog::log calls in init(), load_dependencies(), initialize_managers(), register_custom_order_statuses(), register_bulk_actions(), etc. With WP_DEBUG_LOG enabled, the log fills with init noise. Useful logs (status transitions, SQL errors) get drowned. Consider gating init logs behind a higher verbosity flag.

3.8 parse_csv line length

fgetcsv($handle, 1000, ',') — the second parameter is the max line length in bytes. 1000 bytes is short for a row that contains a long order note or product title plus diacritics. Passing 0 removes the cap. Worth raising explicitly.

3.9 Imported external_ref_ord_date is stored verbatim

class-db-manager.php:185 stores $row['externalorderdate'] without any date parsing or validation. If the provider sends 31/12/2025 while WP expects ISO format elsewhere, downstream filters and the order-detail panel may render an empty value. The "Print Information" panel does not display this field at all — it is only readable via direct meta access.

3.10 external_ref_ord_date is never exposed in the UI

The field is written by import_inprint_protocol() but no manager renders it. Neither Order_Fields_Manager nor Custom_Columns_Manager references it. If the print-shop's order date matters operationally, it is invisible to the operator.

3.11 No uninstall / deactivation cleanup

There is no uninstall.php, no register_deactivation_hook, no register_uninstall_hook. Custom post statuses are registered every init, so they vanish when the plugin is deactivated — but order meta keys (to_print_date, external_ref_ord_no, etc.) remain in the DB indefinitely. For long-term hygiene on test sites, consider documenting that removal requires a manual SQL cleanup.

3.12 No version constant

The plugin version lives only in the header comment (Version: 1.3.2). To reference it in code (e.g., for asset cache-busting or upgrade routines) one must call get_file_data(). Defining define('STUDIOU_WC_OPS_VERSION', '1.3.2'); once in the bootstrap would be a minor consistency improvement and matches the pattern used in sibling plugins per CLAUDE.md.

3.13 HPOS column callback parameter naming

Custom_Columns_Manager::add_custom_shop_order_column_content($column, $post_id) — under HPOS the second argument is actually a WC_Order object, not a post ID. wc_get_order() accepts both, so it works, but the parameter name is misleading and shadows the legacy CPT contract. Renaming to $order_or_id or capturing both branches explicitly would aid readers.


4. Low — documentation, cosmetic, style

4.1 Translation reference (docs/translations.txt) is incomplete

Strings present in code but absent from the EN→CS reference:

  • "Order automatically set to processing by Studiou WC Order Print Statuses plugin." (class-order-status-manager.php:72)
  • "Payment received, order is now processing." (class-order-status-manager.php:70)
  • "Search by External Order Number" (class-order-search-manager.php:30)
  • "You do not have sufficient permissions to access this page." (class-import-manager.php:48, 79)
  • "Error parsing CSV file." (class-import-manager.php:61, 93)
  • "External Order Delivered" field label (class-order-fields-manager.php:38)
  • "In Print Date" field label (class-order-fields-manager.php:25)

These strings would compile into the .pot via wp i18n make-pot but were never reviewed by the translator.

4.2 README/CLAUDE.md claim full HPOS compatibility

Both documents advertise HPOS compatibility without qualification. Given finding §1.1 (search broken under HPOS) and §1.2 (status-transition stamps go to the wrong table), the claim is overstated until those are fixed.

4.3 CLAUDE.md is the only place where the SQL-prepare claim appears

Mentioned in §3.6 — CLAUDE.md should be corrected regardless of whether the code is changed, because it currently misrepresents the security posture.

4.4 Inconsistent comment language

Some class files open with // visualized (in class-bulk-actions-manager.php, class-import-manager.php, class-order-fields-manager.php, class-custom-columns-manager.php) — apparently a leftover marker from an earlier review pass. It conveys nothing to a fresh reader and should be removed or replaced with a proper docblock.

4.5 Order_Status_Manager::add_custom_order_statuses_to_list formatting

class-order-status-manager.php:48-49:

            }        }

Two } on the same line, asymmetric indentation. Cosmetic only.

4.6 No Network: declaration in plugin header

Minor — only relevant for multisite installs.

4.7 README architecture table omits Studiou_DB_Manager and UtilsLog from the manager pattern explanation

Already listed in the table but worth noting that DB Manager is a different category (static service) from the other "manager" classes (hook-registering instances). The README treats them as peers.


5. Spec compliance matrix (against docs/instructions.txt)

Spec item Requirement Status Notes
1 Custom statuses wc-to-print, wc-in-print after wc-processing ✅ Implemented Inserted correctly via wc_order_statuses filter.
2 Add fields to-print-date, in-print-date, external-ref-ord-no, external-ref-ord-delivered ⚠️ Partial Stored with underscore keys (acceptable). external_ref_ord_date exists in code but is not in the spec and not surfaced in UI.
3 Search orders by external-ref-ord-no near Search Order button ❌ Broken See §1.1 — three independent defects.
4 Three bulk actions ✅ Implemented Behaviour matches; concerns about non-atomic export (§2.3) and category multiplication (§2.4).
5 Three columns appended to orders grid ✅ Implemented Inserted after order_total rather than strictly "at the end", but visible and functional.
6 Import InPrint protocol with deduplication ⚠️ Partial Import works; deduplication not implemented (§2.1). CSV headers diverge from spec casing (§2.2).
7 Import Delivered protocol with deduplication ⚠️ Partial Same as item 6.

6. Risk-prioritised summary

Issues most likely to cause a user-reported bug, ordered by expected impact:

  1. §1.1 Search by External Order Number does not work (advertised feature, three defects).
  2. §1.2 Status-transition timestamps lost under HPOS for non-bulk transitions (silent data loss).
  3. §2.5 CSV export mojibake / Excel locale mismatch (operator-facing, daily friction).
  4. §2.1 Import dedup missing (silently double-applies status changes).
  5. §2.6 Payment-complete demotion of print-state orders (workflow regression).
  6. §2.3 Bulk export is non-atomic (rare, but unrecoverable when it triggers).
  7. §3.1 UtilsLog::message() is dead (no user feedback after bulk actions).
  8. §3.4 No status-machine guards (cancelled/refunded orders can be re-activated by bulk action).
  9. §2.2 CSV header casing drift vs. original spec (provider-side surprise).
  10. §3.6 + §4.3 CLAUDE.md misrepresents SQL safety (documentation, not code).

7. Out of scope for this review

  • Performance profiling under large order volumes (the export query is unbounded).
  • Browser/JS interaction — there is no front-end code in this plugin.
  • Cross-plugin compatibility with studiou-wc-product-cat-manage or studiou-wc-free-photo-product (mentioned as siblings in recent git history).
  • Functional testing in a live WP/WC install. All findings above are derived from source reading.

End of review.