Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 123 additions & 0 deletions src/components/challenge-modal/challenge-modal.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
// @vitest-environment jsdom

import * as React from 'react';
import { createElement } from 'react';
import { createRoot, type Root } from 'react-dom/client';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import ChallengeModal from './challenge-modal';
import useChallengesStore from '../../stores/use-challenges-store';

(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
const act = (React as { act?: (callback: () => void | Promise<void>) => void | Promise<void> }).act as (callback: () => void | Promise<void>) => void | Promise<void>;

vi.mock('react-i18next', () => ({
useTranslation: () => ({
t: (key: string, options?: Record<string, unknown>) => (key === 'challenge_counter' ? `${options?.index}/${options?.total}` : key),
}),
}));

vi.mock('@bitsocial/bitsocial-react-hooks', () => ({
useAccount: () => ({ author: { address: '0xabc' } }),
useComment: () => undefined,
}));

vi.mock('../../hooks/use-theme', () => ({
default: () => ['light'],
}));

let container: HTMLDivElement;
let root: Root;

const createChallenge = (prompt: string, publishChallengeAnswers = vi.fn()) =>
[{ challenges: [{ challenge: prompt, type: 'text/plain' }] }, { communityAddress: 'example.bso', content: prompt, publishChallengeAnswers }] as never;

const renderModal = async () => {
await act(async () => {
root.render(createElement(ChallengeModal));
});
};

const clickButton = async (text: string) => {
const button = Array.from(container.querySelectorAll('button')).find((candidate) => candidate.textContent === text);
expect(button).toBeDefined();
await act(async () => {
button?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
await Promise.resolve();
});
};

const enterAnswer = async (value: string) => {
const input = container.querySelector<HTMLInputElement>('input');
expect(input).not.toBeNull();
await act(async () => {
const valueSetter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set;
valueSetter?.call(input, value);
input?.dispatchEvent(new Event('input', { bubbles: true }));
input?.dispatchEvent(new Event('change', { bubbles: true }));
});
};

describe('ChallengeModal', () => {
beforeEach(() => {
vi.clearAllMocks();
useChallengesStore.setState({ challenges: [] });
container = document.createElement('div');
document.body.appendChild(container);
root = createRoot(container);
});

afterEach(() => {
act(() => root.unmount());
container.remove();
useChallengesStore.setState({ challenges: [] });
});

it('abandons exactly the current challenge when Cancel is clicked', async () => {
const firstAbandon = vi.fn().mockResolvedValue(undefined);
const secondAbandon = vi.fn().mockResolvedValue(undefined);
useChallengesStore.getState().addChallenge(createChallenge('first prompt'), firstAbandon);
useChallengesStore.getState().addChallenge(createChallenge('second prompt'), secondAbandon);
await renderModal();

await clickButton('cancel');

expect(firstAbandon).toHaveBeenCalledOnce();
expect(secondAbandon).not.toHaveBeenCalled();
expect(useChallengesStore.getState().challenges).toHaveLength(1);
expect(container.textContent).toContain('second prompt');
});

it('abandons exactly the current challenge once when Escape is pressed', async () => {
const firstAbandon = vi.fn().mockResolvedValue(undefined);
const secondAbandon = vi.fn().mockResolvedValue(undefined);
useChallengesStore.getState().addChallenge(createChallenge('first prompt'), firstAbandon);
useChallengesStore.getState().addChallenge(createChallenge('second prompt'), secondAbandon);
await renderModal();

await act(async () => {
document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }));
await Promise.resolve();
});

expect(firstAbandon).toHaveBeenCalledOnce();
expect(secondAbandon).not.toHaveBeenCalled();
expect(useChallengesStore.getState().challenges).toHaveLength(1);
});

