Skip to content

Commit aae5802

Browse files
authored
[backport cloud/1.47] fix(workspace): gate billing controls by role (#13834)
## Summary Backports #13812 to `cloud/1.47` after the [automatic cherry-pick](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/29704878740) conflicted. This branch is stacked on the current #13814 head. ## Changes - **What**: Applies role-gated billing mutations, Personal/default creator protection, additional-workspace creator actions, and execution-time permission rechecks. - **Dependencies**: Depends on #13814; merge it first so this PR narrows to the #13812 backport. ## Review Focus - Combines #13814 plan-driven Members access with #13812 role and creator policy. - Corrects the cloud workspace fixture to use `consolidated_billing_enabled` for Personal billing routing while leaving `billing_control_enabled` independent. - Keeps the `BillingStatusBanner` story/test and Storybook workspace mock deleted because their runtime surface is absent on this branch. ## Validation - Focused unit suite: 18 files, 397 tests passed. - Main and browser typechecks passed; focused formatting passed. - Cloud `memberRoleChange.spec.ts`: 7 passed. ## Screenshots | Personal/default workspace creator | Additional workspace creator | | --- | --- | | <img width="720" alt="Personal workspace creator menu with billing actions but no Leave or Delete action" src="https://github.com/user-attachments/assets/1fa2e62f-9b93-4d8c-b9bb-d02a2a671531" /> | <img width="720" alt="Additional workspace creator menu with Leave and subscription-locked Delete actions" src="https://github.com/user-attachments/assets/3cb7255d-2e41-44d4-a7f8-feddcb90644d" /> | <img width="720" alt="Another owner can change the additional workspace creator role or remove the creator" src="https://github.com/user-attachments/assets/8fb7b5a5-63b4-4124-a99a-85121f8fa374" />
1 parent 4ee2e3b commit aae5802

45 files changed

Lines changed: 2268 additions & 659 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

browser_tests/fixtures/data/cloudWorkspace.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ import type { RemoteConfig } from '@/platform/remoteConfig/types'
99
// `/api/features` is the remote-config source: production builds resolve the
1010
// workspaces flag from it (the `ff:` localStorage override is dev-only).
1111
export const WORKSPACE_FEATURE_FLAG: RemoteConfig = {
12-
team_workspaces_enabled: true
12+
team_workspaces_enabled: true,
13+
consolidated_billing_enabled: true
1314
}
1415

1516
export const TEAM_WORKSPACE: WorkspaceWithRole = {
@@ -67,21 +68,21 @@ export const DEFAULT_TEAM_MEMBERS: Member[] = [
6768
MEMBER_JOHN
6869
]
6970

71+
const TEAM_PLAN_SLUG = 'team-pro-monthly'
72+
7073
export const TEAM_BILLING_STATUS: BillingStatusResponse = {
7174
is_active: true,
7275
subscription_status: 'active',
7376
subscription_tier: 'PRO',
7477
subscription_duration: 'MONTHLY',
75-
plan_slug: 'pro-monthly',
78+
plan_slug: TEAM_PLAN_SLUG,
7679
billing_status: 'paid',
7780
has_funds: true,
7881
renewal_date: '2099-02-20T00:00:00Z'
7982
}
8083

81-
// `max_seats > 1` on the current plan is what flips `isOnTeamPlan`, which gates
82-
// the whole role-management UI.
8384
export const TEAM_PRO_PLAN: Plan = {
84-
slug: 'pro-monthly',
85+
slug: TEAM_PLAN_SLUG,
8586
tier: 'PRO',
8687
duration: 'MONTHLY',
8788
price_cents: 10000,

browser_tests/fixtures/helpers/CloudWorkspaceMockHelper.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import type { Page, Route } from '@playwright/test'
22

3-
import type { Member } from '@/platform/workspace/api/workspaceApi'
3+
import type {
4+
Member,
5+
WorkspaceWithRole
6+
} from '@/platform/workspace/api/workspaceApi'
47

58
import { mockSystemStats } from '@e2e/fixtures/data/systemStats'
69
import {
@@ -41,18 +44,22 @@ export class CloudWorkspaceMockHelper {
4144
constructor(private readonly page: Page) {}
4245

4346
async setup(
44-
members: Member[] = DEFAULT_TEAM_MEMBERS
47+
members: Member[] = DEFAULT_TEAM_MEMBERS,
48+
activeWorkspace: WorkspaceWithRole = TEAM_WORKSPACE
4549
): Promise<MemberMockState> {
46-
const state = await this.mockBoot(members)
50+
const state = await this.mockBoot(members, activeWorkspace)
4751
await new CloudAuthHelper(this.page).mockAuth()
48-
await this.page.addInitScript(() => {
52+
await this.page.addInitScript((workspaceId) => {
4953
localStorage.setItem('Comfy.userId', 'test-user-e2e')
50-
localStorage.setItem('Comfy.Workspace.LastWorkspaceId', 'ws-team')
51-
})
54+
localStorage.setItem('Comfy.Workspace.LastWorkspaceId', workspaceId)
55+
}, activeWorkspace.id)
5256
return state
5357
}
5458

55-
private async mockBoot(members: Member[]): Promise<MemberMockState> {
59+
private async mockBoot(
60+
members: Member[],
61+
activeWorkspace: WorkspaceWithRole
62+
): Promise<MemberMockState> {
5663
const state: MemberMockState = {
5764
members: members.map((m) => ({ ...m })),
5865
patches: []
@@ -93,11 +100,11 @@ export class CloudWorkspaceMockHelper {
93100
await page.route('**/api/auth/session', (r) =>
94101
r.fulfill(jsonRoute({ token: 'mock-workspace-token' }))
95102
)
96-
await mockWorkspaceTokenMint(page, TEAM_WORKSPACE)
103+
await mockWorkspaceTokenMint(page, activeWorkspace)
97104
await page.route('**/releases**', (r) => r.fulfill(jsonRoute([])))
98105

99106
await page.route('**/api/workspaces', (r) =>
100-
r.fulfill(jsonRoute({ workspaces: [TEAM_WORKSPACE] }))
107+
r.fulfill(jsonRoute({ workspaces: [activeWorkspace] }))
101108
)
102109

103110
await page.route('**/api/workspace/members**', (route: Route) => {
@@ -140,7 +147,10 @@ export class CloudWorkspaceMockHelper {
140147
)
141148
await page.route('**/api/billing/plans', (r) =>
142149
r.fulfill(
143-
jsonRoute({ current_plan_slug: 'pro-monthly', plans: [TEAM_PRO_PLAN] })
150+
jsonRoute({
151+
current_plan_slug: TEAM_PRO_PLAN.slug,
152+
plans: [TEAM_PRO_PLAN]
153+
})
144154
)
145155
)
146156

browser_tests/tests/dialogs/memberRoleChange.spec.ts

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,26 +6,18 @@ import type { Member } from '@/platform/workspace/api/workspaceApi'
66
import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage'
77
import {
88
CREATOR,
9+
DEFAULT_TEAM_MEMBERS,
910
MEMBER_JANE,
1011
MEMBER_JOHN,
1112
VIEWER
1213
} from '@e2e/fixtures/data/cloudWorkspace'
1314
import { CloudWorkspaceMockHelper } from '@e2e/fixtures/helpers/CloudWorkspaceMockHelper'
15+
import { workspace } from '@e2e/fixtures/utils/workspaceMocks'
1416

1517
// Drives a raw `page` (not the `comfyPage` fixture) so the cloud app boots
1618
// against fully mocked endpoints; `comfyPage` would try to reach the OSS
1719
// devtools backend during setup.
1820

19-
/**
20-
* Member role change (Settings ▸ Workspace ▸ Members) — Figma 2993-15512.
21-
*
22-
* The viewer is a promoted owner (not the workspace creator), so the spec can
23-
* distinguish the creator guard from the self guard: the creator row and the
24-
* viewer's own row hide the row menu, every other row exposes
25-
* "Change role ›" (Owner / Member) plus "Remove member". Promoting a member
26-
* sends PATCH /api/workspace/members/:id {role}, flips the Role column,
27-
* re-sorts the row under the creator, and the promoted owner stays demotable.
28-
*/
2921
const APP_URL = process.env.PLAYWRIGHT_TEST_URL || 'http://localhost:8188'
3022

3123
async function openMembersTab(page: Page): Promise<Locator> {
@@ -70,31 +62,84 @@ async function openChangeRoleSubmenu(page: Page) {
7062
).toBeVisible()
7163
}
7264

65+
test.describe('Members plan gating', { tag: '@cloud' }, () => {
66+
test('personal workspace with a Team plan gets member management', async ({
67+
page
68+
}) => {
69+
await new CloudWorkspaceMockHelper(page).setup(
70+
DEFAULT_TEAM_MEMBERS,
71+
workspace('personal', 'owner')
72+
)
73+
const content = await openMembersTab(page)
74+
75+
const inviteButton = content.getByRole('button', {
76+
name: 'Invite member'
77+
})
78+
await expect(inviteButton).toBeEnabled()
79+
await expect(
80+
content.getByRole('button', { name: 'Role', exact: true })
81+
).toBeVisible()
82+
await expect(
83+
content.getByText(MEMBER_JANE.email, { exact: true })
84+
).toBeVisible()
85+
await expect(
86+
content.getByRole('button', { name: 'Upgrade to Team' })
87+
).toHaveCount(0)
88+
await expect(menuButton(memberRow(content, CREATOR.email))).toHaveCount(0)
89+
90+
await inviteButton.click()
91+
await expect(
92+
page.getByRole('heading', {
93+
name: 'Invite members to this workspace'
94+
})
95+
).toBeVisible()
96+
})
97+
})
98+
7399
test.describe('Member role change (Members tab)', { tag: '@cloud' }, () => {
74100
test.describe.configure({ timeout: 60_000 })
75101

76-
test('row menus respect creator and self guards', async ({ page }) => {
77-
await new CloudWorkspaceMockHelper(page).setup()
102+
test('additional workspace creator has actions while self does not', async ({
103+
page
104+
}) => {
105+
const state = await new CloudWorkspaceMockHelper(page).setup()
78106
const content = await openMembersTab(page)
107+
const creatorRow = memberRow(content, CREATOR.email)
79108

80-
// US8/US9 — no row actions on the creator row (Liz) nor on the viewer's
81-
// own row; the two plain members each expose a menu.
82109
await expect(
83110
menuButton(memberRow(content, MEMBER_JOHN.email))
84111
).toBeVisible()
85112
await expect(
86113
menuButton(memberRow(content, MEMBER_JANE.email))
87114
).toBeVisible()
88-
await expect(menuButton(memberRow(content, CREATOR.email))).toHaveCount(0)
115+
await expect(menuButton(creatorRow)).toBeVisible()
89116
await expect(menuButton(memberRow(content, VIEWER.email))).toHaveCount(0)
90117

91-
// US1/US12 — the row menu exposes Change role and the FE-768 remove flow.
92-
await menuButton(memberRow(content, MEMBER_JANE.email)).click()
118+
await menuButton(creatorRow).click()
93119
await expect(
94120
page.getByRole('menuitem', { name: 'Change role' })
95121
).toBeVisible()
96122
await page.getByRole('menuitem', { name: 'Remove member' }).click()
97123
await expect(page.getByText('Remove this member?')).toBeVisible()
124+
await page.getByRole('button', { name: 'Cancel', exact: true }).click()
125+
126+
await menuButton(creatorRow).click()
127+
await openChangeRoleSubmenu(page)
128+
await page
129+
.getByRole('menuitemradio', { name: 'Member', exact: true })
130+
.click()
131+
await expect(
132+
page.getByRole('heading', { name: 'Demote Liz to member?' })
133+
).toBeVisible()
134+
await page.getByRole('button', { name: 'Demote to member' }).click()
135+
136+
await expect(creatorRow.getByText('Member', { exact: true })).toBeVisible()
137+
expect(state.patches).toEqual([
138+
{
139+
url: expect.stringContaining('/api/workspace/members/u-liz'),
140+
role: 'member'
141+
}
142+
])
98143
})
99144

100145
test('selecting the current role is a no-op', async ({ page }) => {
@@ -146,9 +191,7 @@ test.describe('Member role change (Members tab)', { tag: '@cloud' }, () => {
146191
page.getByText('Manage members, payment methods, and workspace settings')
147192
).toBeVisible()
148193
await expect(
149-
page.getByText(
150-
'Promote and demote other owners (except the workspace creator).'
151-
)
194+
page.getByText('Promote members and demote eligible owners.')
152195
).toBeVisible()
153196

154197
await page.getByRole('button', { name: 'Cancel', exact: true }).click()

src/components/dialog/content/subscription/CancelSubscriptionDialogContent.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ const mockCloseDialog = vi.hoisted(() => vi.fn())
5454
const mockToastAdd = vi.hoisted(() => vi.fn())
5555
const mockTier = vi.hoisted(() => ({ value: 'STANDARD' as string | null }))
5656
const mockTrackCancellation = vi.hoisted(() => vi.fn())
57+
const mockShouldUseWorkspaceBilling = vi.hoisted(() => ({ value: false }))
58+
const mockCanManageSubscriptionLifecycle = vi.hoisted(() => ({ value: true }))
5759

5860
vi.mock('@/composables/billing/useBillingContext', () => ({
5961
useBillingContext: vi.fn(() => ({
@@ -64,6 +66,25 @@ vi.mock('@/composables/billing/useBillingContext', () => ({
6466
}))
6567
}))
6668

69+
vi.mock('@/composables/billing/useBillingRouting', () => ({
70+
useBillingRouting: () => ({
71+
shouldUseWorkspaceBilling: mockShouldUseWorkspaceBilling
72+
})
73+
}))
74+
75+
vi.mock('@/platform/workspace/composables/useWorkspaceUI', () => ({
76+
useWorkspaceUI: () => ({
77+
permissions: {
78+
get value() {
79+
return {
80+
canManageSubscriptionLifecycle:
81+
mockCanManageSubscriptionLifecycle.value
82+
}
83+
}
84+
}
85+
})
86+
}))
87+
6788
vi.mock('@/platform/telemetry', () => ({
6889
useTelemetry: () => ({
6990
trackSubscriptionCancellation: mockTrackCancellation
@@ -107,6 +128,8 @@ describe('CancelSubscriptionDialogContent', () => {
107128
beforeEach(() => {
108129
vi.clearAllMocks()
109130
mockTier.value = 'STANDARD'
131+
mockShouldUseWorkspaceBilling.value = false
132+
mockCanManageSubscriptionLifecycle.value = true
110133
})
111134

112135
describe('cancellation telemetry', () => {
@@ -240,6 +263,26 @@ describe('CancelSubscriptionDialogContent', () => {
240263
)
241264
})
242265

266+
it('does not cancel after the workspace role loses permission', async () => {
267+
mockSubscription.value = null
268+
mockShouldUseWorkspaceBilling.value = true
269+
mockCanManageSubscriptionLifecycle.value = true
270+
271+
renderComponent()
272+
mockCanManageSubscriptionLifecycle.value = false
273+
await userEvent.click(
274+
screen.getByRole('button', { name: /^cancel subscription$/i })
275+
)
276+
277+
expect(mockCancelSubscription).not.toHaveBeenCalled()
278+
expect(mockTrackCancellation).not.toHaveBeenCalledWith(
279+
'confirmed',
280+
expect.anything()
281+
)
282+
expect(mockToastAdd).not.toHaveBeenCalled()
283+
expect(mockCloseDialog).not.toHaveBeenCalled()
284+
})
285+
243286
it('does not track cancellation failure when status refresh fails after cancellation succeeds', async () => {
244287
mockSubscription.value = null
245288
mockCancelSubscription.mockResolvedValueOnce(undefined)

src/components/dialog/content/subscription/CancelSubscriptionDialogContent.vue

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ import { useI18n } from 'vue-i18n'
5050
5151
import Button from '@/components/ui/button/Button.vue'
5252
import { useBillingContext } from '@/composables/billing/useBillingContext'
53+
import { useBillingRouting } from '@/composables/billing/useBillingRouting'
5354
import { useTelemetry } from '@/platform/telemetry'
5455
import type { SubscriptionCancellationMetadata } from '@/platform/telemetry/types'
56+
import { useWorkspaceUI } from '@/platform/workspace/composables/useWorkspaceUI'
5557
import { useDialogStore } from '@/stores/dialogStore'
5658
import { parseIsoDateSafe } from '@/utils/dateTimeUtil'
5759
import { getErrorMessage } from '@/utils/errorUtil'
@@ -65,6 +67,8 @@ const dialogStore = useDialogStore()
6567
const toast = useToast()
6668
const { cancelSubscription, fetchStatus, subscription, tier } =
6769
useBillingContext()
70+
const { shouldUseWorkspaceBilling } = useBillingRouting()
71+
const { permissions } = useWorkspaceUI()
6872
const telemetry = useTelemetry()
6973
7074
const isLoading = ref(false)
@@ -119,6 +123,13 @@ function onClose() {
119123
}
120124
121125
async function onConfirmCancel() {
126+
if (
127+
shouldUseWorkspaceBilling.value &&
128+
!permissions.value.canManageSubscriptionLifecycle
129+
) {
130+
return
131+
}
132+
122133
telemetry?.trackSubscriptionCancellation('confirmed', cancellationMetadata())
123134
isLoading.value = true
124135
try {

src/locales/en/main.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2929,7 +2929,7 @@
29292929
"promoteIntro": "They'll be able to:",
29302930
"promotePermissionCredits": "Add additional credits",
29312931
"promotePermissionManage": "Manage members, payment methods, and workspace settings",
2932-
"promotePermissionRoles": "Promote and demote other owners (except the workspace creator).",
2932+
"promotePermissionRoles": "Promote members and demote eligible owners.",
29332933
"promoteConfirm": "Make owner",
29342934
"demoteTitle": "Demote {name} to member?",
29352935
"demoteMessage": "They'll lose admin access.",

0 commit comments

Comments
 (0)