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

refactor: DPM loading lag for successful payments redirect #6

Merged
merged 4 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/payment/PageLoading.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { getConfig } from '@edx/frontend-platform';
import { logInfo } from '@edx/frontend-platform/logging';

export default class PageLoading extends Component {
renderSrMessage() {
Expand All @@ -15,6 +17,17 @@ export default class PageLoading extends Component {
}

render() {
const { shouldRedirectToReceipt, orderNumber } = this.props;

if (shouldRedirectToReceipt) {
logInfo(`Dynamic Payment Methods payment succeeded for edX order number ${orderNumber}, redirecting to ecommerce receipt page.`);
const queryParams = `order_number=${orderNumber}&disable_back_button=${Number(true)}&dpm_enabled=${true}`;
if (getConfig().ENVIRONMENT !== 'test') {
/* istanbul ignore next */
global.location.assign(`${getConfig().ECOMMERCE_BASE_URL}/checkout/receipt/?${queryParams}`);
}
}

return (
<div>
<div
Expand All @@ -34,4 +47,11 @@ export default class PageLoading extends Component {

PageLoading.propTypes = {
srMessage: PropTypes.string.isRequired,
shouldRedirectToReceipt: PropTypes.bool,
orderNumber: PropTypes.string,
};

PageLoading.defaultProps = {
shouldRedirectToReceipt: false,
orderNumber: null,
};
31 changes: 15 additions & 16 deletions src/payment/PaymentPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@ import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { loadStripe } from '@stripe/stripe-js';
import { getConfig } from '@edx/frontend-platform';
import {
FormattedMessage,
getLocale,
injectIntl,
intlShape,
} from '@edx/frontend-platform/i18n';
import { logInfo } from '@edx/frontend-platform/logging';
import { AppContext } from '@edx/frontend-platform/react';
import { sendPageEvent } from '@edx/frontend-platform/analytics';

Expand Down Expand Up @@ -95,15 +93,6 @@ class PaymentPage extends React.Component {
}
};

redirectToReceiptPage(orderNumber) {
logInfo(`Payment succeeded for edX order number ${orderNumber}, redirecting to ecommerce receipt page.`);
const queryParams = `order_number=${orderNumber}&disable_back_button=${Number(true)}&dpm_enabled=${true}`;
if (getConfig().ENVIRONMENT !== 'test') {
/* istanbul ignore next */
global.location.assign(`${getConfig().ECOMMERCE_BASE_URL}/checkout/receipt/?${queryParams}`);
}
}

renderContent() {
const { isEmpty, isRedirect, isPaymentRedirect } = this.props;

Expand All @@ -117,13 +106,10 @@ class PaymentPage extends React.Component {
orderNumber,
} = this.state;

// If this is a redirect from Stripe Dynamic Payment Methods with a successful payment, redirect to the receipt page
if (paymentStatus === 'succeeded') {
this.redirectToReceiptPage(orderNumber);
}
const shouldRedirectToReceipt = paymentStatus === 'succeeded';

// If this is a redirect from Stripe Dynamic Payment Methods, show loading icon until getPaymentStatus is done.
if (isPaymentRedirect && paymentStatus === null) {
if (isPaymentRedirect && paymentStatus !== undefined && paymentStatus === null) {
return (
<PageLoading
srMessage={this.props.intl.formatMessage(messages['payment.loading.payment'])}
Expand All @@ -148,6 +134,19 @@ class PaymentPage extends React.Component {
);
}

// If this is a redirect from Stripe Dynamic Payment Methods with a successful payment,
// redirect to the receipt page. PageLoading render is required first otherwise there is a
// lag between when the paymentStatus is no longer null but the redirect hasn't happened yet.
if (shouldRedirectToReceipt) {
return (
<PageLoading
srMessage={this.props.intl.formatMessage(messages['payment.loading.payment'])}
shouldRedirectToReceipt={shouldRedirectToReceipt}
orderNumber={orderNumber}
/>
);
}

// In all other cases, we want to render the basket content. This is used before we've loaded
// anything, during loading, and after we've loaded a basket with a product in it.

Expand Down
2 changes: 1 addition & 1 deletion src/payment/cart/CouponForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class CouponForm extends Component {

return (
<form onSubmit={this.handleAddSubmit} className="summary-row d-flex align-items-end">
<Form.Group controlId={id} invalid={false} className="mb-0 mr-2">
<Form.Group controlId={id} invalid={false.toString()} className="mb-0 mr-2">
<label className="h6 d-block" htmlFor={id}>
<FormattedMessage
id="payment.coupon.label"
Expand Down
1 change: 1 addition & 0 deletions src/payment/cart/__snapshots__/Cart.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ exports[`<Cart /> renders a basic, one product cart with coupon form 1`] = `
>
<div
class="pgn__form-group mb-0 mr-2"
invalid="false"
>
<label
class="h6 d-block"
Expand Down
1 change: 1 addition & 0 deletions src/payment/cart/__snapshots__/CouponForm.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ exports[`CouponForm should render a form when there is no coupon 1`] = `
>
<div
class="pgn__form-group mb-0 mr-2"
invalid="false"
>
<label
class="h6 d-block"
Expand Down
6 changes: 3 additions & 3 deletions src/payment/checkout/payment-form/StripePaymentForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ const StripePaymentForm = ({

// Check if should show PaymentMethodMessagingElement, as it only renders
// for specific countries, if country code and currency are known, and they must match
const userLocationCountryCode = new Cookies().get(getConfig().LOCATION_OVERRIDE_COOKIE)
|| new Cookies().get(getConfig().USER_LOCATION_COOKIE_NAME);
const userLocationCountryCode = new Cookies().get(getConfig().USER_LOCATION_COOKIE_NAME)
|| new Cookies().get(getConfig().LOCATION_OVERRIDE_COOKIE);
const shouldDisplayPaymentMethodMessagingElement = (
(!!userLocationCountryCode || !!locationCountryCode) && !!orderTotal && !!currency
!!(userLocationCountryCode || locationCountryCode) && !!orderTotal && !!currency
);

// Loading button should appear when: basket and stripe elements are loading, quantity is updating and not submitting
Expand Down
2 changes: 1 addition & 1 deletion src/payment/payment-methods/stripe/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default async function checkout(
const postData = formurlencoded({
payment_intent_id: result.paymentIntent.id,
skus,
dynamic_payment_methods_enabled: basket.isDynamicPaymentMethodsEnabled,
dynamic_payment_methods_enabled: basket.isDynamicPaymentMethodsEnabled || false,
});
await getAuthenticatedHttpClient()
.post(
Expand Down
Loading