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: Show the fetching state when visiting ManagePurchase after renewing #699

Merged
merged 2 commits into from
Nov 25, 2015

Conversation

drewblaisdell
Copy link
Contributor

This PR updates ManagePurchase to render the fetching state when data is fetching after a renewal, to hide any stale data related to the pre-renewal purchase.

screen shot 2015-11-24 at 2 51 50 pm

Previously, we attempted to just obscure the renewal date, but there are other aspects of the purchase (the price, the expired style, etc) that we would need to change temporarily as well. The simplest solution is to just hide the purchase until a new one has been fetched from the API.

Fixes #161.

Testing

  • Visit http://calypso.localhost:3000/purchases and renew an expired purchase.
  • Assert that you are redirected to the ManagePurchase page for the given purchase, see the success notice, and that the fetching state is displayed below for a moment instead of stale data.
  • QA review
  • Code review

@drewblaisdell drewblaisdell 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 24, 2015
@drewblaisdell drewblaisdell self-assigned this Nov 24, 2015
@drewblaisdell drewblaisdell added this to the Purchases: v1 milestone Nov 24, 2015
@scruffian
Copy link
Member

We need to do something about the notices too:

screenshot 2015-11-25 11 05 41

@scruffian
Copy link
Member

I added ca5291a to hide the renewal notice (and the expiring CC notice) after renewal too.

I'm good with your commit 👍

@fabianapsimoes
Copy link
Contributor

QA 👍

@drewblaisdell
Copy link
Contributor Author

@scruffian Thanks for the commit. Code is 👍 for this, then.

@drewblaisdell drewblaisdell 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 25, 2015
drewblaisdell added a commit that referenced this pull request Nov 25, 2015
Purchases: Show the fetching state when visiting `ManagePurchase` after renewing
@drewblaisdell drewblaisdell merged commit daa675a into master Nov 25, 2015
@drewblaisdell drewblaisdell deleted the update/hide-stale-purchase-data branch November 25, 2015 18:07
@stephanethomas
Copy link
Contributor

I just wanted to mention that I think it's indeed the best option we have for now. I've spent quite some time trying to figure out a better approach to no avail.

The fundamental flaw here is that the /me/transactions/ API endpoint doesn't return the new list of purchases. It means that the list of purchases stored in PurchasesStore is stale as soon the transaction is successful. We can ask this store to fetch again data from the /me/purchases endpoint but users would still see the former data on the Manage Purchase page in the mean time.

The best workaround I can think of is to ask the store to update the purchase just renewed: this wouldn't perform any call to the API but just update local data. The thing is, it's easy to deduct basic changes client-side such as the new expiration date and perform this optimistic update. However, it's tricky to determine others such as the new price when the user cancels privacy protection. We could reset the corresponding properties in the store though but it doesn't solve everything. For example what if the user uses a different payment method during renewal? Again we would display the former payment method when users are redirected on the Manage Purchase page.

So this solution to update the data optimistically isn't a good one, which leaves us with either updating /me/transactions/ to return the new list of purchases, or display a skeleton while we load the up-to-date data :).

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. [Type] Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants