fix(frontend): hide Edit and Delete on system-managed groups

frontend/src/__tests__/groups-system-managed-919.test.ts

@@ -0,0 +1,107 @@
+/**
+ * Issue #919: system-managed groups must not offer Edit or Delete (the API
+ * refuses both with 403), and a refused Delete must show the server's
+ * reason rather than a generic "Failed to delete group".
+ */
+import './setup';
+
+jest.mock('../api', () => ({
+  deleteGroup: jest.fn(),
+  getGroup: jest.fn(),
+}));
+
+jest.mock('../users/userActions', () => ({
+  loadUsers: jest.fn().mockResolvedValue(undefined),
+}));
+
+const mockConfirmDialog = jest.fn<Promise<boolean>, [unknown]>(() => Promise.resolve(true));
+jest.mock('../confirmDialog', () => ({
+  confirmDialog: (opts: unknown) => mockConfirmDialog(opts),
+}));
+
+import * as api from '../api';
+import type { APIGroup } from '../api';
+import * as userState from '../users/state';
+import { renderGroups } from '../groups/groupList';
+import { deleteGroup } from '../groups/groupActions';
+import { SYSTEM_GROUP_NOTE } from '../groups/systemGroups';
+
+const SYSTEM: APIGroup = {
+  id: '00000000-0000-5000-8000-000000000001', name: 'Administrators', description: '',
+  permissions: [{ action: 'admin', resource: '*' }], system_managed: true,
+};
+const CUSTOM: APIGroup = {
+  id: 'g-custom', name: 'Finance', description: '',
+  permissions: [{ action: 'view', resource: 'plans' }],
+};
+
+function card(name: string): HTMLElement {
+  return Array.from(document.querySelectorAll<HTMLElement>('.group-card'))
+    .find(c => c.querySelector('h4')?.textContent === name)!;
+}
+
+beforeEach(() => {
+  document.body.innerHTML = '<div id="groups-list"></div>';
+  userState.setAllUsers([]);
+  userState.setAvailableGroups([SYSTEM, CUSTOM]);
+  jest.clearAllMocks();
+  mockConfirmDialog.mockResolvedValue(true);
+  jest.spyOn(console, 'error').mockImplementation(() => undefined);
+});
+
+afterEach(() => {
+  jest.restoreAllMocks();
+});
+
+describe('group cards (#919)', () => {
+  it('hides Edit and Delete on a system-managed group and says why', () => {
+    renderGroups([SYSTEM, CUSTOM]);
+    const sys = card('Administrators');
+    expect(sys.querySelector('.edit-group-btn')).toBeNull();
+    expect(sys.querySelector('.delete-group-btn')).toBeNull();
+    // Duplicate still works: it makes an editable copy.
+    expect(sys.querySelector('.duplicate-group-btn')).not.toBeNull();
+    const badge = sys.querySelector('.system-group-badge') as HTMLElement;
+    expect(badge.getAttribute('title')).toBe(SYSTEM_GROUP_NOTE);
+    expect(badge.querySelector('.sr-only')?.textContent).toContain(SYSTEM_GROUP_NOTE);
+  });
+
+  it('keeps Edit and Delete on custom groups', () => {
+    renderGroups([SYSTEM, CUSTOM]);
+    const custom = card('Finance');
+    expect(custom.querySelector('.edit-group-btn')).not.toBeNull();
+    expect(custom.querySelector('.delete-group-btn')).not.toBeNull();
+    expect(custom.querySelector('.system-group-badge')).toBeNull();
+  });
+
+  it('treats a missing or false system_managed as a custom group', () => {
+    renderGroups([{ ...CUSTOM, system_managed: false }]);
+    expect(document.querySelector('.edit-group-btn')).not.toBeNull();
+    expect(document.querySelector('.delete-group-btn')).not.toBeNull();
+  });
+});
+
+describe('deleteGroup (#919)', () => {
+  it('refuses a system-managed group without asking or calling the API', async () => {
+    await deleteGroup(SYSTEM.id);
+    expect(mockConfirmDialog).not.toHaveBeenCalled();
+    expect(api.deleteGroup).not.toHaveBeenCalled();
+    expect(document.querySelector('.toast-error')?.textContent).toContain(SYSTEM_GROUP_NOTE);
+  });
+
+  it('shows the server reason when the API refuses a delete', async () => {
+    (api.deleteGroup as jest.Mock).mockRejectedValue(
+      new Error('system-managed group cannot be modified: "Finance" is seeded and maintained by migrations'),
+    );
+    await deleteGroup(CUSTOM.id);
+    expect(document.querySelector('.toast-error')?.textContent).toContain(
+      'Failed to delete group: system-managed group cannot be modified: "Finance" is seeded and maintained by migrations',
+    );
+  });
+
+  it('falls back to the generic message when the error has no text', async () => {
+    (api.deleteGroup as jest.Mock).mockRejectedValue(new Error(''));
+    await deleteGroup(CUSTOM.id);
+    expect(document.querySelector('.toast-error')?.textContent).toContain('Failed to delete group');
+  });
+});

frontend/src/groups/groupActions.ts

@@ -7,13 +7,20 @@ import { availableGroups } from '../users/state';
 import { showError, showSuccess } from '../users/utils';
 import { loadUsers } from '../users/userActions';
 import { confirmDialog } from '../confirmDialog';
