fix(pending-post): keep retrying failed posts on the pending route (#1161)

* fix(pending-post): keep retrying failed posts on the pending route

Retrying a failed post deletes the pending row before republishing, which
briefly leaves the pending comment non-addressable with no active challenge —
the same state PendingPost's abandoned-challenge guard uses to redirect back to
the board. A shared use-failed-post-retry-store marks the index being retried so
PendingPost skips that redirect until the republished row is created and its own
navigation to the new pending route takes over.

* fix(pending-post): harden retry flag lifecycle against abandoned-challenge race

Address Cursor Bugbot review on the retry flow. Clear the retry flag (and
isRetryRedirectPending) when the republish challenge is abandoned so an abandon
mid-retry falls through to the normal board redirect instead of stranding an
empty pending view. Also navigate to the new pending row before clearing the
flag in the redirect effect so PendingPost never observes an old-index route
with the flag already cleared.
This commit is contained in:
Tommaso Casaburi
2026-06-08 16:08:51 +07:00
committed by GitHub
parent c2b222c4d5
commit bd12b47db6
5 changed files with 122 additions and 7 deletions
@@ -4,6 +4,7 @@ import { createRoot, type Root } from 'react-dom/client';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import useDeleteFailedPost, { getFailedPostRetryPublishOptions } from '../use-delete-failed-post';
import useChallengesStore from '../../stores/use-challenges-store';
import useFailedPostRetryStore from '../../stores/use-failed-post-retry-store';
(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
const act = (React as { act?: (cb: () => void | Promise<void>) => void | Promise<void> }).act as (cb: () => void | Promise<void>) => void | Promise<void>;
@@ -97,6 +98,7 @@ describe('useDeleteFailedPost', () => {
testState.publishIndex = undefined;
vi.stubGlobal('alert', testState.alertMock);
useChallengesStore.setState({ challenges: [] });
useFailedPostRetryStore.setState({ retryingAccountCommentIndex: null });
container = document.createElement('div');
document.body.appendChild(container);
@@ -168,6 +170,39 @@ describe('useDeleteFailedPost', () => {
expect(testState.navigateMock).toHaveBeenCalledWith('/pending/12', { replace: true });
});
it('flags the failed row as mid-retry while republishing and clears it after the redirect', async () => {
let flaggedIndexDuringPublish: number | null = null;
testState.publishCommentMock.mockImplementationOnce(async () => {
flaggedIndexDuringPublish = useFailedPostRetryStore.getState().retryingAccountCommentIndex;
testState.publishIndex = 12;
});
await act(async () => {
await latestValue.onRetryFailedPost();
});
// The pending view reads this flag to avoid redirecting away during the delete/republish gap.
expect(flaggedIndexDuringPublish).toBe(failedPost.index);
renderHook();
await flushEffects();
expect(testState.navigateMock).toHaveBeenCalledWith('/pending/12', { replace: true });
expect(useFailedPostRetryStore.getState().retryingAccountCommentIndex).toBeNull();
});
it('clears the mid-retry flag when republishing throws', async () => {
testState.publishCommentMock.mockImplementationOnce(async () => {
throw new Error('offline');
});
await act(async () => {
await latestValue.onRetryFailedPost();
});
expect(useFailedPostRetryStore.getState().retryingAccountCommentIndex).toBeNull();
});
it('redirects after deleting a failed post when a redirect path is provided', async () => {
renderHook(failedPost, '/mu');
@@ -194,4 +229,19 @@ describe('useDeleteFailedPost', () => {
expect(testState.abandonPublishMock).toHaveBeenCalledTimes(1);
});
it('clears the mid-retry flag when the republish challenge is abandoned', async () => {
useFailedPostRetryStore.setState({ retryingAccountCommentIndex: failedPost.index });
await act(async () => {
await testState.lastPublishOptions?.onChallenge('captcha', 'nonce');
});
await act(async () => {
await useChallengesStore.getState().abandonCurrentChallenge();
});
expect(testState.abandonPublishMock).toHaveBeenCalledTimes(1);
expect(useFailedPostRetryStore.getState().retryingAccountCommentIndex).toBeNull();
});
});
+23 -4
View File
@@ -3,6 +3,7 @@ import { useNavigate } from 'react-router-dom';
import { ChallengeVerification, Comment, PublishCommentOptions, deleteComment, usePublishComment } from '@bitsocial/bitsocial-react-hooks';
import { alertChallengeVerificationFailed } from '../lib/utils/challenge-utils';
import useChallengesStore from '../stores/use-challenges-store';
import useFailedPostRetryStore from '../stores/use-failed-post-retry-store';
import { getCommentCommunityAddress } from '../lib/utils/comment-utils';
const retryExcludedFields = new Set([
@@ -73,11 +74,17 @@ const useDeleteFailedPost = (post?: FailedPost, deleteRedirectPath?: string) =>
const [isRetryingFailedPost, setIsRetryingFailedPost] = useState(false);
const [isRetryRedirectPending, setIsRetryRedirectPending] = useState(false);
const addChallenge = useChallengesStore((state) => state.addChallenge);
const startRetry = useFailedPostRetryStore((state) => state.startRetry);
const endRetry = useFailedPostRetryStore((state) => state.endRetry);
const navigate = useNavigate();
const abandonPublishRef = useRef<(() => Promise<void>) | undefined>(undefined);
const abandonCurrentPublish = useCallback(async () => {
// Abandoning the republish challenge ends the retry. Clear the flag first so PendingPost resumes its
// normal abandoned-challenge handling (back to the board) instead of staying stuck on an empty row.
setIsRetryRedirectPending(false);
endRetry();
await abandonPublishRef.current?.();
}, []);
}, [endRetry]);
const canDeleteFailedPost = post?.state === 'failed' && typeof post?.index === 'number';
const retryPublishOptions = useMemo(() => getFailedPostRetryPublishOptions(post), [post]);
@@ -95,11 +102,12 @@ const useDeleteFailedPost = (post?: FailedPost, deleteRedirectPath?: string) =>
onError: (error: Error) => {
console.error('Failed to retry failed post:', error);
setIsRetryRedirectPending(false);
endRetry();
alert(`Failed to retry post: ${error.message}`);
},
}
: undefined,
[abandonCurrentPublish, addChallenge, retryPublishOptions],
[abandonCurrentPublish, addChallenge, endRetry, retryPublishOptions],
);
const { abandonPublish, index: retryPostIndex, publishComment } = usePublishComment(publishOptionsWithCallbacks);
abandonPublishRef.current = abandonPublish;
@@ -110,8 +118,11 @@ const useDeleteFailedPost = (post?: FailedPost, deleteRedirectPath?: string) =>
}
setIsRetryRedirectPending(false);
// Navigate to the new pending row before clearing the retry flag so PendingPost never observes a
// committed "old index + no retry flag + no addressable post" state that would bounce to the board.
navigate(`/pending/${retryPostIndex}`, { replace: true });
}, [isRetryRedirectPending, navigate, retryPostIndex]);
endRetry();
}, [endRetry, isRetryRedirectPending, navigate, retryPostIndex]);
const onDeleteFailedPost = useCallback(() => {
if (isDeletingFailedPost || isRetryingFailedPost || !canDeleteFailedPost) {
@@ -150,8 +161,15 @@ const useDeleteFailedPost = (post?: FailedPost, deleteRedirectPath?: string) =>
return;
}
const accountCommentIndex = post?.index;
setIsRetryingFailedPost(true);
setIsRetryRedirectPending(true);
// Mark this pending row as mid-retry so the pending view does not treat the brief
// post-delete gap (no addressable comment, no active challenge yet) as an abandoned challenge.
if (typeof accountCommentIndex === 'number') {
startRetry(accountCommentIndex);
}
try {
await deleteComment(targetComment);
@@ -159,11 +177,12 @@ const useDeleteFailedPost = (post?: FailedPost, deleteRedirectPath?: string) =>
} catch (error) {
console.error('Failed to retry failed post:', error);
setIsRetryRedirectPending(false);
endRetry();
alert(`Failed to retry post: ${error instanceof Error ? error.message : 'Unknown error'}`);
} finally {
setIsRetryingFailedPost(false);
}
}, [canRetryFailedPost, isDeletingFailedPost, isRetryingFailedPost, post, publishComment]);
}, [canRetryFailedPost, endRetry, isDeletingFailedPost, isRetryingFailedPost, post, publishComment, startRetry]);
return {
canDeleteFailedPost,
+19
View File
@@ -0,0 +1,19 @@
import { create } from 'zustand';
interface FailedPostRetryState {
// Account-comment index of the failed post currently being retried, or null when no retry is in flight.
// Retrying deletes the old pending row and republishes, so the pending route briefly has no addressable
// comment and no active challenge. The pending view reads this to avoid mistaking that gap for an
// abandoned challenge and redirecting away.
retryingAccountCommentIndex: number | null;
startRetry: (accountCommentIndex: number) => void;
endRetry: () => void;
}
const useFailedPostRetryStore = create<FailedPostRetryState>((set) => ({
retryingAccountCommentIndex: null,
startRetry: (accountCommentIndex) => set({ retryingAccountCommentIndex: accountCommentIndex }),
endRetry: () => set({ retryingAccountCommentIndex: null }),
}));
export default useFailedPostRetryStore;
@@ -22,6 +22,7 @@ const testState = vi.hoisted(() => ({
locationState: null as { boardPath?: string } | null,
navigateMock: vi.fn(),
post: undefined as TestComment | undefined,
retryingAccountCommentIndex: null as number | null,
}));
vi.mock('react-router-dom', async () => {
@@ -58,6 +59,10 @@ vi.mock('../../../stores/use-challenges-store', () => ({
default: (selector: (state: { challenges: unknown[] }) => unknown) => selector({ challenges: Array.from({ length: testState.challengeCount }) }),
}));
vi.mock('../../../stores/use-failed-post-retry-store', () => ({
default: (selector: (state: { retryingAccountCommentIndex: number | null }) => unknown) => selector({ retryingAccountCommentIndex: testState.retryingAccountCommentIndex }),
}));
vi.mock('../../post', () => ({
Post: ({ post }: { post?: TestComment }) => createElement('div', { 'data-testid': 'post-view' }, post?.cid ?? 'no-post'),
}));
@@ -93,6 +98,7 @@ describe('PendingPost', () => {
testState.locationState = null;
testState.navigateMock.mockReset();
testState.post = undefined;
testState.retryingAccountCommentIndex = null;
window.scrollTo = scrollToMock;
@@ -181,6 +187,19 @@ describe('PendingPost', () => {
expect(testState.navigateMock).toHaveBeenCalledWith('/mu', { replace: true });
});
it('keeps a mid-retry pending post in place while the failed row is deleted and republished', async () => {
testState.accountCommentIndex = '0';
testState.accountComments = [];
testState.challengeCount = 0;
testState.locationState = { boardPath: 'mu' };
testState.post = undefined;
testState.retryingAccountCommentIndex = 0;
await renderPendingPost();
expect(testState.navigateMock).not.toHaveBeenCalled();
});
it('redirects missing sparse pending account comment indices to not found', async () => {
testState.accountCommentIndex = '0';
testState.accountComments = [{ index: 1 }];
+11 -3
View File
@@ -6,6 +6,7 @@ import useSafeAccountComment from '../../hooks/use-safe-account-comment';
import { getCommentCommunityAddress } from '../../lib/utils/comment-utils';
import { getBoardPath } from '../../lib/utils/route-utils';
import useChallengesStore from '../../stores/use-challenges-store';
import useFailedPostRetryStore from '../../stores/use-failed-post-retry-store';
import { Post } from '../post';
type PendingAccountComment = {
@@ -56,6 +57,8 @@ const PendingPost = () => {
const postBoardPath = postCommunityAddress ? getBoardPath(postCommunityAddress, directories) : undefined;
const pendingBoardPath = postBoardPath || routeBoardPath;
const hasActiveChallenge = useChallengesStore((state) => state.challenges.length > 0);
const retryingAccountCommentIndex = useFailedPostRetryStore((state) => state.retryingAccountCommentIndex);
const isRetryingThisPendingPost = retryingAccountCommentIndex !== null && retryingAccountCommentIndex === normalizedAccountCommentIndex;
const lastPendingBoardRef = useRef<{ accountCommentIndex: number; boardPath: string } | null>(null);
useEffect(() => window.scrollTo(0, 0), []);
@@ -74,13 +77,18 @@ const PendingPost = () => {
hasPendingAccountCommentIndex(accountComments, normalizedAccountCommentIndex));
useEffect(() => {
// A retry deletes this pending row before republishing, briefly invalidating the index. Stay put;
// useDeleteFailedPost redirects to the new pending row once the republished comment is created.
if (isRetryingThisPendingPost) {
return;
}
if (!isValidAccountCommentIndex) {
const lastPendingBoard = lastPendingBoardRef.current;
const abandonedBoardPath =
!hasActiveChallenge && lastPendingBoard && lastPendingBoard.accountCommentIndex === normalizedAccountCommentIndex ? lastPendingBoard.boardPath : undefined;
navigate(abandonedBoardPath ? `/${abandonedBoardPath}` : '/not-found', { replace: true });
}
}, [hasActiveChallenge, isValidAccountCommentIndex, navigate, normalizedAccountCommentIndex]);
}, [hasActiveChallenge, isRetryingThisPendingPost, isValidAccountCommentIndex, navigate, normalizedAccountCommentIndex]);
useEffect(() => {
if (post?.cid && postBoardPath) {
@@ -89,7 +97,7 @@ const PendingPost = () => {
}, [post?.cid, postBoardPath, navigate]);
useEffect(() => {
if (hasAddressablePost || !isValidAccountCommentIndex) {
if (isRetryingThisPendingPost || hasAddressablePost || !isValidAccountCommentIndex) {
return;
}
@@ -99,7 +107,7 @@ const PendingPost = () => {
if (abandonedBoardPath) {
navigate(`/${abandonedBoardPath}`, { replace: true });
}
}, [hasActiveChallenge, hasAddressablePost, isValidAccountCommentIndex, navigate, normalizedAccountCommentIndex]);
}, [hasActiveChallenge, hasAddressablePost, isRetryingThisPendingPost, isValidAccountCommentIndex, navigate, normalizedAccountCommentIndex]);
return <Post post={post} />;
};