-
Notifications
You must be signed in to change notification settings - Fork 4
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
Entrepreneur my home goes to Calypso instead #1467
Conversation
Size Change: 0 B Total Size: 197 kB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Juan 👋🏼 I haven't tested the changes yet, but they look good to me. Left one inline comment only so far.
Just checking first whether you had a chance to https://github.com/Automattic/dotcom-forge/issues/6932#issuecomment-2098324092?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! The changes look good and work as expected for the "full" Entrepreneur plan. There's just one more thing I noticed. Please see the inline comment below.
*/ | ||
protected function has_entrepreneur_plan() { | ||
$current_plan = get_option( 'jetpack_active_plan', array() ); | ||
return ! empty( $current_plan ) && 'ecommerce-bundle' === $current_plan['product_slug']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will work for the "full" Entrepreneur plan, however, the Entrepreneur Trial plan currently has the follwing product_slug
: ecommerce-trial-bundle-monthly
.
I think this product_slug
shouldn't change when launching. Maybe @andregardi could confirm. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, we are not changing the product slug. Good catch @ivan-ottinger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per @ivan-ottinger's comment this will not work for entrepreneur trial plan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now! 👍
☢️☢️☢️ Don't merge yet ☢️☢️☢️
Changes proposed in this Pull Request:
This PR changes the URL of My Home link in the main menu. If the site plan is Entrepreneur, that link will go to Calypso My Home instead.
Closes https://github.com/Automattic/dotcom-forge/issues/6932
How to test the changes in this Pull Request:
Other information:
FOR PR REVIEWER ONLY: