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
This commit is contained in:
Fringg
2026-03-04 07:56:19 +03:00
parent 58cf1e3b50
commit aa26059e00
3 changed files with 34 additions and 21 deletions

View File

@@ -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;
};

View File

@@ -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,

View File

@@ -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 <ErrorState />;
}
// Loading
if (isLoading) {
return <LoadingSkeleton />;