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

Fix: Browser back button error in Payment page #3

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
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
21 changes: 19 additions & 2 deletions src/payment/PaymentPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { AppContext } from '@edx/frontend-platform/react';
import { getConfig } from '@edx/frontend-platform';
import { sendPageEvent } from '@edx/frontend-platform/analytics';

import messages from './PaymentPage.messages';
Expand Down Expand Up @@ -42,8 +43,24 @@ class PaymentPage extends React.Component {
}

componentDidMount() {
sendPageEvent();
this.props.fetchBasket();
const rawSkus = localStorage.getItem('skus');
const skus = JSON.parse(rawSkus);

// Check if SKU is not null
if (skus !== null) {
const baseURL = getConfig().ECOMMERCE_BASE_URL;
// Constructing the URL with the sku parameters
let ecommerceBasketURL = `${baseURL}/basket/add/?`;
// Appending each sku value to the URL
Object.values(skus).forEach(sku => { ecommerceBasketURL += `sku=${sku}&`; });
// Removing the extra '&' character at the end
ecommerceBasketURL = ecommerceBasketURL.slice(0, -1);
window.location.href = ecommerceBasketURL;
localStorage.removeItem('skus');
} else {
this.props.fetchBasket();
sendPageEvent();
}
Copy link
Member

Choose a reason for hiding this comment

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

Have a couple of questions:

  1. Can you make this logic specific to PayPal? We don't want the window location to be set to another URL for Stripe purchases.
  2. Have you tested that regular credit card (Stripe) purchases work?
  3. Have you tested what happens on re-load of the page?

Also, continuing on the other comment I added, you could dispatch an action from this component to set localStorage

Copy link
Author

Choose a reason for hiding this comment

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

Regarding point 1 I'll update the code so it is specific to PayPal only.
Point 2. Yes I've tested regular credit card purchases
Point 3. On page reload a new basket is created that contains courses/program user was about to purchase. Previously new empty basket was created.

}

renderContent() {
Expand Down
10 changes: 10 additions & 0 deletions src/payment/checkout/Checkout.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ class Checkout extends React.Component {
);

this.props.submitPayment({ method: 'paypal' });

// eslint-disable-next-line react/prop-types
const { products } = this.props;
const skus = [];

// eslint-disable-next-line no-restricted-syntax
for (const product of products) {
skus.push(product.sku);
}
localStorage.setItem('skus', JSON.stringify(skus));
};

// eslint-disable-next-line react/no-unused-class-component-methods
Expand Down
10 changes: 10 additions & 0 deletions src/payment/checkout/Checkout.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,16 @@ describe('<Checkout />', () => {
expect(store.getActions().pop()).toEqual(submitPayment({ method: 'paypal' }));
});

it('should call submitPayment and store skus in localStorage when handleSubmitPayPal is called', async () => {
const paypalButton = await screen.findByTestId('PayPalButton');
fireEvent.click(paypalButton);

expect(store.getActions().pop()).toEqual(submitPayment({ method: 'paypal' }));
// Check if skus are stored in localStorage
const storedSkus = JSON.parse(localStorage.getItem('skus'));
expect(storedSkus.length).toBeGreaterThan(0);
});

// Apple Pay temporarily disabled per REV-927 - https://github.com/openedx/frontend-app-payment/pull/256

it('submits and tracks the payment form', () => {
Expand Down
3 changes: 2 additions & 1 deletion src/payment/data/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ const basket = (state = basketInitialState, action = null) => {
loaded: true,
};

case BASKET_DATA_RECEIVED: return { ...state, ...action.payload };
case BASKET_DATA_RECEIVED:
return { ...state, ...action.payload };

case BASKET_PROCESSING: return {
...state,
Expand Down
17 changes: 17 additions & 0 deletions src/setupTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,20 @@ mergeConfig({
APPLE_PAY_MERCHANT_CAPABILITIES: process.env.APPLE_PAY_MERCHANT_CAPABILITIES && process.env.APPLE_PAY_MERCHANT_CAPABILITIES.split(','),
WAFFLE_FLAGS: {},
});

const localStorageMock = jest.fn(() => {
let store = {};
return {
getItem: (key) => (store[key] || null),
setItem: (key, value) => {
store[key] = value.toString();
},
clear: () => {
store = {};
},
removeItem: (key) => {
delete store[key];
},
};
})();
Object.defineProperty(window, 'localStorage', { value: localStorageMock });
Loading