google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.22k stars 278 forks source link

Remove scheduled events upon deactivation or uninstall #6992

Open jamesozzie opened 1 year ago

jamesozzie commented 1 year ago

Bug Description

As suggested by a user in the forums, consider allowing users to remove any cron events upon via a hook, or an option to remove such events upon uninstall or deactivation.

The cron event highlighted by the user is googlesitekit_cron_update_remote_features


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

  1. Turn on the conversionReporting feature flag (as that includes one of the cron events).
  2. Set up Site Kit with the Analytics, Ads, and AdSense modules.
  3. Install & activate the WP Crontrol plugin.
  4. Go to "WordPress Dashboard -> Tools -> Cron Events" and search for "googlesitekit".
  5. You should be able to see some events from the Site Kit plugin here.
  6. Now, try deactivating, deleting, or resetting Site Kit.
  7. Perform step 4 again and verify that there are no Site Kit events.

Changelog entry

jamesozzie commented 1 year ago

@aaemnnosttv tagging you here for visibility.

ivonac4 commented 8 months ago

@aaemnnosttv Are you still planning to work on this soon? If not, could you please unassign yourself so someone else can pick it up? Thank you!

aaemnnosttv commented 2 months ago

Closing in favor of #8988

webd-uk commented 2 months ago

I will comment on #8988 as well but I do not agree that this should be closed in favour of that issue.

As per WordPress WP Cron recommendations, tasks should be unscheduled when they are no-longer required and doing so on a deactivation hook (as recommended by WordPress) cannot have a detrimental effect because the plugin is no longer active!

Should the plugin be re-activated, the missing / required tasks will be added again by the plugin anyway.

aaemnnosttv commented 2 months ago

Thanks @webd-uk, I agree, this issue is still valid 👍

eugene-manuilov commented 1 month ago

Update google-site-kit.php

  • Include register_uninstall_hook function, and re-use the googlesitekit_deactivate_plugin as callback. You can invoke this function bellow the register_deactivation_hook, since uninstalling and deactivating can share the same cleanup logic.

@zutigrm it should be done in the includes/Core/Util/Uninstallation.php file.

Update Google\Site_Kit\Plugin::register:

  • Hook into googlesitekit_deactivation and googlesitekit_reset and remove all listed CRON events.

The same here, we should do it in the Uninstallation class.

zutigrm commented 1 month ago

@eugene-manuilov Thanks! Didn't see we have that file. IB Updated

eugene-manuilov commented 1 month ago

Thanks. IB ✔️