Add dependency guards for PMPro and MemberPress option data#6
Add dependency guards for PMPro and MemberPress option data#6dparker1005 wants to merge 2 commits into
Conversation
- Bail out of the admin menu registration and all Action Scheduler callbacks when Paid Memberships Pro is not active. Previously, deactivating PMPro while migration tasks were still queued caused fatal errors in the Action Scheduler runner. - Guard array access to the 'integrations' and 'custom_fields' keys of the mepr_options option, which may be missing if MemberPress options were partially cleaned up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
flintfromthebasement
left a comment
There was a problem hiding this comment.
PR: #6 — Add dependency guards for PMPro and MemberPress option data
dparker1005 → dev | 3 files, +29 -2
#6
Summary
The guards are correctly placed and solve the stated fatal — ! defined( 'PMPRO_VERSION' ) is the right idiom, and the isset() && is_array() pattern for the mepr_options keys is solid. One issue worth addressing before merge: the Action Scheduler callbacks silently return on bail, which causes AS to mark jobs as "complete" even though nothing actually ran.
Issues
-
Major
pmpro-memberpress-migration-toolkit.php:133–137, 249–253, 277–281, 99–103— All four AS callbacks bail with a plainreturn;when PMPro is inactive. Action Scheduler treats a callback that returns without throwing as successfully completed. If PMPro is deactivated mid-migration (plugin conflict, update, accidental deactivation), every queued job drains silently — the AS "complete" list looks clean, but no users or content restrictions were actually migrated. The operator has no signal that anything went wrong and would need to re-run the migration from scratch.Fix: throw instead of return so AS marks the job as failed and it's visible in the failed queue:
if ( ! defined( 'PMPRO_VERSION' ) ) { throw new \Exception( 'PMPro inactive — migration task skipped.' ); }
Or at minimum add an
error_log()before the return so there's a trace. Throwing is better — it makes the failure visible in the AS admin UI and preserves retry semantics. -
Minor
pmpro-memberpress-migration-toolkit.php:277— Inpmprompmt_migrate_content_restriction, the bail guard is placed after theglobaldeclaration, while the other three functions put the guard before any globals. Cosmetic inconsistency but breaks the visual pattern. Move it above thegloballine to match.
Looks Good
class-pmprompmt-migration-step-users.php:107 and class-pmprompmt-migration-step-other.php:260 — the isset() && is_array() ternary with an array() fallback is the right pattern here; no PHP 8 deprecation exposure from feeding null to foreach.
The guard in pmprompmt_menu() is sufficient to protect pmprompmt_page() — WordPress won't register the page callback if the menu registration bails, so direct URL access gets a clean 403. No need to duplicate the guard there.
# Conflicts: # pmpro-memberpress-migration-toolkit.php
Problem
Nothing checked that Paid Memberships Pro is active:
pmprompmt_migrate_user,pmprompmt_queue_user_migrations, etc.) instantiateMemberOrderand callPMPro_Action_Scheduler::instance()unconditionally. Deactivating PMPro while migration tasks are still queued causes fatals in the queue runner.mepr_options['integrations']andmepr_options['custom_fields']exist — undefined-key warnings /foreachon null when MemberPress options are partial or already cleaned up.Fix
! defined( 'PMPRO_VERSION' )bail in the menu registration and all four Action Scheduler callbacks.isset()+is_array()guards around the twomepr_optionsarray accesses.cc @flintfromthebasement
🤖 Generated with Claude Code