Skip to content

Display Posts Widget: Properly handle situations where Jetpack is not in jetpack plugin directory or jetpack.php is under a different name.#3303

Merged
dereksmart merged 2 commits intoAutomattic:masterfrom
bisko:fix/dpw_properly_handle_JP_activate_deactivate
Jan 20, 2016
Merged

Display Posts Widget: Properly handle situations where Jetpack is not in jetpack plugin directory or jetpack.php is under a different name.#3303
dereksmart merged 2 commits intoAutomattic:masterfrom
bisko:fix/dpw_properly_handle_JP_activate_deactivate

Conversation

@bisko
Copy link
Copy Markdown
Contributor

@bisko bisko commented Jan 19, 2016

The Display Posts Widget didn't properly handle situations where jetpack and jetpack.php could be under different names. This lead to the cron not activating/deactivating when doing such to the Jetpack plugin.

Now these cases are handled by properly detecting Jetpack plugin file name and directory.

… is not in `jetpack` plugin directory or `jetpack.php` is under a different name.

The Display Posts Widget didn't properly handle situations where `jetpack` and `jetpack.php` could be under different names. This lead to the cron not activating/deactivating when doing such to the Jetpack plugin.

Now these cases are handled by properly detecting Jetpack plugin file name and directory.
@mdawaffe
Copy link
Copy Markdown
Member

Cool - thanks.

@zinigor zinigor added Bug When a feature is broken and / or not performing as intended [Feature] Extra Sidebar Widgets [Status] Needs Review This PR is ready for review. labels Jan 20, 2016
@zinigor zinigor added this to the 3.9 milestone Jan 20, 2016
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks to me now that the entire method doesn't work because the activated_plugin hook does not run if it's set in the plugin that is being currently activated, much like the register_activation_hook method:
https://codex.wordpress.org/Function_Reference/register_activation_hook

It seems to me that we need some another way of activating cron on Jetpack activation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it does get run, took me a bit digging to figure out how it was working:

When you run the activation action, Jetpack gets activated, but while doing so it gets loaded up - jetpack.php is included and all the hooks are added that are there, including the plugins_loaded hook, which runs Jetpack::load_modules and if Extra Sidebar Widgets is activated, it will include the wordpress-post-widget.php file, adding the hook, mentioned below

The thing is that this happens after the hook in register_activation_hook('jetpack/jetpack.php') is called, which means that it is of no use. But after digging a bit in the WP core code, it seems that the activated_plugin action is called after Jetpack has been initialized after the activation. This means that we get the action where we can handle it in an inner part of the plugin! :)

That's why I hooked this method to the activated_plugin action.

@zinigor zinigor added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Jan 20, 2016
@zinigor
Copy link
Copy Markdown
Contributor

zinigor commented Jan 20, 2016

Thank you, tested, looks good!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bisko can you please add spaces in the plugin_basename()'s

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am so sorry about that. Fixed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So fast! Thank you :)

@dereksmart
Copy link
Copy Markdown
Contributor

LGTM after minor spacing

dereksmart added a commit that referenced this pull request Jan 20, 2016
…te_deactivate

Display Posts Widget: Properly handle situations where Jetpack is not in `jetpack` plugin directory or `jetpack.php` is under a different name.
@dereksmart dereksmart merged commit 229e4ad into Automattic:master Jan 20, 2016
@dereksmart dereksmart removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jan 20, 2016
tbradsha added a commit that referenced this pull request Sep 15, 2023
* Add dropdown option if current status is non-standard

* Save untranslated invoice status in database

* Add translated status label to invoice object

* Use translated status label when appropriate

* Add changelog

* Set invoice to UnPaid if Draft when emailed

* Fix invoice bulk action status change

* Fixed Woo mapping to output translated status

* Code cleanup

* Translated → Untranslated

* Compare untranslated strings

* Cleanup

* Fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug When a feature is broken and / or not performing as intended [Feature] Extra Sidebar Widgets Touches WP.com Files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants