-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Purchases: Reduxify Stored Cards on Secure Payment page #6710
Conversation
@@ -95,7 +98,7 @@ const Checkout = React.createClass( { | |||
props = props || this.props; | |||
|
|||
analytics.tracks.recordEvent( 'calypso_checkout_page_view', { | |||
saved_cards: props.cards.get().length, | |||
saved_cards: props.cards.length, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we shouldn't record this event only when the list of cards have loaded from the server, otherwise we might just provide stale data to Tracks. This is how it currently works though.
7c7ddb0
to
eb4f30e
Compare
Product 👍 Thanks the detailed notes and comments! |
eb4f30e
to
d0f3a74
Compare
d0f3a74
to
ef09216
Compare
ef09216
to
57d5970
Compare
Code 👍 I also tested this, and updated the last commit to remove a misplaced space here (not introduced by this PR). |
… during mounting This is to avoid having to explicitly fetch stored cards when a user uses a new credit card during purchase or adds a new payment method to an existing purchase - as otherwise the Billing History page will display stale information if it was accessed by the user beforehand during a same session. It indeed doesn't really make sense to perform this call in these two use cases since there is little chance that the user visits the Billing History page afterwards. It seems to be more effective to fetch stored cards every time the user access this specific page.
…story page only if no data has been fetched ever
…stored cards This also removes the singleton that was used originally to get this data.
57d5970
to
4aee69d
Compare
This pull request is a follow-up of #6613, #6628, and #6665 that updates the
Secure Payment
page in the checkout to use the global state tree to manage credit cards (stored during previous purchases or when a user added a new payment method to an existing purchase) instead of the old singleton:Testing instructions
git checkout update/reduxify-checkout
and start your server, or open a live branchBilling History
pageManage Your Credit Cards
sectionSecure Payment
pagePay
buttonAdditional notes
This pull requests also updates the query component for stored cards to fetch data as soon as it is mounted, and only in that case. This is to avoid having to explicitly fetch stored cards when a user uses a new credit card during purchase or adds a new payment method to an existing purchase - as otherwise the
Billing History
page will display stale information if it was accessed by the user beforehand during a same session.The logic here is that it indeed doesn't make sense to perform this call in these two use cases since there is little chance that the user visits the
Billing History
page afterwards. It seems to be more effective to fetch stored cards every time the user access this specific page, which I assume doesn't receive lots of traffic.Reviews
@Automattic/sdev-feed