feat(workspace): add per-member monthly credit limit UI (FE-1277)#13716
Open
dante01yoon wants to merge 6 commits into
Open
feat(workspace): add per-member monthly credit limit UI (FE-1277)#13716dante01yoon wants to merge 6 commits into
dante01yoon wants to merge 6 commits into
Conversation
Owners can set or clear an optional monthly credit limit per workspace member from Settings > Members, so no single member consumes a disproportionate share of the shared credit pool. - Set credit limit dialog: radio "limit to" amount / "no limit", with positive safe-integer validation and a warning when the proposed cap is at or below usage already spent - Members table gains a "Credits used this month" column: capped rows show used / limit with a usage progress bar, uncapped rows show usage - Row overflow menu gains "Set credit limit"; the creator and the current user get a self-cap-only menu, members see no actions - Local-only store action; backend persistence and runtime enforcement are tracked separately (FE-1278) Built on the existing owner|member role model; the Admin role remains out of scope.
dosubot
Bot
added
the
size:L
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ✅ 1713 passed, 0 failed · 2 flaky📊 Browser Reports
📦 Bundle: 7.91 MB gzip 🔴 +3.16 kBDetailsSummary
Category Glance App Entry Points — 47.3 kB (baseline 47.3 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.25 MB (baseline 1.25 MB) • 🔴 +1 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 109 kB (baseline 109 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 14 added / 14 removed Panels & Settings — 550 kB (baseline 550 kB) • 🔴 +1 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed / 14 unchanged User & Accounts — 29.1 kB (baseline 29.1 kB) • 🔴 +1 BAuthentication, profile, and account management bundles
Status: 9 added / 9 removed / 1 unchanged Editors & Dialogs — 121 kB (baseline 121 kB) • 🔴 +1 BModals, dialogs, drawers, and in-app editors
Status: 6 added / 6 removed UI Components — 56.8 kB (baseline 56.8 kB) • 🔴 +1 BReusable component library chunks
Status: 13 added / 13 removed Data & Services — 270 kB (baseline 270 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 16 added / 16 removed Utilities & Hooks — 3.4 MB (baseline 3.4 MB) • 🔴 +921 BHelpers, composables, and utility bundles
Status: 26 added / 26 removed / 10 unchanged Vendor & Third-Party — 15.7 MB (baseline 15.7 MB) • 🔴 +21 BExternal libraries and shared vendor chunks
Status: 6 added / 6 removed / 10 unchanged Other — 11.9 MB (baseline 11.9 MB) • 🔴 +12.7 kBBundles that do not match a named category
Status: 110 added / 109 removed / 54 unchanged ⚡ Performance
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughWorkspace member credit usage and monthly limits are added to workspace data, local state, member menus, a feature-gated members-table column, and a localized limit-management dialog with validation. ChangesMember credit limit management
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant MemberList
participant useMembersPanel
participant dialogService
participant SetMemberCreditLimitDialogContent
participant teamWorkspaceStore
MemberList->>useMembersPanel: select set credit limit
useMembersPanel->>dialogService: showSetMemberCreditLimitDialog(member details)
dialogService->>SetMemberCreditLimitDialogContent: open dialog
SetMemberCreditLimitDialogContent->>teamWorkspaceStore: setMemberCreditLimit(userId, limit)
teamWorkspaceStore-->>MemberList: updated local member limit
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/workspace/components/dialogs/settings/MemberListItem.vue`:
- Around line 106-110: Use vue-i18n number formatting for both affected credit
displays: in
src/platform/workspace/components/dialogs/settings/MemberListItem.vue lines
106-110, obtain useI18n().n() and format both used credits and
monthlyCreditLimit with it; in
src/platform/workspace/components/dialogs/SetMemberCreditLimitDialogContent.vue
lines 80-83, pass $n(creditsUsed) to the translated warning instead of
browser-locale formatting.
In `@src/platform/workspace/composables/useMembersPanel.test.ts`:
- Line 630: Update the remove-member tests around memberMenuItems to select the
action by its visible label rather than the positional index [2]. Apply the same
change to the additional occurrence, preserving the existing behavioral
assertions while making them independent of menu ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 87df12aa-e9fe-47f2-b30c-e7a201475c10
📒 Files selected for processing (13)
src/locales/en/main.jsonsrc/platform/workspace/api/workspaceApi.tssrc/platform/workspace/components/dialogs/SetMemberCreditLimitDialogContent.test.tssrc/platform/workspace/components/dialogs/SetMemberCreditLimitDialogContent.vuesrc/platform/workspace/components/dialogs/settings/MemberListItem.vuesrc/platform/workspace/components/dialogs/settings/MembersPanelContent.test.tssrc/platform/workspace/components/dialogs/settings/MembersPanelContent.vuesrc/platform/workspace/composables/useMembersPanel.test.tssrc/platform/workspace/composables/useMembersPanel.tssrc/platform/workspace/composables/useWorkspaceUI.test.tssrc/platform/workspace/composables/useWorkspaceUI.tssrc/platform/workspace/stores/teamWorkspaceStore.tssrc/services/dialogService.ts
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #13716 +/- ##
==========================================
- Coverage 78.49% 78.23% -0.26%
==========================================
Files 1677 1684 +7
Lines 106121 96954 -9167
Branches 38118 34923 -3195
==========================================
- Hits 83297 75852 -7445
+ Misses 22022 20642 -1380
+ Partials 802 460 -342
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 476 files with indirect coverage changes 🚀 New features to boost your workflow:
|
dante01yoon
changed the title
With the new four-column members grid the right-aligned Role cell butted against the left-aligned Credits column (e.g. "Owner546 / 3,000"). Left- align Role in both the header and rows, matching the Figma layout.
Contributor
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 (2)
src/platform/workspace/components/dialogs/settings/MemberListItem.vue (1)
61-61: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winGuard optional
menuItemsbefore reading.length.
menuItemsis optional, and the parent passesmemberMenus.get(member.id), which can beundefinedfor members without actions. AccessingmenuItems.lengththen throws during rendering and can break the member list.Proposed fix
- <DropdownMenu v-if="menuItems.length > 0" :entries="menuItems"> + <DropdownMenu v-if="menuItems?.length" :entries="menuItems">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/workspace/components/dialogs/settings/MemberListItem.vue` at line 61, Update the DropdownMenu condition in MemberListItem to guard menuItems before accessing its length, so undefined values from memberMenus.get(member.id) skip rendering safely while non-empty menus continue to render.src/platform/workspace/components/dialogs/settings/MembersPanelContent.vue (1)
135-140: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the credits header aligned with the row cells.
The credits header is gated only by
permissions.canManageMembers, whileMemberListItemrenders the credits and action cells only whencanManageMembers && !isSingleSeatPlan(isSingleSeatPlanis passed as!isOnTeamPlan). If member management is enabled on a single-seat workspace, the header adds columns that the rows omit.Gate both the credits header and the empty action header with the same team-plan condition.
Proposed fix
- v-if="permissions.canManageMembers" + v-if="permissions.canManageMembers && isOnTeamPlan" ... - <div v-if="permissions.canManageMembers" /> + <div v-if="permissions.canManageMembers && isOnTeamPlan" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/workspace/components/dialogs/settings/MembersPanelContent.vue` around lines 135 - 140, Update the credits header and empty action header conditions in the members panel to require both permissions.canManageMembers and the team-plan condition, matching MemberListItem’s canManageMembers && !isSingleSeatPlan rendering logic. Reuse the existing isOnTeamPlan value so single-seat workspaces omit both headers when rows omit those cells.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/platform/workspace/components/dialogs/settings/MemberListItem.vue`:
- Line 61: Update the DropdownMenu condition in MemberListItem to guard
menuItems before accessing its length, so undefined values from
memberMenus.get(member.id) skip rendering safely while non-empty menus continue
to render.
In `@src/platform/workspace/components/dialogs/settings/MembersPanelContent.vue`:
- Around line 135-140: Update the credits header and empty action header
conditions in the members panel to require both permissions.canManageMembers and
the team-plan condition, matching MemberListItem’s canManageMembers &&
!isSingleSeatPlan rendering logic. Reuse the existing isOnTeamPlan value so
single-seat workspaces omit both headers when rows omit those cells.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a7899ed4-b96b-40bc-a8e6-04314e03fedf
📒 Files selected for processing (2)
src/platform/workspace/components/dialogs/settings/MemberListItem.vuesrc/platform/workspace/components/dialogs/settings/MembersPanelContent.vue
The credit-limit UI is mock-backed until FE-1278 wires the backend, so shipping it enabled would expose a Credits-used column reading 0 for everyone and a Set credit limit action that silently drops writes. Add a member_credit_limits_enabled server flag (default off). While off, the Members panel is unchanged: original three-column grid, right-aligned Role, no credits column, and self/creator rows have no menu. When on, the credits column, progress bars, and Set credit limit action appear. Flip the flag with FE-1278. Also expose the usage bar as an accessible progressbar.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/platform/workspace/components/dialogs/settings/MemberListItem.vue (1)
112-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
vue-i18nconsistently for credit-number formatting.The UI currently formats values using the native browser locale instead of the application locale. As per coding guidelines, use
vue-i18nfor formatting user-facing strings and numbers in the Composition API.Note: This concern was flagged in a previous review and remains unresolved.
♻️ Proposed refactor
Ensure you import
useI18nfromvue-i18nin the script setup, then apply this pattern:+const { n } = useI18n() + const creditsLabel = computed(() => { - const used = (member.creditsUsedThisMonth ?? 0).toLocaleString() - return member.monthlyCreditLimit - ? `${used} / ${member.monthlyCreditLimit.toLocaleString()}` - : used + const used = n(member.creditsUsedThisMonth ?? 0) + return member.monthlyCreditLimit + ? `${used} / ${n(member.monthlyCreditLimit)}` + : used })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/workspace/components/dialogs/settings/MemberListItem.vue` around lines 112 - 117, Update the creditsLabel computed value in MemberListItem to use vue-i18n number formatting instead of native toLocaleString. Import and initialize useI18n in the script setup, then format both creditsUsedThisMonth and monthlyCreditLimit through the i18n formatter while preserving the existing fallback and “used / limit” display behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/platform/workspace/components/dialogs/settings/MemberListItem.vue`:
- Around line 112-117: Update the creditsLabel computed value in MemberListItem
to use vue-i18n number formatting instead of native toLocaleString. Import and
initialize useI18n in the script setup, then format both creditsUsedThisMonth
and monthlyCreditLimit through the i18n formatter while preserving the existing
fallback and “used / limit” display behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 26bdea51-a769-4a90-bb13-cc41e7f98e30
📒 Files selected for processing (9)
src/composables/useFeatureFlags.tssrc/platform/remoteConfig/types.tssrc/platform/workspace/components/dialogs/settings/MemberListItem.test.tssrc/platform/workspace/components/dialogs/settings/MemberListItem.vuesrc/platform/workspace/components/dialogs/settings/MembersPanelContent.vuesrc/platform/workspace/composables/useMembersPanel.test.tssrc/platform/workspace/composables/useMembersPanel.tssrc/platform/workspace/composables/useWorkspaceUI.test.tssrc/platform/workspace/composables/useWorkspaceUI.ts
Contributor
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/services/dialogService.ts (1)
360-362: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify redundant width utilities.
The
max-wandsm:max-wclasses duplicate the exact same dimension constraint as the base width and each other. You can simplify this down to standard responsive boundaries (e.g.,w-[360px] max-w-[95vw]), which natively restricts the 360px dialog width on smaller screens without needing arbitrary min-functions or responsive prefix redundancy.♻️ Proposed refactor
- contentClass: - 'w-[min(360px,95vw)] max-w-[min(360px,95vw)] sm:max-w-[min(360px,95vw)] border-0 bg-transparent shadow-none' + contentClass: + 'w-[360px] max-w-[95vw] border-0 bg-transparent shadow-none'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/dialogService.ts` around lines 360 - 362, Simplify the contentClass width utilities in the dialog configuration by replacing the redundant min-function and responsive max-width classes with standard responsive boundaries: a fixed 360px width constrained by a 95vw maximum. Preserve the existing border, background, and shadow classes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/services/dialogService.ts`:
- Around line 360-362: Simplify the contentClass width utilities in the dialog
configuration by replacing the redundant min-function and responsive max-width
classes with standard responsive boundaries: a fixed 360px width constrained by
a 95vw maximum. Preserve the existing border, background, and shadow classes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d6befda5-9b7b-470f-a097-702ea4348a7b
📒 Files selected for processing (7)
src/locales/en/main.jsonsrc/platform/workspace/api/workspaceApi.tssrc/platform/workspace/composables/useMembersPanel.test.tssrc/platform/workspace/composables/useMembersPanel.tssrc/platform/workspace/composables/useWorkspaceUI.test.tssrc/platform/workspace/composables/useWorkspaceUI.tssrc/services/dialogService.ts
| const modeGroupName = useId() | ||
| const mode = ref<'limited' | 'unlimited'>( | ||
| currentLimit ? 'limited' : 'unlimited' | ||
| ) |
Contributor
There was a problem hiding this comment.
issue: currentLimit=0 is treated as falsy, so the dialog opens in "No limit" mode when a member already has a 0-credit cap. Should be:
currentLimit !== null ? "limited" : "unlimited"Same falsy-0 pattern affects MemberListItem.vue (v-if="member.monthlyCreditLimit" skips the progress bar and the used / limit label for a zero-limit member).
Collaborator
Author
There was a problem hiding this comment.
Fixed in 3b22457. Limit presence now uses explicit nullish checks in both the dialog and member row. A zero cap opens in limited mode and renders safely without dividing by zero. The focused suite passes 194 tests, plus typecheck, lint, format check, and knip.
| <template> | ||
| <div | ||
| class="flex w-lg max-w-full flex-col rounded-2xl border border-border-default bg-base-background" | ||
| > |
Contributor
There was a problem hiding this comment.
issue: (non-blocking) The dialog container is a plain <div> with no ARIA dialog semantics. Screen readers won't identify it as a dialog or announce its title. Needs role="dialog" aria-modal="true" and aria-labelledby pointing to the <h2>. No focus trap is implemented either -- Tab will escape to elements behind the overlay.
Collaborator
Author
There was a problem hiding this comment.
Addressed the actual accessibility gap in 3b22457. GlobalDialog already renders Reka DialogContent, which provides the dialog/modal semantics and modal focus scope, so adding a second dialog role or focus trap inside the content would duplicate them. I connected its existing aria-labelledby value to the h2 and added an integration test that asserts the rendered dialog has the translated accessible name. All focused validation passes.
| aria-valuemin="0" | ||
| aria-valuemax="100" | ||
| :aria-label="creditsLabel" | ||
| > |
Contributor
There was a problem hiding this comment.
suggestion: (non-blocking) The progressbar has no aria-valuetext, so screen readers announce a raw percentage (e.g. "73%") with no context. Adding :aria-valuetext="creditsLabel" (or a more descriptive variant like "546 of 3,000 credits used") would give assistive tech the full picture.
Collaborator
Author
There was a problem hiding this comment.
Applied in 3b22457. The progress bar now has a localized Monthly credits label and contextual aria-valuetext such as 546 of 3,000 monthly credits used. The component test covers the accessible name and value text; all focused validation passes.
| isOriginalOwner: member.is_original_owner ?? false | ||
| isOriginalOwner: member.is_original_owner ?? false, | ||
| creditsUsedThisMonth: member.credits_used_this_month ?? 0, | ||
| monthlyCreditLimit: member.monthly_credit_limit ?? null |
Contributor
There was a problem hiding this comment.
suggestion: creditsUsedThisMonth defaults to 0 for a field the backend does not send yet. With the credits column visible (flag on), every member shows 0 -- indistinguishable from a real zero-usage value. Keeping it undefined (matching the ? on WorkspaceMember.creditsUsedThisMonth) would let the UI render a dash or skeleton until FE-1278 delivers real values.
Collaborator
Author
There was a problem hiding this comment.
Applied in 3b22457. The API mapping now preserves absent usage as undefined. The row renders an em dash and omits the progress bar until usage is available, and the dialog no longer infers zero usage for warning logic. Tests cover absent versus explicit zero values; all focused validation passes.
| "limitOption": "Limit monthly credit usage to:", | ||
| "noLimit": "No limit", | ||
| "warning": "This user has already spent {credits} credits. Setting this cap will prevent further runs until next month.", | ||
| "invalidLimit": "Enter a credit limit greater than zero.", |
Contributor
There was a problem hiding this comment.
nitpick: (non-blocking) "Setting this cap will prevent further runs until next month" implies any cap triggers a block, but the warning only shows when the proposed limit is at or below current spend. Something like "Setting this limit at or below their current usage will prevent further runs until next month" matches the actual condition more precisely.
Collaborator
Author
There was a problem hiding this comment.
Applied in 3b22457. The warning now says that setting the limit at or below current usage will prevent further runs until next month. Format and JSON validation pass.
| "role": "Role" | ||
| "role": "Role", | ||
| "creditsUsed": "Credits used this month" | ||
| }, |
Contributor
There was a problem hiding this comment.
nitpick: (non-blocking) The column header "Credits used this month" only describes the uncapped display (plain usage number). When a cap is set the cell shows 546 / 3,000, making the header incomplete. "Monthly credits" or "Credits (this month)" covers both states.
Collaborator
Author
There was a problem hiding this comment.
Applied in 3b22457. The column is now labeled Monthly credits so it covers both usage-only and used-over-limit displays. Format and JSON validation pass.
| // Per-member monthly credit limit UI (FE-1277). The cloud OpenAPI carries | ||
| // neither usage nor limit yet; persistence and real usage land in FE-1278. | ||
| credits_used_this_month?: number | ||
| monthly_credit_limit?: number | null |
Contributor
There was a problem hiding this comment.
nitpick: (non-blocking) credits_used_this_month?: number and monthly_credit_limit?: number | null use both ? and | null, giving a three-valued type (undefined | number | null). The ?? null fallback in mapApiMemberToWorkspaceMember collapses undefined and null together, losing the "field absent vs cap cleared" distinction. A // TODO(FE-1278): remove ? once backend sends these fields comment would prevent the optionals from silently outliving their purpose.
Collaborator
Author
There was a problem hiding this comment.
Addressed the underlying distinction in 3b22457. The mapping now preserves undefined versus explicit null for the limit, and undefined versus explicit zero for usage, with tests for both cases. I did not add another tracking comment because the adjacent API contract note already identifies FE-1278 and explains that these fields are not yet in the cloud OpenAPI.
| key: 'set-member-credit-limit', | ||
| component, | ||
| props, | ||
| dialogComponentProps: workspaceDialogProps |
Contributor
There was a problem hiding this comment.
nitpick: (non-blocking) The static key "set-member-credit-limit" means rapid re-open overwrites the first dialog's props. An owner who opens the dialog for member A and immediately triggers it for member B will commit the limit for whichever memberId is in props at submission time. Other workspace dialogs share this pattern, but the local-only mutation makes silent overwrites harder to detect. Keying on set-member-credit-limit-${props.memberId} would make each member's dialog independent.
Collaborator
Author
There was a problem hiding this comment.
Checked, but I kept the shared modal key. The member list cannot be activated while this modal is open, and dialogStore.showDialog only raises an existing keyed dialog; it does not overwrite its contentProps. The described cross-member overwrite therefore cannot occur through this flow. Per-member keys would instead allow stacked copies and require dynamic close-key plumbing.
| } | ||
|
|
||
| function memberMenuItems(member: WorkspaceMember): MenuItem[] { | ||
| const creditLimitItem: MenuItem = { |
Contributor
There was a problem hiding this comment.
nitpick: (non-blocking) memberMenuItems() builds credit limit items for all non-self, non-creator rows without checking the caller's role. It works today because MemberListItem gates the whole action column on canManageMembers, but the composable could be more self-contained with an early guard:
if (!permissions.value.canManageMembers) return []
Collaborator
Author
There was a problem hiding this comment.
Applied in 3b22457. memberMenuItems now returns early when canManageMembers is false, with a focused test covering the non-manager path. All focused validation passes.
| @@ -198,9 +203,24 @@ function useWorkspaceUIInternal() { | |||
| ) | |||
Contributor
There was a problem hiding this comment.
question: The isOriginalOwnerByEmail helper (below the diff) re-implements creator detection locally with its own tie-breaking by id.localeCompare, rather than deriving from store.isCurrentUserOriginalOwner, which the store documents as the single source of truth. Could these two signals ever disagree? If not, would it be safe to replace the local computation with store.isCurrentUserOriginalOwner?
Collaborator
Author
There was a problem hiding this comment.
Good catch. The signals could disagree when the explicit original-owner field or the owner-only fallback differs from the local earliest-member calculation. In 3b22457 I removed the duplicate helper and now derive this UI state from store.isCurrentUserOriginalOwner, while preserving the personal-workspace behavior. The focused tests now exercise the store signal directly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/workspace/composables/useMembersPanel.test.ts`:
- Around line 677-693: Update the test in the “preserves unavailable usage and
limit values for the dialog” case to select the credit-limit menu action by its
stable translation key or label instead of indexing memberMenuItems(member) at
position 1; keep the existing command invocation and dialog assertion unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9ba6ed6b-8e1c-4579-8465-9ee1278d43d3
📒 Files selected for processing (13)
src/components/dialog/GlobalDialog.test.tssrc/locales/en/main.jsonsrc/platform/workspace/components/dialogs/SetMemberCreditLimitDialogContent.test.tssrc/platform/workspace/components/dialogs/SetMemberCreditLimitDialogContent.vuesrc/platform/workspace/components/dialogs/settings/MemberListItem.test.tssrc/platform/workspace/components/dialogs/settings/MemberListItem.vuesrc/platform/workspace/composables/useMembersPanel.test.tssrc/platform/workspace/composables/useMembersPanel.tssrc/platform/workspace/composables/useWorkspaceUI.test.tssrc/platform/workspace/composables/useWorkspaceUI.tssrc/platform/workspace/stores/teamWorkspaceStore.test.tssrc/platform/workspace/stores/teamWorkspaceStore.tssrc/services/dialogService.ts
Comment on lines
+677
to
+693
| it('preserves unavailable usage and limit values for the dialog', async () => { | ||
| const panel = await setup() | ||
| const member = createMember({ id: 'mem-9', name: 'Jane' }) | ||
| const limitItem = panel.memberMenuItems(member)[1] | ||
|
|
||
| limitItem.command?.({ | ||
| originalEvent: new Event('click'), | ||
| item: limitItem | ||
| }) | ||
|
|
||
| expect(mockShowSetMemberCreditLimitDialog).toHaveBeenCalledWith({ | ||
| memberId: 'mem-9', | ||
| memberName: 'Jane', | ||
| creditsUsed: undefined, | ||
| currentLimit: undefined | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Select menu actions by label instead of array index.
To prevent unrelated menu ordering changes from breaking behavioral tests, find the target item by its translation key/label rather than hardcoding its index.
As per path instructions, prefer stable behavioral tests and avoid change-detector expectations.
♻️ Proposed refactor
- const limitItem = panel.memberMenuItems(member)[1]
+ const limitItem = panel
+ .memberMenuItems(member)
+ .find(
+ (item) =>
+ item.label === 'workspacePanel.members.actions.setCreditLimit'
+ )!🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/platform/workspace/composables/useMembersPanel.test.ts` around lines 677
- 693, Update the test in the “preserves unavailable usage and limit values for
the dialog” case to select the credit-limit menu action by its stable
translation key or label instead of indexing memberMenuItems(member) at position
1; keep the existing command invocation and dialog assertion unchanged.
Source: Path instructions
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
UI prototype: #13653
Owners can set or clear an optional monthly credit limit per workspace member from Settings > Members, so no single member consumes a disproportionate share of the shared credit pool. UI only; backend persistence and enforcement are tracked in FE-1278.
Changes
SetMemberCreditLimitDialogContent.vue— radio "Limit monthly credit usage to" + amount, or "No limit"; positive safe-integer validation; warning when the proposed cap is at or below usage already spentused / limitwith a usage progress bar, uncapped rows show usage onlyteamWorkspaceStore.setMemberCreditLimit(local-only) + optionalmonthlyCreditLimit/creditsUsedThisMonthon the member modelReview Focus
billing_control_enabledflag throughflags.billingControlEnabled; while disabled, the Members panel remains unchanged until FE-1278 provides persistence and enforcement.owner | memberrole model. The Admin role remains out of scope (deferred, per the Slack decision thread); role labels/submenu are unchanged.MemberListItemmoves from!isCurrentUser && !isOriginalOwnertomenuItems.length > 0, so owners can now cap themselves and the creator (DES-504 permits self-caps).creditsUsedThisMonthhas no backend source yet, so usage renders as 0 until real usage lands (DES-479 / FE-1249); the "Last activity" column and credits sorting are out of scope here.main.screenshots