Improved the support for plugins with free and premium versions that … #727

fajardoleo · 2024-07-23T14:41:44Z

…can be activated in parallel.

swashata

Please see my suggestions, thanks.

swashata · 2024-08-11T06:38:12Z

includes/class-freemius.php

+                ! empty( $fs_active_plugins->newest->plugin_path ) &&
+                $fs_active_plugins->newest->plugin_path === $plugin_basename
+            ) {
+                fs_fallback_to_newest_active_sdk();


@fajardoleo Can't we always call the fs_fallback_to_newset_active_sdk? Will it harm anything? I find always calling it safer than relying on some uncontrolled data-structure.

@swashata It was actually supposed to reduce risks since it's called only when it's necessary. However, in the current code, we are always calling it anyway. I have reverted the changes.

swashata · 2024-08-11T06:39:57Z

includes/class-freemius.php

         */
-        private function remove_sdk_reference() {
+        private function remove_sdk_reference( $plugin_basename = null ) {


You can create a global function

function fs_remove_sdk_reference_by_basename( $basename )

and call that directly. This function will then be refactored to

private function remove_sdk_reference() { fs_remove_sdk_reference_by_basename( $this->_plugin_basename ); }

The reasons are

$this->remove_sdk_reference better remove just its own reference instead of something else.

We already have fs_fallback_to_newest_active_sdk, so having a complimentary fs_remove_sdk_reference_by_basename makes sense. They both work on $fs_active_plugins global.

Ideally, if we had a model class against the $fs_active_plugins it would've been better, but no need to refactor at this moment 😄

swashata · 2024-08-11T06:41:23Z

includes/class-freemius.php

@@ -7407,6 +7407,11 @@ function _activate_plugin_event_hook() {
                    $this->apply_filters( 'deactivate_on_activation', true )
                ) {
                    deactivate_plugins( $other_version_basename );
+                } else if ( $is_premium_version_activation ) {


@fajardoleo would it be good to always remove the sdk reference in case this is active?

if ( $is_premium_version_activation && is_plugin_active( $other_version_basename ) ) { fs_remove_sdk_reference_by_basename( $other_version_basename ); }

Now thinking about it again, perhaps we would want to also handle "Free plugin activation", for example user activates pro version, then deactivates free version on their own. Then notices the pro doesn't work without the Free, so they activate the Free version again. So instead of the else part here, we can do something like (pseudo code)

// If both free and premium version are active, then remove SDK from the Free version to avoid having it load the configuration. if (is_plugin_active( $other_version_basename ) ) { fs_remove_sdk_reference_by_basename( $this->_free_plugin_basename ); }

But please double check me.

@swashata In that use case, when the premium version is activated, the latest SDK is already set to the SDK that comes with the premium version (when using the code in this PR). When the free version is deactivated and reactivated, it will not have any effect on the latest SDK information. Please let me know if I missed something :) Thanks!

swashata · 2024-08-19T11:06:15Z

includes/fs-essential-functions.php

+    if ( ! function_exists( 'fs_remove_sdk_reference_by_basename' ) ) {
+        /**
+         * @author Leo Fajardo (@leorw)
+         * @since 2.7.4


@since 2.8.0 or 2.7.3.6. I don't think there will be a 2.7.0 version.

…can be activated in parallel.

swashata · 2024-08-20T05:59:12Z

includes/class-freemius.php

+         * @param string $plugin
+         * @param bool   $network_wide
+         */
+        function _activated_plugin( $plugin, $network_wide ) {


Just for the sake of properly handling bulk activation, can we also do this?

if ( $plugin !== $this->premium_plugin_basename() || $plugin !== $this->_free_plugin_basename ) { return; } $this->move_free_plugin_to_end_of_active_plugins();

swashata · 2024-08-20T06:00:10Z

includes/class-freemius.php

+         */
+        function _activated_plugin( $plugin, $network_wide ) {
+            // We cannot rely on fs_remove_sdk_reference_by_basename() for reordering, as the latest SDK reference might be associated with a different product, leading to the reordering of that product instead of the intended one.
+            $this->move_free_plugin_to_end_of_active_plugins( $this->_free_plugin_basename );


I suggest not to pass the $this->_free_plugin_basename as the function can already get it from the instance. (Just for code readability)

swashata · 2024-08-20T06:02:03Z

includes/class-freemius.php

+                /**
+                 * To avoid placing the free version at the start of the active plugins option when the SDK is loaded from it, remove the SDK reference linked to the free version if it is the newest.
+                 */
+                fs_remove_sdk_reference_by_basename( $this->_free_plugin_basename );


Will it hurt us even if the SDK reference from the free version is at top? I don't mind doing this "just in case", but just asking for my understanding.

swashata · 2024-08-20T06:04:31Z

includes/class-freemius.php

+         *
+         * @param string $plugin
+         */
+        private function move_free_plugin_to_end_of_active_plugins( $plugin ) {


Please remove the $plugin parameter and rely solely on $this->_free_plugin_basename. If that's not set, just bail early.

if ( empty( $this->_free_plugin_basename ) ) { return; }

swashata · 2024-08-20T06:05:17Z

includes/class-freemius.php

+
+            // Move the free version to the last position in the active plugins option.
+            if ( false !== ( $key = array_search( $plugin, $active_plugins ) ) ) {
+                unset( $active_plugins[ $key ] );


Let's either use array_splice or right after unset, let's reindex the array with $active_plugins = array_values( $active_plugins ).

swashata · 2024-08-20T06:06:13Z

includes/class-freemius.php

+         *
+         * @param string $plugin
+         */
+        private function move_free_plugin_to_end_of_active_plugins( $plugin ) {


Don't we want to handle network wide activation too?

Maybe it will be difficult to support network wide. Since I see the wp_get_active_network_plugins function does a sort when being called. If that's the case then please add some comment here.

swashata · 2024-08-20T06:12:26Z

includes/class-freemius.php

+         * @param string $plugin
+         * @param bool   $network_wide
+         */
+        function _activated_plugin( $plugin, $network_wide ) {


Proposing to rename this function to _handle_free_and_premium_parallel_activation or something like that, just for readability.

fajardoleo requested a review from swashata July 23, 2024 14:41

fajardoleo self-assigned this Jul 23, 2024

fajardoleo force-pushed the feature/free-premium-parallel-activation-support-enhancements branch from 3395977 to f49b565 Compare July 23, 2024 14:51

swashata reviewed Aug 11, 2024

View reviewed changes

fajardoleo force-pushed the feature/free-premium-parallel-activation-support-enhancements branch from f49b565 to 3b639fd Compare August 18, 2024 05:05

swashata reviewed Aug 19, 2024

View reviewed changes

fajardoleo force-pushed the feature/free-premium-parallel-activation-support-enhancements branch 2 times, most recently from ed9d106 to 2293c14 Compare August 19, 2024 17:10

Improved the support for plugins with free and premium versions that …

a281bef

…can be activated in parallel.

fajardoleo force-pushed the feature/free-premium-parallel-activation-support-enhancements branch from 2293c14 to a281bef Compare August 19, 2024 17:11

swashata requested changes Aug 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved the support for plugins with free and premium versions that … #727

Improved the support for plugins with free and premium versions that … #727

fajardoleo commented Jul 23, 2024

swashata left a comment

swashata Aug 11, 2024

fajardoleo Aug 18, 2024

swashata Aug 11, 2024

swashata Aug 11, 2024

swashata Aug 11, 2024

fajardoleo Aug 18, 2024

swashata Aug 19, 2024

swashata Aug 20, 2024

swashata Aug 20, 2024

swashata Aug 20, 2024

swashata Aug 20, 2024

swashata Aug 20, 2024

swashata Aug 20, 2024

swashata Aug 20, 2024

Improved the support for plugins with free and premium versions that … #727

Are you sure you want to change the base?

Improved the support for plugins with free and premium versions that … #727

Conversation

fajardoleo commented Jul 23, 2024

swashata left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment