mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-22 05:19:03 +00:00
Revert "fix(cloud): stop bouncing working users to /cloud/survey mid-session" (#12344)
Reverts Comfy-Org/ComfyUI_frontend#12301 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12344-Revert-fix-cloud-stop-bouncing-working-users-to-cloud-survey-mid-session-3656d73d365081119ebad749a2e0d403) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -1,91 +0,0 @@
|
||||
import { fromPartial } from '@total-typescript/shoehorn'
|
||||
import { beforeEach, describe, expect, test, vi } from 'vitest'
|
||||
|
||||
import { getSurveyCompletedStatus } from './auth'
|
||||
|
||||
const fetchApi = vi.fn()
|
||||
|
||||
vi.mock('@/scripts/api', () => ({
|
||||
api: {
|
||||
fetchApi: (...args: unknown[]) => fetchApi(...args)
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@sentry/vue', () => ({
|
||||
addBreadcrumb: vi.fn(),
|
||||
captureException: vi.fn()
|
||||
}))
|
||||
|
||||
function mockResponse({
|
||||
ok,
|
||||
status,
|
||||
body
|
||||
}: {
|
||||
ok: boolean
|
||||
status: number
|
||||
body?: unknown
|
||||
}): Response {
|
||||
return fromPartial<Response>({
|
||||
ok,
|
||||
status,
|
||||
statusText: '',
|
||||
json: async () => body
|
||||
})
|
||||
}
|
||||
|
||||
describe('getSurveyCompletedStatus', () => {
|
||||
beforeEach(() => {
|
||||
fetchApi.mockReset()
|
||||
})
|
||||
|
||||
test('200 with non-empty value → true', async () => {
|
||||
fetchApi.mockResolvedValueOnce(
|
||||
mockResponse({ ok: true, status: 200, body: { value: { q1: 'a' } } })
|
||||
)
|
||||
await expect(getSurveyCompletedStatus()).resolves.toBe(true)
|
||||
})
|
||||
|
||||
test('200 with empty value → false (the only "not completed" signal)', async () => {
|
||||
fetchApi.mockResolvedValueOnce(
|
||||
mockResponse({ ok: true, status: 200, body: { value: {} } })
|
||||
)
|
||||
await expect(getSurveyCompletedStatus()).resolves.toBe(false)
|
||||
})
|
||||
|
||||
test('200 with null value → false', async () => {
|
||||
fetchApi.mockResolvedValueOnce(
|
||||
mockResponse({ ok: true, status: 200, body: { value: null } })
|
||||
)
|
||||
await expect(getSurveyCompletedStatus()).resolves.toBe(false)
|
||||
})
|
||||
|
||||
test('404 → true (do not bounce on missing key)', async () => {
|
||||
fetchApi.mockResolvedValueOnce(mockResponse({ ok: false, status: 404 }))
|
||||
await expect(getSurveyCompletedStatus()).resolves.toBe(true)
|
||||
})
|
||||
|
||||
test('500 → true (do not bounce on transient backend error)', async () => {
|
||||
fetchApi.mockResolvedValueOnce(mockResponse({ ok: false, status: 500 }))
|
||||
await expect(getSurveyCompletedStatus()).resolves.toBe(true)
|
||||
})
|
||||
|
||||
// 401/403 fall under the same "ambiguous => treat as completed" policy.
|
||||
// The dedicated auth layer handles re-authentication on the next API
|
||||
// call; this function deliberately does not try to disambiguate auth
|
||||
// failures from other non-OK responses. Locking with tests so the
|
||||
// policy can't drift back to a "throw on auth error" branch.
|
||||
test('401 → true (auth layer handles re-auth on next call)', async () => {
|
||||
fetchApi.mockResolvedValueOnce(mockResponse({ ok: false, status: 401 }))
|
||||
await expect(getSurveyCompletedStatus()).resolves.toBe(true)
|
||||
})
|
||||
|
||||
test('403 → true (auth layer handles re-auth on next call)', async () => {
|
||||
fetchApi.mockResolvedValueOnce(mockResponse({ ok: false, status: 403 }))
|
||||
await expect(getSurveyCompletedStatus()).resolves.toBe(true)
|
||||
})
|
||||
|
||||
test('network rejection → true (do not bounce on network error)', async () => {
|
||||
fetchApi.mockRejectedValueOnce(new TypeError('Network request failed'))
|
||||
await expect(getSurveyCompletedStatus()).resolves.toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -96,24 +96,23 @@ export async function getSurveyCompletedStatus(): Promise<boolean> {
|
||||
}
|
||||
})
|
||||
if (!response.ok) {
|
||||
// Ambiguous response (404/5xx/etc). Treat as completed to avoid
|
||||
// bouncing working customers to /cloud/survey on transient hiccups.
|
||||
// Real "not completed" only comes from a 200 with empty value.
|
||||
// Not an error case - survey not completed is a valid state
|
||||
Sentry.addBreadcrumb({
|
||||
category: 'auth',
|
||||
message: 'Survey status check returned non-ok response',
|
||||
level: 'warning',
|
||||
level: 'info',
|
||||
data: {
|
||||
status: response.status,
|
||||
endpoint: `/settings/${ONBOARDING_SURVEY_KEY}`
|
||||
}
|
||||
})
|
||||
return true
|
||||
return false
|
||||
}
|
||||
const data = await response.json()
|
||||
// Check if data exists and is not empty
|
||||
return !isEmpty(data.value)
|
||||
} catch (error) {
|
||||
// Network/parse failure — same policy as ambiguous HTTP responses.
|
||||
// Network error - still capture it as it's not thrown from above
|
||||
Sentry.captureException(error, {
|
||||
tags: {
|
||||
api_endpoint: '/settings/{key}',
|
||||
@@ -125,7 +124,7 @@ export async function getSurveyCompletedStatus(): Promise<boolean> {
|
||||
},
|
||||
level: 'warning'
|
||||
})
|
||||
return true
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user