Skip to content

fix(woocommerce): stop dropping Tutor LMS course orders from sales notifications#137

Open
sadmansakibnadvi wants to merge 1 commit into
WPDevelopers:masterfrom
sadmansakibnadvi:fix/woo-sales-tutor-course-orders-dropped
Open

fix(woocommerce): stop dropping Tutor LMS course orders from sales notifications#137
sadmansakibnadvi wants to merge 1 commit into
WPDevelopers:masterfrom
sadmansakibnadvi:fix/woo-sales-tutor-course-orders-dropped

Conversation

@sadmansakibnadvi

Copy link
Copy Markdown

Summary

WooCommerce Sales notifications silently discard every order whose product is linked to a Tutor LMS course. On stores that sell courses through WooCommerce, no course orders ever appear and newly placed orders are never imported — the feature is effectively non-functional. A secondary defect stores the discarded orders as corrupt data => false entries, producing repeated PHP 8 Trying to access array offset on false warnings on the frontend.

Reproducible regardless of HPOS vs. legacy order storage. Observed on NotificationX 3.2.10 / Pro 3.1.4.

Root cause

Defect 1 — hardcoded course exclusion (primary)

Course-linked items were rejected unconditionally in four places, using two different detectors:

Site Detector Action
ordered_product() product_belongs_with_course() return false
status_transition() product_belongs_with_course() continue
manual_order() _tutor_product post meta continue
get_orders() _tutor_product post meta continue

Both the live import path (status_transition()) and the bulk rebuild path (get_orders()) applied a filter, which is why new orders were never imported and regeneration didn't help. There was no setting or filter guarding this behavior.

Defect 2 — invalid entries stored, not skipped (secondary)

get_orders() pushed ordered_product()'s return value unconditionally. Because get_orders() pre-filters by _tutor_product meta while ordered_product() rejects by product_belongs_with_course(), any product that belongs to a course without _tutor_product meta slipped past the get_orders() guard, returned false from ordered_product(), and that false was persisted by get_notification_ready() as 'data' => false. On the frontend, check_order_status() then evaluated false['status'] → PHP 8 warning and a hidden entry.

What changed (single file: includes/Extensions/WooCommerce/WooCommerce.php)

Fix A — gate the exclusion behind an off-by-default filter, at all four sites.
Introduced one helper, is_excluded_course_item(), that unifies both Tutor detectors and is gated behind a new filter:

add_filter( 'nx_woocommerce_exclude_tutor_course_products', '__return_true' ); // opt back into the old behavior

Default (false) imports course orders as expected. All four call sites now route through this helper, so free and Pro behave consistently (Pro's WooCommerce extends this class and overrides none of these methods — verified — so no Pro change is needed).

Fix B — never store invalid entries; harden the reader.

  • get_orders() now only stores non-empty array entries (skips every false branch of ordered_product(), not just the course one).
  • check_order_status() guards against a missing/non-array data before reading ['status'], eliminating the line-461 warning for all false-return branches.

Backward compatibility

  • Behavioral change is intentional and matches user expectation: course orders now appear. Stores that genuinely want them hidden can restore the old behavior with the new filter above.
  • No schema/DB migration.

Verification

  • php -l clean on the changed file.
  • The two changed method bodies were extracted verbatim and driven under PHP 8.2 with a strict error handler (any notice/warning = failure). All 10 checks pass, including: course product imported at the default filter; opt-in exclusion works via both detectors; and data => false returns false with no array offset on false warning.

Note: the repo's PHPUnit suite is a full WordPress integration suite (bin/install-wp-tests.sh + WooCommerce + Tutor LMS) and could not be exercised in the authoring sandbox. Suggested regression tests to add there are listed below.

Post-deploy migration (runtime, not code)

Existing datasets already contain corrupt data => false entries. After deploying: regenerate each affected WooCommerce/Sales notification (rebuilds from live orders), then clear page/server cache. No DB migration required.

Suggested regression tests (WP integration suite)

  • Order with a course-linked product on a Sales notification → entry imported (filter default false).
  • With nx_woocommerce_exclude_tutor_course_productstrue → course entry excluded.
  • Order with a deleted/invalid product → skipped, not stored as false, no PHP warning on render.
  • Mixed order (course + non-course) → non-course item still imports.
  • Newly placed order via status_transition() (Processing/Completed) → imported consistently with regeneration.

🤖 Generated with Claude Code

…tifications

WooCommerce Sales notifications silently discarded every order whose product
is linked to a Tutor LMS course, so on course-selling stores no sales
notifications appeared and newly placed orders were never imported. A second
defect stored the discarded orders as corrupt `data => false` entries, which
produced repeated PHP 8 "array offset on false" warnings on the frontend.

Defect 1 - hardcoded course exclusion (primary)
Course-linked items were rejected unconditionally in four places, using two
different detectors: product_belongs_with_course() in ordered_product() and
status_transition(), and the _tutor_product post meta in manual_order() and
get_orders(). All four now route through a single is_excluded_course_item()
helper gated behind a new off-by-default filter,
`nx_woocommerce_exclude_tutor_course_products`. Default behaviour imports
course orders; the old behaviour is available via:

    add_filter('nx_woocommerce_exclude_tutor_course_products', '__return_true');

Because both detectors are unified in the helper, the free and Pro plugins
(Pro's WooCommerce extends this class and overrides none of these methods)
behave consistently.

Defect 2 - invalid entries stored, not skipped (secondary)
get_orders() pushed ordered_product()'s return value unconditionally, so its
false branches were persisted as `data => false` by get_notification_ready().
get_orders() now skips non-array entries, and check_order_status() guards
against a missing/non-array `data` before reading `['status']`, eliminating
the line-461 warning for every false-return branch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant