Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion src/layout/CompetitionLayout/CompetitionLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export const CompetitionLayout = () => {
dispatch(
uploadCurrentWCIFChanges((e) => {
if (e) {
enqueueSnackbar('Error saving changes', { variant: 'error' });
enqueueSnackbar(`Error saving changes: ${e.message}`, { variant: 'error' });
} else {
enqueueSnackbar('Saved!', { variant: 'success' });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ describe('CompetitionLayout', () => {
const errorCallback = uploadCurrentWCIFChangesMock.mock.calls[1][0];
errorCallback(new Error('save failed'));

expect(enqueueSnackbarMock).toHaveBeenCalledWith('Error saving changes', { variant: 'error' });
expect(enqueueSnackbarMock).toHaveBeenCalledWith('Error saving changes: save failed', {
variant: 'error',
});
});
});
29 changes: 29 additions & 0 deletions src/lib/api/wcaAPI.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, it, vi, afterEach } from 'vitest';
import {
checkWcif,
getMe,
getPastManageableCompetitions,
getUpcomingManageableCompetitions,
Expand Down Expand Up @@ -52,6 +53,34 @@ describe('wcaAPI', () => {
await expect(wcaApiFetch('/me')).rejects.toThrow('Something went wrong: Status code 418');
});

it('uses an API error response when one is available', async () => {
mockFetch({
ok: false,
status: 400,
statusText: 'Bad Request',
json: vi.fn().mockResolvedValue({ error: 'WCIF formatVersion is required' }),
});

await expect(wcaApiFetch('/me')).rejects.toThrow('WCIF formatVersion is required');
});

it('checks a complete WCIF without parsing the empty success response', async () => {
const json = vi.fn();
const wcif = { id: 'Comp', formatVersion: '1.1' } as any;
mockFetch({ json });

await checkWcif(wcif);

expect(globalThis.fetch).toHaveBeenCalledWith(
'https://wca.test/api/v0/competitions/wcif/check',
expect.objectContaining({
method: 'PUT',
body: JSON.stringify(wcif),
})
);
expect(json).not.toHaveBeenCalled();
});

it('builds upcoming and past competition queries', async () => {
vi.spyOn(Date, 'now').mockReturnValue(0);
mockFetch({ json: vi.fn().mockResolvedValue([]) });
Expand Down
33 changes: 32 additions & 1 deletion src/lib/api/wcaAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ export const patchWcif = (
body: JSON.stringify(wcif),
});

export const checkWcif = (wcif: Competition): Promise<void> =>
wcaApiFetch(
'/competitions/wcif/check',
{
method: 'PUT',
body: JSON.stringify(wcif),
},
false
);

export const saveWcifChanges = (
previousWcif: Competition,
newWcif: Competition
Expand All @@ -82,7 +92,8 @@ export const getUser = (userId: number): Promise<{ user: WcaUser }> =>

export const wcaApiFetch = async <T = unknown>(
path: string,
fetchOptions: RequestInit = {}
fetchOptions: RequestInit = {},
parseJsonResponse = true
): Promise<T> => {
const baseApiUrl = `${WCA_ORIGIN}/api/v0`;

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

if (!res.ok) {
const error = await errorFromResponse(res);
if (error) throw new Error(error);

if (res.statusText) {
throw new Error(`${res.status}: ${res.statusText}`);
} else {
throw new Error(`Something went wrong: Status code ${res.status}`);
}
}

if (!parseJsonResponse) return undefined as T;

return await res.json();
};

const errorFromResponse = async (res: Response): Promise<string | undefined> => {
try {
const body: unknown = await res.json();
if (Array.isArray(body)) return body.map(String).join('\n');

if (body && typeof body === 'object' && 'error' in body) {
const error = body.error;
if (Array.isArray(error)) return error.map(String).join('\n');
if (typeof error === 'string') return error;
}
} catch {
// Fall back to the HTTP status when the API does not return JSON.
}
};
26 changes: 25 additions & 1 deletion src/store/actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
import type { Assignment, Competition } from '@wca/helpers';
import type { Extension } from '@wca/helpers/lib/models/extension';
import { describe, expect, it, vi } from 'vitest';
import { getUpcomingManageableCompetitions, getWcif, patchWcif } from '../lib/api';
import { checkWcif, getUpcomingManageableCompetitions, getWcif, patchWcif } from '../lib/api';
import { sortWcifEvents } from '../lib/domain/events';
import { validateWcif } from '../lib/wcif/validation';
import type { AppState } from './initialState';
Expand All @@ -41,6 +41,7 @@ import {
vi.mock('../lib/api', () => ({
getUpcomingManageableCompetitions: vi.fn(),
getWcif: vi.fn(),
checkWcif: vi.fn(),
patchWcif: vi.fn(),
}));

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

const getUpcomingManageableCompetitionsMock = vi.mocked(getUpcomingManageableCompetitions);
const getWcifMock = vi.mocked(getWcif);
const checkWcifMock = vi.mocked(checkWcif);
const patchWcifMock = vi.mocked(patchWcif);
const sortWcifEventsMock = vi.mocked(sortWcifEvents);
const validateWcifMock = vi.mocked(validateWcif);
Expand Down Expand Up @@ -311,6 +313,7 @@ describe('store actions', () => {
wcif,
changedKeys: new Set(['events']),
}) as unknown as AppState;
checkWcifMock.mockResolvedValueOnce(undefined);
patchWcifMock.mockResolvedValueOnce(wcif);

uploadCurrentWCIFChanges(cb)(dispatch, getState);
Expand All @@ -320,6 +323,7 @@ describe('store actions', () => {
type: ActionType.UPLOADING_WCIF,
uploading: true,
});
expect(checkWcifMock).toHaveBeenCalledWith(wcif);
expect(patchWcifMock).toHaveBeenCalledWith('Comp1', {
formatVersion: wcif.formatVersion,
events: wcif.events,
Expand All @@ -343,6 +347,7 @@ describe('store actions', () => {
changedKeys: new Set(['events']),
}) as unknown as AppState;
const error = new Error('Upload failed');
checkWcifMock.mockResolvedValueOnce(undefined);
patchWcifMock.mockRejectedValueOnce(error);
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {});

Expand All @@ -360,4 +365,23 @@ describe('store actions', () => {
expect(cb).toHaveBeenCalledWith(error);
consoleError.mockRestore();
});

it('does not patch when the WCIF schema check fails', async () => {
vi.clearAllMocks();
const dispatch = vi.fn();
const cb = vi.fn();
const wcif = { ...buildWcif([], []), id: 'Comp1' };
const error = new Error('WCIF formatVersion is required');
const getState = () =>
({ wcif, changedKeys: new Set(['events']) }) as unknown as AppState;
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {});
checkWcifMock.mockRejectedValueOnce(error);

uploadCurrentWCIFChanges(cb)(dispatch, getState);
await flushPromises();

expect(patchWcifMock).not.toHaveBeenCalled();
expect(cb).toHaveBeenCalledWith(error);
consoleError.mockRestore();
});
});
5 changes: 3 additions & 2 deletions src/store/actions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getUpcomingManageableCompetitions, getWcif, patchWcif } from '../lib/api';
import { checkWcif, getUpcomingManageableCompetitions, getWcif, patchWcif } from '../lib/api';
import { sortWcifEvents } from '../lib/domain/events';
import { type BulkInProgressAssignments } from '../lib/types';
import { validateWcif, type ValidationError } from '../lib/wcif/validation';
Expand Down Expand Up @@ -163,7 +163,8 @@ export const uploadCurrentWCIFChanges =
const changes = pick(wcif, keysForPatch);

dispatch(updateUploading(true));
patchWcif(competitionId, changes)
checkWcif(wcif)
.then(() => patchWcif(competitionId, changes))
.then(() => {
dispatch(updateUploading(false));
cb();
Expand Down
Loading