Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions changelog.d/SEP-1760.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
React UI: keep the unsaved-changes prompt after a blocked submit, stop an enable/disable toggle from clearing a scheduled task's arguments, reject silently-truncated numeric input and stop a whitespace-only numeric field from submitting as 0, keep step-less task log lines, let a failed executor-host lookup be retried by reopening the field, bound execution-event stream reconnects, and no longer render a white table in dark mode.
5 changes: 3 additions & 2 deletions frontend/oxlintrc.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"$schema": "https://raw.githubusercontent.com/oxc-project/oxc/main/npm/oxlint/configuration_schema.json",
"$schema": "./node_modules/oxlint/configuration_schema.json",
"plugins": ["react", "import", "typescript", "unicorn", "oxc"],
"rules": {
"no-console": "error",
"no-debugger": "error",
Expand Down Expand Up @@ -35,5 +36,5 @@
"unicorn/prefer-array-flat-map": "warn",
"unicorn/prefer-includes": "warn"
},
"ignorePatterns": ["dist", "node_modules", "storybook-static", "*.config.ts", "*.config.js"]
"ignorePatterns": ["dist", "node_modules", "storybook-static"]
}
16 changes: 15 additions & 1 deletion frontend/packages/api/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,21 @@ export function refreshAccessToken(): Promise<string | null> {
refreshInFlight = null;
});
}
_onRefreshed(data.access_token, data.expires_in);
try {
_onRefreshed(data.access_token, data.expires_in);
} catch (handlerError) {
// A throwing auth-layer handler must not invalidate a cookie rotation
// that already succeeded on the backend: it would reject the shared
// promise and force-logout every awaiting caller. It does leave the
// auth layer without the new token or its expiry while this function
// still returns the token, so record that inconsistency — never the
// token or expiry themselves.
// eslint-disable-next-line no-console
console.error(
'[api] onRefreshed handler threw; the rotated token was not recorded',
handlerError,
);
}
return data.access_token;
})();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ describe('HostSelector', () => {
expect(matches.length).toBeGreaterThanOrEqual(1);
});

