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.

Sign up for GitHub

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

Jump to bottom

Allow one click upsell for the plan upgrade nudge #47774

Merged
merged 5 commits into from
Dec 18, 2020
Merged

Allow one click upsell for the plan upgrade nudge #47774

merged 5 commits into from
Dec 18, 2020

Conversation

ddc22
Copy link
Contributor

@ddc22 ddc22 commented Nov 26, 2020 •
edited
Loading

Changes proposed in this Pull Request

Support feature provided in #45350 for plan upgrade upsell nudge.

Testing instructions

1-Register new site by going to http://calypso.localhost:3000/start/domains
2-Select the premium plan in the plans step
3-Complete purchase on checkout using a credit card
3-In the up-sell nudge click on Yes, I need plugins for my site!
4-The following modal should appear
image

@ddc22 ddc22 requested a review from a team as a code owner November 26, 2020 03:36
@matticbot
Copy link
Contributor

Test live: https://calypso.live/?branch=update/support-upsell-for--plan-upgrades

@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 26, 2020
@matticbot
Copy link
Contributor

matticbot commented Nov 26, 2020 •
edited
Loading

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@ddc22 ddc22 force-pushed the update/support-upsell-for--plan-upgrades branch from d474f78 to 6b1142c Compare November 27, 2020 11:47
@ddc22 ddc22 requested a review from a team November 30, 2020 06:46
@ddc22 ddc22 force-pushed the update/support-upsell-for--plan-upgrades branch 2 times, most recently from 5b8750f to 55f9ec4 Compare November 30, 2020 16:45
niranjan-uma-shankar
niranjan-uma-shankar reviewed Dec 1, 2020
View reviewed changes
Copy link
Contributor

@niranjan-uma-shankar niranjan-uma-shankar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Everything works well 👍

Found one issue when I upgrade from Calypso's /plans page to a Premium plan - I am getting the wrong Thank You page.

To reproduce:

  1. Log in to Calypso, head to /plans page, and upgrade to Premium
  2. After Premium purchase completes, accept the Business bump offer
  3. Complete the one-click purchase of the Business plan

I saw a Thank You page for the Premium plan whereas I expected it to be for the Business plan I just purchased.

1cu

client/my-sites/checkout/controller.jsx Outdated
@@ -193,13 +197,13 @@ export function upsellNudge( context, next ) {
let upgradeItem;

if ( context.path.includes( 'offer-quickstart-session' ) ) {
upsellType = 'concierge-quickstart-session';
upsellType = CONCIERGE_QUICKSTART_SESSION;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this refactor.

sirbrillig
sirbrillig approved these changes Dec 1, 2020
View reviewed changes
Copy link
Member

@sirbrillig sirbrillig left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks fine from a checkout point-of-view, although Niranjan's comment above is relevant. Please, if you modify the getThankYouUrl function, update the associated tests!

niranjan-uma-shankar
niranjan-uma-shankar reviewed Dec 2, 2020
View reviewed changes
Copy link
Contributor

@niranjan-uma-shankar niranjan-uma-shankar left a comment

Choose a reason for hiding this comment

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

If I purchase a Premium monthly plan, and then upgrade via the offer page, then the one-click modal does not indicate that the plan is of type monthly.

For comparison, if you upgrade from Premium monthly to Business monthly, then this is how the checkout page looks. It shows that plan is of type monthly, and displays a striked out price:

Screenshot 2020-12-02 at 11 58 42 AM

This is how it looks in the one-click modal. Notice that both are missing.

Screenshot 2020-12-02 at 11 58 13 AM

This is probably not a blocker for this PR and can be taken as a follow up. cc @victorespigares for opinion.

niranjan-uma-shankar
niranjan-uma-shankar reviewed Dec 2, 2020
View reviewed changes
client/my-sites/checkout/upsell-nudge/index.jsx Outdated
@@ -46,6 +46,14 @@ import { isMonthly } from 'calypso/lib/plans/constants';
* Style dependencies
*/
import './style.scss';
import { getPlanByPathSlug } from 'calypso/lib/plans';
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be under Internal dependencies, instead of under Style dependencies.

@ddc22 ddc22 force-pushed the update/support-upsell-for--plan-upgrades branch 2 times, most recently from b579797 to 321ef22 Compare December 8, 2020 09:33
@sirbrillig
Copy link
Member

Please, if you modify the getThankYouUrl function, update the associated tests!

I forgot, the upsell still uses the old handleCheckoutCompleteRedirect system, which is not covered by tests, rather than the newer useGetThankYouUrl. Once the upsell is converted to the new system, this fix may need to be applied there too.

@sirbrillig sirbrillig mentioned this pull request Dec 8, 2020
Closed
@ddc22
Copy link
Contributor Author

ddc22 commented Dec 10, 2020

I have addressed everything except the monthly pricing bug, @niranjan-uma-shankar can you have another look?

niranjan-uma-shankar
niranjan-uma-shankar approved these changes Dec 10, 2020
View reviewed changes
Copy link
Contributor

@niranjan-uma-shankar niranjan-uma-shankar left a comment

Choose a reason for hiding this comment

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

LGTM

@niranjan-uma-shankar niranjan-uma-shankar force-pushed the update/support-upsell-for--plan-upgrades branch from 321ef22 to adcfdec Compare December 18, 2020 05:09
@niranjan-uma-shankar niranjan-uma-shankar merged commit 1e075ab into trunk Dec 18, 2020
@niranjan-uma-shankar niranjan-uma-shankar deleted the update/support-upsell-for--plan-upgrades branch December 18, 2020 06:23
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 18, 2020
sarayourfriend pushed a commit that referenced this pull request Dec 23, 2020
@ddc22 @niranjan-uma-shankar @sarayourfriend
Allow one click upsell for the plan upgrade nudge (#47774)
a68f9dc 
* Allow one click upsell for plan upgrade nudge

* Dynamically resolve product slug based on the upsell type and upgraded item if applicable

* Map correct reciept id to be shown in thank you page

* Remove merge conflict

* Remove duplicate import and other merge conflicts

Co-authored-by: Niranjan Uma Shankar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sirbrillig sirbrillig sirbrillig approved these changes

@niranjan-uma-shankar niranjan-uma-shankar niranjan-uma-shankar approved these changes

@taggon taggon Awaiting requested review from taggon

Assignees
No one assigned
Labels
Post Checkout
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ddc22 @matticbot @sirbrillig @niranjan-uma-shankar