Skip to content

Commit ea954f8

Browse files
authored
fix(schedule-send): serialize scheduled message cancellation per room (#1564)
<!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> ### Description nothing stopped you double-clicking cancel on a scheduled message, and a failed cancel was silent. cancels go through a per-room queue now, button spinners and disables while in flight, failures show an inline retry. Fixes # #### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings ### AI disclosure: - [ ] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). <!-- Write any explanation required here, but do not generate the explanation using AI!! You must prove you understand what the code in this PR does. -->
2 parents cb16d97 + 7f9d042 commit ea954f8

4 files changed

Lines changed: 363 additions & 7 deletions

File tree

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
/* oxlint-disable typescript/no-explicit-any, typescript/no-extraneous-class, unicorn/consistent-function-scoping, vitest/require-mock-type-parameters */
2+
3+
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
4+
import { beforeEach, describe, expect, it, vi } from 'vitest';
5+
import { ScheduledMessagesList } from './ScheduledMessagesList';
6+
import type * as MatrixSdkModule from '$types/matrix-sdk';
7+
8+
const testState = vi.hoisted(() => ({
9+
cancelDelayedEvent: vi.fn(),
10+
coordinatorRun: vi.fn(),
11+
invalidateQueries: vi.fn(),
12+
matrix: {
13+
getSafeUserId: vi.fn(() => '@me:example.org'),
14+
},
15+
}));
16+
17+
vi.mock('$hooks/useMatrixClient', () => ({
18+
useMatrixClient: () => testState.matrix,
19+
}));
20+
21+
vi.mock('$utils/delayedEvents', () => ({
22+
cancelDelayedEvent: testState.cancelDelayedEvent,
23+
getDelayedEvents: vi.fn(),
24+
}));
25+
26+
vi.mock('$state/room/roomScheduleCoordinator', () => ({
27+
roomScheduleCoordinator: {
28+
run: testState.coordinatorRun,
29+
},
30+
}));
31+
32+
vi.mock('@tanstack/react-query', () => ({
33+
useQuery: () => ({
34+
data: {
35+
delayed_events: [
36+
{
37+
delay_id: 'delay-1',
38+
room_id: '!room:example.org',
39+
type: 'm.room.message',
40+
content: { body: 'Scheduled message' },
41+
running_since: 1_000,
42+
delay: 60_000,
43+
},
44+
],
45+
},
46+
}),
47+
useQueryClient: () => ({ invalidateQueries: testState.invalidateQueries }),
48+
}));
49+
50+
vi.mock('$state/scheduledMessages', async () => {
51+
const { atom } = await import('jotai');
52+
const scheduledTimeAtom = atom<Date | null>(null);
53+
const editingScheduledDelayIdAtom = atom<string | null>(null);
54+
55+
return {
56+
delayedEventsSupportedAtom: atom(true),
57+
roomIdToScheduledTimeAtomFamily: () => scheduledTimeAtom,
58+
roomIdToEditingScheduledDelayIdAtomFamily: () => editingScheduledDelayIdAtom,
59+
};
60+
});
61+
62+
vi.mock('$state/hooks/settings', () => ({
63+
useSetting: (_atom: unknown, key: string) => [
64+
key === 'hour24Clock' ? false : 'YYYY-MM-DD',
65+
vi.fn(),
66+
],
67+
}));
68+
69+
vi.mock('$state/settings', () => ({ settingsAtom: {} }));
70+
71+
vi.mock('$types/matrix-sdk', async (importOriginal) => ({
72+
...(await importOriginal<typeof MatrixSdkModule>()),
73+
MatrixEvent: class MatrixEvent {},
74+
}));
75+
76+
vi.mock('$utils/time', () => ({
77+
timeDayMonthYear: () => '2026-07-28',
78+
timeHourMinute: () => '00:01',
79+
}));
80+
81+
vi.mock('$components/message-preview', () => ({
82+
MessagePreview: ({ actions, event }: any) => (
83+
<div>
84+
<span>{event.getContent?.().body ?? 'Scheduled message'}</span>
85+
{actions}
86+
</div>
87+
),
88+
useRoomMessagePreviewRenderer: () => vi.fn(),
89+
}));
90+
91+
vi.mock('$components/icons/phosphor', () => ({
92+
CaretDown: 'CaretDown',
93+
CaretUp: 'CaretUp',
94+
Clock: 'Clock',
95+
Lock: 'Lock',
96+
PencilSimple: 'PencilSimple',
97+
X: 'X',
98+
chipIcon: () => null,
99+
}));
100+
101+
vi.mock('folds', () => {
102+
const Box = ({ children, ...props }: any) => <div {...props}>{children}</div>;
103+
const Text = ({ children, ...props }: any) => <span {...props}>{children}</span>;
104+
const Button = ({ children, ...props }: any) => <button {...props}>{children}</button>;
105+
106+
return {
107+
Box,
108+
Chip: Button,
109+
IconButton: Button,
110+
Spinner: () => <span role="progressbar">Cancelling</span>,
111+
Text,
112+
config: {
113+
borderWidth: { B300: '1px' },
114+
space: { S100: '1px', S200: '2px', S400: '4px' },
115+
},
116+
toRem: (value: number) => `${value / 16}rem`,
117+
};
118+
});
119+
120+
vi.mock('./SchedulePickerDialog', () => ({
121+
SchedulePickerDialog: () => null,
122+
}));
123+
124+
const room = { roomId: '!room:example.org' } as any;
125+
126+
function deferred<T>() {
127+
let resolve!: (value: T) => void;
128+
let reject!: (reason?: unknown) => void;
129+
const promise = new Promise<T>((promiseResolve, promiseReject) => {
130+
resolve = promiseResolve;
131+
reject = promiseReject;
132+
});
133+
return { promise, resolve, reject };
134+
}
135+
136+
function renderExpandedList() {
137+
render(<ScheduledMessagesList room={room} />);
138+
fireEvent.click(screen.getByRole('button', { name: /1 scheduled message/i }));
139+
return screen.getByRole('button', { name: 'Cancel scheduled message' });
140+
}
141+
142+
describe('ScheduledMessagesList cancellation', () => {
143+
beforeEach(() => {
144+
testState.cancelDelayedEvent.mockReset();
145+
testState.coordinatorRun.mockReset();
146+
testState.invalidateQueries.mockReset();
147+
testState.coordinatorRun.mockImplementation(
148+
(_mx: unknown, _roomId: string, operation: () => unknown) => operation()
149+
);
150+
});
151+
152+
it('blocks duplicate cancellation clicks and shows pending state', () => {
153+
const pending = deferred<void>();
154+
testState.cancelDelayedEvent.mockReturnValue(pending.promise);
155+
156+
const cancel = renderExpandedList();
157+
fireEvent.click(cancel);
158+
fireEvent.click(cancel);
159+
160+
expect(testState.coordinatorRun).toHaveBeenCalledWith(
161+
testState.matrix,
162+
room.roomId,
163+
expect.any(Function)
164+
);
165+
expect(testState.cancelDelayedEvent).toHaveBeenCalledOnce();
166+
expect(cancel).toBeDisabled();
167+
expect(cancel).toHaveAttribute('aria-busy', 'true');
168+
expect(screen.getByRole('progressbar')).toBeInTheDocument();
169+
});
170+
171+
it('shows a retryable alert when cancellation fails', async () => {
172+
testState.cancelDelayedEvent.mockRejectedValueOnce(new Error('Cancel failed'));
173+
const cancel = renderExpandedList();
174+
175+
fireEvent.click(cancel);
176+
177+
await waitFor(() =>
178+
expect(screen.getByRole('alert')).toHaveTextContent(
179+
'Failed to cancel scheduled message. Try again.'
180+
)
181+
);
182+
expect(cancel).not.toBeDisabled();
183+
expect(cancel).not.toHaveAttribute('aria-busy', 'true');
184+
});
185+
186+
it('successfully retries cancellation after a failure', async () => {
187+
testState.cancelDelayedEvent
188+
.mockRejectedValueOnce(new Error('Cancel failed'))
189+
.mockResolvedValueOnce(undefined);
190+
const cancel = renderExpandedList();
191+
192+
fireEvent.click(cancel);
193+
await waitFor(() => expect(screen.getByRole('alert')).toBeInTheDocument());
194+
195+
fireEvent.click(cancel);
196+
197+
await waitFor(() => expect(testState.cancelDelayedEvent).toHaveBeenCalledTimes(2));
198+
await waitFor(() => expect(testState.invalidateQueries).toHaveBeenCalledOnce());
199+
expect(screen.queryByRole('alert')).not.toBeInTheDocument();
200+
});
201+
});

src/app/features/room/schedule-send/ScheduledMessagesList.tsx

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { useCallback, useMemo, useState } from 'react';
1+
import { useCallback, useMemo, useRef, useState } from 'react';
22
import { useQuery, useQueryClient } from '@tanstack/react-query';
3-
import { Box, Text, Chip, IconButton } from 'folds';
3+
import { Box, Text, Chip, IconButton, Spinner } from 'folds';
44
import {
55
CaretDown,
66
CaretUp,
@@ -27,6 +27,7 @@ import {
2727
import { timeHourMinute, timeDayMonthYear } from '$utils/time';
2828
import { useSetting } from '$state/hooks/settings';
2929
import { settingsAtom } from '$state/settings';
30+
import { roomScheduleCoordinator } from '$state/room/roomScheduleCoordinator';
3031
import { MessagePreview, useRoomMessagePreviewRenderer } from '$components/message-preview';
3132
import { SchedulePickerDialog } from './SchedulePickerDialog';
3233
import * as css from './ScheduledMessagesList.css';
@@ -44,6 +45,12 @@ type ScheduledMessageRowProps = {
4445
hour24Clock: boolean;
4546
onEdit: (delayId: string, body: string, formattedBody?: string, scheduledTs?: number) => void;
4647
onCancel: (delayId: string) => void;
48+
cancellationState: CancellationState;
49+
};
50+
51+
type CancellationState = {
52+
status: 'idle' | 'pending' | 'error';
53+
error?: string;
4754
};
4855

4956
function ScheduledMessageRow({
@@ -52,6 +59,7 @@ function ScheduledMessageRow({
5259
hour24Clock,
5360
onEdit,
5461
onCancel,
62+
cancellationState,
5563
}: ScheduledMessageRowProps) {
5664
const mx = useMatrixClient();
5765
const [dateFormatString] = useSetting(settingsAtom, 'dateFormatString');
@@ -63,6 +71,8 @@ function ScheduledMessageRow({
6371
!isEncrypted && typeof event.content.formatted_body === 'string'
6472
? event.content.formatted_body
6573
: undefined;
74+
const isCancelling = cancellationState.status === 'pending';
75+
const cancelIcon = isCancelling ? <Spinner size="100" /> : chipIcon(X);
6676
const matrixEvent = useMemo(
6777
() =>
6878
new MatrixEvent({
@@ -106,9 +116,11 @@ function ScheduledMessageRow({
106116
variant="Critical"
107117
radii="300"
108118
onClick={() => onCancel(event.delay_id)}
119+
disabled={isCancelling}
120+
aria-busy={isCancelling}
109121
aria-label="Cancel scheduled message"
110122
>
111-
{chipIcon(X)}
123+
{cancelIcon}
112124
</IconButton>
113125
</Box>
114126
}
@@ -126,12 +138,19 @@ function ScheduledMessageRow({
126138
variant="Critical"
127139
radii="300"
128140
onClick={() => onCancel(event.delay_id)}
141+
disabled={isCancelling}
142+
aria-busy={isCancelling}
129143
aria-label="Cancel scheduled message"
130144
>
131-
{chipIcon(X)}
145+
{cancelIcon}
132146
</IconButton>
133147
)}
134148
</Box>
149+
{cancellationState.status === 'error' && (
150+
<Text size="T200" priority="300" role="alert" aria-live="polite">
151+
{cancellationState.error}
152+
</Text>
153+
)}
135154
</Box>
136155
);
137156
}
@@ -146,6 +165,10 @@ export function ScheduledMessagesList({ room, onEditMessage }: ScheduledMessages
146165
const [editingDelayId, setEditingDelayId] = useAtom(
147166
roomIdToEditingScheduledDelayIdAtomFamily(room.roomId)
148167
);
168+
const [cancellationStates, setCancellationStates] = useState<Record<string, CancellationState>>(
169+
{}
170+
);
171+
const pendingCancellations = useRef(new Set<string>());
149172

150173
const { data } = useQuery({
151174
queryKey: ['delayedEvents', room.roomId],
@@ -167,10 +190,35 @@ export function ScheduledMessagesList({ room, onEditMessage }: ScheduledMessages
167190

168191
const handleCancel = useCallback(
169192
async (delayId: string) => {
170-
await cancelDelayedEvent(mx, delayId);
171-
invalidateEvents();
193+
if (pendingCancellations.current.has(delayId)) return;
194+
195+
pendingCancellations.current.add(delayId);
196+
setCancellationStates((states) => ({
197+
...states,
198+
[delayId]: { status: 'pending' },
199+
}));
200+
201+
try {
202+
await roomScheduleCoordinator.run(mx, room.roomId, () => cancelDelayedEvent(mx, delayId));
203+
invalidateEvents();
204+
setCancellationStates((states) => {
205+
const next = { ...states };
206+
delete next[delayId];
207+
return next;
208+
});
209+
} catch {
210+
setCancellationStates((states) => ({
211+
...states,
212+
[delayId]: {
213+
status: 'error',
214+
error: 'Failed to cancel scheduled message. Try again.',
215+
},
216+
}));
217+
} finally {
218+
pendingCancellations.current.delete(delayId);
219+
}
172220
},
173-
[mx, invalidateEvents]
221+
[mx, room.roomId, invalidateEvents]
174222
);
175223

176224
const handleEdit = useCallback(
@@ -218,6 +266,7 @@ export function ScheduledMessagesList({ room, onEditMessage }: ScheduledMessages
218266
hour24Clock={hour24Clock}
219267
onEdit={handleEdit}
220268
onCancel={handleCancel}
269+
cancellationState={cancellationStates[event.delay_id] ?? { status: 'idle' }}
221270
/>
222271
))}
223272
</Box>

0 commit comments

Comments
 (0)