fix(admin): address PR #623 second-pass review feedback
- Fix VALID_STATUSES fallback to use uppercase enum value - Add TODO comment on InviteListItem cast pending type regeneration - Guard revoke action against null id (returns fail 400) - Add request: to delete action mock events for Sentry consistency - Add expiresAt forwarding test for create action - Add null-id guard test for revoke action Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
import { fail } from '@sveltejs/kit';
|
||||
import { createApiClient } from '$lib/shared/api.server';
|
||||
import { getErrorMessage } from '$lib/shared/errors';
|
||||
import type { Actions, PageServerLoad } from './$types';
|
||||
import type { components } from '$lib/generated/api';
|
||||
|
||||
@@ -25,9 +26,9 @@ type InviteStatus = (typeof VALID_STATUSES)[number];
|
||||
|
||||
export const load: PageServerLoad = async ({ url, fetch }) => {
|
||||
const rawStatus = url.searchParams.get('status');
|
||||
const status: InviteStatus | 'active' = VALID_STATUSES.includes(rawStatus as InviteStatus)
|
||||
const status: InviteStatus = VALID_STATUSES.includes(rawStatus as InviteStatus)
|
||||
? (rawStatus as InviteStatus)
|
||||
: 'active';
|
||||
: 'ACTIVE';
|
||||
const api = createApiClient(fetch);
|
||||
|
||||
const [invitesResult, groupsResult] = await Promise.all([
|
||||
@@ -41,6 +42,7 @@ export const load: PageServerLoad = async ({ url, fetch }) => {
|
||||
const code = (invitesResult.error as unknown as { code?: string })?.code;
|
||||
loadError = code ?? 'INTERNAL_ERROR';
|
||||
} else {
|
||||
// TODO: remove cast after next npm run generate:api — shareableUrl is now @Schema(requiredMode=REQUIRED)
|
||||
invites = (invitesResult.data ?? []) as unknown as InviteListItem[];
|
||||
}
|
||||
|
||||
@@ -79,12 +81,14 @@ export const actions = {
|
||||
return fail(result.response.status, { createError: code ?? 'INTERNAL_ERROR' });
|
||||
}
|
||||
|
||||
// TODO: remove cast after next npm run generate:api — shareableUrl is now @Schema(requiredMode=REQUIRED)
|
||||
return { created: result.data! as unknown as InviteListItem };
|
||||
},
|
||||
|
||||
revoke: async ({ request, fetch }) => {
|
||||
const formData = await request.formData();
|
||||
const id = formData.get('id') as string;
|
||||
const id = formData.get('id') as string | null;
|
||||
if (!id) return fail(400, { revokeError: getErrorMessage('VALIDATION_ERROR') });
|
||||
|
||||
const api = createApiClient(fetch);
|
||||
const result = await api.DELETE('/api/invites/{id}', { params: { path: { id } } });
|
||||
|
||||
@@ -203,6 +203,22 @@ describe('admin/invites create action', () => {
|
||||
|
||||
expect(result).toMatchObject({ status: 500, data: { createError: 'INTERNAL_ERROR' } });
|
||||
});
|
||||
|
||||
it('includes expiresAt in POST body when provided', async () => {
|
||||
const fd = new FormData();
|
||||
fd.append('expiresAt', '2026-12-31');
|
||||
mockFetch.mockResolvedValueOnce(mockResponse(true, successBody, 201));
|
||||
|
||||
await actions.create({
|
||||
request: new Request('http://localhost', { method: 'POST', body: fd }),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any);
|
||||
|
||||
const [req] = mockFetch.mock.calls[0] as [Request, unknown];
|
||||
const sent = await req.json();
|
||||
expect(sent.expiresAt).toBe('2026-12-31');
|
||||
});
|
||||
});
|
||||
|
||||
describe('admin/invites revoke action', () => {
|
||||
@@ -254,4 +270,15 @@ describe('admin/invites revoke action', () => {
|
||||
|
||||
expect(result).toMatchObject({ status: 404, data: { revokeError: 'NOT_FOUND' } });
|
||||
});
|
||||
|
||||
it('returns fail(400) when revoke id is missing', async () => {
|
||||
const result = await actions.revoke({
|
||||
request: new Request('http://localhost', { method: 'POST', body: new FormData() }),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any);
|
||||
|
||||
expect(mockFetch).not.toHaveBeenCalled();
|
||||
expect(result).toMatchObject({ status: 400 });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -165,7 +165,8 @@ describe('admin/users/[id] delete action', () => {
|
||||
function makeEvent() {
|
||||
return {
|
||||
params: { id: 'user-123' },
|
||||
fetch: vi.fn() as unknown as typeof fetch
|
||||
fetch: vi.fn() as unknown as typeof fetch,
|
||||
request: new Request('http://localhost/admin/users/user-123')
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user