Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions lint-staged.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import path from 'node:path'

export default {
'./**/*.js': (stagedFiles) => formatAndEslint(stagedFiles),

'./**/*.{ts,tsx,vue,mts}': (stagedFiles) => [
...formatAndEslint(stagedFiles),
'pnpm typecheck'
]
Comment on lines +6 to +9
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider the performance impact of project-wide typecheck.

The pnpm typecheck command runs on the entire project whenever any TypeScript/Vue file is staged, rather than checking only the staged files. For large codebases, this could slow down the commit process.

However, this is likely intentional since TypeScript type checking often requires full project context to catch type errors that might arise from changes. If typecheck performance becomes an issue, you could explore alternatives like tsc --incremental or checking only affected files, but the current approach is safer for correctness.

}
Comment on lines +3 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider adding handlers for .jsx, .json, and .css files.

Based on project conventions, Prettier formatting should apply to .jsx, .json, and .css files, but these aren't currently covered by the lint-staged configuration. If your project uses these file types, they won't be automatically formatted on commit.

🔎 Proposed configuration with additional file types
 export default {
   './**/*.js': (stagedFiles) => formatAndEslint(stagedFiles),
 
+  './**/*.jsx': (stagedFiles) => formatAndEslint(stagedFiles),
+
+  './**/*.{json,css}': (stagedFiles) => {
+    const relativePaths = stagedFiles.map((f) => path.relative(process.cwd(), f))
+    const joinedPaths = relativePaths.map((p) => `"${p}"`).join(' ')
+    return `pnpm exec prettier --cache --write ${joinedPaths}`
+  },
+
   './**/*.{ts,tsx,vue,mts}': (stagedFiles) => [
     ...formatAndEslint(stagedFiles),
     'pnpm typecheck'
   ]
 }

Note: JSON and CSS files typically only need Prettier formatting, not ESLint/oxlint.

Based on learnings, the project's formatting standards should cover these file types.

📝 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
export default {
'./**/*.js': (stagedFiles) => formatAndEslint(stagedFiles),
'./**/*.{ts,tsx,vue,mts}': (stagedFiles) => [
...formatAndEslint(stagedFiles),
'pnpm typecheck'
]
}
export default {
'./**/*.js': (stagedFiles) => formatAndEslint(stagedFiles),
'./**/*.jsx': (stagedFiles) => formatAndEslint(stagedFiles),
'./**/*.{json,css}': (stagedFiles) => {
const relativePaths = stagedFiles.map((f) => path.relative(process.cwd(), f))
const joinedPaths = relativePaths.map((p) => `"${p}"`).join(' ')
return `pnpm exec prettier --cache --write ${joinedPaths}`
},
'./**/*.{ts,tsx,vue,mts}': (stagedFiles) => [
...formatAndEslint(stagedFiles),
'pnpm typecheck'
]
}
🤖 Prompt for AI Agents
In @lint-staged.config.mjs around lines 3 - 10, Update the lint-staged file
patterns to include .jsx, .json, and .css so they get Prettier/formatting on
commit: extend the existing glob entries referencing the current handlers (the
function formatAndEslint used for JS/TS/TSX/VUE files) to also match .jsx files,
and add separate patterns for .json and .css that run only the formatting step
(not ESLint) to avoid running linters that don't apply; ensure you reference the
same formatAndEslint symbol for applicable globs and only the formatter for
JSON/CSS.


function formatAndEslint(fileNames) {
// Convert absolute paths to relative paths for better ESLint resolution
const relativePaths = fileNames.map((f) => path.relative(process.cwd(), f))
const joinedPaths = relativePaths.map((p) => `"${p}"`).join(' ')
return [
`pnpm exec prettier --cache --write ${joinedPaths}`,
`pnpm exec oxlint --fix ${joinedPaths}`,
`pnpm exec eslint --cache --fix --no-warn-ignored ${joinedPaths}`
]
}


10 changes: 7 additions & 3 deletions src/composables/auth/useFirebaseAuthActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ export const useFirebaseAuthActions = () => {
}, reportError)

