My Jetpack: Add "Expired" & "Expires soon" statuses to product cards #39816
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
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. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! |
elliottprogrammer
added
[Status] Needs Team Review
and removed
[Status] In Progress
labels
projects/packages/my-jetpack/_inc/components/product-card/style.module.scss
Outdated
Show resolved
Hide resolved
|
It seems like you need to update the PR before merging |
There was a problem hiding this comment.
I mentioned this is the other PR but wanted to make sure it didn't get lost. I don't think we should be showing the "expired" status if it's been expired for a very long time. My gut reaction is we probably shouldn't show it if it's been expired for longer than a month
I noticed the "needs connection" error shows as a red error. I'm wondering if it should be yellow since the status is also yellow? Also, if the "Needs connection" error has a red or maybe yellow border, should the "Needs plan" status also have one?
Other than those thoughts, the plan recognition upgrade seems to work great! Thank you for your hard work on this
| * | ||
| * @return boolean | ||
| */ | ||
| public static function has_paid_plan_for_product() { | ||
| // First check site features (if there's a feature that identifies the paid plan) | ||
| if ( static::$feature_identifying_paid_plan ) { |
There was a problem hiding this comment.
Seems like having this check fulfills most of what this issue needs to be fixed 😄 Once this is done, it should be a quick patch to get this one done finally 🥳
There was a problem hiding this comment.
@grzegorz-cp would you be able to follow up on this, and get the issue done?
|
First, thanks very much for the reviews @CodeyGuyDylan! 😃
For not showing a product as "expired" after it's been expired for a certain amount of time, here's my thoughts:
Anyway, so let me know your thoughts. Let me know if maybe I'm missing something, or maybe over-thinking it? I did however make a change in the code where I added in the ability to hide/don't show the expiration status after 2 months after the expiration date. I didn't turn it on by default. But it would only be a matter of passing in a
Hmm, I changed/fixed this prior to your review.. I wonder if your branch (or Jetpack Beta branch) was not up to the latest? |
This is a good point and maybe fixes the issue I had with it 🤔 I don't know that, as a user, I would find it clear that the way to make the notification go away is to remove the subscription altogether, though I guess I would probably try it eventually. (I hate notifications 😆). I think we are probably okay to keep it how it is then if that's the case. Could we add to tracks the ability to see how long a subscription has been expired when viewing a card with an expired plan? That way we could check in tracks later to see if users are leaving that notification there for long periods of time or not later on.
Personally I don't think that's very good UX either 😅. I am not every user but personally that would bug the heck out of me |
Yeah, so true, I think we all hate notifications! 😆 Looking into it and thinking about this further, I'm noticing there's a way to show a close(X) icon on the notice! I'll look now to see how much it would take to close the expired/expiring notices with some persistence! This would be ideal, I think. Do you agree? 😃
This is a great idea! Yes, I'll do this! 🙌
Yeah, I agree. |
There was a problem hiding this comment.
More of a question instead of a requested change. In the scenario below, Boost, Social, and Protect all have expired plans. I see that some statuses override the expired plan status such as these. I think that is fine, as it wouldn't work anyway without the plugin being installed or active, so the statuses shown are still integral to getting the plugin to work. I just wanted to make sure it was expected behavior 😄
One note, and this probably isn't feasible, but I think it would be great if we could redirect them back to My Jetpack after renewing or removing a product after getting to /me/purchases from My Jetpack. Maybe we could do that sometime in the future
Other than that I think all of my issues have been addressed. Thank you for all the changes! I'll put my conversation with payments about the renewal stuff here since that answers the question as to why we don't need to cut off renewals older than a month old 😄
Slack: p1732730061873749-slack-C02BEP610
@CodeyGuyDylan, Yes, that is expected behavior. |
| return array( | ||
| 'jetpack_social_v1_yearly', | ||
| 'jetpack_social_v1_monthly', | ||
| 'jetpack_social_v1_bi_yearly', | ||
| 'jetpack_social_basic_yearly', | ||
| 'jetpack_social_monthly', | ||
| 'jetpack_social_basic_monthly', | ||
| 'jetpack_social_basic_bi_yearly', | ||
| 'jetpack_social_advanced_yearly', | ||
| 'jetpack_social_advanced_monthly', | ||
| 'jetpack_social_advanced_bi_yearly', | ||
| ); |
There was a problem hiding this comment.
Should this contain all the legacy products that are no longer used anymore?
Social currently has only those *_v1_* products.
There was a problem hiding this comment.
Hey @manzoorwanijk! 👋 Thanks for the question! 😃
Should this contain all the legacy products that are no longer used anymore?
Social currently has only those v1 products.
To answer your question: Yes, it should contain ALL possible Social product slugs (including legacy products, but not including Bundle plans)
Yes, ultimately has_paid_plan_for_product() uses this function in question (get_paid_plan_product_slugs()) as well as get_paid_bundles_that_include_product() to get All the paid plan product_slugs and bundle plan product_slugs that include Social, to ultimately determine if the site has a paid plan for the product (in this case Social).
Some sites/users might still be renewing a legacy Social product that they bought previously, so we need to know if the site/user has ANY Social paid plan (including legacy plans, or even bundled plans that include Social within it). Basically, if the site/user has Any paid plan that includes Social, we want to know about it. 😉
This PR adds an "Expired" or "Expiring soon" status to the product cards of products with an expired or expiring paid plan subscription. We also make the product CTA say "Renew my plan" (or "Resume my plan" if expired) and link to the specific subscription in purchase management section of their wordpress.com account so that they can renew or resume their subscription.
Screenshots:
.
Proposed changes:
Other information:
Jetpack product discussion
Project Thread: pbNhbs-bJN-p2
Design Thread: p1HpG7-umC-p2
Does this pull request change what data or activity we track or use?
No
Testing instructions:
To test this PR, skip down to "Testing this PR":
Otherwise, if testing issue: Backup Card: My Jetpack shows "Needs Plan" for backup even when there is a Jetpack plan from a hosting partner, follow the instructions below:
Testing issue #39762 :
(Backup Card: My Jetpack shows "Needs Plan" for backup even when there is a Jetpack plan from a hosting partner.)
Testing this PR:
add/mj-expired-products-cardsbranch. In the JN interface, you can automatically assign paid product licenses for all the products shown on the My Jetpack page, See screenshot: