Fix: Browser back button error in Payment page #3
Conversation
…app-payment into zubair-back-button-fix
Let me look into it if we can add any. Thanks |
src/payment/data/reducers.js
Outdated
| case BASKET_DATA_RECEIVED: return { ...state, ...action.payload }; | ||
| case BASKET_DATA_RECEIVED: | ||
| if (action.payload.products && action.payload.products.length > 0) { | ||
| localStorage.setItem('sku', action.payload.products[0].sku); |
There was a problem hiding this comment.
Noted you have new code to handle baskets with more than 1 product.
Apart from that, I don't think it's recommended to directly update localStorage within reducers, as they should be pure functions, without any side effects.
You can update it instead within the PaymentPage component or in actions.
There was a problem hiding this comment.
Alright 👍 I'll update the code accordingly. Thanks
| } else { | ||
| this.props.fetchBasket(); | ||
| sendPageEvent(); | ||
| } |
There was a problem hiding this comment.
Have a couple of questions:
- Can you make this logic specific to PayPal? We don't want the window location to be set to another URL for Stripe purchases.
- Have you tested that regular credit card (Stripe) purchases work?
- 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
There was a problem hiding this comment.
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.
Anyone merging to this repository is expected to promptly release and monitor their changes; if you are not able to do this DO NOT MERGE, please coordinate with someone who can to ensure that the changes are released.
Description
Supporting information
Testing instructions
Other information
Checklist