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

[dashboard] Re-implement 'Cancel Downgrade' in Plans page #3873

Merged
merged 5 commits into from
Apr 20, 2021

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented Apr 9, 2021

Fixes #3795

Has no effect on Core / OSS.

How to test:

  • Log in to https://jx-cancel-downgrade.staging.gitpod-io-dev.com/plans
  • Upgrade to Professional (and complete billing)
  • Once the upgrade is through, downgrade to Personal
  • Once the downgrade is scheduled, and the "Cancel" link appears, click on it
  • The modal should allow you to cancel the scheduled downgrade
  • (Also, creating Team and using the invite link yourself should still work)

@jankeromnes jankeromnes force-pushed the jx/cancel-downgrade branch 5 times, most recently from 2f03f6d to a7ebf75 Compare April 14, 2021 08:15
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Apr 14, 2021

🎰
/werft run

👍 started the job as gitpod-build-jx-cancel-downgrade.9

@jankeromnes jankeromnes force-pushed the jx/cancel-downgrade branch 3 times, most recently from e1b77ab to efe3566 Compare April 15, 2021 13:56
@jankeromnes
Copy link
Contributor Author

IO preview: https://jx-cancel-downgrade.staging.gitpod-io-dev.com/workspaces/

Currently looks like this:

Screenshot 2021-04-14 at 11 04 24

Screenshot 2021-04-14 at 11 04 34

We might want to adjust the texts a bit.

@AlexTugarev and/or @gtsiolis, could you please take a look when you have time?

@jankeromnes jankeromnes marked this pull request as ready for review April 15, 2021 15:32
@jankeromnes jankeromnes requested review from AlexTugarev and gtsiolis and removed request for AlexTugarev April 15, 2021 15:32
@jankeromnes jankeromnes force-pushed the jx/cancel-downgrade branch 2 times, most recently from 8471f0b to 0d002fd Compare April 19, 2021 08:55
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Apr 19, 2021

Alright, should be good to go now. 🚀

@AlexTugarev & @gtsiolis please take another look. 😊 🙏

@AlexTugarev
Copy link
Member

AlexTugarev commented Apr 19, 2021

Just tested the downgrade. Here is what I noticed:

  • Downgrade scheduled is rendered with a cursor pointer
  • Cancel link does not appear immediately, actually it took a couple of reloads to get it.

Suggestion: on hover of Downgrade scheduled you can render Cancel downgrade. WDYT?

Cancellation is also not an instant thing, right? After refresh it still presents with Cancel button. It looks like there are a lot of intermediate states with Chargebee :-(

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Apr 19, 2021

Just tested the downgrade.

Awesome, thanks!

  • Downgrade scheduled is rendered with a cursor pointer

Good catch. I can fix that.

  • Cancel link does not appear immediately, actually it took a couple of reloads to get it.

That's expected, as we wait for the Chargebee callback here. (When there is a pendingDowngradeToPlan, we optimistically show that the downgrade is scheduled, and poll for an update every 10 seconds. Next, when there finally is a scheduledDowngradePlanId, we remove the pending state, and show the Cancel action.)

P.S. You don't need to reload, as the polling will already update the state when Chargebee finally calls back.

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Apr 19, 2021

Cancellation is also not an instant thing, right? After refresh it still presents with Cancel button. It looks like there are a lot of intermediate states with Chargebee :-(

That's a bug in my pendingDowngradeCancellation then (if you cancel the downgrade, it should never show the Cancel button again, unless Chargebee doesn't call back within 5 minutes -- even if you reload multiple times).

EDIT: Found the bug:

https://github.com/gitpod-io/gitpod/blob/1a6d6a197e72406276ee43dc8522830d64c5ef02/components/dashboard/src/settings/Plans.tsx#L199-L201

Condition for successful downgrade cancellation should be !!accountStatement && !scheduledDowngrade (because everything is undefined on load until we get the accountStatement). Fixed ✅

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Curious why we need this cancel button in the first place? Is there a need for an undo action?

Since there's now a) confirmation to upgrade or downgrade through modals and b) adequate information on the action and potential account changes through alert components, this undo action seems to be redundant, right? 😭

What do you think? Let me know if I'm missing something. 💭

FWIW, an undo action on the workspaces deletion action could be great! ⭐

components/dashboard/src/settings/Plans.tsx Show resolved Hide resolved
components/dashboard/src/settings/Plans.tsx Show resolved Hide resolved
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Two more minor comments! 🧡

components/dashboard/src/settings/Plans.tsx Show resolved Hide resolved
components/dashboard/src/settings/Plans.tsx Show resolved Hide resolved
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

One last comment for this round:

@jankeromnes if you think this a) covers some edge cases and b) restores some broken functionality it should be fine to merge. However, it terms of UX I'd vote for removing this undo functionality with the cancel button in the long run unless needed for technical reasons due to the integration with Chargebee. 🤷

See #3873 (comment).

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Apr 19, 2021

Many thanks for the reviews, @AlexTugarev and @gtsiolis!

@gtsiolis This was more about re-implementing a feature that was available in the old dashboard, but maybe it's okay to leave it out? The use case goes something like this:

  • I upgrade to the Professional plan. It's great!
  • Actually maybe I want the Personal plan, so I downgrade to it. However, the will only happen at the end of my billing cycle (i.e. in 3 weeks or so), so it says "Downgrade scheduled" for several days
  • Actually, the Professional plan is pretty great. I now want to stick with it!

Currently: The downgrade is still schedule for 2-3 weeks from now. To cancel it, I need to navigate Billing very deep in order to find "delete pending changes" or similar. (Otherwise, I forget about this, then the downgrade happens automatically, and maybe I "forget" to upgrade again.)

With this feature: I can click on "Cancel" to cancel the scheduled downgrade, and stick with Professional.

@gtsiolis
Copy link
Contributor

gtsiolis commented Apr 19, 2021

Thanks for explaining this in #3873 (comment) @jankeromnes! 🙏

Makes sense now. Since we're not charging per hour usage or something similar this approach seems better for now. 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[dashboard] Re-implement 'Cancel Downgrade'
3 participants