+import { isSystemManagedGroup, SYSTEM_GROUP_NOTE } from './systemGroups';
 
 /**
  * Delete group
  */
 export async function deleteGroup(groupId: string): Promise<void> {
   const group = availableGroups.find(g => g.id === groupId);
   if (!group) return;
+  // The card hides Delete for these (#919); refuse here too rather than
+  // ask for confirmation of a delete the API will reject.
+  if (isSystemManagedGroup(group)) {
+    showError(SYSTEM_GROUP_NOTE);
+    return;
+  }
 
   const ok = await confirmDialog({
     title: `Delete group "${group.name}"?`,
@@ -29,7 +36,10 @@ export async function deleteGroup(groupId: string): Promise<void> {
     showSuccess('Group deleted successfully');
   } catch (error) {
     console.error('Failed to delete group:', error);
-    showError('Failed to delete group');
+    // Surface the server's reason (e.g. the system-managed refusal), as the
+    // Edit path already does (#919 review comment).
+    const err = error as Error;
+    showError(err?.message ? `Failed to delete group: ${err.message}` : 'Failed to delete group');
   }
 }
 

frontend/src/groups/groupList.ts

@@ -7,6 +7,7 @@ import { allUsers } from '../users/state';
 import { escapeHtml } from '../users/utils';
 import { openEditGroupModal, deleteGroup } from './groupActions';
 import { openDuplicateGroupModal } from './groupModals';
+import { isSystemManagedGroup, SYSTEM_GROUP_NOTE } from './systemGroups';
 
 /**
  * Format permission constraints as a readable string for tooltips
@@ -49,6 +50,7 @@ export function renderGroups(groups: APIGroup[]): void {
       ? members.map(m => `<span class="member-pill">${escapeHtml(m.email)}</span>`).join('')
       : '<span class="text-muted">No members</span>';
 
+    const locked = isSystemManagedGroup(group);
     const description = group.description
       ? `<p class="group-description">${escapeHtml(group.description)}</p>`
       : '';
@@ -62,9 +64,11 @@ export function renderGroups(groups: APIGroup[]): void {
           </div>
           <div class="group-card-actions">
             <span class="badge">${memberCount} member${memberCount !== 1 ? 's' : ''}</span>
-            <button class="btn-small edit-group-btn" data-group-id="${escapeHtml(group.id)}">Edit</button>
+            ${locked
+              ? `<span class="badge badge-muted system-group-badge" title="${escapeHtml(SYSTEM_GROUP_NOTE)}">System group<span class="sr-only">. ${escapeHtml(SYSTEM_GROUP_NOTE)}</span></span>`
+              : `<button class="btn-small edit-group-btn" data-group-id="${escapeHtml(group.id)}">Edit</button>`}
             <button class="btn-small duplicate-group-btn" data-group-id="${escapeHtml(group.id)}" title="Duplicate">Duplicate</button>
-            <button class="btn-small btn-danger delete-group-btn" data-group-id="${escapeHtml(group.id)}">Delete</button>
+            ${locked ? '' : `<button class="btn-small btn-danger delete-group-btn" data-group-id="${escapeHtml(group.id)}">Delete</button>`}
           </div>
         </div>
         <div class="group-card-body">

frontend/src/groups/systemGroups.ts

@@ -0,0 +1,16 @@
+/**
+ * System-managed groups (issue #919).
+ */
+
+import type { APIGroup } from '../api';
+
+// Issue #919: the API refuses edits and deletes of system-managed groups
+// (seeded by migrations: Administrators, Purchaser, Standard Users, ...)
+// with ErrSystemManagedGroup / 403, so offering Edit and Delete there only
+// leads to an error. Duplicate stays: it creates a separate, editable copy.
+export const SYSTEM_GROUP_NOTE =
+  'Built-in group maintained by CUDly. It cannot be edited or deleted; change its members from the Users list.';
+
+export function isSystemManagedGroup(group: Pick<APIGroup, 'system_managed'> | undefined): boolean {
+  return group?.system_managed === true;
+}

frontend/tests-e2e/system-managed-groups.spec.ts

@@ -0,0 +1,27 @@
+import { test, expect } from '@playwright/test';
+import { mockApi, seedAuth } from './fixtures/recs';
+
+// Issue #919: system-managed groups are refused by the API on edit and
+// delete, so their cards must not offer those actions.
+test('system-managed group cards hide Edit and Delete', async ({ page }) => {
+  await page.addInitScript(() => localStorage.setItem('cudly:purchaser-prompt-dismissed', '1'));
+  await seedAuth(page);
+  await mockApi(page);
+  await page.route('**/api/users', route => route.fulfill({ json: { users: [] } }));
+  await page.route('**/api/groups', route => route.fulfill({ json: { groups: [
+    { id: '00000000-0000-5000-8000-000000000001', name: 'Administrators', description: '',
+      permissions: [{ action: 'admin', resource: '*' }], allowed_accounts: [], system_managed: true },
+    { id: 'g-custom', name: 'Finance', description: '',
+      permissions: [{ action: 'view', resource: 'plans' }], allowed_accounts: [] },
+  ] } }));
+
+  await page.goto('/admin/users');
+  const system = page.locator('.group-card', { has: page.locator('h4', { hasText: 'Administrators' }) });
+  const custom = page.locator('.group-card', { has: page.locator('h4', { hasText: 'Finance' }) });
+  await expect(system.locator('.system-group-badge')).toBeVisible();
+  await expect(system.locator('.edit-group-btn')).toHaveCount(0);
+  await expect(system.locator('.delete-group-btn')).toHaveCount(0);
+  await expect(system.locator('.duplicate-group-btn')).toBeVisible();
+  await expect(custom.locator('.edit-group-btn')).toBeVisible();
+  await expect(custom.locator('.delete-group-btn')).toBeVisible();
+});