Add plugin uninstall cleanup: remove tables, cron hooks, and options#48
Open
MathieuLamiot wants to merge 6 commits intodevelopfrom
Open
Add plugin uninstall cleanup: remove tables, cron hooks, and options#48MathieuLamiot wants to merge 6 commits intodevelopfrom
MathieuLamiot wants to merge 6 commits intodevelopfrom
Conversation
- Add `uninstall.php` entry point that runs `Uninstaller::run()` on plugin deletion - Add `Uninstaller` class with focused methods: `drop_tables()`, `clear_cron_hooks()`, `delete_options()` - Refactor `DatabaseManager::get_table_names()` to static (no-instantiation, no side effects) - Add `Sybgo::get_cron_hooks()` static method; refactor `init_cron_schedules()` and `deactivate()` to use it - Add `Settings_Page::LEGACY_OPTION_EMAIL_RECIPIENTS` constant and `get_option_names()` static method - Add `UninstallerTest` covering drop_tables, clear_cron_hooks, delete_options, and run() - Stub `register_activation_hook`, `register_deactivation_hook`, `add_action` in unit test bootstrap Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Uninstaller Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…o instance - Constructor now only initializes table names (no side effects) - New maybe_create_tables() method handles table creation + migration - get_table_names() and drop_table() converted from static to instance methods - Factory::create_database_manager() calls maybe_create_tables() explicitly - Uninstaller uses a plain new DatabaseManager() instance (no table creation) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes: #47
When a user deletes the Sybgo plugin from the WordPress admin, all plugin-created data is now removed: the four database tables, the four scheduled cron hooks, and the two WordPress options. This follows the WordPress plugin uninstall guidelines.
Type of change
Detailed scenario
What was tested
Scenario 1 — Full uninstall on a live WordPress site
wp_sybgo_events,wp_sybgo_reports,wp_sybgo_email_log,wp_sybgo_aggregated_events) via phpMyAdmin.sybgo_settings,sybgo_email_recipients) viawp option get sybgo_settings.wp cron event list | grep sybgo.wp option get sybgo_settingsreturned "Error: Could not get 'sybgo_settings' option."wp cron event list | grep sybgoreturned no results.Scenario 2 — Deactivation does not remove data
How to test
Affected Features & Quality Assurance Scope
sybgo_settings,sybgo_email_recipients)Technical description
Documentation
Updated: wp-plugin/docs/development.md — new "Plugin Uninstall" section explains the three-step cleanup and the single-source-of-truth pattern.
Implementation details
uninstall.phpis the WordPress-recommended approach (vsregister_uninstall_hook) because it runs in a clean context without loading the full plugin, avoiding the singleton constructor, DB setup, and action registrations.Sybgo\Admin\Uninstaller::run()delegates to three focused methods:drop_tables()— callsDatabaseManager::get_table_names()(static, no side effects, no table creation) and issuesDROP TABLE IF EXISTSfor each.clear_cron_hooks()— callsSybgo::get_cron_hooks()(new static method) and passes each towp_clear_scheduled_hook().delete_options()— callsSettings_Page::get_option_names()(new static method returningOPTION_NAMEandLEGACY_OPTION_EMAIL_RECIPIENTS) and passes each todelete_option().To avoid identifier duplication, three refactors were made:
DatabaseManager::get_table_names()converted tostatic— the constructor now uses it instead of repeating the prefix+name strings. TheFactorycall sites ($db_manager->get_table_names()) continue to work since PHP allows calling static methods on instances.Sybgo::get_cron_hooks()added aspublic static;init_cron_schedules()anddeactivate()refactored to call it.Settings_Page::LEGACY_OPTION_EMAIL_RECIPIENTSconstant added;activate()refactored to use it.Unit tests in
UninstallerTestcover all three methods andrun(), using Brain\Monkey + Mockery. TheMockeryPHPUnitIntegrationtrait is used to ensure Mockery expectations count as PHPUnit assertions.New dependencies
None.
Risks
DatabaseManager: Calling a static method via an instance ($obj->staticMethod()) is valid PHP but triggers a deprecation notice in strict tools. The existingFactorycall sites do this — no behaviour change, but can be cleaned up in a follow-up.Mandatory Checklist
Code validation
Code style
Unticked items justification
None.
Additional Checks
Unticked additional checks justification
$wpdb->query,wp_clear_scheduled_hook, anddelete_optionare WP core functions with their own error handling. The uninstall context does not support surfacing errors to the user, so wrapping them would add complexity with no benefit.