Skip to content

refactor: rename isSubscribed for better expressiveness and avoid initializing cloud modules on local #7127

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

Closed
christian-byrne wants to merge 8 commits into from
Closed

refactor: rename isSubscribed for better expressiveness and avoid initializing cloud modules on local #7127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9d88919
rename isSubscribed and dont import cloud on local
christian-byrne Dec 3, 2025
afaceb5
dont import cloud in core commands module
christian-byrne Dec 4, 2025
cd15c27
improve test
christian-byrne Dec 6, 2025
5fd6aa6
don't use compound name, use isSubscriptionRequirementMet
christian-byrne Dec 6, 2025
6fb677f
chore: keep legacy subscription alias
christian-byrne Dec 6, 2025
1ebdab9
fix: use subscription requirement flag in cloud remote config
christian-byrne Dec 6, 2025
8ce0c9e
fix: skip subscription check when not required
christian-byrne Dec 6, 2025
b8f563d
refactor: split subscription popover section
christian-byrne Dec 9, 2025
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
6 changes: 3 additions & 3 deletions src/components/actionbar/ComfyRunButton/CloudRunButtonWrapper.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<template>
<component
:is="currentButton"
:key="isActiveSubscription ? 'queue' : 'subscribe'"
:key="isSubscriptionRequirementMet ? 'queue' : 'subscribe'"
/>
</template>
<script setup lang="ts">
Expand All @@ -11,9 +11,9 @@ import ComfyQueueButton from '@/components/actionbar/ComfyRunButton/ComfyQueueBu
import SubscribeToRunButton from '@/platform/cloud/subscription/components/SubscribeToRun.vue'
import { useSubscription } from '@/platform/cloud/subscription/composables/useSubscription'

const { isActiveSubscription } = useSubscription()
const { isSubscriptionRequirementMet } = useSubscription()

const currentButton = computed(() =>
isActiveSubscription.value ? ComfyQueueButton : SubscribeToRunButton
isSubscriptionRequirementMet.value ? ComfyQueueButton : SubscribeToRunButton
)
</script>
9 changes: 7 additions & 2 deletions src/components/dialog/content/setting/CreditsPanel.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<UserCredit text-class="text-3xl font-bold" />
<Skeleton v-if="loading" width="2rem" height="2rem" />
<Button
v-else-if="isActiveSubscription"
v-else-if="isSubscriptionRequirementMet"
:label="$t('credits.purchaseCredits')"
:loading="loading"
@click="handlePurchaseCreditsClick"
Expand Down Expand Up @@ -125,6 +125,7 @@ import UsageLogsTable from '@/components/dialog/content/setting/UsageLogsTable.v
import { useFirebaseAuthActions } from '@/composables/auth/useFirebaseAuthActions'
import { useExternalLink } from '@/composables/useExternalLink'
import { useSubscription } from '@/platform/cloud/subscription/composables/useSubscription'
import { isCloud } from '@/platform/distribution/types'
import { useTelemetry } from '@/platform/telemetry'
import { useDialogService } from '@/services/dialogService'
import { useCommandStore } from '@/stores/commandStore'
Expand All @@ -144,7 +145,11 @@ const authStore = useFirebaseAuthStore()
const authActions = useFirebaseAuthActions()
const commandStore = useCommandStore()
const telemetry = useTelemetry()
const { isActiveSubscription } = useSubscription()
const subscription = isCloud ? useSubscription() : null
const isSubscriptionRequirementMet = computed(() => {
if (!isCloud) return true
return subscription?.isSubscriptionRequirementMet.value ?? false
})
const loading = computed(() => authStore.loading)
const balanceLoading = computed(() => authStore.isFetchingBalance)

