Update/statuses to make more sense with ownership #38390

CodeyGuyDylan · 2024-07-17T19:50:18Z

Proposed changes:

Update statuses to avoid showing the "Learn More/Inactive" state when a product is "owned"
If product is owned, allow activation if there is a free offering that does not require a plan
If product is owned but requires a plan (even a free one) show a "Needs plan" status with a "Get plan" CTA
If product needs first site connection, but product is owned, show a "Needs Connection" status with the CTA of "Connect" instead of "Learn More/Inactive"

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?
Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

P2: pbNhbs-b6Y-p2

Does this pull request change what data or activity we track or use?

No

Testing instructions:

Create a new JN site with Jetpack Beta installed (no Jetpack plugin)
Activate Jetpack Protect with this branch
Deactivate the Akismet plugin
Go to My Jetpack and confirm you see Protect with the status of "Needs Connection" and a CTA of "Connect" with Akismet being "Inactive" but with the option to "Activate"
Activate the base Jetpack plugin with this branch and install the Jetpack VaultPress Backup standalone plugin. Make sure you see the new "Needs Plan" status with the CTA of "Get plan"
Install the Boost plugin and make sure you see the option to Activate Boost in My Jetpack
Connect your site and make sure the statuses look correct
Now connect your user and make sure they still look correct
Uninstall Jetpack Protect and add a paid plan for it from SA and make sure you see the "Needs plugin" status
Try activating some of the plugins (such as Boost) and make sure they remain active
Try disconnecting your user account and activating a plugin that requires a user connection and make sure the card updates to needing a connection

Statuses have a lot of different scenarios, so I can't account for all of them here so feel free to play around and try out anything you can think of

github-actions · 2024-07-17T19:51:10Z

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin, and enable the update/statuses-to-make-more-sense-with-ownership branch.

To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack update/statuses-to-make-more-sense-with-ownership

Interested in more tips and information?

In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
Read more about our development workflow here: PCYsg-eg0-p2
Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

github-actions · 2024-07-17T19:51:13Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
🔴 Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.

Videopress plugin:

Next scheduled release: August 6, 2024.
Scheduled code freeze: July 29, 2024.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

…o do so

elliottprogrammer

This tested very good, as described. Code looks good and reasonable. I don't see any issues.
LGTM! 👍

elliottprogrammer · 2024-07-19T16:38:21Z

projects/packages/my-jetpack/_inc/components/product-card/index.jsx

@@ -88,12 +75,13 @@ const ProductCard = inprops => {
 		onMouseLeave,
 	} = props;

+	const { ownedProducts } = getMyJetpackWindowInitialState( 'lifecycleStats' );


Oh! Wow, TIL you can pass a key into getMyJetpackInitialState( key )! I didn't know that previously. Thanks! 😊

Oh yes! @robertsreberski had that idea when we were updating the state in My Jetpack 😃

jboland88

This LGTM - it tested well. Overall, I think the statuses are more clear as a result of the change. It seems more clear what to do to get each product to "Active".

I had a few notes, but no blockers.

I noticed when clicking "Activate" on Boost - the status did not update to "Active" without me reloading the page. Let's add a separate maintenance item to follow up there.

Not for this PR, but would be nice to have upgrade and view options for Creator as well

jboland88 · 2024-07-19T21:05:22Z

