diff --git a/web/app/components/app/app-publisher/index.tsx b/web/app/components/app/app-publisher/index.tsx index 801345798b..2dc45e1337 100644 --- a/web/app/components/app/app-publisher/index.tsx +++ b/web/app/components/app/app-publisher/index.tsx @@ -21,6 +21,7 @@ import { import { useKeyPress } from 'ahooks' import Divider from '../../base/divider' import Loading from '../../base/loading' +import Toast from '../../base/toast' import Tooltip from '../../base/tooltip' import { getKeyboardKeyCodeBySystem, getKeyboardKeyNameBySystem } from '../../workflow/utils' import AccessControl from '../app-access-control' @@ -41,6 +42,7 @@ import type { InputVar, Variable } from '@/app/components/workflow/types' import { appDefaultIconBackground } from '@/config' import { useGlobalPublicStore } from '@/context/global-public-context' import { useFormatTimeFromNow } from '@/hooks/use-format-time-from-now' +import { useAsyncWindowOpen } from '@/hooks/use-async-window-open' import { AccessMode } from '@/models/access-control' import { useAppWhiteListSubjects, useGetUserCanAccessApp } from '@/service/access-control' import { fetchAppDetailDirect } from '@/service/apps' @@ -49,7 +51,6 @@ import { AppModeEnum } from '@/types/app' import type { PublishWorkflowParams } from '@/types/workflow' import { basePath } from '@/utils/var' import UpgradeBtn from '@/app/components/billing/upgrade-btn' -import { useAsyncWindowOpen } from '@/hooks/use-async-window-open' const ACCESS_MODE_MAP: Record = { [AccessMode.ORGANIZATION]: { @@ -153,6 +154,7 @@ const AppPublisher = ({ const { data: userCanAccessApp, isLoading: isGettingUserCanAccessApp, refetch } = useGetUserCanAccessApp({ appId: appDetail?.id, enabled: false }) const { data: appAccessSubjects, isLoading: isGettingAppWhiteListSubjects } = useAppWhiteListSubjects(appDetail?.id, open && systemFeatures.webapp_auth.enabled && appDetail?.access_mode === AccessMode.SPECIFIC_GROUPS_MEMBERS) + const openAsyncWindow = useAsyncWindowOpen() const noAccessPermission = useMemo(() => systemFeatures.webapp_auth.enabled && appDetail && appDetail.access_mode !== AccessMode.EXTERNAL_MEMBERS && !userCanAccessApp?.result, [systemFeatures, appDetail, userCanAccessApp]) const disabledFunctionButton = useMemo(() => (!publishedAt || missingStartNode || noAccessPermission), [publishedAt, missingStartNode, noAccessPermission]) @@ -216,23 +218,20 @@ const AppPublisher = ({ setPublished(false) }, [disabled, onToggle, open]) - const { openAsync } = useAsyncWindowOpen() - - const handleOpenInExplore = useCallback(() => { - if (!appDetail?.id) return - - openAsync( - async () => { - const { installed_apps }: { installed_apps?: { id: string }[] } = await fetchInstalledAppList(appDetail.id) || {} - if (installed_apps && installed_apps.length > 0) - return `${basePath}/explore/installed/${installed_apps[0].id}` - throw new Error('No app found in Explore') + const handleOpenInExplore = useCallback(async () => { + await openAsyncWindow(async () => { + if (!appDetail?.id) + throw new Error('App not found') + const { installed_apps }: any = await fetchInstalledAppList(appDetail?.id) || {} + if (installed_apps?.length > 0) + return `${basePath}/explore/installed/${installed_apps[0].id}` + throw new Error('No app found in Explore') + }, { + onError: (err) => { + Toast.notify({ type: 'error', message: `${err.message || err}` }) }, - { - errorMessage: 'Failed to open app in Explore', - }, - ) - }, [appDetail?.id, openAsync]) + }) + }, [appDetail?.id, openAsyncWindow]) const handleAccessControlUpdate = useCallback(async () => { if (!appDetail) diff --git a/web/app/components/apps/app-card.tsx b/web/app/components/apps/app-card.tsx index 407df23913..b8da0264e4 100644 --- a/web/app/components/apps/app-card.tsx +++ b/web/app/components/apps/app-card.tsx @@ -7,7 +7,7 @@ import { useTranslation } from 'react-i18next' import { RiBuildingLine, RiGlobalLine, RiLockLine, RiMoreFill, RiVerifiedBadgeLine } from '@remixicon/react' import cn from '@/utils/classnames' import { type App, AppModeEnum } from '@/types/app' -import { ToastContext } from '@/app/components/base/toast' +import Toast, { ToastContext } from '@/app/components/base/toast' import { copyApp, deleteApp, exportAppConfig, updateAppInfo } from '@/service/apps' import type { DuplicateAppModalProps } from '@/app/components/app/duplicate-modal' import AppIcon from '@/app/components/base/app-icon' @@ -27,11 +27,11 @@ import { fetchWorkflowDraft } from '@/service/workflow' import { fetchInstalledAppList } from '@/service/explore' import { AppTypeIcon } from '@/app/components/app/type-selector' import Tooltip from '@/app/components/base/tooltip' +import { useAsyncWindowOpen } from '@/hooks/use-async-window-open' import { AccessMode } from '@/models/access-control' import { useGlobalPublicStore } from '@/context/global-public-context' import { formatTime } from '@/utils/time' import { useGetUserCanAccessApp } from '@/service/access-control' -import { useAsyncWindowOpen } from '@/hooks/use-async-window-open' import dynamic from 'next/dynamic' const EditAppModal = dynamic(() => import('@/app/components/explore/create-app-modal'), { @@ -65,6 +65,7 @@ const AppCard = ({ app, onRefresh }: AppCardProps) => { const { isCurrentWorkspaceEditor } = useAppContext() const { onPlanInfoChanged } = useProviderContext() const { push } = useRouter() + const openAsyncWindow = useAsyncWindowOpen() const [showEditModal, setShowEditModal] = useState(false) const [showDuplicateModal, setShowDuplicateModal] = useState(false) @@ -243,24 +244,25 @@ const AppCard = ({ app, onRefresh }: AppCardProps) => { e.preventDefault() setShowAccessControl(true) } - const { openAsync } = useAsyncWindowOpen() - - const onClickInstalledApp = (e: React.MouseEvent) => { + const onClickInstalledApp = async (e: React.MouseEvent) => { e.stopPropagation() props.onClick?.() e.preventDefault() - - openAsync( - async () => { - const { installed_apps }: { installed_apps?: { id: string }[] } = await fetchInstalledAppList(app.id) || {} - if (installed_apps && installed_apps.length > 0) + try { + await openAsyncWindow(async () => { + const { installed_apps }: any = await fetchInstalledAppList(app.id) || {} + if (installed_apps?.length > 0) return `${basePath}/explore/installed/${installed_apps[0].id}` throw new Error('No app found in Explore') - }, - { - errorMessage: 'Failed to open app in Explore', - }, - ) + }, { + onError: (err) => { + Toast.notify({ type: 'error', message: `${err.message || err}` }) + }, + }) + } + catch (e: any) { + Toast.notify({ type: 'error', message: `${e.message || e}` }) + } } return (
diff --git a/web/app/components/billing/billing-page/index.tsx b/web/app/components/billing/billing-page/index.tsx index adb676cde1..590219c2d5 100644 --- a/web/app/components/billing/billing-page/index.tsx +++ b/web/app/components/billing/billing-page/index.tsx @@ -9,33 +9,28 @@ import PlanComp from '../plan' import { useAppContext } from '@/context/app-context' import { useProviderContext } from '@/context/provider-context' import { useBillingUrl } from '@/service/use-billing' +import { useAsyncWindowOpen } from '@/hooks/use-async-window-open' const Billing: FC = () => { const { t } = useTranslation() const { isCurrentWorkspaceManager } = useAppContext() const { enableBilling } = useProviderContext() const { data: billingUrl, isFetching, refetch } = useBillingUrl(enableBilling && isCurrentWorkspaceManager) + const openAsyncWindow = useAsyncWindowOpen() const handleOpenBilling = async () => { - // Open synchronously to preserve user gesture for popup blockers - if (billingUrl) { - window.open(billingUrl, '_blank', 'noopener,noreferrer') - return - } - - const newWindow = window.open('', '_blank', 'noopener,noreferrer') - try { + await openAsyncWindow(async () => { const url = (await refetch()).data - if (url && newWindow) { - newWindow.location.href = url - return - } - } - catch (err) { - console.error('Failed to fetch billing url', err) - } - // Close the placeholder window if we failed to fetch the URL - newWindow?.close() + if (url) + return url + return null + }, { + immediateUrl: billingUrl, + features: 'noopener,noreferrer', + onError: (err) => { + console.error('Failed to fetch billing url', err) + }, + }) } return ( diff --git a/web/app/components/billing/pricing/plans/cloud-plan-item/index.tsx b/web/app/components/billing/pricing/plans/cloud-plan-item/index.tsx index 164ad9061a..52c2883b81 100644 --- a/web/app/components/billing/pricing/plans/cloud-plan-item/index.tsx +++ b/web/app/components/billing/pricing/plans/cloud-plan-item/index.tsx @@ -43,6 +43,7 @@ const CloudPlanItem: FC = ({ const isCurrentPaidPlan = isCurrent && !isFreePlan const isPlanDisabled = isCurrentPaidPlan ? false : planInfo.level <= ALL_PLANS[currentPlan].level const { isCurrentWorkspaceManager } = useAppContext() + const openAsyncWindow = useAsyncWindowOpen() const btnText = useMemo(() => { if (isCurrent) @@ -55,8 +56,6 @@ const CloudPlanItem: FC = ({ })[plan] }, [isCurrent, plan, t]) - const { openAsync } = useAsyncWindowOpen() - const handleGetPayUrl = async () => { if (loading) return @@ -75,13 +74,16 @@ const CloudPlanItem: FC = ({ setLoading(true) try { if (isCurrentPaidPlan) { - await openAsync( - () => fetchBillingUrl().then(res => res.url), - { - errorMessage: 'Failed to open billing page', - windowFeatures: 'noopener,noreferrer', + await openAsyncWindow(async () => { + const res = await fetchBillingUrl() + if (res.url) + return res.url + throw new Error('Failed to open billing page') + }, { + onError: (err) => { + Toast.notify({ type: 'error', message: err.message || String(err) }) }, - ) + }) return } diff --git a/web/hooks/use-async-window-open.spec.ts b/web/hooks/use-async-window-open.spec.ts new file mode 100644 index 0000000000..63ec9185da --- /dev/null +++ b/web/hooks/use-async-window-open.spec.ts @@ -0,0 +1,116 @@ +import { act, renderHook } from '@testing-library/react' +import { useAsyncWindowOpen } from './use-async-window-open' + +describe('useAsyncWindowOpen', () => { + const originalOpen = window.open + + beforeEach(() => { + jest.clearAllMocks() + }) + + afterAll(() => { + window.open = originalOpen + }) + + it('opens immediate url synchronously without calling async getter', async () => { + const openSpy = jest.fn() + window.open = openSpy + const getUrl = jest.fn() + const { result } = renderHook(() => useAsyncWindowOpen()) + + await act(async () => { + await result.current(getUrl, { + immediateUrl: 'https://example.com', + target: '_blank', + features: 'noopener,noreferrer', + }) + }) + + expect(openSpy).toHaveBeenCalledWith('https://example.com', '_blank', 'noopener,noreferrer') + expect(getUrl).not.toHaveBeenCalled() + }) + + it('sets opener to null and redirects when async url resolves', async () => { + const close = jest.fn() + const mockWindow: any = { + location: { href: '' }, + close, + opener: 'should-be-cleared', + } + const openSpy = jest.fn(() => mockWindow) + window.open = openSpy + const { result } = renderHook(() => useAsyncWindowOpen()) + + await act(async () => { + await result.current(async () => 'https://example.com/path') + }) + + expect(openSpy).toHaveBeenCalledWith('about:blank', '_blank', undefined) + expect(mockWindow.opener).toBeNull() + expect(mockWindow.location.href).toBe('https://example.com/path') + expect(close).not.toHaveBeenCalled() + }) + + it('closes placeholder and forwards error when async getter throws', async () => { + const close = jest.fn() + const mockWindow: any = { + location: { href: '' }, + close, + opener: null, + } + const openSpy = jest.fn(() => mockWindow) + window.open = openSpy + const onError = jest.fn() + const { result } = renderHook(() => useAsyncWindowOpen()) + + const error = new Error('fetch failed') + await act(async () => { + await result.current(async () => { + throw error + }, { onError }) + }) + + expect(close).toHaveBeenCalled() + expect(onError).toHaveBeenCalledWith(error) + expect(mockWindow.location.href).toBe('') + }) + + it('closes placeholder and reports when no url is returned', async () => { + const close = jest.fn() + const mockWindow: any = { + location: { href: '' }, + close, + opener: null, + } + const openSpy = jest.fn(() => mockWindow) + window.open = openSpy + const onError = jest.fn() + const { result } = renderHook(() => useAsyncWindowOpen()) + + await act(async () => { + await result.current(async () => null, { onError }) + }) + + expect(close).toHaveBeenCalled() + expect(onError).toHaveBeenCalled() + const errArg = onError.mock.calls[0][0] as Error + expect(errArg.message).toBe('No url resolved for new window') + }) + + it('reports failure when window.open returns null', async () => { + const openSpy = jest.fn(() => null) + window.open = openSpy + const getUrl = jest.fn() + const onError = jest.fn() + const { result } = renderHook(() => useAsyncWindowOpen()) + + await act(async () => { + await result.current(getUrl, { onError }) + }) + + expect(onError).toHaveBeenCalled() + const errArg = onError.mock.calls[0][0] as Error + expect(errArg.message).toBe('Failed to open new window') + expect(getUrl).not.toHaveBeenCalled() + }) +}) diff --git a/web/hooks/use-async-window-open.ts b/web/hooks/use-async-window-open.ts index 582ab28be4..e3d7910217 100644 --- a/web/hooks/use-async-window-open.ts +++ b/web/hooks/use-async-window-open.ts @@ -1,72 +1,49 @@ import { useCallback } from 'react' -import Toast from '@/app/components/base/toast' -export type AsyncWindowOpenOptions = { - successMessage?: string - errorMessage?: string - windowFeatures?: string - onError?: (error: any) => void - onSuccess?: (url: string) => void +type GetUrl = () => Promise + +type AsyncWindowOpenOptions = { + immediateUrl?: string | null + target?: string + features?: string + onError?: (error: Error) => void } -export const useAsyncWindowOpen = () => { - const openAsync = useCallback(async ( - fetchUrl: () => Promise, - options: AsyncWindowOpenOptions = {}, - ) => { - const { - successMessage, - errorMessage = 'Failed to open page', - windowFeatures = 'noopener,noreferrer', - onError, - onSuccess, - } = options +export const useAsyncWindowOpen = () => useCallback(async (getUrl: GetUrl, options?: AsyncWindowOpenOptions) => { + const { + immediateUrl, + target = '_blank', + features, + onError, + } = options ?? {} - const newWindow = window.open('', '_blank', windowFeatures) + if (immediateUrl) { + window.open(immediateUrl, target, features) + return + } - if (!newWindow) { - const error = new Error('Popup blocked by browser') - onError?.(error) - Toast.notify({ - type: 'error', - message: 'Popup blocked. Please allow popups for this site.', - }) + const newWindow = window.open('about:blank', target, features) + if (!newWindow) { + onError?.(new Error('Failed to open new window')) + return + } + + try { + newWindow.opener = null + } + catch { /* noop */ } + + try { + const url = await getUrl() + if (url) { + newWindow.location.href = url return } - - try { - const url = await fetchUrl() - - if (url) { - newWindow.location.href = url - onSuccess?.(url) - - if (successMessage) { - Toast.notify({ - type: 'success', - message: successMessage, - }) - } - } - else { - newWindow.close() - const error = new Error('Invalid URL received') - onError?.(error) - Toast.notify({ - type: 'error', - message: errorMessage, - }) - } - } - catch (error) { - newWindow.close() - onError?.(error) - Toast.notify({ - type: 'error', - message: errorMessage, - }) - } - }, []) - - return { openAsync } -} + newWindow.close() + onError?.(new Error('No url resolved for new window')) + } + catch (error) { + newWindow.close() + onError?.(error instanceof Error ? error : new Error(String(error))) + } +}, [])