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

Purchases: Fill credit card form with card holder name upon loading Edit Card Details page #485

Merged
merged 3 commits into from
Nov 24, 2015

Conversation

stephanethomas
Copy link
Contributor

This pull request fixes #193 partially by updating the Edit Card Details page to display the name attached to the stored credit card associated to the current purchase when it loads:

screenshot

It makes use of the Stored Cards store that relies on the /me/stored-cards API endpoint. We only fill in the form with the card holder name for the time being but we should probably display the country and the postal code as well - these are probably the only fields that won't be updated by users in 95% of the cases. It doesn't make sense to display the last four digits as well as the expiration date for the opposite reason.

This pull request doesn't address disabling of input fields - this is handled as a separate issue in #260.

Testing instructions

  1. Get a new test account and blog
  2. Enable the store sandbox
  3. Select a plan by clicking the Upgrade Now button on the Plans page
  4. Submit the form on the Secure Payment page with fake credit card information
  5. Check that the purchase was successful
  6. Head to the Purchases page
  7. Click the new plan to display the corresponding Manage Purchase page
  8. Check that the Payment Method section shows the right credit card logo and last digits
  9. Click the Edit Payment Method navigation link to display the Edit Card Details page
  10. Check that card holder name entered during checkout appears in the form
  11. Enter new credit card information and click the Update Card button
  12. Check that the update was successful
  13. Check that the Manage Purchase page reflects these changes
  14. Check that the new stored credit card appears at the bottom of the Billing History page

Notes

I think we should remove the country and postal code returned with the payment data in the /me/purchases endpoint and update the /me/stored-cards endpoint to return it. That way everything is located in the Stored Cards store. What do you think?

@stephanethomas stephanethomas added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] Purchase Management Related to managing purchases such as subscriptions, plans, history, auto-renew, cancellation, etc. labels Nov 23, 2015
@stephanethomas stephanethomas self-assigned this Nov 23, 2015
@stephanethomas stephanethomas added this to the Purchases: v1 milestone Nov 23, 2015
@scruffian
Copy link
Member

One bug; when I edit my card, the local copy of the data doesn't update, so when I navigate back to edit the card, the old details are displayed until they reload. It probably makes sense to fix this in a different PR.

@scruffian
Copy link
Member

That way everything is located in the Stored Cards store.

I like this idea, but we will need to store some data in the purchases endpoint for Paypal. Either way this is probably a v2 change :)

@scruffian
Copy link
Member

Check that the new stored credit card appears at the bottom of the Billing History page

It actually appears at the top :)

@scruffian scruffian added [Status] Needs Author Reply [Status] Awaiting Fixes and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Author Reply labels Nov 23, 2015
@stephanethomas
Copy link
Contributor Author

One bug; when I edit my card, the local copy of the data doesn't update, so when I navigate back to edit the card, the old details are displayed until they reload. It probably makes sense to fix this in a different PR.

Could you be more specific? If I update a card on the Edit Card Details page succesfully, I am then redirected to the Manage Purchase page with the correct payment information. If I navigate to the Edit Card Details page afterwards I see the new data in the Name on Card field.

@stephanethomas stephanethomas added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Awaiting Fixes labels Nov 23, 2015
@stephanethomas stephanethomas force-pushed the update/edit-card-details branch 2 times, most recently from 0c56195 to dafb5c2 Compare November 24, 2015 10:56
@gziolo
Copy link
Member

gziolo commented Nov 24, 2015

I think we should remove the country and postal code returned with the payment data in the /me/purchases endpoint and update the /me/stored-cards endpoint to return it. That way everything is located in the Stored Cards store. What do you think?

I agree, you can create follow up task.

Code looks good 👍

@gziolo
Copy link
Member

gziolo commented Nov 24, 2015

It doesn't work when I try to update. I see the following errors on the console:
screen shot 2015-11-24 at 12 59 32

Looks like merge issue.

@gziolo gziolo added [Status] Awaiting Fixes and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 24, 2015
} );
};

if ( this.props.card ) {
Copy link
Member

Choose a reason for hiding this comment

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

I hope we can get rid of this condition once we introduce loading placeholder :)

Copy link
Member

Choose a reason for hiding this comment

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

BTW, there is some delay with filling up form field with card holder name, but this should also be deprecated with loader implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@stephanethomas
Copy link
Contributor Author

It doesn't work when I try to update. I see the following errors on the console:

This specific Javascript error has been fixed separately in #621.

@stephanethomas stephanethomas added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Awaiting Fixes labels Nov 24, 2015
@stephanethomas stephanethomas changed the title Purchases: Fill credit card form with data upon loading Edit Card Details page Purchases: Fill credit card form with card holder name upon loading Edit Card Details page Nov 24, 2015
@scruffian
Copy link
Member

Here's another issue caused by the lack of a placeholder loading state:
issue2

@scruffian
Copy link
Member

Ok I think this is good to merge, as long as we work on the placeholder state soon :)

@scruffian scruffian added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 24, 2015
@gziolo
Copy link
Member

gziolo commented Nov 24, 2015

@scruffian: Sounds good. I can start implementing loading placeholders with this page tomorrow (see #275) :)

stephanethomas added a commit that referenced this pull request Nov 24, 2015
 Purchases: Fill credit card form with card holder name upon loading Edit Card Details page
@stephanethomas stephanethomas merged commit a440ad1 into master Nov 24, 2015
@stephanethomas stephanethomas deleted the update/edit-card-details branch November 24, 2015 16:27
* @param card
* @param fields
*/
mergeCard( card, fields: {} ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intended to be fields = {} to provide the default argument? This line broke after dropping Flow plugin in the Babel 6 upgrade (#1515) and is assumed to be corrected in 4b06d6e.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it looks like a default param. A few lines above it's called without 2nd param, so that makes perfect sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Purchase Management Related to managing purchases such as subscriptions, plans, history, auto-renew, cancellation, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Purchases: Fill credit card form with data upon loading Edit Card Details page
5 participants