-
Notifications
You must be signed in to change notification settings - Fork 673
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
|
@@ -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: () => ({ | ||
|
|
@@ -121,6 +126,13 @@ const WorkspaceSwitcherPopoverStub = defineComponent({ | |
| ` | ||
| }) | ||
|
|
||
| const SubscribeButtonStub = defineComponent({ | ||
| props: { | ||
| label: { type: String, required: true } | ||
| }, | ||
| template: '<button type="button">{{ label }}</button>' | ||
| }) | ||
|
|
||
| const i18n = createI18n({ | ||
| legacy: false, | ||
| locale: 'en', | ||
|
|
@@ -149,7 +161,7 @@ function renderComponent( | |
| }, | ||
| stubs: { | ||
| WorkspaceSwitcherPopover: WorkspaceSwitcherPopoverStub, | ||
| SubscribeButton: true, | ||
| SubscribeButton: SubscribeButtonStub, | ||
| UserAvatar: true, | ||
| WorkspaceProfilePic: true, | ||
| Skeleton: true, | ||
|
|
@@ -161,6 +173,7 @@ function renderComponent( | |
|
|
||
| describe('CurrentUserPopoverWorkspace', () => { | ||
| beforeEach(() => { | ||
| state.isCloud = true | ||
| state.billingStatus = 'paid' | ||
| state.canAccessSubscriptionFeatures = true | ||
| state.isFreeTier = false | ||
|
|
@@ -307,6 +320,41 @@ describe('CurrentUserPopoverWorkspace', () => { | |
| ).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('keeps Subscribe hidden on Local after switching to an unsubscribed workspace', async () => { | ||
| state.isCloud = false | ||
| state.canAccessSubscriptionFeatures = false | ||
| state.canManageSubscription = true | ||
| const { rerender } = renderComponent('personal') | ||
|
|
||
| expect( | ||
| screen.queryByRole('button', { name: 'Subscribe' }) | ||
| ).not.toBeInTheDocument() | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| if (!workspaceStoreMock.store) throw new Error('Workspace store not ready') | ||
| workspaceStoreMock.store.workspaceName = 'Team Workspace' | ||
| workspaceStoreMock.store.isInPersonalWorkspace = false | ||
| await rerender({}) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.getByTestId('workspace-switcher-trigger')).toHaveTextContent( | ||
| 'Team Workspace' | ||
| ) | ||
| expect( | ||
| screen.queryByRole('button', { name: 'Subscribe' }) | ||
| ).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| it('keeps Resubscribe hidden on Local for a cancelled plan', () => { | ||
| state.isCloud = false | ||
| state.isCancelled = true | ||
| state.canManageSubscriptionLifecycle = true | ||
|
|
||
| renderComponent('team') | ||
|
|
||
| expect( | ||
| screen.queryByRole('button', { name: 'Resubscribe' }) | ||
| ).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| it.for([ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (non-blocking): The existing
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in b42290b with an explicit Local cancelled-plan case: |
||
| { | ||
| name: 'allows a lifecycle manager to resubscribe a cancelled plan', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -326,10 +326,11 @@ const showManagePlan = computed( | |
| ) | ||
| const showSubscribeAction = computed( | ||
| () => | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| (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 = () => { | ||
|
|
||
There was a problem hiding this comment.
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 fromsaveWorkflow()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.There was a problem hiding this comment.
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.