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

Update plans to Development/Production/Enterprise without deploy-api code changes #953

Merged
merged 3 commits into from
Oct 16, 2024

Conversation

fancyremarker
Copy link
Member

This PR changes the plans we show on /plans to match . The new plans already exist in deploy-api in production (see related aptible/deploy-api#1586). Note that the new Development and Production plans have no included resources and no base fee. This is intentional, and should make it easier to simplify or deprecate ActivePlans in the future.

To show how the /plans change will change for existing users, I created a Notion doc showing the before/after for (nearly) every type of plan/payment info combination that exists today. 👉 QA: Changes to /plans (Notion)

The approach of making no changes to deploy-api has one downside: customers can "downgrade" to Development even if they have an existing dedicated stack. However, because this has no impact on their invoice (Development and Production both have no included resources and no base fee), combined with the fact that a similar bug exists today, I think this is acceptable.

To summarize, here are the expected acceptance criteria for this feature (each of which I've tested):

  • ✅ A user without a payment method (i.e. a trial user) should not be able to upgrade to Development or Production
  • ✅ A user currently on the Growth or Scale plan should be able to "upgrade" to Production
  • ✅ A user currently on the Enterprise plan should not be able to switch plans
  • ✅ A user should not see the Starter, Growth or Scale plans unless they are currently on that plan
  • ✅ A user on the Starter, Growth or Scale plan should see that plan as "Current Plan"
  • 🚫 A user with a dedicated stack should not be able to "downgrade" to Development (see above, I'm saying this is OK for now and we can create a ticket to fix later)

Related PRs

  • aptible/deploy-api#1586
  • aptible/deploy-ui#797

Updated mock plan `name` to match descriptive text
const canRequestStack = ["growth", "scale", "enterprise"].includes(
selectedPlan.name,
);
const canRequestStack = [
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: we might consider moving this logic to deploy-api in the future.

Copy link
Member

@neurosnap neurosnap left a comment

Choose a reason for hiding this comment

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

Nice work! I have one addition to add a loading state to the plan buttons:

diff --git a/src/ui/shared/plan.tsx b/src/ui/shared/plan.tsx
index 8f288d65..b9bc1bf2 100644
--- a/src/ui/shared/plan.tsx
+++ b/src/ui/shared/plan.tsx
@@ -1,5 +1,5 @@
-import { selectPlansForView } from "@app/deploy";
-import { useSelector } from "@app/react";
+import { selectPlansForView, updateActivePlan } from "@app/deploy";
+import { useLoader, useSelector } from "@app/react";
 import { capitalize } from "@app/string-utils";
 import type { DeployActivePlan, DeployPlan, PlanName } from "@app/types";
 import { Button, ButtonLinkExternal } from "./button";
@@ -52,6 +52,7 @@ const PlanButton = ({
   onClick: (e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => void;
   selected: boolean;
 }) => {
+  const loader = useLoader(updateActivePlan);
   if (contactUs) {
     return (
       <ButtonLinkExternal href="https://aptible.com/contact" className="w-full">
@@ -77,7 +78,7 @@ const PlanButton = ({
   }
 
   return (
-    <Button className="w-full" onClick={onClick}>
+    <Button className="w-full" onClick={onClick} isLoading={loader.isLoading}>
       Select Plan
     </Button>
   );
@@ -381,7 +382,7 @@ export const Plans = ({
     <div
       className={`grid ${col} md:grid-cols-2 grid-cols-1 gap-4 lg:mx-0 mx-10`}
     >
-      {plansToShow.map((plan, index) => (
+      {plansToShow.map((plan) => (
         <PlanCard
           key={plan.name}
           plan={plan}

Copy link
Member

@neurosnap neurosnap left a comment

Choose a reason for hiding this comment

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

nice!

@fancyremarker fancyremarker merged commit 2077ddf into main Oct 16, 2024
6 checks passed
@fancyremarker fancyremarker deleted the plan-changes branch October 16, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants