choret: enforce discord-only provider type across notifications API and UI
- Added validation to reject non-discord provider types in create, update, test, and preview operations. - Updated the notifications form to automatically normalize non-discord types to discord. - Modified UI to display explicit messaging for deprecated and non-dispatch statuses for non-discord providers. - Enhanced tests to cover new validation logic and UI changes for provider types.
This commit is contained in:
GitHub Actions
@@ -88,6 +88,16 @@ describe('notifications api', () => {
|
||||
expect(mockedClient.delete).toHaveBeenCalledWith('/notifications/providers/new')
|
||||
})
|
||||
|
||||
it('rejects non-discord type before submit for provider mutations and preview', async () => {
|
||||
await expect(createProvider({ name: 'Bad', type: 'slack' })).rejects.toThrow('Only discord notification providers are supported')
|
||||
await expect(updateProvider('bad', { type: 'generic' })).rejects.toThrow('Only discord notification providers are supported')
|
||||
await expect(testProvider({ id: 'bad', type: 'email' })).rejects.toThrow('Only discord notification providers are supported')
|
||||
await expect(previewProvider({ id: 'bad', type: 'gotify' })).rejects.toThrow('Only discord notification providers are supported')
|
||||
|
||||
expect(mockedClient.post).not.toHaveBeenCalled()
|
||||
expect(mockedClient.put).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fetches templates and previews provider payloads with data', async () => {
|
||||
mockedClient.get.mockResolvedValueOnce({ data: [{ id: 'tpl', name: 'default' }] })
|
||||
mockedClient.post.mockResolvedValue({ data: { preview: 'ok' } })
|
||||
|
||||
@@ -32,6 +32,12 @@ const withDiscordType = (data: Partial<NotificationProvider>): Partial<Notificat
|
||||
return { ...data, type: DISCORD_PROVIDER_TYPE };
|
||||
};
|
||||
|
||||
const assertDiscordOnlyInput = (data: Partial<NotificationProvider>): void => {
|
||||
if (typeof data.type === 'string' && data.type.toLowerCase() !== DISCORD_PROVIDER_TYPE) {
|
||||
throw new Error('Only discord notification providers are supported');
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Fetches all notification providers.
|
||||
* @returns Promise resolving to array of NotificationProvider objects
|
||||
@@ -49,6 +55,7 @@ export const getProviders = async () => {
|
||||
* @throws {AxiosError} If creation fails
|
||||
*/
|
||||
export const createProvider = async (data: Partial<NotificationProvider>) => {
|
||||
assertDiscordOnlyInput(data);
|
||||
const response = await client.post<NotificationProvider>('/notifications/providers', withDiscordType(data));
|
||||
return response.data;
|
||||
};
|
||||
@@ -61,6 +68,7 @@ export const createProvider = async (data: Partial<NotificationProvider>) => {
|
||||
* @throws {AxiosError} If update fails or provider not found
|
||||
*/
|
||||
export const updateProvider = async (id: string, data: Partial<NotificationProvider>) => {
|
||||
assertDiscordOnlyInput(data);
|
||||
const response = await client.put<NotificationProvider>(`/notifications/providers/${id}`, withDiscordType(data));
|
||||
return response.data;
|
||||
};
|
||||
@@ -80,6 +88,7 @@ export const deleteProvider = async (id: string) => {
|
||||
* @throws {AxiosError} If test fails
|
||||
*/
|
||||
export const testProvider = async (provider: Partial<NotificationProvider>) => {
|
||||
assertDiscordOnlyInput(provider);
|
||||
await client.post('/notifications/providers/test', withDiscordType(provider));
|
||||
};
|
||||
|
||||
@@ -107,6 +116,7 @@ export interface NotificationTemplate {
|
||||
* @throws {AxiosError} If preview fails
|
||||
*/
|
||||
export const previewProvider = async (provider: Partial<NotificationProvider>, data?: Record<string, unknown>) => {
|
||||
assertDiscordOnlyInput(provider);
|
||||
const payload: Record<string, unknown> = withDiscordType(provider) as Record<string, unknown>;
|
||||
if (data) payload.data = data;
|
||||
const response = await client.post('/notifications/providers/preview', payload);
|
||||
|
||||
@@ -16,7 +16,7 @@ const supportsJSONTemplates = (providerType: string | undefined): boolean => {
|
||||
return providerType.toLowerCase() === DISCORD_PROVIDER_TYPE;
|
||||
};
|
||||
|
||||
const isDeprecatedProvider = (providerType: string | undefined): boolean => {
|
||||
const isNonDiscordProvider = (providerType: string | undefined): boolean => {
|
||||
if (!providerType) {
|
||||
return false;
|
||||
}
|
||||
@@ -110,6 +110,12 @@ const ProviderForm: FC<{
|
||||
};
|
||||
|
||||
const type = watch('type');
|
||||
useEffect(() => {
|
||||
if (type !== DISCORD_PROVIDER_TYPE) {
|
||||
setValue('type', DISCORD_PROVIDER_TYPE, { shouldDirty: false, shouldTouch: false });
|
||||
}
|
||||
}, [type, setValue]);
|
||||
|
||||
const { data: builtins } = useQuery({ queryKey: ['notificationTemplates'], queryFn: getTemplates });
|
||||
const { data: externalTemplates } = useQuery({ queryKey: ['externalTemplates'], queryFn: getExternalTemplates });
|
||||
const template = watch('template');
|
||||
@@ -153,6 +159,8 @@ const ProviderForm: FC<{
|
||||
<select
|
||||
{...register('type')}
|
||||
data-testid="provider-type"
|
||||
disabled
|
||||
aria-readonly="true"
|
||||
className="mt-1 block w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 dark:bg-gray-700 dark:border-gray-600 dark:text-white sm:text-sm"
|
||||
>
|
||||
<option value="discord">Discord</option>
|
||||
@@ -555,7 +563,7 @@ const Notifications: FC = () => {
|
||||
<div className="grid gap-4">
|
||||
{providers?.map((provider) => (
|
||||
<Card key={provider.id} className="p-4" data-testid={`provider-row-${provider.id}`}>
|
||||
{editingId === provider.id && !isDeprecatedProvider(provider.type) ? (
|
||||
{editingId === provider.id && !isNonDiscordProvider(provider.type) ? (
|
||||
<ProviderForm
|
||||
initialData={provider}
|
||||
onClose={() => setEditingId(null)}
|
||||
@@ -569,43 +577,46 @@ const Notifications: FC = () => {
|
||||
</div>
|
||||
<div>
|
||||
<h3 className="font-medium text-gray-900 dark:text-white">{provider.name}</h3>
|
||||
{isDeprecatedProvider(provider.type) && (
|
||||
<div className="flex items-center gap-2 mt-1" data-testid={`provider-deprecated-status-${provider.id}`}>
|
||||
<span
|
||||
data-testid={`provider-deprecated-badge-${provider.id}`}
|
||||
className="uppercase text-xs font-bold bg-amber-100 text-amber-700 dark:bg-amber-900/40 dark:text-amber-300 px-2 py-0.5 rounded"
|
||||
>
|
||||
{t('notificationProviders.deprecatedReadOnly')}
|
||||
</span>
|
||||
<span
|
||||
data-testid={`provider-nondispatch-badge-${provider.id}`}
|
||||
className="uppercase text-xs font-bold bg-gray-200 text-gray-700 dark:bg-gray-700 dark:text-gray-300 px-2 py-0.5 rounded"
|
||||
>
|
||||
{t('notificationProviders.nonDispatch')}
|
||||
</span>
|
||||
</div>
|
||||
)}
|
||||
{updateIndicatorId === provider.id && (
|
||||
<span className="text-xs text-green-600" data-testid={`provider-update-indicator-${provider.id}`}>
|
||||
{t('common.saved')}
|
||||
</span>
|
||||
)}
|
||||
{isNonDiscordProvider(provider.type) && (
|
||||
<div className="mt-2 space-y-1">
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
<span
|
||||
className="text-xs font-semibold bg-amber-100 text-amber-800 dark:bg-amber-900 dark:text-amber-200 px-2 py-0.5 rounded"
|
||||
data-testid={`provider-deprecated-status-${provider.id}`}
|
||||
>
|
||||
{t('notificationProviders.deprecatedReadOnly')}
|
||||
</span>
|
||||
<span
|
||||
className="text-xs font-semibold bg-gray-200 text-gray-700 dark:bg-gray-700 dark:text-gray-200 px-2 py-0.5 rounded"
|
||||
data-testid={`provider-nondispatch-status-${provider.id}`}
|
||||
>
|
||||
{t('notificationProviders.nonDispatch')}
|
||||
</span>
|
||||
</div>
|
||||
<p
|
||||
className="text-xs text-gray-600 dark:text-gray-300"
|
||||
data-testid={`provider-deprecated-message-${provider.id}`}
|
||||
>
|
||||
{t('notificationProviders.deprecatedProviderMessage')}
|
||||
</p>
|
||||
</div>
|
||||
)}
|
||||
<div className="text-sm text-gray-500 dark:text-gray-400 flex items-center gap-2">
|
||||
<span className="uppercase text-xs font-bold bg-gray-100 dark:bg-gray-700 px-2 py-0.5 rounded">
|
||||
{provider.type}
|
||||
</span>
|
||||
<span className="truncate max-w-xs opacity-50">{provider.url}</span>
|
||||
</div>
|
||||
{isDeprecatedProvider(provider.type) && (
|
||||
<p className="text-xs text-amber-700 dark:text-amber-300 mt-1" data-testid={`provider-deprecated-message-${provider.id}`}>
|
||||
{t('notificationProviders.deprecatedProviderMessage')}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div className="flex items-center gap-2">
|
||||
{!isDeprecatedProvider(provider.type) && (
|
||||
{!isNonDiscordProvider(provider.type) && (
|
||||
<Button
|
||||
variant="secondary"
|
||||
size="sm"
|
||||
@@ -616,7 +627,7 @@ const Notifications: FC = () => {
|
||||
<Send className="w-4 h-4" />
|
||||
</Button>
|
||||
)}
|
||||
{!isDeprecatedProvider(provider.type) && (
|
||||
{!isNonDiscordProvider(provider.type) && (
|
||||
<Button variant="secondary" size="sm" onClick={() => setEditingId(provider.id)}>
|
||||
<Edit2 className="w-4 h-4" />
|
||||
</Button>
|
||||
|
||||
@@ -145,6 +145,28 @@ describe('Notifications', () => {
|
||||
|
||||
expect(options).toHaveLength(1)
|
||||
expect(options[0].value).toBe('discord')
|
||||
expect(typeSelect.disabled).toBe(true)
|
||||
})
|
||||
|
||||
it('normalizes stale non-discord type to discord on submit', async () => {
|
||||
const user = userEvent.setup()
|
||||
renderWithQueryClient(<Notifications />)
|
||||
|
||||
await user.click(await screen.findByTestId('add-provider-btn'))
|
||||
await user.type(screen.getByTestId('provider-name'), 'Normalized Provider')
|
||||
await user.type(screen.getByTestId('provider-url'), 'https://example.com/webhook')
|
||||
|
||||
const typeSelect = screen.getByTestId('provider-type') as HTMLSelectElement
|
||||
expect(typeSelect.value).toBe('discord')
|
||||
|
||||
await user.click(screen.getByTestId('provider-save-btn'))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(notificationsApi.createProvider).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
const payload = vi.mocked(notificationsApi.createProvider).mock.calls[0][0]
|
||||
expect(payload.type).toBe('discord')
|
||||
})
|
||||
|
||||
it('shows and hides the update indicator after save', async () => {
|
||||
@@ -344,7 +366,7 @@ describe('Notifications', () => {
|
||||
confirmSpy.mockRestore()
|
||||
})
|
||||
|
||||
it('renders non-discord providers as deprecated read-only rows', async () => {
|
||||
it('renders non-discord providers with explicit deprecated and non-dispatch messaging', async () => {
|
||||
const legacyProvider: NotificationProvider = {
|
||||
...baseProvider,
|
||||
id: 'legacy-provider',
|
||||
@@ -358,9 +380,9 @@ describe('Notifications', () => {
|
||||
renderWithQueryClient(<Notifications />)
|
||||
|
||||
const legacyRow = await screen.findByTestId('provider-row-legacy-provider')
|
||||
expect(await screen.findByTestId('provider-deprecated-badge-legacy-provider')).toBeInTheDocument()
|
||||
expect(screen.getByTestId('provider-nondispatch-badge-legacy-provider')).toBeInTheDocument()
|
||||
expect(screen.getByTestId('provider-deprecated-message-legacy-provider')).toBeInTheDocument()
|
||||
expect(within(legacyRow).getAllByRole('button')).toHaveLength(1)
|
||||
expect(screen.getByTestId('provider-deprecated-status-legacy-provider')).toHaveTextContent('notificationProviders.deprecatedReadOnly')
|
||||
expect(screen.getByTestId('provider-nondispatch-status-legacy-provider')).toHaveTextContent('notificationProviders.nonDispatch')
|
||||
expect(screen.getByTestId('provider-deprecated-message-legacy-provider')).toHaveTextContent('notificationProviders.deprecatedProviderMessage')
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user