projects/packages/my-jetpack/_inc/components/product-card/action-button.jsx

 					label: __( 'Learn more', 'jetpack-my-jetpack' ),
 					onClick: onAdd,
 					...( primaryActionOverride &&
 						PRODUCT_STATUSES.NEEDS_FIRST_SITE_CONNECTION in primaryActionOverride &&
 						primaryActionOverride[ PRODUCT_STATUSES.NEEDS_FIRST_SITE_CONNECTION ] ),
 				};
 			case PRODUCT_STATUSES.NEEDS_PURCHASE: {
+				const getPlanText = __( 'Get plan', 'jetpack-my-jetpack' );
+				const learnMoreText = __( 'Learn more', 'jetpack-my-jetpack' );
+				const buttonText = isOwned ? getPlanText : learnMoreText;


Clever - this is a nice little text variance that keeps the same CTA action 👍

jboland88 · 2024-07-19T21:08:41Z

projects/packages/my-jetpack/src/products/class-product.php

+	 *
+	 * @return boolean
+	 */
+	public static function is_owned() {


Nice - I like that this is a method on the product now.

jboland88 · 2024-07-19T21:10:25Z

projects/packages/my-jetpack/src/products/class-product.php

@@ -472,19 +496,19 @@ public static function get_status() {
 			// However if the standalone plugin for this product is active, then we will defer to showing errors that prevent the module from being active
 			// This is because if a standalone plugin is installed, we expect the product to not show as "inactive" on My Jetpack
 			if ( static::$requires_plan || ( ! static::has_any_plan_for_product() && static::$has_standalone_plugin && ! self::is_plugin_active() ) ) {
-				$status = static::$has_free_offering ? Products::STATUS_NEEDS_PURCHASE_OR_FREE : Products::STATUS_NEEDS_PURCHASE;
+				$status = static::is_owned() && static::$has_free_offering && ! static::$requires_plan ? Products::STATUS_NEEDS_ACTIVATION : Products::STATUS_NEEDS_PURCHASE;


So in this case, I suppose "NEEDS_PURCHASE" will mean it needs a plan of some kind, even if that purchase is $0?

It would be good to document what we mean by "NEEDS_PURCHASE" so folks don't assume it's only paid stuff.

We could refactor it to be "NEEDS_PLAN" or something else more descriptive in a later iterations 🤔

Yes that is correct, and I think you're right. NEEDS_PLAN would probably be a better name for this. I'll go ahead and update that in this PR since I'm already updating one of the status names

jboland88 · 2024-07-19T21:14:30Z

projects/packages/my-jetpack/src/products/class-product.php

@@ -455,7 +479,7 @@ public static function get_status() {
 			if ( static::$requires_site_connection && ! ( new Connection_Manager() )->is_connected() ) {
 				// Site has never been connected before
 				if ( ! Jetpack_Options::get_option( 'id' ) ) {
-					$status = Products::STATUS_NEEDS_FIRST_SITE_CONNECTION;
+					$status = static::is_owned() ? Products::STATUS_SITE_CONNECTION_ERROR : Products::STATUS_NEEDS_FIRST_SITE_CONNECTION;


I see what you mean about the conditionals getting insane here 😅

You could potentially add && !static::is_owned to the if statement above to remove the ternary if here - slightly cleaner.

jboland88 · 2024-07-19T21:16:38Z

projects/packages/my-jetpack/src/products/class-product.php

 			} elseif ( static::$requires_site_connection && ! ( new Connection_Manager() )->is_connected() ) {
 				// Site has never been connected before
 				if ( ! Jetpack_Options::get_option( 'id' ) ) {
-					$status = Products::STATUS_NEEDS_FIRST_SITE_CONNECTION;
+					$status = static::is_owned() ? Products::STATUS_SITE_CONNECTION_ERROR : Products::STATUS_NEEDS_FIRST_SITE_CONNECTION;


Same here, you might be able to add !static::is_owned() to the containing if condition to remove the need for the ternary

jboland88 · 2024-07-19T21:46:44Z

projects/packages/my-jetpack/src/class-products.php

@@ -176,23 +176,16 @@ public static function get_products() {
 	 * @return array
 	 */
 	public static function get_products_by_ownership( $type ) {


Not related to this PR directly - but this is a method that we should avoid calling outside of the My Jetpack context. Even though we don't load the full product array by calling get_products, the get_status call is still going to do checks for the presence of plans by sending requests to WPCOM.

That should be fine in the context of My Jetpack where we have already loaded products once and cached the results for that request lifecycle. But if we call this elsewhere, it will likely end up making some unintended API requests.

Yes I also had that in mind 😅 Fortunately we really only need this for the segmentation of the products (for now) so we should be okay 😄

jboland88

Reviewing changes since last review - code LGTM 👍

github-actions bot added the [Package] My Jetpack label Jul 17, 2024

github-actions bot added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Tests] Includes Tests labels Jul 17, 2024

CodeyGuyDylan added 6 commits July 17, 2024 14:47

Update statuses

203478b

changelog

6396d60

Fix build errors

34e59a8

Fix build errors

b766b7a

Fix tests

be1326d

Handle products that can be activated but need to be user connected t…

710e190

…o do so

CodeyGuyDylan force-pushed the update/statuses-to-make-more-sense-with-ownership branch from 2bb9bae to 710e190 Compare July 17, 2024 21:06

github-actions bot added [Plugin] VideoPress A standalone plugin to add high-quality VideoPress videos to your site. Docs E2E Tests [Plugin] Dumb Videopress labels Jul 17, 2024

CodeyGuyDylan added 2 commits July 17, 2024 15:25

Fix unowned statuses

ade8272

Remove accidental changes

02c4512

CodeyGuyDylan force-pushed the update/statuses-to-make-more-sense-with-ownership branch from 1737bad to 02c4512 Compare July 17, 2024 21:28

elliottprogrammer previously approved these changes Jul 19, 2024

View reviewed changes

CodeyGuyDylan removed the [Plugin] Dumb Videopress label Jul 19, 2024

jboland88 previously approved these changes Jul 19, 2024

View reviewed changes

jboland88 reviewed Jul 19, 2024

View reviewed changes

CodeyGuyDylan added 2 commits July 22, 2024 11:10

Update NEEDS_PURCHASE to NEEDS_PLAN for clarity

d6f76f0

Remove unnecessary ternary

cb7ab07

CodeyGuyDylan dismissed stale reviews from jboland88 and elliottprogrammer via cb7ab07 July 22, 2024 17:37

jboland88 approved these changes Jul 22, 2024

View reviewed changes

CodeyGuyDylan merged commit a6a641f into trunk Jul 22, 2024
70 checks passed

CodeyGuyDylan deleted the update/statuses-to-make-more-sense-with-ownership branch July 22, 2024 17:53

github-actions bot removed the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Jul 22, 2024

github-actions bot added this to the videopress/1.7.0 milestone Jul 22, 2024

This was referenced Jul 22, 2024

My Jetpack: replace Creator card with Newsletter #38418

Open

Extend ProductCard component and create recommendations UI #38319

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update/statuses to make more sense with ownership #38390

Update/statuses to make more sense with ownership #38390

CodeyGuyDylan commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

elliottprogrammer left a comment

elliottprogrammer Jul 19, 2024

CodeyGuyDylan Jul 19, 2024

jboland88 left a comment

jboland88 Jul 19, 2024

jboland88 Jul 19, 2024

jboland88 Jul 19, 2024

CodeyGuyDylan Jul 22, 2024

jboland88 Jul 19, 2024

jboland88 Jul 19, 2024

jboland88 Jul 19, 2024

CodeyGuyDylan Jul 22, 2024

jboland88 left a comment

Update/statuses to make more sense with ownership #38390

Update/statuses to make more sense with ownership #38390

Conversation

CodeyGuyDylan commented Jul 17, 2024 • edited Loading

Proposed changes:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

github-actions bot commented Jul 17, 2024 • edited Loading

github-actions bot commented Jul 17, 2024 • edited Loading

elliottprogrammer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jboland88 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

jboland88 left a comment

Choose a reason for hiding this comment

CodeyGuyDylan commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading