Skip to content

Commit 6ac0f24

Browse files
committed
Preflight WCIF saves
Validate the complete WCIF with the WCA check endpoint before PATCHing changed fields, and surface API error details to delegates.
1 parent dd5d8f5 commit 6ac0f24

6 files changed

Lines changed: 93 additions & 6 deletions

File tree

src/layout/CompetitionLayout/CompetitionLayout.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export const CompetitionLayout = () => {
8383
dispatch(
8484
uploadCurrentWCIFChanges((e) => {
8585
if (e) {
86-
enqueueSnackbar('Error saving changes', { variant: 'error' });
86+
enqueueSnackbar(`Error saving changes: ${e.message}`, { variant: 'error' });
8787
} else {
8888
enqueueSnackbar('Saved!', { variant: 'success' });
8989
}

src/layout/CompetitionLayout/_tests_/CompetitionLayout.test.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ describe('CompetitionLayout', () => {
155155
const errorCallback = uploadCurrentWCIFChangesMock.mock.calls[1][0];
156156
errorCallback(new Error('save failed'));
157157

158-
expect(enqueueSnackbarMock).toHaveBeenCalledWith('Error saving changes', { variant: 'error' });
158+
expect(enqueueSnackbarMock).toHaveBeenCalledWith('Error saving changes: save failed', {
159+
variant: 'error',
160+
});
159161
});
160162
});

src/lib/api/wcaAPI.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, it, vi, afterEach } from 'vitest';
22
import {
3+
checkWcif,
34
getMe,
45
getPastManageableCompetitions,
56
getUpcomingManageableCompetitions,
@@ -52,6 +53,34 @@ describe('wcaAPI', () => {
5253
await expect(wcaApiFetch('/me')).rejects.toThrow('Something went wrong: Status code 418');
5354
});
5455

56+
it('uses an API error response when one is available', async () => {
57+
mockFetch({
58+
ok: false,
59+
status: 400,
60+
statusText: 'Bad Request',
61+
json: vi.fn().mockResolvedValue({ error: 'WCIF formatVersion is required' }),
62+
});
63+
64+
await expect(wcaApiFetch('/me')).rejects.toThrow('WCIF formatVersion is required');
65+
});
66+
67+
it('checks a complete WCIF without parsing the empty success response', async () => {
68+
const json = vi.fn();
69+
const wcif = { id: 'Comp', formatVersion: '1.1' } as any;
70+
mockFetch({ json });
71+
72+
await checkWcif(wcif);
73+
74+
expect(globalThis.fetch).toHaveBeenCalledWith(
75+
'https://wca.test/api/v0/competitions/wcif/check',
76+
expect.objectContaining({
77+
method: 'PUT',
78+
body: JSON.stringify(wcif),
79+
})
80+
);
81+
expect(json).not.toHaveBeenCalled();
82+
});
83+
5584
it('builds upcoming and past competition queries', async () => {
5685
vi.spyOn(Date, 'now').mockReturnValue(0);
5786
mockFetch({ json: vi.fn().mockResolvedValue([]) });

src/lib/api/wcaAPI.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,16 @@ export const patchWcif = (
5656
body: JSON.stringify(wcif),
5757
});
5858

59+
export const checkWcif = (wcif: Competition): Promise<void> =>
60+
wcaApiFetch(
61+
'/competitions/wcif/check',
62+
{
63+
method: 'PUT',
64+
body: JSON.stringify(wcif),
65+
},
66+
false
67+
);
68+
5969
export const saveWcifChanges = (
6070
previousWcif: Competition,
6171
newWcif: Competition
@@ -82,7 +92,8 @@ export const getUser = (userId: number): Promise<{ user: WcaUser }> =>
8292

8393
export const wcaApiFetch = async <T = unknown>(
8494
path: string,
85-
fetchOptions: RequestInit = {}
95+
fetchOptions: RequestInit = {},
96+
parseJsonResponse = true
8697
): Promise<T> => {
8798
const baseApiUrl = `${WCA_ORIGIN}/api/v0`;
8899

@@ -97,12 +108,32 @@ export const wcaApiFetch = async <T = unknown>(
97108
);
98109

99110
if (!res.ok) {
111+
const error = await errorFromResponse(res);
112+
if (error) throw new Error(error);
113+
100114
if (res.statusText) {
101115
throw new Error(`${res.status}: ${res.statusText}`);
102116
} else {
103117
throw new Error(`Something went wrong: Status code ${res.status}`);
104118
}
105119
}
106120

121+
if (!parseJsonResponse) return undefined as T;
122+
107123
return await res.json();
108124
};
125+
126+
const errorFromResponse = async (res: Response): Promise<string | undefined> => {
127+
try {
128+
const body: unknown = await res.json();
129+
if (Array.isArray(body)) return body.map(String).join('\n');
130+
131+
if (body && typeof body === 'object' && 'error' in body) {
132+
const error = body.error;
133+
if (Array.isArray(error)) return error.map(String).join('\n');
134+
if (typeof error === 'string') return error;
135+
}
136+
} catch {
137+
// Fall back to the HTTP status when the API does not return JSON.
138+
}
139+
};

src/store/actions.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
import type { Assignment, Competition } from '@wca/helpers';
2727
import type { Extension } from '@wca/helpers/lib/models/extension';
2828
import { describe, expect, it, vi } from 'vitest';
29-
import { getUpcomingManageableCompetitions, getWcif, patchWcif } from '../lib/api';
29+
import { checkWcif, getUpcomingManageableCompetitions, getWcif, patchWcif } from '../lib/api';
3030
import { sortWcifEvents } from '../lib/domain/events';
3131
import { validateWcif } from '../lib/wcif/validation';
3232
import type { AppState } from './initialState';
@@ -41,6 +41,7 @@ import {
4141
vi.mock('../lib/api', () => ({
4242
getUpcomingManageableCompetitions: vi.fn(),
4343
getWcif: vi.fn(),
44+
checkWcif: vi.fn(),
4445
patchWcif: vi.fn(),
4546
}));
4647

@@ -54,6 +55,7 @@ vi.mock('../lib/wcif/validation', () => ({
5455

5556
const getUpcomingManageableCompetitionsMock = vi.mocked(getUpcomingManageableCompetitions);
5657
const getWcifMock = vi.mocked(getWcif);
58+
const checkWcifMock = vi.mocked(checkWcif);
5759
const patchWcifMock = vi.mocked(patchWcif);
5860
const sortWcifEventsMock = vi.mocked(sortWcifEvents);
5961
const validateWcifMock = vi.mocked(validateWcif);
@@ -311,6 +313,7 @@ describe('store actions', () => {
311313
wcif,
312314
changedKeys: new Set(['events']),
313315
}) as unknown as AppState;
316+
checkWcifMock.mockResolvedValueOnce(undefined);
314317
patchWcifMock.mockResolvedValueOnce(wcif);
315318

316319
uploadCurrentWCIFChanges(cb)(dispatch, getState);
@@ -320,6 +323,7 @@ describe('store actions', () => {
320323
type: ActionType.UPLOADING_WCIF,
321324
uploading: true,
322325
});
326+
expect(checkWcifMock).toHaveBeenCalledWith(wcif);
323327
expect(patchWcifMock).toHaveBeenCalledWith('Comp1', {
324328
formatVersion: wcif.formatVersion,
325329
events: wcif.events,
@@ -343,6 +347,7 @@ describe('store actions', () => {
343347
changedKeys: new Set(['events']),
344348
}) as unknown as AppState;
345349
const error = new Error('Upload failed');
350+
checkWcifMock.mockResolvedValueOnce(undefined);
346351
patchWcifMock.mockRejectedValueOnce(error);
347352
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {});
348353

@@ -360,4 +365,23 @@ describe('store actions', () => {
360365
expect(cb).toHaveBeenCalledWith(error);
361366
consoleError.mockRestore();
362367
});
368+
369+
it('does not patch when the WCIF schema check fails', async () => {
370+
vi.clearAllMocks();
371+
const dispatch = vi.fn();
372+
const cb = vi.fn();
373+
const wcif = { ...buildWcif([], []), id: 'Comp1' };
374+
const error = new Error('WCIF formatVersion is required');
375+
const getState = () =>
376+
({ wcif, changedKeys: new Set(['events']) }) as unknown as AppState;
377+
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {});
378+
checkWcifMock.mockRejectedValueOnce(error);
379+
380+
uploadCurrentWCIFChanges(cb)(dispatch, getState);
381+
await flushPromises();
382+
383+
expect(patchWcifMock).not.toHaveBeenCalled();
384+
expect(cb).toHaveBeenCalledWith(error);
385+
consoleError.mockRestore();
386+
});
363387
});

src/store/actions.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getUpcomingManageableCompetitions, getWcif, patchWcif } from '../lib/api';
1+
import { checkWcif, getUpcomingManageableCompetitions, getWcif, patchWcif } from '../lib/api';
22
import { sortWcifEvents } from '../lib/domain/events';
33
import { type BulkInProgressAssignments } from '../lib/types';
44
import { validateWcif, type ValidationError } from '../lib/wcif/validation';
@@ -163,7 +163,8 @@ export const uploadCurrentWCIFChanges =
163163
const changes = pick(wcif, keysForPatch);
164164

165165
dispatch(updateUploading(true));
166-
patchWcif(competitionId, changes)
166+
checkWcif(wcif)
167+
.then(() => patchWcif(competitionId, changes))
167168
.then(() => {
168169
dispatch(updateUploading(false));
169170
cb();

0 commit comments

Comments
 (0)