From aa26059e004dc7ce96b3b0953343ace5e86696c3 Mon Sep 17 00:00:00 2001 From: Fringg Date: Wed, 4 Mar 2026 07:56:19 +0300 Subject: [PATCH] fix: second round review fixes for merge UI - Fix false success toast when response.success is false (CRITICAL) - Remove mergeToken! non-null assertion in queryFn - Add early return for missing mergeToken param - Zero-pad minutes in formatCountdown (MM:SS format) - Clamp negative seconds in formatCountdown - Block all unlink buttons while any unlink mutation is pending - Clear OAuth state only after validation succeeds (not before) - Split getAndClearLinkOAuthState into read + clear functions --- src/pages/ConnectedAccounts.tsx | 1 + src/pages/LinkOAuthCallback.tsx | 16 +++++++++----- src/pages/MergeAccounts.tsx | 38 +++++++++++++++++++-------------- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/pages/ConnectedAccounts.tsx b/src/pages/ConnectedAccounts.tsx index bb449f1..dddfd87 100644 --- a/src/pages/ConnectedAccounts.tsx +++ b/src/pages/ConnectedAccounts.tsx @@ -106,6 +106,7 @@ export default function ConnectedAccounts() { const canUnlink = (provider: LinkedProvider): boolean => { if (!provider.linked) return false; if (!isOAuthProvider(provider.provider)) return false; + if (unlinkMutation.isPending) return false; const linkedCount = data?.providers.filter((p) => p.linked).length ?? 0; return linkedCount > 1; }; diff --git a/src/pages/LinkOAuthCallback.tsx b/src/pages/LinkOAuthCallback.tsx index bb57e2e..913df31 100644 --- a/src/pages/LinkOAuthCallback.tsx +++ b/src/pages/LinkOAuthCallback.tsx @@ -8,15 +8,18 @@ import { useToast } from '../components/Toast'; export const LINK_OAUTH_STATE_KEY = 'link_oauth_state'; export const LINK_OAUTH_PROVIDER_KEY = 'link_oauth_provider'; -function getAndClearLinkOAuthState(): { state: string; provider: string } | null { +function getLinkOAuthState(): { state: string; provider: string } | null { const state = sessionStorage.getItem(LINK_OAUTH_STATE_KEY); const provider = sessionStorage.getItem(LINK_OAUTH_PROVIDER_KEY); - sessionStorage.removeItem(LINK_OAUTH_STATE_KEY); - sessionStorage.removeItem(LINK_OAUTH_PROVIDER_KEY); if (!state || !provider) return null; return { state, provider }; } +function clearLinkOAuthState(): void { + sessionStorage.removeItem(LINK_OAUTH_STATE_KEY); + sessionStorage.removeItem(LINK_OAUTH_PROVIDER_KEY); +} + export default function LinkOAuthCallback() { const { t } = useTranslation(); const navigate = useNavigate(); @@ -41,8 +44,8 @@ export default function LinkOAuthCallback() { return; } - // Get saved state from sessionStorage - const saved = getAndClearLinkOAuthState(); + // Get saved state from sessionStorage (read only, don't clear yet) + const saved = getLinkOAuthState(); if (!saved) { showToast({ type: 'error', message: t('profile.accounts.linkError') }); navigate('/profile/accounts', { replace: true }); @@ -56,6 +59,9 @@ export default function LinkOAuthCallback() { return; } + // State validated — clear it now (one-time use) + clearLinkOAuthState(); + try { const response = await authApi.linkProviderCallback( saved.provider, diff --git a/src/pages/MergeAccounts.tsx b/src/pages/MergeAccounts.tsx index c1f36f7..8b236e3 100644 --- a/src/pages/MergeAccounts.tsx +++ b/src/pages/MergeAccounts.tsx @@ -112,9 +112,10 @@ function ProviderBadgeIcon({ provider }: { provider: string }) { } function formatCountdown(seconds: number): string { - const min = Math.floor(seconds / 60); - const sec = seconds % 60; - return `${min}:${sec.toString().padStart(2, '0')}`; + const clamped = Math.max(0, seconds); + const min = Math.floor(clamped / 60); + const sec = clamped % 60; + return `${min.toString().padStart(2, '0')}:${sec.toString().padStart(2, '0')}`; } function formatDate(dateStr: string | null): string { @@ -356,7 +357,10 @@ export default function MergeAccounts() { // Fetch merge preview (no auth required) const { data, isLoading, error } = useQuery({ queryKey: ['merge-preview', mergeToken], - queryFn: () => authApi.getMergePreview(mergeToken!), + queryFn: () => { + if (!mergeToken) return Promise.reject(new Error('Missing merge token')); + return authApi.getMergePreview(mergeToken); + }, enabled: !!mergeToken, retry: false, staleTime: Infinity, @@ -409,25 +413,22 @@ export default function MergeAccounts() { return authApi.executeMerge(mergeToken, selectedUserId); }, onSuccess: async (response) => { - if (response.success && response.access_token && response.refresh_token) { + if (!response.success) { + showToast({ type: 'error', message: t('merge.error') }); + return; + } + + if (response.access_token && response.refresh_token) { const { setTokens, setUser, checkAdminStatus } = useAuthStore.getState(); setTokens(response.access_token, response.refresh_token); if (response.user) { setUser(response.user); } await checkAdminStatus(); - showToast({ - type: 'success', - message: t('merge.success'), - }); - navigate('/', { replace: true }); - } else { - showToast({ - type: 'success', - message: t('merge.success'), - }); - navigate('/', { replace: true }); } + + showToast({ type: 'success', message: t('merge.success') }); + navigate('/', { replace: true }); }, onError: () => { showToast({ @@ -452,6 +453,11 @@ export default function MergeAccounts() { const canConfirm = selectedUserId !== null && !isExpired && !mergeMutation.isPending; const combinedBalance = data ? data.primary.balance_kopeks + data.secondary.balance_kopeks : 0; + // Missing token param + if (!mergeToken) { + return ; + } + // Loading if (isLoading) { return ;