it('renders error state and disables the input when the endpoint rejects', async () => {
it('renders error state but stays enabled when the endpoint rejects, so opening retries', async () => {
mocked.get.mockRejectedValueOnce(new ApiError({ kind: 'http', status: 502, message: 'boom' }));
const client = makeClient();
render(
Expand All @@ -113,7 +113,17 @@ describe('HostSelector', () => {
);
await screen.findByText('boom');
const input = screen.getByLabelText('Host');
expect(input).toBeDisabled();
// `onOpen` holds the only retry trigger, and a disabled Autocomplete never
// opens, so disabling here would wedge the field until the page remounts.
expect(input).not.toBeDisabled();

mocked.get.mockResolvedValueOnce(
makeResponse([{ id: 'nomad-1', name: 'db-mysql-prod-01', address: '10.0.0.1' }]),
);
const user = userEvent.setup();
await user.click(input);

expect(await screen.findByText('db-mysql-prod-01')).toBeInTheDocument();
});

it('shows a loading message before the endpoint resolves', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/

import { useEffect, useRef, type ReactNode } from 'react';
import { useFormContext } from 'react-hook-form';
import { get, useFormContext } from 'react-hook-form';
import { AutoCompleteInput } from '@percona/percona-ui';
import { useSnackbar } from 'notistack';
import { useHosts, type HostOption } from '../../hooks/useHosts';
Expand Down Expand Up @@ -213,9 +213,16 @@ export function HostSelector({
const { data, isLoading, isError, error, refetch } = useHosts();
const hosts = data ?? EMPTY_HOSTS;
const empty = !isLoading && !isError && hosts.length === 0;
const fieldError = errors[name]?.message as string | undefined;
// Path-aware: a one-of branch field carries a dotted name (`source.host`),
// which `errors[name]` would never resolve.
const fieldError = get(errors, name)?.message as string | undefined;
const freeSolo = Boolean(allowCustom);
const inputDisabled = Boolean(disabled) || isError;
// A failed hosts query does not disable the control: `onOpen` holds the only
// `refetch()` trigger and a disabled Autocomplete never opens, so one failure
// would wedge the field until the page remounts. The error stays visible
// through `text` / `showError` / the snackbar. Same reasoning as
// StandaloneHostSelector.
const inputDisabled = Boolean(disabled);
const showError = isError || Boolean(fieldError);
const text = resolveHelperText({
helperText,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe('StandaloneHostSelector', () => {
expect(handleChange).toHaveBeenCalledWith('');
});

it('disables the input and shows error text when the endpoint rejects', async () => {
it('shows error text but stays enabled when the endpoint rejects, so opening retries', async () => {
mocked.get.mockRejectedValueOnce(
new ApiError({ kind: 'http', status: 502, message: 'network error' }),
);
Expand All @@ -111,7 +111,18 @@ describe('StandaloneHostSelector', () => {
);

await screen.findByText('network error');
expect(screen.getByLabelText('Execution Host')).toBeDisabled();
const input = screen.getByLabelText('Execution Host');
// `onOpen` holds the only retry trigger, so a disabled input would wedge the
// control until the page remounts.
expect(input).not.toBeDisabled();

mocked.get.mockResolvedValueOnce(
makeResponse([{ id: 'nomad-1', name: 'db-mysql-prod-01', address: '10.0.0.1' }]),
);
const user = userEvent.setup();
await user.click(input);

expect(await screen.findByRole('option', { name: 'db-mysql-prod-01' })).toBeInTheDocument();
});

it('surfaces upstream Tasks-API failure via snackbar', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ export function StandaloneHostSelector({
loading={isLoading}
loadingText="Loading hosts…"
noOptionsText="No hosts available"
disabled={disabled || isError}
// Stays enabled on a failed hosts query: `onOpen` holds the only retry
// trigger, and a disabled Autocomplete never opens — one failure would
// otherwise wedge the control until the page remounts.
disabled={disabled}
renderInput={(params) => (
<TextField
{...params}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,26 @@ export function ScheduledTasksPanel({

const handleToggleEnabled = async (task: PeriodicTaskResponse, nextEnabled: boolean) => {
// PeriodicTaskUpdate requires `kwargs` and `description`, but
// PeriodicTaskResponse exposes only `description`. Tasks created elsewhere
// with non-default kwargs will have them reset to '{}' on toggle. Tracked
// upstream as a backend schema gap.
// PeriodicTaskResponse declares only `description`. Preserve `kwargs` when
// the response happens to carry it so a plain enable/disable toggle does
// not silently wipe a task's arguments; '{}' stays the last-resort
// fallback. Tracked upstream as a backend schema gap.
const rawKwargs = (task as { kwargs?: unknown }).kwargs;
// The wire shape is unverified either way, so accept both: a JSON string
// passes through, a decoded object is re-serialised. Anything else (or a
// blank value) falls back to '{}' — the only case that still loses data.
let preservedKwargs = '{}';
if (typeof rawKwargs === 'string' && rawKwargs.trim() !== '') {
preservedKwargs = rawKwargs;
} else if (rawKwargs !== null && typeof rawKwargs === 'object') {
preservedKwargs = JSON.stringify(rawKwargs);
}
const body: PeriodicTaskUpdate = {
name: task.name,
task: task.task,
enabled: nextEnabled,
description: task.description,
kwargs: '{}',
kwargs: preservedKwargs,
start_time: task.start_time,
interval: task.interval ?? null,
crontab: task.crontab ?? null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
useAppTask,
useAppTasks,
type DetailSection,
type ListView,
type SepComponents,
type AppEntitySchema,
type AppSchema,
Expand Down Expand Up @@ -249,6 +250,9 @@ const BASELINE_OVERVIEW_HIDDEN_FIELDS = [
'anonymized_entities',
] as const;

/** Stable empty column set so a schema without a `list_view` never re-memoizes. */
const EMPTY_LIST_COLUMNS: ListView['columns'] = [];

function formatLabel(key: string): string {
return key.replace(/_/g, ' ').replace(/\b\w/g, (c) => c.toUpperCase());
}
Expand Down Expand Up @@ -406,7 +410,10 @@ function OverviewTab({
const navState = (location.state ?? null) as { connectivityWarning?: unknown } | null;
const connectivityWarning = task.connectivity_warning ?? navState?.connectivityWarning;
const taskName = typeof task.name === 'string' && task.name.trim() ? task.name.trim() : undefined;
const columns = schema.list_view!.columns;
// `list_view` is optional: an entity schema reached through an unresolved
// detail route has no top-level list view, so fall back to the task's own
// fields (rendered below as `extraEntries`) rather than crashing.
const columns = schema.list_view?.columns ?? EMPTY_LIST_COLUMNS;
const schemaHiddenFields = schema.list_view?.overview_hidden_fields;

const suppressedFields = useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,14 @@ export function AppListPage({
: rows.filter((row) => RUNNING_STATUSES.has(row.status as TaskHistoryStatus)).length,
[multi, rows],
);
const listView = multi ? entitySchema!.list_view : schema.list_view!;
// `list_view` is optional on the top-level schema, and an unresolved entity
// route (unknown `entityName` on an entity schema) falls back to it, so it can
// be absent here — guarded below once every hook has run.
const listView = multi ? entitySchema!.list_view : schema.list_view;
const title = multi ? entitySchema!.display_name : schema.display_name;
const description = multi ? entitySchema?.description : schema.description;

const hasActionsColumn = listView.columns.some((c) => c.format === 'actions');
const hasActionsColumn = listView?.columns.some((c) => c.format === 'actions') ?? false;
const deleteEntity = useDeleteAppEntity(
pluginName,
entityName ?? '',
Expand Down Expand Up @@ -203,6 +206,14 @@ export function AppListPage({
});
};

if (!listView) {
return (
<Box sx={{ py: 2 }}>
<Typography variant="h5">Not found</Typography>
</Box>
);
}

return (
<Box>
{multi && schema.entities && !hideEntityTabs && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,4 +335,45 @@ describe('normalizeChoiceDefaults', () => {
const result = normalizeChoiceDefaults({ upload: ['RSYNC'] }, sections);
expect(result.upload).toEqual(['RSYNC']);
});

it('normalizes a one-of branch field stored at its dotted path', () => {
const oneOfSections = [
{
title: 'Source',
fields: [
{
type: 'one_of' as const,
name: 'source',
label: 'Source',
discriminator: 'source.mode',
branches: [
{
value: 'rsync',
label: 'Rsync',
fields: [
{
type: 'choice' as const,
name: 'source.transport',
label: 'Transport',
required: false,
choices: [
{ value: 'SSH', label: 'SSH' },
{ value: 'DAEMON', label: 'Daemon' },
],
},
],
},
],
},
],
},
];
const form = { source: { mode: 'rsync', transport: 'ssh' } };

const result = normalizeChoiceDefaults(form, oneOfSections);

expect(result.source).toEqual({ mode: 'rsync', transport: 'SSH' });
// The input is not mutated: `setAtPath` clones the intermediates it walks.
expect(form.source.transport).toBe('ssh');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ import Typography from '@mui/material/Typography';
import ArrowBackIcon from '@mui/icons-material/ArrowBack';
import { useSnackbar } from 'notistack';
import { useAppTask, useUpdateAppTask, type AppSchema } from '@sep/api';
import { SchemaFormRenderer, coerceFormValues, flattenSectionFields } from '../SchemaFormRenderer';
import {
SchemaFormRenderer,
coerceFormValues,
flattenSectionFields,
getAtPath,
setAtPath,
} from '../SchemaFormRenderer';
import type { FormSection, RenderFieldOverride } from '../SchemaFormRenderer/types';
import type { RenderFormSlot } from './types';
import { getStoredForm } from './storedForm';
Expand All @@ -51,16 +57,24 @@ export function normalizeChoiceDefaults(
continue;
}
const choiceMap = new Map(field.choices.map((c) => [c.value.toLowerCase(), c.value]));
const raw = out[field.name];
// Path-aware: `flattenSectionFields` also returns one-of branch fields,
// whose names are dotted paths (`source.mode`) stored nested in the form.
// `setAtPath` shallow-clones the intermediates it walks, so the shallow
// copy above is enough to leave `form` untouched.
const raw = getAtPath(out, field.name);
if (field.type === 'multi_choice' && Array.isArray(raw)) {
out[field.name] = raw.map((v) => {
const canonical = typeof v === 'string' ? choiceMap.get(v.toLowerCase()) : undefined;
return canonical ?? v;
});
setAtPath(
out,
field.name,
raw.map((v) => {
const canonical = typeof v === 'string' ? choiceMap.get(v.toLowerCase()) : undefined;
return canonical ?? v;
}),
);
} else if (field.type === 'choice' && typeof raw === 'string') {
const canonical = choiceMap.get(raw.toLowerCase());
if (canonical !== undefined) {
out[field.name] = canonical;
setAtPath(out, field.name, canonical);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,10 @@ function AppEditPage({
onSubmit: handleSubmit,
loading: updateEntity.isPending,
defaultValues,
capabilities: schema.capabilities,
renderField,
submitError,
fieldErrors,
}) ?? (
<SchemaFormRenderer
sections={sections}
Expand Down
Loading
Loading