it('submits a text answer without abandoning and resets state for the next queue entry', async () => {
const publishChallengeAnswers = vi.fn();
const firstAbandon = vi.fn().mockResolvedValue(undefined);
useChallengesStore.getState().addChallenge(createChallenge('first prompt', publishChallengeAnswers), firstAbandon);
useChallengesStore.getState().addChallenge(createChallenge('second prompt'));
await renderModal();

await enterAnswer('four');
await clickButton('submit');

expect(publishChallengeAnswers).toHaveBeenCalledWith(['four']);
expect(firstAbandon).not.toHaveBeenCalled();
expect(useChallengesStore.getState().challenges).toHaveLength(1);
expect(container.textContent).toContain('second prompt');
expect(container.querySelector<HTMLInputElement>('input')?.value).toBe('');
});
});
67 changes: 35 additions & 32 deletions src/components/challenge-modal/challenge-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ const ChallengeHeader = ({ publicationType, votePreview, parentCid, parentAddres
interface RegularChallengeContentProps {
challenge: ChallengeType;
closeModal: () => void;
abandonModal: () => void;
}

const RegularChallengeContent = ({ challenge, closeModal }: RegularChallengeContentProps) => {
const RegularChallengeContent = ({ challenge, closeModal, abandonModal }: RegularChallengeContentProps) => {
const { t } = useTranslation();
const account = useAccount();
const [theme] = useTheme();
Expand Down Expand Up @@ -92,7 +93,7 @@ const RegularChallengeContent = ({ challenge, closeModal }: RegularChallengeCont
closeModal();
}, [challenge, answers, closeModal]);

const onIframeClose = useCallback(() => {
const onIframeDone = useCallback(() => {
// Submit empty string as answer for iframe challenges
challenge[1].publishChallengeAnswers(['']);
closeModal();
Expand Down Expand Up @@ -163,7 +164,7 @@ const RegularChallengeContent = ({ challenge, closeModal }: RegularChallengeCont
} catch (error) {
console.error('Invalid iframe challenge URL', { error });
alert('Error: Invalid URL for authentication challenge');
closeModal();
abandonModal();
}
};

Expand Down Expand Up @@ -196,20 +197,6 @@ const RegularChallengeContent = ({ challenge, closeModal }: RegularChallengeCont
}
}, [iframeOrigin, iframeUrlState, sendThemeToIframe, showIframeConfirmation]);

useEffect(() => {
const onEscapeKey = (e: KeyboardEvent) => {
if (e.key === 'Escape') {
if (isIframeChallenge) {
onIframeClose();
} else {
closeModal();
}
}
};
document.addEventListener('keydown', onEscapeKey);
return () => document.removeEventListener('keydown', onEscapeKey);
}, [isIframeChallenge, onIframeClose, closeModal]);

const communityShortAddress = getDisplayAddress(shortCommunityAddress || shortSubplebbitAddress || communityAddress || subplebbitAddress || '');

// Render iframe challenge
Expand Down Expand Up @@ -238,8 +225,12 @@ const RegularChallengeContent = ({ challenge, closeModal }: RegularChallengeCont
</div>
<div className={styles.challengeFooter}>
<span className={styles.buttons}>
<button onClick={handleLoadIframe}>{t('open', { defaultValue: 'open' })}</button>
<button onClick={closeModal}>{t('cancel')}</button>
<button type='button' onClick={handleLoadIframe}>
{t('open', { defaultValue: 'open' })}
</button>
<button type='button' onClick={abandonModal}>
{t('cancel')}
</button>
</span>
</div>
</>
Expand All @@ -260,7 +251,9 @@ const RegularChallengeContent = ({ challenge, closeModal }: RegularChallengeCont
{t('iframe_challenge_keep_open', { defaultValue: 'Complete the challenge in the box above. Keep this window open until done.' })}
</div>
<div className={styles.iframeCloseButton}>
<button onClick={onIframeClose}>{t('done', { defaultValue: 'done' })}</button>
<button type='button' onClick={onIframeDone}>
{t('done', { defaultValue: 'done' })}
</button>
</div>
</div>
</>
Expand Down Expand Up @@ -299,42 +292,52 @@ const RegularChallengeContent = ({ challenge, closeModal }: RegularChallengeCont
<div className={styles.counter}>{t('challenge_counter', { index: currentChallengeIndex + 1, total: challenges?.length })}</div>
<span className={styles.buttons}>
{!challenges?.[currentChallengeIndex + 1] && (
<button onClick={onSubmit} disabled={!isValidAnswer(currentChallengeIndex)}>
<button type='button' onClick={onSubmit} disabled={!isValidAnswer(currentChallengeIndex)}>
{t('submit')}
</button>
)}
<button onClick={closeModal}>{t('cancel')}</button>
<button type='button' onClick={abandonModal}>
{t('cancel')}
</button>
{challenges && challenges.length > 1 && (
<button disabled={!challenges[currentChallengeIndex - 1]} onClick={() => setCurrentChallengeIndex((prev) => prev - 1)}>
<button type='button' disabled={!challenges[currentChallengeIndex - 1]} onClick={() => setCurrentChallengeIndex((prev) => prev - 1)}>
{t('previous')}
</button>
)}
{challenges?.[currentChallengeIndex + 1] && <button onClick={() => setCurrentChallengeIndex((prev) => prev + 1)}>{t('next')}</button>}
{challenges?.[currentChallengeIndex + 1] && (
<button type='button' onClick={() => setCurrentChallengeIndex((prev) => prev + 1)}>
{t('next')}
</button>
)}
</span>
</div>
</>
);
};

const ChallengeContent = ({ challenge, closeModal }: { challenge?: ChallengeType; closeModal: () => void }) => {
const ChallengeContent = ({ challenge, closeModal, abandonModal }: { challenge?: ChallengeType; closeModal: () => void; abandonModal: () => void }) => {
if (challenge) {
return <RegularChallengeContent challenge={challenge} closeModal={closeModal} />;
return <RegularChallengeContent challenge={challenge} closeModal={closeModal} abandonModal={abandonModal} />;
}

return null;
};

const ChallengeModal = () => {
const { challenges, removeChallenge } = useChallengesStore();
const { challenges, removeChallenge, abandonCurrentChallenge } = useChallengesStore();

const isOpen = !!challenges.length;
const closeModal = () => {
removeChallenge();
const closeModal = () => removeChallenge();
const abandonModal = () => {
void abandonCurrentChallenge();
};
const currentChallenge = challenges[0];

const { refs, context } = useFloating({
open: isOpen,
onOpenChange: closeModal,
onOpenChange: (open) => {
if (!open) abandonModal();
},
});
const click = useClick(context);
const dismiss = useDismiss(context, { outsidePress: false });
Expand All @@ -344,11 +347,11 @@ const ChallengeModal = () => {

return (
<>
{isOpen && (
{isOpen && currentChallenge && (
<FloatingFocusManager context={context} modal={false}>
<div className={styles.modal} ref={refs.setFloating} aria-labelledby={headingId} {...getFloatingProps()}>
<div className={styles.container}>
<ChallengeContent challenge={challenges[0]} closeModal={closeModal} />
<ChallengeContent key={currentChallenge.id} challenge={currentChallenge.challenge} closeModal={closeModal} abandonModal={abandonModal} />
</div>
</div>
</FloatingFocusManager>
Expand Down
71 changes: 71 additions & 0 deletions src/hooks/use-publish-reply.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// @vitest-environment jsdom

import * as React from 'react';
import { createElement } from 'react';
import { createRoot, type Root } from 'react-dom/client';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import usePublishReply from './use-publish-reply';
import useChallengesStore from '../stores/use-challenges-store';
import usePublishReplyStore from '../stores/use-publish-reply-store';

(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
const act = (React as { act?: (callback: () => void | Promise<void>) => void | Promise<void> }).act as (callback: () => void | Promise<void>) => void | Promise<void>;

const testState = vi.hoisted(() => ({
abandonPublish: vi.fn().mockResolvedValue(undefined),
lastOptions: undefined as Record<string, any> | undefined,
}));

vi.mock('@bitsocial/bitsocial-react-hooks', () => ({
usePublishComment: (options: Record<string, any>) => {
testState.lastOptions = options;
return { abandonPublish: testState.abandonPublish, index: undefined, publishComment: vi.fn() };
},
}));

let container: HTMLDivElement;
let latestValue: ReturnType<typeof usePublishReply>;
let root: Root;

const HookHarness = () => {
latestValue = usePublishReply({ cid: 'parent-cid', communityAddress: 'example.bso', postCid: 'post-cid' });
return null;
};

describe('usePublishReply', () => {
beforeEach(() => {
vi.clearAllMocks();
testState.lastOptions = undefined;
useChallengesStore.setState({ challenges: [] });
usePublishReplyStore.getState().resetReplyStore('parent-cid');
container = document.createElement('div');
document.body.appendChild(container);
root = createRoot(container);
act(() => root.render(createElement(HookHarness)));
});

afterEach(() => {
act(() => root.unmount());
container.remove();
useChallengesStore.setState({ challenges: [] });
usePublishReplyStore.getState().resetReplyStore('parent-cid');
});

it('routes challenge cancellation to the current usePublishComment abandonPublish', async () => {
await act(async () => {
latestValue.setPublishReplyOptions.content('Reply body');
});

await act(async () => {
await testState.lastOptions?.onChallenge({ challenges: [] }, { content: 'Reply body' });
});

expect(useChallengesStore.getState().challenges).toHaveLength(1);

await act(async () => {
await useChallengesStore.getState().abandonCurrentChallenge();
});

expect(testState.abandonPublish).toHaveBeenCalledOnce();
});
});
23 changes: 21 additions & 2 deletions src/hooks/use-publish-reply.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useMemo } from 'react';
import { useCallback, useEffect, useMemo, useRef } from 'react';
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
import { usePublishComment } from '@bitsocial/bitsocial-react-hooks';
import usePublishReplyStore from '../stores/use-publish-reply-store';
import useChallengesStore from '../stores/use-challenges-store';

const usePublishReply = ({ cid, communityAddress, postCid }: { cid: string; communityAddress: string; postCid: string | undefined }) => {
const parentCid = cid;
Expand All @@ -15,6 +16,11 @@ const usePublishReply = ({ cid, communityAddress, postCid }: { cid: string; comm

const setReplyStore = usePublishReplyStore((state) => state.setReplyStore);
const resetReplyStore = usePublishReplyStore((state) => state.resetReplyStore);
const addChallenge = useChallengesStore((state) => state.addChallenge);
const abandonPublishRef = useRef<(() => Promise<void>) | undefined>(undefined);
const abandonCurrentPublish = useCallback(async () => {
await abandonPublishRef.current?.();
}, []);

const setPublishReplyOptions = useMemo(
() => ({
Expand Down Expand Up @@ -64,7 +70,20 @@ const usePublishReply = ({ cid, communityAddress, postCid }: { cid: string; comm

const resetPublishReplyOptions = useMemo(() => () => resetReplyStore(parentCid), [parentCid, resetReplyStore]);

const { index, publishComment } = usePublishComment(publishCommentOptions);
const publishOptionsWithAbandon = useMemo(
() => ({
...publishCommentOptions,
onChallenge: async (...args: any[]) => {
addChallenge(args, abandonCurrentPublish);
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale abandon on queued challenges

Medium Severity

Each queued challenge stores the same abandonCurrentPublish callback, which reads abandonPublishRef when cancel runs—not when the challenge was enqueued. If two publishes from the same hook are queued, canceling the first modal can invoke the latest abandonPublish and miss the publication that actually owns that challenge.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2a790f8. Configure here.

}),
[abandonCurrentPublish, addChallenge, publishCommentOptions],
);

const { index, publishComment, abandonPublish } = usePublishComment(publishOptionsWithAbandon);
useEffect(() => {
abandonPublishRef.current = abandonPublish;
}, [abandonPublish]);

return { setPublishReplyOptions, resetPublishReplyOptions, replyIndex: index, publishReply: publishComment, publishReplyOptions };
};
Expand Down
Loading
Loading