Skip to content

fix(workspace): hide subscription CTA on Local #15601

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.

Sign up for GitHub

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

Merged
christian-byrne merged 4 commits into from
Aug 23, 2026
+58 −6
Merged

fix(workspace): hide subscription CTA on Local #15601

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a384a68
fix(workspace): hide subscribe action on local
dante01yoon Aug 22, 2026
6cd4e98
test: verify local workspace transition
dante01yoon Aug 22, 2026
b42290b
test: cover local subscription actions
dante01yoon Aug 23, 2026
ff8f062
test: render accessible subscribe stub
dante01yoon Aug 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions browser_tests/tests/workspace/localWorkspaceSwitcher.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ test.describe('Local workspace switcher', { tag: '@auth' }, () => {
document.body.dataset.workspaceSwitchDocument = 'original'
})

@christian-byrne christian-byrne Aug 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick (non-blocking): closeToasts() is load-bearing here — the success toast from saveWorkflow() can overlap the "Current user" button and cause the click to miss. Worth a brief inline comment so future readers don't treat it as optional defensive cleanup.

@dante01yoon dante01yoon Aug 23, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I kept this unchanged. closeToasts() directly expresses the required precondition immediately before clicking Current user, while an inline comment would only restate the operation and conflict with the repository guidance to avoid redundant comments.

await comfyPage.toast.closeToasts()
await page.getByRole('button', { name: 'Current user' }).click()
await expect(page.getByRole('button', { name: 'Subscribe' })).toHaveCount(0)
await expect(page.getByTestId('workspace-switcher-trigger')).toContainText(
PERSONAL_WORKSPACE_NAME
)
Expand All @@ -72,6 +74,7 @@ test.describe('Local workspace switcher', { tag: '@auth' }, () => {
await expect(page.getByTestId('workspace-switcher-trigger')).toContainText(
TEAM_WORKSPACE_NAME
)
await expect(page.getByRole('button', { name: 'Subscribe' })).toHaveCount(0)
await expect.poll(() => billingRequestUrls.length).toBeGreaterThan(0)
expect(tokenRequestBody).toEqual({ workspace_id: 'ws-team' })
expect(billingRequestUrls).toEqual(
Expand Down
24 changes: 23 additions & 1 deletion src/platform/workspace/components/CurrentUserPopoverWorkspace.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import enMessages from '@/locales/en/main.json'
import CurrentUserPopoverWorkspace from './CurrentUserPopoverWorkspace.vue'

const state = vi.hoisted(() => ({
isCloud: true,
billingStatus: 'paid',
canAccessSubscriptionFeatures: true,
isFreeTier: false,
Expand Down Expand Up @@ -91,7 +92,11 @@ vi.mock('@/platform/settings/composables/useSettingsDialog', () => ({
useSettingsDialog: () => ({ show: state.showSettingsDialog })
}))

vi.mock('@/platform/distribution/types', () => ({ isCloud: true }))
vi.mock('@/platform/distribution/types', () => ({
get isCloud() {
return state.isCloud
}
}))

vi.mock('@/services/dialogService', () => ({
useDialogService: () => ({
Expand Down Expand Up @@ -161,6 +166,7 @@ function renderComponent(

describe('CurrentUserPopoverWorkspace', () => {
beforeEach(() => {
state.isCloud = true
state.billingStatus = 'paid'
state.canAccessSubscriptionFeatures = true
state.isFreeTier = false
Expand Down Expand Up @@ -307,6 +313,22 @@ describe('CurrentUserPopoverWorkspace', () => {
).toBeInTheDocument()
})

it('keeps Subscribe hidden on Local after switching to an unsubscribed workspace', async () => {
state.isCloud = false
const { rerender } = renderComponent('personal')

state.canAccessSubscriptionFeatures = false
state.canManageSubscription = true
if (!workspaceStoreMock.store) throw new Error('Workspace store not ready')
workspaceStoreMock.store.workspaceName = 'Team Workspace'
workspaceStoreMock.store.isInPersonalWorkspace = false
await rerender({})

@christian-byrne christian-byrne Aug 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion (non-blocking): The test verifies Subscribe is absent after the workspace switch but doesn't assert it was also absent before the switch (while still on the personal workspace with isCloud=false). Adding a queryByRole assertion before the store mutation would catch a regression where the initial render is correct but a re-render re-introduces the button.

@dante01yoon dante01yoon Aug 23, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in b42290b. The component test now asserts Subscribe is absent on the initial Local personal-workspace render, then verifies the transition to Team Workspace and asserts it remains absent after rerender.


expect(
screen.queryByRole('button', { name: 'Subscribe' })
).not.toBeInTheDocument()
})

Comment thread
coderabbitai[bot] marked this conversation as resolved.
it.for([

@christian-byrne christian-byrne Aug 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion (non-blocking): The existing it.for matrix for Resubscribe/Subscribe runs with isCloud=true (reset in beforeEach). There is no test for isCloud=false + isCancelled=true + canManageSubscriptionLifecycle=true — the "Resubscribe on Local" path. Since the isCloud gate also hides Resubscribe on Local, a test case for it would make that decision explicit and guard against regression.

@dante01yoon dante01yoon Aug 23, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in b42290b with an explicit Local cancelled-plan case: isCloud=false, isCancelled=true, and lifecycle-management permission enabled still keeps Resubscribe hidden.

{
name: 'allows a lifecycle manager to resubscribe a cancelled plan',
Expand Down
9 changes: 5 additions & 4 deletions src/platform/workspace/components/CurrentUserPopoverWorkspace.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,11 @@ const showManagePlan = computed(
)
const showSubscribeAction = computed(
() =>

@christian-byrne christian-byrne Aug 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: showPlansAndPricing and showManagePlan both lack an isCloud gate, which leaves two Cloud-only UI actions reachable on Local/Desktop for any authenticated workspace owner. canManageBilling = hasActiveWorkspace && role === 'owner' in useWorkspaceUI.ts has no distribution check, so a Local owner of an active workspace gets canManageSubscription=true and both menu items render. Clicking "Plans & Pricing" invokes handleOpenPlansAndPricing → subscriptionDialog.showPricingTable() — a Cloud-only API call — on Local. This PR fixes showSubscribeAction but leaves the same class of bug on the two sibling computed values. Suggest adding isCloud && prefix to both, matching the pattern used here.

@dante01yoon dante01yoon Aug 23, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. This is the same Local profile-menu regression tracked and fixed separately in #15611: #15611. That PR gates both showPlansAndPricing and showManagePlan with isCloud, adds the Figma-defined Local Plans and credits action, and includes component/E2E regression coverage. I am keeping this PR scoped to QA findings 3 and 6 (Subscribe/Resubscribe) to avoid duplicating the same component changes across two open branches.

(isCancelled.value && permissions.value.canManageSubscriptionLifecycle) ||
(!canAccessSubscriptionFeatures.value &&
!hasDelinquentSubscription.value &&
permissions.value.canManageSubscription)
isCloud &&
((isCancelled.value && permissions.value.canManageSubscriptionLifecycle) ||
(!canAccessSubscriptionFeatures.value &&
!hasDelinquentSubscription.value &&
permissions.value.canManageSubscription))
)

const handleOpenUserSettings = () => {
Expand Down
Loading