Purchases: Add option to remove expiring purchases #2602

drewblaisdell · 2016-01-20T01:02:22Z

Fixes #1402 by adding an option to ManagePurchase that opens a dialog to remove an expiring purchase.

Testing

Point your hosts file at your sandbox.
Apply this patch: D906-code
Visit /purchases/:site/:purchase_id for a site/purchase that has auto-renew disabled and is outside the refund period.
You should see an option to 'Remove [product]'. Clicking it will open the dialog above.
Click 'Remove' and assert that the buttons are disabled, and that you are eventually redirected to /purchases with a notice indicating that the purchase was removed.
Assert that the purchase that was removed no longer appears in the list below.
Product review
Code review

stephanethomas · 2016-01-20T17:50:28Z

client/me/purchases/remove-purchase/index.jsx

+
+	getInitialState() {
+		return {
+			isDialogOpen: false


Shouldn't we name this property showDialog as we did in other components? That's also what the readme file suggests in its examples.

I'm a fan of prefixing boolean values with "is" to indicate that they are boolean. showDialog without context sounds like it actually shows the dialog (like closeDialog, which is a method name this file uses). A couple checkout components use isDialogVisible for this value, presumably for the same reason.

I'm this sort of fan as well - I just wanted to stay consistent with the rest of our code. Looks like you updated it to isDialogVisible, that's perfect.

drewblaisdell · 2016-01-21T01:37:19Z

One outstanding issue here (possibly the only one) is that when the /me/purchases/:purchase_id/remove endpoint returns new purchases, these new purchases are added to PurchasesStore, but the purchase that was removed remains in memory and still appears on PurchasesList until the page is refreshed.

stephanethomas · 2016-01-21T11:11:51Z

client/me/purchases/manage-purchase/index.jsx

@@ -21,6 +22,7 @@ import NoticeAction from 'components/notice/notice-action';
 import { oldShowcaseUrl } from 'lib/themes/helpers';
 import paths from '../paths';
 import PaymentLogo from 'components/payment-logo';
+import RemovePurchase from '../remove-purchase';


This should be one line below in order to keep imports sorted.

mikeshelton1503 · 2016-01-21T13:02:31Z

For the remove option, let's use the Trash pattern by adding the trash can icon. Also, let's remove the arrow icon from the right since it's not going taking you to a new screen:

stephanethomas · 2016-01-21T16:06:12Z

I've removed some old code, fixed expired purchases that weren't taken into account, and updated the remove link to look as requested.

drewblaisdell · 2016-01-22T01:13:42Z

Thanks @stephanethomas. I added da94c39 to remove purchases from the PurchasesStore when they are not present in the response from either endpoint that returns purchases. This should be ready for review now.

drewblaisdell · 2016-01-22T01:15:10Z

client/me/purchases/remove-purchase/index.jsx

+
+		return (
+			<CompactCard className="remove-purchase__card" onClick={ this.openDialog }>
+				<a >


@stephanethomas Do you think it is better to use an anchor with no href attribute here, or use cursor: pointer;?

The problem with using an hrefattribute in this anchor is that the cursor will only change if you hover the link itself whereas we probably want to change it whenever the cursor enters the card (especially since the onclick event applies to the whole card). So cursor: pointer; would be needed anyway (@ebinnion or @lezama: that's probably something you want to fix in the <DeleteUser /> component).

Note I had to inject this anchor in this CompactCard because I wasn't able to use its href prop in a way which would give me similar results.

Note I don't mind adding href="#" to this anchor since this is pretty much a convention. If not, we should get rid of this unnecessary white space though.

fabianapsimoes · 2016-01-22T11:19:36Z

We have a different copy for the dialog when removing domains (#1402 (comment)):

Remove [domain name]
This will remove [domain name] from your account. By removing, you are canceling the domain registration. This may stop you from using it again, even with another service.

Right now, it shows the same copy for all kinds of upgrades:

I wonder if we should tweak the confirmation notice as well. Right now it says The purchase was removed from [site], but maybe we want to give more context for domain removals? Not sure. cc @mikeshelton1503

fabianapsimoes · 2016-01-22T11:22:24Z

This doesn't look great in smaller screens if the domain name is a bit long:

fabianapsimoes · 2016-01-22T11:25:31Z

I tried removing the domain fabianaupgrades20151120125457.net from my account, but that left the Domain Mapping associated to it behind:

I haven't been able to test this with a domain that had privacy enabled as well. I'd expect that to leave a subscription in the admin tools which won't be displayed in /purchases.

Edit: Just tested that case. It does exactly what I expected above ^.

fabianapsimoes · 2016-01-22T12:02:33Z

Right now, removing a plan will leave the domain associated to it behind. That can be confusing to users looking to get rid of all their purchases with WordPress.com. We should add some messaging explaining the domain will remain in the account. cc @mikeshelton1503

fabianapsimoes · 2016-01-22T12:05:28Z

Related to the issue I reported here, I noticed that removing a domain does not cancel its registration. I've tested this with both testingupgrades.org and fabianaupgrades20151120125457.net, and the registrations should have been canceled with WWD.

fabianapsimoes · 2016-01-22T12:15:15Z

@Lochlaer I'm curious how well this flow works for Google Apps. Could you give it a try?

You should see:

"Cancel and Refund" within the refund window, which will remove the subscriptions and trigger a refund.
"Cancel" outside of the refund window, which will just disable auto-renewal and leave the subscriptions behind, still active until expiration.
"Remove" also outside the refund window after clicking "Cancel" (it might take a few seconds for the option to appear). This will remove the subscriptions and should terminate Google Apps use.

Seems everything works as expected on WordPress.com. Is it all OK from Google's end as well?

stephanethomas · 2016-01-22T15:25:26Z

I have updated the dialog to display a custom text for domain registrations. I have also fixed the heading on small screens. I'm not a big fan of this fix but otherwise it would mean updating styles from the <Dialog /> shared component - something that is always tricky (note we used a similar approach in the checkout).

`ManagePurchase`

…se page We don't need this text any longer since we moved removal of purchase into a dedicated component.

Users should now be able to remove any purchase that is not subscribed expect when it is included in a plan.

…e page

This also removes the chevron on the right.

Previously, purchases that were absent from the request to `/me/purchases` were kept in memory.

…urchases

…ge Purchase page This fixes a display issue that happens with small screens when the product name or the domain name are too long.

… Purchase page This fixes a display issue that happens with small screens when the product name or the domain name are too long and can't be wrapped.

…ponent

…Purchase page This also adds a default url to an empty anchor.

This adds the name of the purchase - except for domains where this removes the site url - to match the copy used in the dialog. This also removes an unnecessary component in a translation as well as a variable.

This also adds information for translators and removes a tag that cannot contain block-level elements.

Purchases: Add option to remove expiring purchases

drewblaisdell added [Status] In Progress [Type] Task [Feature] Purchase Management Related to managing purchases such as subscriptions, plans, history, auto-renew, cancellation, etc. labels Jan 20, 2016

drewblaisdell changed the title ~~Purchases: Add option to remove a purchase~~ Purchases: Add option to remove expired purchases Jan 20, 2016

drewblaisdell self-assigned this Jan 20, 2016

stephanethomas reviewed Jan 20, 2016
View reviewed changes

drewblaisdell changed the title ~~Purchases: Add option to remove expired purchases~~ Purchases: Add option to remove expiring purchases Jan 20, 2016

drewblaisdell force-pushed the add/remove-option-purchases branch 4 times, most recently from 361fa31 to b7a4342 Compare January 21, 2016 01:31

stephanethomas reviewed Jan 21, 2016
View reviewed changes

drewblaisdell force-pushed the add/remove-option-purchases branch 2 times, most recently from 5b25c11 to da94c39 Compare January 22, 2016 01:05

drewblaisdell added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Jan 22, 2016

drewblaisdell reviewed Jan 22, 2016
View reviewed changes

fabianapsimoes added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 22, 2016

stephanethomas mentioned this pull request Jan 26, 2016

Purchases: Move purchases logic in the API #2783

Closed

drewblaisdell and others added 21 commits January 26, 2016 11:08

Purchases: Use CompactCard instead of VerticalNavItem in

3cb00c2

`ManagePurchase`

Purchases: Add dialog for removing a purchase

22edc94

Purchases: Add max-width to RemovePurchase

e10eb46

Purchases: Add action for removing a purchase

1ab2135

Purchases: Display a notice when removing a purchase

dcc1a37

Purchases: Disable the dialog buttons when removing a purchase

8465068

Purchases: Remove former remove purchase information in Manage Purcha…

7c05831

…se page We don't need this text any longer since we moved removal of purchase into a dedicated component.

Purchases: Allow almost all expiring and expired purchases to be removed

42a5d1d

Users should now be able to remove any purchase that is not subscribed expect when it is included in a plan.

Purchase: Allow expired purchases to be removed in the Manage Purchas…

1e937b1

…e page

Purchases: Add trash icon to remove link on Manage Purchase page

ba6b71f

This also removes the chevron on the right.

Purchases: Remove purchases from PurchasesStore when they are deleted

e818775

Previously, purchases that were absent from the request to `/me/purchases` were kept in memory.

Purchases: Display custom warning when removing domain registration p…

abbbe4f

…urchases

Purchases: Add missing prop validation in Remove Purchase component

2cef4f2

Purchases: Fix heading that overlaps on text in remove dialog in Mana…

db93904

…ge Purchase page This fixes a display issue that happens with small screens when the product name or the domain name are too long.

Purchases: Add domain-specific notice when removing a domain purchase

5dec2e6

Purchases: Show ellipsis for long headings in remove dialog in Manage…

6e94d32

… Purchase page This fixes a display issue that happens with small screens when the product name or the domain name are too long and can't be wrapped.

Purchases: Remove unnecessary string templates in Remove Purchase com…

71033b8

…ponent

Purchases: Update incorrect variable name in remove dialog in Manage …

d9053a0

…Purchase page This also adds a default url to an empty anchor.

Puchases: Update removal success notices in Manage Purchase page

8688131

This adds the name of the purchase - except for domains where this removes the site url - to match the copy used in the dialog. This also removes an unnecessary component in a translation as well as a variable.

Purchases: Add message for plans with an included domain

a153bc8

Purchases: Update copy in remove dialog in Manage Purchase page

4eb6160

This also adds information for translators and removes a tag that cannot contain block-level elements.

drewblaisdell force-pushed the add/remove-option-purchases branch from 4e058a6 to 4eb6160 Compare January 26, 2016 19:09

drewblaisdell added a commit that referenced this pull request Jan 26, 2016

Merge pull request #2602 from Automattic/add/remove-option-purchases

9790bc7

Purchases: Add option to remove expiring purchases

drewblaisdell merged commit 9790bc7 into master Jan 26, 2016

drewblaisdell deleted the add/remove-option-purchases branch January 26, 2016 19:18

drewblaisdell removed the [Status] Ready to Merge label Jan 26, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Purchases: Add option to remove expiring purchases #2602

Purchases: Add option to remove expiring purchases #2602

drewblaisdell commented Jan 20, 2016

stephanethomas Jan 20, 2016

drewblaisdell Jan 20, 2016

stephanethomas Jan 21, 2016

drewblaisdell commented Jan 21, 2016

stephanethomas Jan 21, 2016

mikeshelton1503 commented Jan 21, 2016

stephanethomas commented Jan 21, 2016

drewblaisdell commented Jan 22, 2016

drewblaisdell Jan 22, 2016

stephanethomas Jan 22, 2016

stephanethomas Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

stephanethomas commented Jan 22, 2016

Purchases: Add option to remove expiring purchases #2602

Purchases: Add option to remove expiring purchases #2602

Conversation

drewblaisdell commented Jan 20, 2016

stephanethomas Jan 20, 2016

Choose a reason for hiding this comment

drewblaisdell Jan 20, 2016

Choose a reason for hiding this comment

stephanethomas Jan 21, 2016

Choose a reason for hiding this comment

drewblaisdell commented Jan 21, 2016

stephanethomas Jan 21, 2016

Choose a reason for hiding this comment

mikeshelton1503 commented Jan 21, 2016

stephanethomas commented Jan 21, 2016

drewblaisdell commented Jan 22, 2016

drewblaisdell Jan 22, 2016

Choose a reason for hiding this comment

stephanethomas Jan 22, 2016

Choose a reason for hiding this comment

stephanethomas Jan 22, 2016

Choose a reason for hiding this comment

fabianapsimoes commented Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

fabianapsimoes commented Jan 22, 2016

stephanethomas commented Jan 22, 2016