Conversation
Console (appwrite/console)Project ID: Tip Teams feature lets you group users with membership management and role permissions |
WalkthroughThe members page component was updated to implement a new service limit-based gating system for the add members button. The change replaces a simple binary free-member check with a more nuanced approach that evaluates whether the organization has reached its member service limit. New imports were added from billing and system stores to access service limit information, read-only status, and grace period override configuration. The button disablement logic now considers the current plan's member limit, read-only state, and any applicable grace period overrides. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/organization-[organization]/members/+page.svelte (1)
81-96:⚠️ Potential issue | 🟡 MinorTooltip message is misleading when the button is disabled due to read-only state.
isButtonDisabledis now true for two distinct reasons — a reached member limit (Case B) and the account being in read-only/billing-freeze state ($readOnly && !GRACE_PERIOD_OVERRIDE, Case A). The tooltip content, however, only accounts for Case B: it shows either "Upgrade to add more members" or "You've reached the members limit for the X plan". When the button is disabled purely because the account is read-only, neither message is accurate, which will confuse users into thinking they need to change their plan instead of resolving a billing issue.Add a third branch that checks the read-only condition first:
🐛 Proposed fix
<div slot="tooltip"> - {!supportsMembers - ? 'Upgrade to add more members' - : `You've reached the members limit for the ${$organization?.billingPlanDetails.name} plan`} + {$readOnly && !GRACE_PERIOD_OVERRIDE + ? 'Your organization is in read-only mode due to a billing issue' + : !supportsMembers + ? 'Upgrade to add more members' + : `You've reached the members limit for the ${$organization?.billingPlanDetails.name} plan`} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(console)/organization-[organization]/members/+page.svelte around lines 81 - 96, The tooltip currently only distinguishes member-limit vs upgrade (via supportsMembers) but not the read-only billing freeze case; update the tooltip rendering inside Tooltip (where Tooltip, isButtonDisabled, supportsMembers, $readOnly, and GRACE_PERIOD_OVERRIDE are used) to check the read-only condition first (if $readOnly && !GRACE_PERIOD_OVERRIDE) and return a clear billing/read-only message (e.g., account is read-only/frozen — resolve billing or contact support) before falling back to the existing supportsMembers branch that shows upgrade or plan-limit messages using $organization?.billingPlanDetails.name.
🧹 Nitpick comments (1)
src/routes/(console)/organization-[organization]/members/+page.svelte (1)
48-49:limit !== 0is redundant — simplifyisLimited.After line 48,
limitcan never be0: the|| Infinitycoerces any falsy return (including0) toInfinity, so thelimit !== 0 &&guard in line 49 is alwaystrue.isLimitedis equivalent to justlimit < Infinity(i.e.,limit !== Infinity).♻️ Proposed simplification
-$: isLimited = limit !== 0 && limit < Infinity; +$: isLimited = limit < Infinity;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(console)/organization-[organization]/members/+page.svelte around lines 48 - 49, The isLimited reactive declaration is using a redundant check (`limit !== 0`) because `limit` is set with `getServiceLimit('members', null, $currentPlan) || Infinity`, so 0 would already coerce to Infinity; update the reactive statement that defines isLimited (the `$: isLimited = ...` line) to remove the `limit !== 0 &&` part and use only `limit < Infinity` (or `limit !== Infinity`) so it correctly reflects whether the members service is actually limited based on the `limit` value returned by `getServiceLimit`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/routes/`(console)/organization-[organization]/members/+page.svelte:
- Around line 81-96: The tooltip currently only distinguishes member-limit vs
upgrade (via supportsMembers) but not the read-only billing freeze case; update
the tooltip rendering inside Tooltip (where Tooltip, isButtonDisabled,
supportsMembers, $readOnly, and GRACE_PERIOD_OVERRIDE are used) to check the
read-only condition first (if $readOnly && !GRACE_PERIOD_OVERRIDE) and return a
clear billing/read-only message (e.g., account is read-only/frozen — resolve
billing or contact support) before falling back to the existing supportsMembers
branch that shows upgrade or plan-limit messages using
$organization?.billingPlanDetails.name.
---
Nitpick comments:
In `@src/routes/`(console)/organization-[organization]/members/+page.svelte:
- Around line 48-49: The isLimited reactive declaration is using a redundant
check (`limit !== 0`) because `limit` is set with `getServiceLimit('members',
null, $currentPlan) || Infinity`, so 0 would already coerce to Infinity; update
the reactive statement that defines isLimited (the `$: isLimited = ...` line) to
remove the `limit !== 0 &&` part and use only `limit < Infinity` (or `limit !==
Infinity`) so it correctly reflects whether the members service is actually
limited based on the `limit` value returned by `getServiceLimit`.
2 participants
What does this PR do?
before:
after:
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit