Self service downgrade: Show refund text for eligible users#101893
Self service downgrade: Show refund text for eligible users#101893claudiucelfilip merged 6 commits intotrunkfrom
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
|
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~229 bytes added 📈 [gzipped]) DetailsSections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
|
Thanks for the ping. A quick broader ping also to @Automattic/dotcom-design for any nuance I'm missing, in my supporting-work capacity. Since this is mostly a text-change (thanks for including the screenshots), it seems like this one can just move forward. However I can't help but look at the mockup and find a lot of details missing: Note that in the following I'm reviewing purely based on the screenshot, as this is a flow that's hard to test in my dev env.—so I may be missing nuance. Let me know. The space below the list of features you're losing is slightly larger than it is above. In the mockup, it's the same spacing: This gap may be addressed by other semantic changes I'll get to in a minute (line-heights, etc)—and we want to avoid that you write custom CSS to address this. But coming from a WordPress core context, we would've used something like a VStack component to ensure perfect spacing. Sharing mostly for awareness. The buttons look like core components. Nice. Please set the "Downgrade to Personal" button to destructive so it's red. I was looking through the general Settings screens, and honestly it looks like the font sizes are a bit all over the place, headings at 14px and default text at 16px, it's a curious mess. Because I don't want you to add additional CSS just for this one screen, I'll defer some of these details to you on how to implement in a clean way with as little CSS as possible, so it's just the componentry. But really, we should have a single font size for all content inside this card. All the text—heading, description, list items, button text, "Need help with purchase", all of that text should be the same font size. Because there's such a mix of font sizes, it's unclear what is the near-term best choice here. But I expect the long term choice to be: 13px font size for all body text. |
|
Thank you @jasmussen for outlining these. |
|
Should we also show the actual refund amount here too? If they're getting money back it's probably a good idea to tell them in advance how much it will be. Note that in the current self-serve downgrade flow that's live in production (i.e. the one that's hidden away in the cancellation flow) we do already show the exact amount of money that will be refunded -- see screenshot below. So you could hopefully look at that code for where the number comes from and use it here too. |
|
@DavidRothstein Thanks for reviewing this!
Good point. I copied over the approach from the hidden downgrade flow from cancellation and applied it here. |
ddc22
left a comment
There was a problem hiding this comment.
LGTM 🚢
Tests well.
We could probably move some logic to a hook. I will try to work on this on a subsequent PR.
|
Hey @claudiucelfilip ! Thanks for reaching out on Slack. A little feedback on the copy mentioned in case it helps:
This is generally clear, but there are a few potential improvements we could make:
As it looks like this is a refund, how about:
Or if the credit's transferred from the older more expensive plan to the new plan, 'refund' might be confusing, and something like this might be a better fit:
Just to cover all bases, if it's one of those 'it depends' moments, a fallback might be:
(Although the more precise we can be, the better!) Hope that helps! |
|
Thank you @michaelpick! |
|
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/17317926 Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday. Thank you @claudiucelfilip for including a screenshot in the description! This is really helpful for our translators. |
|
Hm, it seems to be showing the wrong refund amount (see screenshot below). $252 is the expected refund for downgrading to Personal (not Premium). $204 is the expected amount for Premium (and the actual amount that I receive if I go through with the downgrade)... However, this may be a separate bug. Can we also change the wording here to remove the word "immediately"? We shouldn't say that because it won't necessarily show up in their bank account right away. We normally say things more like "you will receive a refund of $X" but I think the wording in this pull request is OK too as long as it doesn't imply that the refund is immediate. Speaking of which, we usually send a confirmation email after a refund (which explains more about how/when the refund will appear in their bank account) but for downgrades we apparently don't. This applies for both the old (hidden) downgrade flow and the new one, so I've made a separate issue about it here: https://github.com/Automattic/payments-shilling/issues/3660 |
| const amount = | ||
| isRefundable( purchase ) && | ||
| Array.isArray( refundOptions ) && | ||
| refundOptions[ 0 ]?.refund_amount |
There was a problem hiding this comment.
I debugged a bit and I think this is the reason it's showing the wrong refund amount (see my earlier comment). The code is using the first element of the array here, but it actually needs to pick the correct element of the array depending on which product is being downgraded to.
As you mentioned, you basically just copied this code from the existing self-serve downgrade flow within cancellation, so a version of the bug exists there too. The code has been there for years, but perhaps it never mattered much in practice (until recently when the server started returning more downgrade options than it did before). Eventually it will need to be fixed in both places.
There was a problem hiding this comment.
Thanks for debugging this. I added a search to match to_product_id to the target plan's.
|
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/17317926 Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday. Thank you @claudiucelfilip for including a screenshot in the description! This is really helpful for our translators. |
|
Translation for this Pull Request has now been finished. |




Proposed Changes
When you downgrade from %(currentPlan)s to %(targetPlan)s, we'll immediately process a refund of %(amount)s to your original payment method.Refund availablebeneath the Downgrade button (similar to Cancel)Why are these changes being made?
Testing Instructions
Refund availabletext in Downgrade buttonRefund availableshould not be visiblePre-merge Checklist