Skip to content
Merged
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
1 change: 1 addition & 0 deletions changelog.d/SEP-1489.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Settings page now separates core SEP settings from app-owned settings, rendering enabled app-owned groups under a dedicated "App settings" section with a per-app label chip; app-owned settings are hidden when their owning app is disabled, and the section is omitted entirely when nothing eligible remains.
64 changes: 64 additions & 0 deletions frontend/packages/e2e/tests/settings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,40 @@ async function mockApis(
}),
],
},
// App-owned group whose app is enabled: shows under "App settings".
{
setting_class: 'AlertsSettings',
is_app_owned: true,
app_id: 'alerts',
app_display_name: 'Alerts',
app_enabled: true,
settings: [
makeSetting({
setting_class: 'AlertsSettings',
key: 'ALERTS_RETENTION_DAYS',
value: 30,
default_value: 30,
type: 'int',
}),
],
},
// App-owned group whose app is disabled: hidden from the page entirely.
{
setting_class: 'InventorySettings',
is_app_owned: true,
app_id: 'inventory',
app_display_name: 'Inventory',
app_enabled: false,
settings: [
makeSetting({
setting_class: 'InventorySettings',
key: 'INVENTORY_SCAN_INTERVAL',
value: 60,
default_value: 60,
type: 'int',
}),
],
},
],
});

Expand Down Expand Up @@ -301,6 +335,36 @@ test.describe('Settings page smoke', () => {
await expect(page.getByTestId('setting-row-STALENESS_THRESHOLD_SECONDS')).toBeVisible();
});

test('groups app-owned settings in the App settings region and hides disabled apps', async ({
page,
}) => {
await mockApis(page, { isAdmin: true });
await page.goto('/settings');

await expect(page.getByTestId('settings-group-SEPSettings')).toBeVisible({ timeout: 10_000 });

// Enabled app's group renders under App settings, tagged with its app name.
const region = page.getByTestId('app-settings-region');
await expect(region).toBeVisible();
await expect(region.getByText('App settings')).toBeVisible();
await expect(region.getByTestId('settings-group-AlertsSettings')).toBeVisible();
await expect(region.getByTestId('settings-group-app-label-AlertsSettings')).toHaveText(
'Alerts',
);

// Core groups stay out of the App settings region.
await expect(region.getByTestId('settings-group-SEPSettings')).toHaveCount(0);

// Disabled app's group is hidden everywhere.
await expect(page.getByTestId('settings-group-InventorySettings')).toHaveCount(0);
await expect(page.getByTestId('setting-row-INVENTORY_SCAN_INTERVAL')).toHaveCount(0);

// Search reaches into the app region too.
await page.getByLabel('Search settings').fill('ALERTS_RETENTION');
await expect(page.getByTestId('setting-row-SYNC_REFRESH_TIME')).toBeHidden();
await expect(region.getByTestId('setting-row-ALERTS_RETENTION_DAYS')).toBeVisible();
});

