Tys paywall #1721
Open
Tys paywall #1721
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updated where the plan info was being pulled from to match the current setup with the pro and basic plan.
Added logic for the title and button text of the pricing card for the current plan.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to e8ad0c9 in 3 minutes and 34 seconds
More details
- Looked at
825lines of code in9files - Skipped
0files when reviewing. - Skipped posting
11drafted comments based on config settings.
1. apps/studio/electron/main/events/payments.ts:9
- Draft comment:
Default plan passed as PRO—verify this is intended for all checkout invocations. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
The comment is asking for verification of intention, which violates our rules. It's not pointing out a clear issue - just asking the author to double-check something. The use of a default parameter is a deliberate programming choice that the author made. If it was incorrect, it would be caught during testing.
Maybe the default plan being PRO could lead to unexpected billing issues if used incorrectly? Maybe this is a security concern?
While billing is important, the comment isn't pointing out an actual issue - it's just asking for verification. If there was a specific security or billing concern, it should be stated directly.
Delete the comment as it violates our rule about not asking authors to verify their intentions or double-check things.
2. packages/models/src/supabase/db.ts:719
- Draft comment:
DB enum for usage_plan_values only supports 'free' and 'pro'; update this to include 'launch' and 'scale' to match the UsagePlanType. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
The comment references a type (UsagePlanType) that isn't visible in this file. Without seeing that type, I can't verify if the enum really needs those values. The PR is explicitly changing the enum from 'basic' to 'free', suggesting this is an intentional simplification. Making cross-file assumptions without evidence violates the review rules.
The comment could be correct - maybe there is a UsagePlanType in another file that needs to stay in sync with this enum. But I can't verify that without more context.
The rules state that if understanding requires more context like seeing other files, we should delete the comment. We can't verify the need for 'launch' and 'scale' values from this file alone.
Delete the comment since it relies on cross-file context we can't verify, and the change from 'basic' to 'free' appears intentional.
3. packages/models/src/usage/index.ts:1
- Draft comment:
Ensure the new plan types (LAUNCH, SCALE) in UsagePlanType are synchronized with your backend/DB enums. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the author to ensure synchronization between the code and the backend/DB enums. It is not a direct suggestion or a specific request for a test or code change. It falls under the category of asking the author to ensure something, which is against the rules.
4. apps/studio/src/components/Modals/Subscription/PricingPage.tsx:273
- Draft comment:
Double-check button text for plan downgrades (e.g., for the Pro plan card, text says 'Downgrade to Pro' even if current plan might not match the intended logic). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to double-check the button text logic, which violates the rule against asking the author to double-check things. It doesn't provide a specific suggestion or point out a clear issue.
5. apps/studio/electron/main/events/payments.ts:9
- Draft comment:
Consider removing the redundant 'await' in the return statement (i.e. use 'return checkoutWithStripe(plan)' instead of 'return await checkoutWithStripe(plan)'). - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%
None
6. apps/studio/src/components/Modals/Subscription/PricingPage.tsx:72
- Draft comment:
The checkout functions (startProCheckout, startLaunchCheckout, startScaleCheckout) share similar logic. Consider refactoring the common parts into a shared helper to reduce duplication. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. apps/studio/src/components/Modals/Subscription/PricingPage.tsx:174
- Draft comment:
In manageSubscription(), setting isCheckingOut to UsagePlanType.FREE might be confusing if 'FREE' is used as a flag for the subscription portal. Confirm that this is the intended identifier for the free/downgrade path. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
The code shows consistent usage of UsagePlanType.FREE throughout. The comment suggests there might be confusion about using FREE as a flag, but there's no evidence of this - it's used the same way as other plan types (PRO, LAUNCH, SCALE) for tracking checkout state. The comment is speculative and asks for confirmation without clear evidence of an issue.
I could be missing some context about how the subscription portal works in the backend. Maybe there's a reason why FREE would be problematic for the portal that isn't visible in this file.
Even if there were backend concerns, those would be handled by the MANAGE_SUBSCRIPTION channel, not the frontend state management. The frontend usage is consistent and clear.
The comment should be deleted as it's speculative, asks for confirmation without evidence of an issue, and the code shows consistent and clear usage of UsagePlanType.FREE.
8. apps/studio/src/lib/user/subscription.ts:16
- Draft comment:
restoreCachedPlan() currently ignores the cachedPlan value and unconditionally sets the plan to PRO. Consider using the cached value if available or document why it’s being overridden. - Reason this comment was not posted:
Marked as duplicate.
9. packages/models/src/supabase/db.ts:718
- Draft comment:
The DB enum 'usage_plan_values' now only includes 'free' and 'pro'. With new plan types (LAUNCH and SCALE) defined in the UI, this DB enum should be updated to include those values to ensure consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The comment makes assumptions about UI values that I can't verify. It's suggesting a schema change based on information not present in this diff or file. The comment also seems to be mixing concerns between UI labels and database schema design. Without seeing the UI code or understanding the full system design, I can't verify if this suggestion is correct.
The comment could be correct - maybe there really are LAUNCH and SCALE plan types in the UI that should be reflected in the database. I don't have enough context to definitively say it's wrong.
However, per the rules, I should only keep comments when there is STRONG EVIDENCE they are correct. Making database schema changes based on unverified UI values would be risky.
Delete the comment since it relies on information outside this file and makes assumptions about system design that can't be verified from the available code.
10. apps/studio/src/components/Modals/Subscription/PricingPage.tsx:17
- Draft comment:
The PR description mentions onboarding docs, but these changes implement a new subscription paywall with updated pricing options. Please update the PR description to accurately reflect the changes. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. apps/studio/src/locales/en/translation.json:130
- Draft comment:
The 'pro' plan feature 'Unlimited monthly chats' may be inconsistent with similar texts (e.g., '{{monthlyMessages}} AI messages a month' in other plans). Consider aligning the terminology for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_zx5oSrA1blOMyhHx
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -14,7 +14,7 @@ export class SubscriptionManager { | |||
|
|
|||
| private restoreCachedPlan() { | |||
| const cachedPlan = localStorage.getItem('currentPlan'); | |||
| this.plan = (cachedPlan as UsagePlanType) || UsagePlanType.BASIC; | |||
| this.plan = UsagePlanType.PRO; | |||
There was a problem hiding this comment.
restoreCachedPlan overwrites the cached plan with PRO, ignoring localStorage value.
Suggested change
| this.plan = UsagePlanType.PRO; | |
| this.plan = cachedPlan ? (cachedPlan as UsagePlanType) : UsagePlanType.PRO; |
| <div className="flex-shrink-0"> | ||
| <h2 className="text-[18px] font-medium">{plan}</h2> | ||
| <p className="text-[40px] font-medium flex items-baseline"> | ||
| <span>{price.split('/')[0]}</span> |
There was a problem hiding this comment.
Using price.split('/') to extract the amount and hardcoding /month could cause issues if the price string format changes. Consider parameterizing the billing period.
| } | ||
| }; | ||
|
|
||
| const startLaunchCheckout = async () => { |
There was a problem hiding this comment.
Hey! I noticed these functions have pretty similar logic — I think it might be cleaner to combine them into one function with a parameter to handle the differences. What do you think?
@thughey
There was a problem hiding this comment.
@spartan-vutrannguyen That sounds good 👍
| buttonText={ | ||
| userManager.subscription.plan === | ||
| UsagePlanType.PRO | ||
| ? `You are on ${t('pricing.plans.pro.name')}` |
There was a problem hiding this comment.
Should we include the full text in the translation file, or just the key parts? What’s your take on that?
There was a problem hiding this comment.
Whichever would be easier for you to manage/edit in the future as the plans need edits.
| </div> | ||
| </motion.div> | ||
| <div className="flex flex-row gap-4 w-full justify-center pb-4 flex-grow"> | ||
| <PricingCard |
There was a problem hiding this comment.
It might be cleaner if we use UsagePlanType directly for the logic checks and text display inside PricingCard, instead of passing it down as a prop. What do you think?
There was a problem hiding this comment.
@spartan-vutrannguyen Yes! If you see a better way to organize - go for it 🙌
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Added three pricing options with logic in the buttons based on the plan a user is currently on.
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Adds new pricing plans and updates subscription logic and UI components to handle these plans.
FREE,LAUNCH, andSCALEinUsagePlanType.checkoutWithStripe()inindex.tsto accept aplanparameter.listenForPaymentMessages()inpayments.tsto handle different plans.getPlanFromServer()insubscription.tsto handle new plan types.PricingCardinPricingCard.tsxto display new plans and handle button logic.SubscriptionModalinPricingPage.tsxto include new plans and checkout logic.translation.jsonfor both English and Chinese.usage_plan_valuesenum indb.tsto includefreeandpro.This description was created by
for e8ad0c9. It will automatically update as commits are pushed.