Expand Down
22 changes: 13 additions & 9 deletions src/components/topbar/CurrentUserPopover.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ vi.mock('@/stores/firebaseAuthStore', () => ({
const mockFetchStatus = vi.fn().mockResolvedValue(undefined)
vi.mock('@/platform/cloud/subscription/composables/useSubscription', () => ({
useSubscription: vi.fn(() => ({
isActiveSubscription: { value: true },
isSubscriptionRequirementMet: { value: true },
fetchStatus: mockFetchStatus
}))
}))
Expand Down Expand Up @@ -121,6 +121,11 @@ describe('CurrentUserPopover', () => {
vi.clearAllMocks()
})

const findButtonByLabel = (wrapper: VueWrapper, label: string) =>
wrapper
.findAllComponents(Button)
.find((button) => button.props('label') === label)!

Comment on lines +124 to +128
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider adding a guard for missing buttons to improve test failure messages.

The non-null assertion ! will throw an unhelpful error if the button isn't found. A small guard would make test failures more debuggable:

  const findButtonByLabel = (wrapper: VueWrapper, label: string) =>
-    wrapper
+  {
+    const button = wrapper
       .findAllComponents(Button)
       .find((button) => button.props('label') === label)!
+    if (!button) throw new Error(`Button with label "${label}" not found`)
+    return button
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const findButtonByLabel = (wrapper: VueWrapper, label: string) =>
wrapper
.findAllComponents(Button)
.find((button) => button.props('label') === label)!
const findButtonByLabel = (wrapper: VueWrapper, label: string) => {
const button = wrapper
.findAllComponents(Button)
.find((button) => button.props('label') === label)
if (!button) throw new Error(`Button with label "${label}" not found`)
return button
}
🤖 Prompt for AI Agents 
In src/components/topbar/CurrentUserPopover.test.ts around lines 124 to 128, the
helper uses a non-null assertion when finding a Button which leads to an
unhelpful runtime error if the button is missing; replace the `!` with a safe
guard that throws a clear, descriptive Error when the button is not found
(include the requested label and optionally wrapper.html() or a brief list of
found labels), or return undefined and let the caller assert with a meaningful
message; this change will make test failures easier to debug.

const mountComponent = (): VueWrapper => {
const i18n = createI18n({
legacy: false,
Expand Down Expand Up @@ -161,8 +166,7 @@ describe('CurrentUserPopover', () => {
const wrapper = mountComponent()

// Find all buttons and get the settings button (third button)
const buttons = wrapper.findAllComponents(Button)
const settingsButton = buttons[2]
const settingsButton = findButtonByLabel(wrapper, 'User Settings')

// Click the settings button
await settingsButton.trigger('click')
Expand All @@ -179,8 +183,7 @@ describe('CurrentUserPopover', () => {
const wrapper = mountComponent()

// Find all buttons and get the logout button (last button)
const buttons = wrapper.findAllComponents(Button)
const logoutButton = buttons[4]
const logoutButton = findButtonByLabel(wrapper, 'Log Out')

// Click the logout button
await logoutButton.trigger('click')
Expand All @@ -197,8 +200,10 @@ describe('CurrentUserPopover', () => {
const wrapper = mountComponent()

// Find all buttons and get the Partner Nodes info button (first one)
const buttons = wrapper.findAllComponents(Button)
const partnerNodesButton = buttons[0]
const partnerNodesButton = findButtonByLabel(
wrapper,
'Partner Nodes pricing table'
)

// Click the Partner Nodes button
await partnerNodesButton.trigger('click')
Expand All @@ -218,8 +223,7 @@ describe('CurrentUserPopover', () => {
const wrapper = mountComponent()

// Find all buttons and get the top-up button (second one)
const buttons = wrapper.findAllComponents(Button)
const topUpButton = buttons[1]
const topUpButton = findButtonByLabel(wrapper, 'Top Up')

// Click the top-up button
await topUpButton.trigger('click')
Expand Down
88 changes: 43 additions & 45 deletions src/components/topbar/CurrentUserPopover.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,37 +23,49 @@
</div>
</div>

<div v-if="isActiveSubscription" class="flex items-center justify-between">
<div class="flex flex-col gap-1">
<UserCredit text-class="text-2xl" />
<component
:is="SubscriptionSection"
v-if="SubscriptionSection"
@top-up="handleTopUp"
@open-partner-info="handleOpenPartnerNodesInfo"
@open-plan-settings="handleOpenPlanAndCreditsSettings"
/>
<template v-else>
<div class="flex items-center justify-between">
<div class="flex flex-col gap-1">
<UserCredit text-class="text-2xl" />
<Button
:label="$t('subscription.partnerNodesCredits')"
severity="secondary"
text
size="small"
class="pl-6 p-0 h-auto justify-start"
:pt="{
root: {
class: 'hover:bg-transparent active:bg-transparent'
}
}"
@click="handleOpenPartnerNodesInfo"
/>
</div>
<Button
:label="$t('subscription.partnerNodesCredits')"
:label="$t('credits.topUp.topUp')"
severity="secondary"
text
size="small"
class="pl-6 p-0 h-auto justify-start"
:pt="{
root: {
class: 'hover:bg-transparent active:bg-transparent'
}
}"
@click="handleOpenPartnerNodesInfo"
@click="handleTopUp"
/>
</div>

<Button
:label="$t('credits.topUp.topUp')"
class="justify-start"
:label="$t(planSettingsLabel)"
icon="pi pi-receipt"
text
fluid
severity="secondary"
size="small"
@click="handleTopUp"
@click="handleOpenPlanAndCreditsSettings"
/>
</div>
<SubscribeButton
v-else
:label="$t('subscription.subscribeToComfyCloud')"
size="small"
variant="gradient"
@subscribed="handleSubscribed"
/>
</template>

<Divider class="my-2" />

Expand All @@ -67,17 +79,6 @@
@click="handleOpenUserSettings"
/>

<Button
v-if="isActiveSubscription"
class="justify-start"
:label="$t(planSettingsLabel)"
icon="pi pi-receipt"
text
fluid
severity="secondary"
@click="handleOpenPlanAndCreditsSettings"
/>

<Divider class="my-2" />

<Button
Expand All @@ -95,15 +96,13 @@
<script setup lang="ts">
import Button from 'primevue/button'
import Divider from 'primevue/divider'
import { onMounted } from 'vue'
import { defineAsyncComponent, onMounted } from 'vue'

import UserAvatar from '@/components/common/UserAvatar.vue'
import UserCredit from '@/components/common/UserCredit.vue'
import { useCurrentUser } from '@/composables/auth/useCurrentUser'
import { useFirebaseAuthActions } from '@/composables/auth/useFirebaseAuthActions'
import { useExternalLink } from '@/composables/useExternalLink'
import SubscribeButton from '@/platform/cloud/subscription/components/SubscribeButton.vue'
import { useSubscription } from '@/platform/cloud/subscription/composables/useSubscription'
import { isCloud } from '@/platform/distribution/types'
import { useTelemetry } from '@/platform/telemetry'
import { useDialogService } from '@/services/dialogService'
Expand All @@ -114,15 +113,18 @@ const emit = defineEmits<{

const { buildDocsUrl } = useExternalLink()

const planSettingsLabel = isCloud
? 'settingsCategories.PlanCredits'
: 'settingsCategories.Credits'
const planSettingsLabel = 'settingsCategories.Credits'

const SubscriptionSection = isCloud
? defineAsyncComponent(
() => import('./CurrentUserPopoverSubscriptionSection.vue')
)
: null
Comment on lines +116 to +122
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider inlining the constant to simplify.

The planSettingsLabel variable is only used once at line 61, and using a constant for a single literal value adds indirection without benefit.

-const planSettingsLabel = 'settingsCategories.Credits'
-
 const SubscriptionSection = isCloud
   ? defineAsyncComponent(
       () => import('./CurrentUserPopoverSubscriptionSection.vue')
     )
   : null

And at line 61:

-        :label="$t(planSettingsLabel)"
+        :label="$t('settingsCategories.Credits')"

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/components/topbar/CurrentUserPopover.vue around lines 116–122, the
constant planSettingsLabel ('settingsCategories.Credits') is declared but only
used once (line 61); remove this unnecessary variable declaration and replace
its single usage with the literal 'settingsCategories.Credits' directly, then
delete the now-unused constant to reduce indirection and keep the file clean.


const { userDisplayName, userEmail, userPhotoUrl, handleSignOut } =
useCurrentUser()
const authActions = useFirebaseAuthActions()
const dialogService = useDialogService()
const { isActiveSubscription, fetchStatus } = useSubscription()

const handleOpenUserSettings = () => {
dialogService.showSettingsDialog('user')
Expand Down Expand Up @@ -161,10 +163,6 @@ const handleLogout = async () => {
emit('close')
}

const handleSubscribed = async () => {
await fetchStatus()
}

onMounted(() => {
void authActions.fetchBalance()
})
Expand Down
78 changes: 78 additions & 0 deletions src/components/topbar/CurrentUserPopoverSubscriptionSection.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<template>
<div v-if="isSubscriptionRequirementMet" class="flex flex-col gap-2">
<div class="flex items-center justify-between">
<div class="flex flex-col gap-1">
<UserCredit text-class="text-2xl" />
<Button
:label="$t('subscription.partnerNodesCredits')"
severity="secondary"
text
size="small"
class="pl-6 p-0 h-auto justify-start"
:pt="{
root: {
class: 'hover:bg-transparent active:bg-transparent'
}
}"
@click="handleOpenPartnerInfo"
/>
</div>
<Button
:label="$t('credits.topUp.topUp')"
severity="secondary"
size="small"
@click="handleTopUp"
/>
</div>

<Button
class="justify-start"
:label="$t('settingsCategories.PlanCredits')"
icon="pi pi-receipt"
text
fluid
severity="secondary"
@click="handleOpenPlanSettings"
/>
</div>
<SubscribeButton
v-else
:label="$t('subscription.subscribeToComfyCloud')"
size="small"
variant="gradient"
class="w-full"
@subscribed="handleSubscribed"
/>
</template>

<script setup lang="ts">
import Button from 'primevue/button'

import UserCredit from '@/components/common/UserCredit.vue'
import SubscribeButton from '@/platform/cloud/subscription/components/SubscribeButton.vue'
import { useSubscription } from '@/platform/cloud/subscription/composables/useSubscription'

const emit = defineEmits<{
'top-up': []
'open-partner-info': []
'open-plan-settings': []
}>()

const { isSubscriptionRequirementMet, fetchStatus } = useSubscription()

const handleSubscribed = async () => {
await fetchStatus()
}

const handleTopUp = () => {
emit('top-up')
}

const handleOpenPartnerInfo = () => {
emit('open-partner-info')
}

const handleOpenPlanSettings = () => {
emit('open-plan-settings')
}
</script>
10 changes: 7 additions & 3 deletions src/composables/auth/useFirebaseAuthActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { useErrorHandling } from '@/composables/useErrorHandling'
import type { ErrorRecoveryStrategy } from '@/composables/useErrorHandling'
import { t } from '@/i18n'
import { isCloud } from '@/platform/distribution/types'
import { useSubscription } from '@/platform/cloud/subscription/composables/useSubscription'
import { useTelemetry } from '@/platform/telemetry'
import { useToastStore } from '@/platform/updates/common/toastStore'
import { useDialogService } from '@/services/dialogService'
Expand Down Expand Up @@ -82,8 +81,13 @@ export const useFirebaseAuthActions = () => {
)

const purchaseCredits = wrapWithErrorHandlingAsync(async (amount: number) => {
const { isActiveSubscription } = useSubscription()
if (!isActiveSubscription.value) return
if (isCloud) {
const { useSubscription } = await import(
'@/platform/cloud/subscription/composables/useSubscription'
)
const { isSubscriptionRequirementMet } = useSubscription()
if (!isSubscriptionRequirementMet.value) return
}

const response = await authStore.initiateCreditPurchase({
amount_micros: usdToMicros(amount),
Expand Down
Loading
Loading