const accessBillingPortal = wrapWithErrorHandlingAsync<
[targetTier?: BillingPortalTargetTier],
[targetTier?: BillingPortalTargetTier, openInNewTab?: boolean],
void
>(async (targetTier) => {
>(async (targetTier, openInNewTab = true) => {
const response = await authStore.accessBillingPortal(targetTier)
if (!response.billing_portal_url) {
throw new Error(
Expand All @@ -115,7 +115,11 @@ export const useFirebaseAuthActions = () => {
})
)
}
window.open(response.billing_portal_url, '_blank')
if (openInNewTab) {
window.open(response.billing_portal_url, '_blank')
} else {
globalThis.location.href = response.billing_portal_url
}
}, reportError)

const fetchBalance = wrapWithErrorHandlingAsync(async () => {
Expand Down
1 change: 1 addition & 0 deletions src/locales/en/main.json
Original file line number Diff line number Diff line change
Expand Up @@ -2129,6 +2129,7 @@
"renderErrorState": "Render Error State"
},
"cloudOnboarding": {
"skipToCloudApp": "Skip to the cloud app",
"survey": {
"title": "Cloud Survey",
"placeholder": "Survey questions placeholder",
Expand Down
14 changes: 11 additions & 3 deletions src/platform/cloud/onboarding/CloudLoginView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,20 @@ import { useRoute, useRouter } from 'vue-router'
import Button from '@/components/ui/button/Button.vue'
import { useFirebaseAuthActions } from '@/composables/auth/useFirebaseAuthActions'
import CloudSignInForm from '@/platform/cloud/onboarding/components/CloudSignInForm.vue'
import { getSafePreviousFullPath } from '@/platform/cloud/onboarding/utils/previousFullPath'
import { useToastStore } from '@/platform/updates/common/toastStore'
import type { SignInData } from '@/schemas/signInSchema'

const { t } = useI18n()
const router = useRouter()
const route = useRoute()
const authActions = useFirebaseAuthActions()
const isSecureContext = window.isSecureContext
const isSecureContext = globalThis.isSecureContext
const authError = ref('')
const toastStore = useToastStore()

const navigateToSignup = () => {
void router.push({ name: 'cloud-signup', query: route.query })
const navigateToSignup = async () => {
await router.push({ name: 'cloud-signup', query: route.query })
}
Comment on lines +99 to 101
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Async keyword unnecessary for navigateToSignup.

The function is now async but only contains a single await router.push(). Since there's no error handling or subsequent awaits, and Vue Router's push is typically fire-and-forget in this context, the async/await adds no value here.

♻️ Optional simplification
-const navigateToSignup = async () => {
-  await router.push({ name: 'cloud-signup', query: route.query })
+const navigateToSignup = () => {
+  void router.push({ name: 'cloud-signup', query: route.query })
 }
📝 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 navigateToSignup = async () => {
await router.push({ name: 'cloud-signup', query: route.query })
}
const navigateToSignup = () => {
void router.push({ name: 'cloud-signup', query: route.query })
}
🤖 Prompt for AI Agents
In @src/platform/cloud/onboarding/CloudLoginView.vue around lines 99 - 101, The
navigateToSignup function is marked async but only calls await router.push()
with no error handling or further awaits; remove the unnecessary async/await by
converting navigateToSignup to a synchronous function and either return
router.push(...) directly or call router.push(...) without await (reference
navigateToSignup and router.push to locate the code).


const onSuccess = async () => {
Expand All @@ -105,6 +106,13 @@ const onSuccess = async () => {
summary: 'Login Completed',
life: 2000
})

const previousFullPath = getSafePreviousFullPath(route.query)
if (previousFullPath) {
await router.replace(previousFullPath)
return
}

await router.push({ name: 'cloud-user-check' })
}

Expand Down
16 changes: 12 additions & 4 deletions src/platform/cloud/onboarding/CloudSignupView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ import { useRoute, useRouter } from 'vue-router'
import SignUpForm from '@/components/dialog/content/signin/SignUpForm.vue'
import Button from '@/components/ui/button/Button.vue'
import { useFirebaseAuthActions } from '@/composables/auth/useFirebaseAuthActions'
import { getSafePreviousFullPath } from '@/platform/cloud/onboarding/utils/previousFullPath'
import { isCloud } from '@/platform/distribution/types'
import { useTelemetry } from '@/platform/telemetry'
import { useToastStore } from '@/platform/updates/common/toastStore'
Expand All @@ -110,13 +111,13 @@ const { t } = useI18n()
const router = useRouter()
const route = useRoute()
const authActions = useFirebaseAuthActions()
const isSecureContext = window.isSecureContext
const isSecureContext = globalThis.isSecureContext
const authError = ref('')
const userIsInChina = ref(false)
const toastStore = useToastStore()

const navigateToLogin = () => {
void router.push({ name: 'cloud-login', query: route.query })
const navigateToLogin = async () => {
await router.push({ name: 'cloud-login', query: route.query })
}

const onSuccess = async () => {
Expand All @@ -125,7 +126,14 @@ const onSuccess = async () => {
summary: 'Sign up Completed',
life: 2000
})
// Direct redirect to main app - email verification removed

const previousFullPath = getSafePreviousFullPath(route.query)
if (previousFullPath) {
await router.replace(previousFullPath)
return
}

// Default redirect to the normal onboarding flow
await router.push({ path: '/', query: route.query })
}

Expand Down
162 changes: 162 additions & 0 deletions src/platform/cloud/onboarding/CloudSubscriptionRedirectView.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
import { mount } from '@vue/test-utils'
import { beforeEach, describe, expect, test, vi } from 'vitest'
import { createI18n } from 'vue-i18n'

import CloudSubscriptionRedirectView from './CloudSubscriptionRedirectView.vue'

const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0))

// Router mocks
let mockQuery: Record<string, unknown> = {}
const mockRouterPush = vi.fn()

vi.mock('vue-router', () => ({
useRoute: () => ({
query: mockQuery
}),
useRouter: () => ({
push: mockRouterPush
})
}))

// Firebase / subscription mocks
const authActionMocks = vi.hoisted(() => ({
reportError: vi.fn(),
accessBillingPortal: vi.fn()
}))

vi.mock('@/composables/auth/useFirebaseAuthActions', () => ({
useFirebaseAuthActions: () => authActionMocks
}))

vi.mock('@/composables/useErrorHandling', () => ({
useErrorHandling: () => ({
wrapWithErrorHandlingAsync:
<T extends (...args: never[]) => unknown>(fn: T) =>
(...args: Parameters<T>) =>
fn(...args)
})
}))

const subscriptionMocks = vi.hoisted(() => ({
isActiveSubscription: { value: false },
isInitialized: { value: true }
}))

vi.mock('@/platform/cloud/subscription/composables/useSubscription', () => ({
useSubscription: () => subscriptionMocks
}))

// Avoid real network / isCloud behavior
const mockPerformSubscriptionCheckout = vi.fn()
vi.mock('@/platform/cloud/subscription/utils/subscriptionCheckoutUtil', () => ({
performSubscriptionCheckout: (...args: unknown[]) =>
mockPerformSubscriptionCheckout(...args)
}))

const createI18nInstance = () =>
createI18n({
legacy: false,
locale: 'en',
messages: {
en: {
cloudOnboarding: {
skipToCloudApp: 'Skip to the cloud app'
},
g: {
comfyOrgLogoAlt: 'Comfy org logo'
},
subscription: {
subscribeTo: 'Subscribe to {plan}',
tiers: {
standard: { name: 'Standard' },
creator: { name: 'Creator' },
pro: { name: 'Pro' }
}
}
}
}
})

const mountView = async (query: Record<string, unknown>) => {
mockQuery = query

const wrapper = mount(CloudSubscriptionRedirectView, {
global: {
plugins: [createI18nInstance()]
}
})

await flushPromises()

return { wrapper }
}

describe('CloudSubscriptionRedirectView', () => {
beforeEach(() => {
vi.clearAllMocks()
mockQuery = {}
subscriptionMocks.isActiveSubscription.value = false
subscriptionMocks.isInitialized.value = true
})

test('redirects to home when subscriptionType is missing', async () => {
await mountView({})

expect(mockRouterPush).toHaveBeenCalledWith('/')
})
Comment on lines +103 to +107
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Test description uses outdated parameter name.

The test description refers to "subscriptionType" but the component uses tier as the query parameter. Update for consistency with the implementation.

🔧 Suggested fix
-  test('redirects to home when subscriptionType is missing', async () => {
+  test('redirects to home when tier is missing', async () => {
     await mountView({})

     expect(mockRouterPush).toHaveBeenCalledWith('/')
   })
🤖 Prompt for AI Agents
In @src/platform/cloud/onboarding/CloudSubscriptionRedirectView.test.ts around
lines 103 - 107, Update the test's description string to match the component's
query parameter name: change "redirects to home when subscriptionType is
missing" to "redirects to home when tier is missing" in the test wrapping the
call to mountView({}) and asserting mockRouterPush toHaveBeenCalledWith('/');
keep the test body (mountView and mockRouterPush assertion) unchanged so it
stays consistent with the component's use of the `tier` query parameter.


test('redirects to home when subscriptionType is invalid', async () => {
await mountView({ tier: 'invalid' })

expect(mockRouterPush).toHaveBeenCalledWith('/')
})

test('shows subscription copy when subscriptionType is valid', async () => {
const { wrapper } = await mountView({ tier: 'creator' })

// Should not redirect to home
expect(mockRouterPush).not.toHaveBeenCalledWith('/')

// Shows copy under logo
expect(wrapper.text()).toContain('Subscribe to Creator')

// Triggers checkout flow
expect(mockPerformSubscriptionCheckout).toHaveBeenCalledWith(
'creator',
'monthly',
false
)

// Shows loading affordances
expect(wrapper.findComponent({ name: 'ProgressSpinner' }).exists()).toBe(
true
)
const skipLink = wrapper.get('a[href="/"]')
expect(skipLink.text()).toContain('Skip to the cloud app')
})
Comment on lines +115 to +137
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Good behavioral coverage, but test name is inconsistent with implementation.

The test body correctly uses tier but the test name says "subscriptionType". Consider updating the test name for consistency.

🔎 Suggested fix:
-  test('shows subscription copy when subscriptionType is valid', async () => {
+  test('shows subscription copy when tier is valid', async () => {
📝 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
test('shows subscription copy when subscriptionType is valid', async () => {
const { wrapper } = await mountView({ tier: 'creator' })
// Should not redirect to home
expect(mockRouterPush).not.toHaveBeenCalledWith('/')
// Shows copy under logo
expect(wrapper.text()).toContain('Subscribe to Creator')
// Triggers checkout flow
expect(mockPerformSubscriptionCheckout).toHaveBeenCalledWith(
'creator',
'monthly',
false
)
// Shows loading affordances
expect(wrapper.findComponent({ name: 'ProgressSpinner' }).exists()).toBe(
true
)
const skipLink = wrapper.get('a[href="/"]')
expect(skipLink.text()).toContain('Skip to the cloud app')
})
test('shows subscription copy when tier is valid', async () => {
const { wrapper } = await mountView({ tier: 'creator' })
// Should not redirect to home
expect(mockRouterPush).not.toHaveBeenCalledWith('/')
// Shows copy under logo
expect(wrapper.text()).toContain('Subscribe to Creator')
// Triggers checkout flow
expect(mockPerformSubscriptionCheckout).toHaveBeenCalledWith(
'creator',
'monthly',
false
)
// Shows loading affordances
expect(wrapper.findComponent({ name: 'ProgressSpinner' }).exists()).toBe(
true
)
const skipLink = wrapper.get('a[href="/"]')
expect(skipLink.text()).toContain('Skip to the cloud app')
})
🤖 Prompt for AI Agents
In
tests-ui/tests/platform/cloud/onboarding/CloudSubscriptionRedirectView.test.ts
around lines 115 to 137 the test title says "shows subscription copy when
subscriptionType is valid" but the implementation uses the prop `tier`; update
the test name to match the implementation — e.g., change the description string
to "shows subscription copy when tier is valid" (or similar wording that
references "tier" instead of "subscriptionType") so the test name accurately
reflects what it's testing.


test('opens billing portal when subscription is already active', async () => {
subscriptionMocks.isActiveSubscription.value = true

await mountView({ tier: 'creator' })

expect(mockRouterPush).not.toHaveBeenCalledWith('/')
expect(authActionMocks.accessBillingPortal).toHaveBeenCalledTimes(1)
expect(mockPerformSubscriptionCheckout).not.toHaveBeenCalled()
})

test('uses first value when subscriptionType is an array', async () => {
const { wrapper } = await mountView({
tier: ['creator', 'pro']
})

expect(mockRouterPush).not.toHaveBeenCalledWith('/')
expect(wrapper.text()).toContain('Subscribe to Creator')
expect(mockPerformSubscriptionCheckout).toHaveBeenCalledWith(
'creator',
'monthly',
false
)
})
Comment on lines +149 to +161
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Test name inconsistency with implementation.

The test body correctly uses tier but the test name says "subscriptionType". For consistency with the actual query parameter used:

🔎 Suggested fix:
-  test('uses first value when subscriptionType is an array', async () => {
+  test('uses first value when tier is an array', async () => {
📝 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
test('uses first value when subscriptionType is an array', async () => {
const { wrapper } = await mountView({
tier: ['creator', 'pro']
})
expect(mockRouterPush).not.toHaveBeenCalledWith('/')
expect(wrapper.text()).toContain('Subscribe to Creator')
expect(mockPerformSubscriptionCheckout).toHaveBeenCalledWith(
'creator',
'monthly',
false
)
})
test('uses first value when tier is an array', async () => {
const { wrapper } = await mountView({
tier: ['creator', 'pro']
})
expect(mockRouterPush).not.toHaveBeenCalledWith('/')
expect(wrapper.text()).toContain('Subscribe to Creator')
expect(mockPerformSubscriptionCheckout).toHaveBeenCalledWith(
'creator',
'monthly',
false
)
})
🤖 Prompt for AI Agents
In
tests-ui/tests/platform/cloud/onboarding/CloudSubscriptionRedirectView.test.ts
around lines 149 to 161, the test name references "subscriptionType" but the
test uses the query parameter `tier`; update the test name to reflect the actual
parameter (e.g., "uses first value when tier is an array") so the description
matches the implementation and run the tests to confirm no other name mismatches
exist.

})
Loading