test('admin can run a connectivity check and see per-service results', async ({ page }) => {
await mockApis(page, { isAdmin: true });
await page.goto('/settings');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ export interface SettingsGroupProps {
defaultExpanded?: boolean;
/** Whether a key search is active (auto-expands nested groups). */
searchActive?: boolean;
/**
* Owning app's display label for app-owned groups. When set, the group is
* tagged with the app it belongs to so admins can tell it apart from core
* SEP settings.
*/
appLabel?: string;
}

/** One expandable section per `setting_class`, listing its setting rows. */
Expand All @@ -42,6 +48,7 @@ export default function SettingsGroup({
settings,
defaultExpanded = true,
searchActive = false,
appLabel,
}: SettingsGroupProps) {
const tree = buildSettingTree(settings);
return (
Expand All @@ -52,6 +59,14 @@ export default function SettingsGroup({
{settingClass}
</Typography>
<Chip size="small" label={tree.length} />
{appLabel && (
<Chip
size="small"
variant="outlined"
label={appLabel}
data-testid={`settings-group-app-label-${settingClass}`}
/>
)}
</Stack>
</AccordionSummary>
<AccordionDetails>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { http, HttpResponse } from 'msw';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { server } from '../../../../tests/msw-server';
import { makeWrapper, sepListResponse, tasksListResponse } from './fixtures';
import { appsListResponse, makeWrapper, sepListResponse, tasksListResponse } from './fixtures';

const authState = vi.hoisted(() => ({ isAdmin: true }));
vi.mock('../../../contexts/auth', () => ({
Expand All @@ -36,9 +36,12 @@ const EXPORT_URL = 'http://localhost/api/sep/admin/settings/export';
const originalCreateObjectURL = URL.createObjectURL;
const originalRevokeObjectURL = URL.revokeObjectURL;

/** SEP aggregates its local classes and the proxied TasksSettings into one list. */
/**
* SEP aggregates its local classes, the proxied TasksSettings, and app-owned
* groups (both enabled and disabled) into one list.
*/
const combinedListResponse = {
groups: [...sepListResponse.groups, ...tasksListResponse.groups],
groups: [...sepListResponse.groups, ...tasksListResponse.groups, ...appsListResponse.groups],
};

function renderPage() {
Expand Down Expand Up @@ -183,4 +186,78 @@ describe('SettingsPage', () => {
expect(within(row).getByTestId('setting-value-API_SECRET')).toHaveTextContent('**********');
expect(within(row).getByLabelText('API_SECRET')).toHaveValue('');
});

it('renders enabled app-owned groups under the App settings region, labeled by app', async () => {
renderPage();
const region = await screen.findByTestId('app-settings-region');
expect(within(region).getByText('App settings')).toBeInTheDocument();

// The enabled app's group renders inside the region, tagged with its app.
const alertsGroup = within(region).getByTestId('settings-group-AlertsSettings');
expect(alertsGroup).toBeInTheDocument();
expect(within(region).getByTestId('settings-group-app-label-AlertsSettings')).toHaveTextContent(
'Alerts',
);
});

it('keeps core groups in the core region, out of the App settings region', async () => {
renderPage();
await screen.findByTestId('app-settings-region');

const region = screen.getByTestId('app-settings-region');
// Core groups are not tagged and not nested under the App settings region.
expect(screen.getByTestId('settings-group-SEPSettings')).toBeInTheDocument();
expect(within(region).queryByTestId('settings-group-SEPSettings')).not.toBeInTheDocument();
expect(within(region).queryByTestId('settings-group-TasksSettings')).not.toBeInTheDocument();
expect(screen.queryByTestId('settings-group-app-label-SEPSettings')).not.toBeInTheDocument();
});

it('hides app-owned groups whose owning app is disabled', async () => {
renderPage();
await screen.findByTestId('settings-group-SEPSettings');
// The disabled app's group and its rows never render.
expect(screen.queryByTestId('settings-group-InventorySettings')).not.toBeInTheDocument();
expect(screen.queryByTestId('setting-row-INVENTORY_SCAN_INTERVAL')).not.toBeInTheDocument();
});

it('keeps disabled apps out of the Class filter dropdown', async () => {
renderPage();
await screen.findByTestId('settings-group-SEPSettings');

await userEvent.click(screen.getByLabelText('Filter by class'));
const listbox = await screen.findByRole('listbox');
// Enabled app's class is selectable; disabled app's class never appears.
expect(within(listbox).getByRole('option', { name: 'AlertsSettings' })).toBeInTheDocument();
expect(
within(listbox).queryByRole('option', { name: 'InventorySettings' }),
).not.toBeInTheDocument();
});

it('omits the App settings region when no enabled app-owned groups remain', async () => {
server.use(
http.get(SEP_URL, () =>
HttpResponse.json({
groups: [...sepListResponse.groups, appsListResponse.groups[1]],
}),
),
);
renderPage();
await screen.findByTestId('settings-group-SEPSettings');
expect(screen.queryByTestId('app-settings-region')).not.toBeInTheDocument();
expect(screen.queryByText('App settings')).not.toBeInTheDocument();
});

it('searches across both core and app-owned regions', async () => {
renderPage();
await screen.findByTestId('app-settings-region');

await userEvent.type(screen.getByLabelText('Search settings'), 'ALERTS_RETENTION');

// App-owned match survives under its region; core rows filter out.
await waitFor(() =>
expect(screen.queryByTestId('setting-row-SYNC_REFRESH_TIME')).not.toBeInTheDocument(),
);
const region = screen.getByTestId('app-settings-region');
expect(within(region).getByTestId('setting-row-ALERTS_RETENTION_DAYS')).toBeInTheDocument();
}, 15_000);
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@
import { describe, expect, it } from 'vitest';
import type { SettingClassGroup } from '@sep/api';

import { DEFAULT_SETTINGS_FILTERS, filterSettingsGroups } from '../filters';
import {
DEFAULT_SETTINGS_FILTERS,
appLabelFor,
filterSettingsGroups,
partitionSettingsGroups,
} from '../filters';
import { makeSetting } from './fixtures';

const groups: SettingClassGroup[] = [
Expand Down Expand Up @@ -125,3 +130,49 @@ describe('filterSettingsGroups', () => {
expect(result).toHaveLength(0);
});
});

const appGroup = (
overrides: Partial<SettingClassGroup> & Pick<SettingClassGroup, 'setting_class'>,
): SettingClassGroup => ({
is_app_owned: true,
settings: [makeSetting({ setting_class: overrides.setting_class, key: 'K' })],
...overrides,
});

describe('partitionSettingsGroups', () => {
it('separates core groups from enabled app-owned groups', () => {
const enabled = appGroup({
setting_class: 'AlertsSettings',
app_id: 'alerts',
app_display_name: 'Alerts',
app_enabled: true,
});
const { core, appOwned } = partitionSettingsGroups([...groups, enabled]);
expect(core.map((g) => g.setting_class)).toEqual(['SEPSettings', 'TasksSettings']);
expect(appOwned.map((g) => g.setting_class)).toEqual(['AlertsSettings']);
});

it('drops app-owned groups whose owning app is disabled or unset', () => {
const disabled = appGroup({
setting_class: 'AlertsSettings',
app_id: 'alerts',
app_enabled: false,
});
const unset = appGroup({ setting_class: 'InventorySettings', app_id: 'inventory' });
const { core, appOwned } = partitionSettingsGroups([...groups, disabled, unset]);
expect(core).toHaveLength(2);
expect(appOwned).toHaveLength(0);
});
});

describe('appLabelFor', () => {
it('prefers the display name, falling back to the app id then undefined', () => {
expect(
appLabelFor(appGroup({ setting_class: 'AlertsSettings', app_display_name: 'Alerts' })),
).toBe('Alerts');
expect(appLabelFor(appGroup({ setting_class: 'AlertsSettings', app_id: 'alerts' }))).toBe(
'alerts',
);
expect(appLabelFor(appGroup({ setting_class: 'AlertsSettings' }))).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,47 @@ export const tasksListResponse = {
],
} satisfies { groups: SettingClassGroup[] };

/**
* App-owned groups: one whose owning app is enabled (must render under the
* "App settings" region) and one whose app is disabled (must be hidden).
*/
export const appsListResponse = {
groups: [
{
setting_class: 'AlertsSettings',
is_app_owned: true,
app_id: 'alerts',
app_display_name: 'Alerts',
app_enabled: true,
settings: [
makeSetting({
setting_class: 'AlertsSettings',
key: 'ALERTS_RETENTION_DAYS',
value: 30,
default_value: 30,
type: 'int',
}),
],
},
{
setting_class: 'InventorySettings',
is_app_owned: true,
app_id: 'inventory',
app_display_name: 'Inventory',
app_enabled: false,
settings: [
makeSetting({
setting_class: 'InventorySettings',
key: 'INVENTORY_SCAN_INTERVAL',
value: 60,
default_value: 60,
type: 'int',
}),
],
},
],
} satisfies { groups: SettingClassGroup[] };

/** Wrap children with the providers the settings tree depends on. */
export function makeWrapper() {
const client = new QueryClient({
Expand Down
31 changes: 31 additions & 0 deletions frontend/packages/shell/src/components/settings/filters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,34 @@ export function filterSettingsGroups(
}))
.filter((group) => group.settings.length > 0);
}

export interface PartitionedSettingsGroups {
/** Core SEP settings groups, rendered in the main region. */
core: SettingClassGroup[];
/** App-owned groups whose owning app is enabled, rendered under "App settings". */
appOwned: SettingClassGroup[];
}

/**
* Split the given groups into core and app-owned regions. App-owned
* groups whose owning app is disabled or not enabled are dropped entirely, so the
* page never shows settings for apps the admin is not running. Core groups are
* always kept regardless of any app metadata.
*/
export function partitionSettingsGroups(groups: SettingClassGroup[]): PartitionedSettingsGroups {
const core: SettingClassGroup[] = [];
const appOwned: SettingClassGroup[] = [];
for (const group of groups) {
if (!group.is_app_owned) {
core.push(group);
} else if (group.app_enabled) {
appOwned.push(group);
}
}
return { core, appOwned };
}

/** The display label for an app-owned group's owning app, or `undefined`. */
export function appLabelFor(group: SettingClassGroup): string | undefined {
return group.app_display_name ?? group.app_id ?? undefined;
}
Loading
Loading