Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add has_required_plan to product info and implement it in Search #22682

Merged
merged 4 commits into from
Feb 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: added

Add has_required_plan to product info and implement method in Search
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Updated package dependencies.
1 change: 1 addition & 0 deletions projects/packages/my-jetpack/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"yoast/phpunit-polyfills": "1.0.3",
"automattic/jetpack-changelogger": "^3.0",
"automattic/jetpack-options": "^1.14",
"automattic/jetpack-search": "^0.7",
"automattic/wordbless": "@dev"
},
"autoload": {
Expand Down
17 changes: 17 additions & 0 deletions projects/packages/my-jetpack/src/products/class-product.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public static function get_info() {
'status' => static::get_status(),
'pricing_for_ui' => static::get_pricing_for_ui(),
'requires_user_connection' => static::$requires_user_connection,
'has_required_plan' => static::has_required_plan(),
'class' => get_called_class(),
);
}
Expand Down Expand Up @@ -141,6 +142,19 @@ abstract public static function get_features();
*/
abstract public static function get_pricing_for_ui();

/**
* Checks whether the current plan (or purchases) of the site already supports the product
*
* Returns true if it supports. Return false if a purchase is still required.
*
* Free products will always return true.
*
* @return boolean
*/
public static function has_required_plan() {
return true;
}

/**
* Undocumented function
*
Expand All @@ -149,6 +163,9 @@ abstract public static function get_pricing_for_ui();
public static function get_status() {
if ( static::is_active() ) {
$status = 'active';
if ( ! static::has_required_plan() ) {
$status = 'needs_purchase';
}
Comment on lines 163 to +168
Copy link
Contributor

Choose a reason for hiding this comment

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

something we've been talking about. I thought.
I though we could check whether the product requires a plan at top of this method:

Suggested change
public static function get_status() {
if ( static::is_active() ) {
$status = 'active';
if ( ! static::has_required_plan() ) {
$status = 'needs_purchase';
}
public static function get_status() {
if ( ! static::has_required_plan() ) {
$status = 'needs_purchase';
} elseif ( static::is_active() ) {
$status = 'active';

It's ok to me, just wanted to put in code the way that we'll have to take to deal with the product activation flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

has_required_plan will return true by default. If the product doesn't need a plan, it will return true.

But I agree that could be useful to have another property that specifically says if the product is free or not

} elseif ( ! self::is_plugin_installed() ) {
$status = 'plugin_absent';
} else {
Expand Down
14 changes: 14 additions & 0 deletions projects/packages/my-jetpack/src/products/class-search.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
namespace Automattic\Jetpack\My_Jetpack\Products;

use Automattic\Jetpack\My_Jetpack\Module_Product;
use Automattic\Jetpack\Search\Plan as Search_Plan;

/**
* Class responsible for handling the Search product
Expand Down Expand Up @@ -86,4 +87,17 @@ public static function get_pricing_for_ui() {
'promotion_percentage' => '50',
);
}

/**
* Checks whether the current plan of the site already supports the product
*
* Returns true if it supports. Return false if a purchase is still required.
*
* Free products will always return true.
*
* @return boolean
*/
public static function has_required_plan() {
return ( new Search_PLan() )->supports_search();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Search_PLan a typo? Looks like it ought to be Search_Plan 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wow, I wonder how it didnt throw any errors

}
}
4 changes: 4 additions & 0 deletions projects/plugins/backup/changelog/add-my_jetpack_has_plan
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

updated lock file
5 changes: 5 additions & 0 deletions projects/plugins/backup/changelog/add-my_jetpack_has_plan#2
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Updated composer.lock.


3 changes: 2 additions & 1 deletion projects/plugins/backup/composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/add-my_jetpack_has_plan
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: compat

update lock file
5 changes: 5 additions & 0 deletions projects/plugins/jetpack/changelog/add-my_jetpack_has_plan#2
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: other
Comment: Updated composer.lock.


3 changes: 2 additions & 1 deletion projects/plugins/jetpack/composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.