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

Plans: Redirect to Plan Overview page after free trial checkout #2238

Merged
merged 6 commits into from
Jan 11, 2016
2 changes: 1 addition & 1 deletion client/my-sites/plans/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ module.exports = function() {
);

page(
'/plans/:domain',
'/plans/:domain/:destinationType?',
adTracking.retarget,
controller.siteSelection,
controller.navigation,
Expand Down
1 change: 1 addition & 0 deletions client/my-sites/plans/main.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ var Plans = React.createClass( {
<PlanOverview
path={ this.props.context.path }
cart={ this.props.cart }
destinationType={ this.props.context.params.destinationType }
plan={ currentPlan }
selectedSite={ this.props.selectedSite } />
);
Expand Down
58 changes: 42 additions & 16 deletions client/my-sites/plans/plan-overview/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,22 @@
* External dependencies
*/
import React from 'react';
import page from 'page';

/**
* Internal dependencies
*/
import Main from 'components/main';
import PlanFeatures from 'my-sites/plans/plan-overview/plan-features';
import PlanStatus from 'my-sites/plans/plan-overview/plan-status';
import Notice from 'components/notice';
import SidebarNavigation from 'my-sites/sidebar-navigation';
import UpgradesNavigation from 'my-sites/upgrades/navigation';

const PlanOverview = React.createClass( {
propTypes: {
cart: React.PropTypes.object.isRequired,
destinationType: React.PropTypes.string,
plan: React.PropTypes.object.isRequired,
path: React.PropTypes.string.isRequired,
selectedSite: React.PropTypes.oneOfType( [
Expand All @@ -23,24 +26,47 @@ const PlanOverview = React.createClass( {
] ).isRequired
},

redirectToDefault() {
page.redirect( `/plans/${ this.props.selectedSite.slug }` );
},

renderNotice() {
if ( 'thank-you' === this.props.destinationType ) {
return (
<Notice onDismissClick={ this.redirectToDefault } status="is-success">
{
this.translate( 'Hooray, you just started your 14 day free trial of %(planName)s. Enjoy!', {
args: { planName: this.props.plan.productName }
} )
}
</Notice>
);
}
},

render() {
return (
<Main className="plan-overview">
<SidebarNavigation />

<UpgradesNavigation
cart={ this.props.cart }
path={ this.props.path }
selectedSite={ this.props.selectedSite } />

<PlanStatus
plan={ this.props.plan }
selectedSite={ this.props.selectedSite } />

<PlanFeatures
plan={ this.props.plan }
selectedSite={ this.props.selectedSite } />
</Main>
<div>

{ this.renderNotice() }

<Main className="plan-overview">
<SidebarNavigation />

<UpgradesNavigation
cart={ this.props.cart }
path={ this.props.path }
selectedSite={ this.props.selectedSite } />

<PlanStatus
plan={ this.props.plan }
selectedSite={ this.props.selectedSite } />

<PlanFeatures
plan={ this.props.plan }
selectedSite={ this.props.selectedSite } />
</Main>
</div>
);
}
} );
Expand Down
18 changes: 16 additions & 2 deletions client/my-sites/upgrades/checkout/checkout.jsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/**
* External dependencies
*/
var isEqual = require( 'lodash/lang/isEqual' ),
var Dispatcher = require( 'dispatcher' ),
isEmpty = require( 'lodash/lang/isEmpty' ),
isEqual = require( 'lodash/lang/isEqual' ),
page = require( 'page' ),
React = require( 'react' );

Expand All @@ -14,6 +15,7 @@ var analytics = require( 'analytics' ),
DomainDetailsForm = require( './domain-details-form' ),
hasDomainDetails = require( 'lib/store-transactions' ).hasDomainDetails,
observe = require( 'lib/mixins/data-observe' ),
planActions = require( 'state/sites/plans/actions' ),
purchasePaths = require( 'me/purchases/paths' ),
SecurePaymentForm = require( './secure-payment-form' ),
upgradesActions = require( 'lib/upgrades/actions' );
Expand Down Expand Up @@ -120,19 +122,31 @@ module.exports = React.createClass( {

if ( cartItems.hasRenewalItem( this.state.previousCart ) ) {
renewalItem = cartItems.getRenewalItems( this.state.previousCart )[ 0 ];

redirectTo = purchasePaths.managePurchase( renewalItem.extra.purchaseDomain, renewalItem.extra.purchaseId );
}
}

page.redirect( redirectTo );

return true;
},

getCheckoutCompleteRedirectPath: function() {
var renewalItem;

if ( cartItems.hasRenewalItem( this.props.cart ) ) {
renewalItem = cartItems.getRenewalItems( this.props.cart )[ 0 ];

return purchasePaths.managePurchaseDestination( renewalItem.extra.purchaseDomain, renewalItem.extra.purchaseId, 'thank-you' );
} else if ( cartItems.hasFreeTrial( this.props.cart ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a big fan of this change. I feel like it doesn't improve the clarity of the code and handles some edge cases in a function which should simply return a path.
In the case of Paypal we don't care about stale data since the whole app is going to reload anyway. So we can ignore this case.

I think it would make sense to me that the code responsible for making the payment request would itself trigger the invalidation (not necessarily handle it but notify of a possible modification in the site data).
For instance this could be handled in SecurePaymentForm.componentWillReceiveProps, once this.props.transaction.step.name === 'received-wpcom-response'

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @Tug's concerns here, but I think we should deal with them in a different PR. @Tug are you ok to open an issue for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure I'll do that ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

For the record: #2274

planActions.clearSitePlans();

Dispatcher.handleServerAction( {
type: 'FETCH_SITES'
} );

return `/plans/${ this.props.sites.getSelectedSite().slug }/thank-you`;
}

return '/checkout/thank-you';
Expand Down Expand Up @@ -162,7 +176,7 @@ module.exports = React.createClass( {
cards={ this.props.cards }
products={ this.props.productsList.get() }
selectedSite={ selectedSite }
redirectTo={ this.getCheckoutCompleteRedirectPath() } />
redirectTo={ this.getCheckoutCompleteRedirectPath } />
);
},

Expand Down
2 changes: 1 addition & 1 deletion client/my-sites/upgrades/checkout/paypal-payment-box.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ module.exports = React.createClass( {
} );

dataForApi = assign( {}, this.state, {
successUrl: origin + this.props.redirectTo,
successUrl: origin + this.props.redirectTo(),
cancelUrl: origin + '/checkout/' + this.props.selectedSite.slug,
cart: cart,
domainDetails: transaction.domainDetails
Expand Down
3 changes: 2 additions & 1 deletion client/my-sites/upgrades/checkout/secure-payment-form.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ var SecurePaymentForm = React.createClass( {
mixins: [ TransactionStepsMixin ],

propTypes: {
products: React.PropTypes.object.isRequired
products: React.PropTypes.object.isRequired,
redirectTo: React.PropTypes.func.isRequired
},

getInitialState: function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ var TransactionStepsMixin = {

defer( () => {
// The Thank You page throws a rendering error if this is not in a defer.
page( this.props.redirectTo );
page( this.props.redirectTo() );
} );
}
};
Expand Down
35 changes: 24 additions & 11 deletions client/state/sites/plans/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,25 @@ import {
} from './action-types';

/**
* Returns an action object to be used in fetching an object containing
* the plans for the given site.
* Clears plans for the given site.
*
* @param {Object} siteId ID of the concerned site
* @return {Object} Action object
* @param {Number} siteId identifier of the site
* @returns {Function} the corresponding action thunk
*/
export function clearSitePlans( siteId ) {
return ( dispatch ) => {
dispatch( {
type: REMOVE_SITE_PLANS,
siteId
} );
}
}

/**
* Fetches plans for the given site.
*
* @param {Number} siteId identifier of the site
* @returns {Function} a promise that will resolve once fetching is completed
*/
export function fetchSitePlans( siteId ) {
return ( dispatch ) => {
Expand All @@ -50,9 +64,9 @@ export function fetchSitePlans( siteId ) {
* Returns an action object to be used in signalling that an object containing
* the plans for a given site have been received.
*
* @param {Object} siteId ID of the concerned site
* @param {Object} plans Plans received
* @return {Object} Action object
* @param {Number} siteId identifier of the site
* @param {Object} plans list of plans received from the API
* @returns {Object} the corresponding action object
*/
export function fetchSitePlansCompleted( siteId, plans ) {
plans = reject( plans, '_headers' );
Expand All @@ -65,11 +79,10 @@ export function fetchSitePlansCompleted( siteId, plans ) {
}

/**
* Returns an action object to be used in updating an object containing
* the plans for the given site.
* Clears plans and fetches them for the given site.
*
* @param {Object} siteId ID of the concerned site
* @return {Object} Action object
* @param {Number} siteId identifier of the site
* @returns {Function} the corresponding action thunk
*/
export function refreshSitePlans( siteId ) {
return ( dispatch ) => {
Expand Down