fix(frontend): address review findings on the public access panel

- Preload columns for every already-public table, not only the ones carrying a
  readableColumns whitelist. An unrestricted table still renders the column
  picker, so it previously showed an empty select with no options until the user
  unchecked and rechecked the table.
- Clear tablesLoading when fetchTables fails. TablesService.fetchTables does not
  swallow errors, so a failure hit an absent error handler and pinned the panel
  body on the content loader indefinitely.
- Disable the table checkboxes and column selects while a save is in flight.
  Editing during that window was the only case where the post-reload re-seed
  could discard an unsaved edit; a one-shot seeding guard was avoided because it
  would latch onto the pre-load empty state and would also suppress legitimate
  re-sync when public access changes elsewhere.
- Type the mocked publicPermissions signals in users.component.spec.ts instead of
  casting them to any.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: gugu

frontend/src/app/components/users/public-access-panel/public-access-panel.component.html

@@ -30,6 +30,7 @@
                     <li class="public-access-table" [class.public-access-table_selected]="selected">
                         <mat-checkbox
                             [checked]="selected"
+                            [disabled]="submitting()"
                             (change)="toggleTable(table.tableName, $event.checked)">
                             {{ table.displayName }}
                         </mat-checkbox>
@@ -42,6 +43,7 @@
                                     <mat-label>Readable columns</mat-label>
                                     <mat-select multiple
                                         [value]="selectedColumns(table.tableName)"
+                                        [disabled]="submitting()"
                                         (valueChange)="setColumns(table.tableName, $event)"
                                         placeholder="All columns">
                                         @for (column of availableColumns(table.tableName); track column) {

frontend/src/app/components/users/public-access-panel/public-access-panel.component.spec.ts

@@ -1,7 +1,7 @@
 import { Signal, signal, WritableSignal } from '@angular/core';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
-import { of } from 'rxjs';
+import { of, throwError } from 'rxjs';
 import { PublicPermissions } from 'src/app/models/user';
 import { ConnectionsService } from 'src/app/services/connections.service';
 import { TablesService } from 'src/app/services/tables.service';
@@ -13,6 +13,7 @@ type PublicAccessPanelTestable = PublicAccessPanelComponent & {
 	selectedCount: Signal<number>;
 	canDisable: Signal<boolean>;
 	tablesLoading: WritableSignal<boolean>;
+	submitting: WritableSignal<boolean>;
 	tables: Signal<Array<{ tableName: string; displayName: string }>>;
 };
 
@@ -107,9 +108,19 @@ describe('PublicAccessPanelComponent', () => {
 		expect(component.selectedColumns('orders')).toEqual([]);
 		expect(testable.statusLabel()).toBe('2 tables');
 		expect(testable.canDisable()).toBe(true);
-		// Columns are only fetched for tables that already carry a whitelist.
+	});
+
+	it('should preload columns for every stored table, restricted or not', () => {
+		publicPermissions.set({
+			enabled: true,
+			tables: [{ tableName: 'customers', readableColumns: ['id'] }, { tableName: 'orders' }],
+		});
+		fixture.detectChanges();
+
+		// An unrestricted table still renders the column picker, so it needs its options too.
 		expect(mockTablesService.fetchTableStructure).toHaveBeenCalledWith(CONNECTION_ID, 'customers');
-		expect(mockTablesService.fetchTableStructure).not.toHaveBeenCalledWith(CONNECTION_ID, 'orders');
+		expect(mockTablesService.fetchTableStructure).toHaveBeenCalledWith(CONNECTION_ID, 'orders');
+		expect(component.availableColumns('orders')).toEqual(['id', 'name', 'secret']);
 	});
 
 	it('should singularize the status label for one table', () => {
@@ -165,6 +176,38 @@ describe('PublicAccessPanelComponent', () => {
 		]);
 	});
 
+	it('should clear the loading state when the table list fails to load', () => {
+		mockTablesService.fetchTables = vi.fn().mockReturnValue(throwError(() => new Error('boom')));
+
+		const failing = TestBed.createComponent(PublicAccessPanelComponent);
+		failing.detectChanges();
+
+		expect((failing.componentInstance as PublicAccessPanelTestable).tablesLoading()).toBe(false);
+	});
+
+	it('should block editing while a save is in flight', async () => {
+		let resolveSave: () => void = () => {};
+		mockUsersService.savePublicPermissions = vi
+			.fn()
+			.mockReturnValue(new Promise<void>((resolve) => (resolveSave = resolve)));
+
+		component.toggleTable('customers', true);
+		const saving = component.save();
+		fixture.detectChanges();
+
+		// The re-seed after reload can only discard an edit made in this window, so it is closed off.
+		expect((component as PublicAccessPanelTestable).submitting()).toBe(true);
+		const host: HTMLElement = fixture.nativeElement;
+		fixture.nativeElement.querySelector('mat-expansion-panel-header').click();
+		fixture.detectChanges();
+		await fixture.whenStable();
+		expect(host.querySelector('mat-checkbox input')?.hasAttribute('disabled')).toBe(true);
+
+		resolveSave();
+		await saving;
+		expect((component as PublicAccessPanelTestable).submitting()).toBe(false);
+	});
+
 	it('should render the table list and column picker once expanded', async () => {
 		publicPermissions.set({ enabled: true, tables: [{ tableName: 'customers', readableColumns: ['id'] }] });
 		fixture.detectChanges();

frontend/src/app/components/users/public-access-panel/public-access-panel.component.ts

@@ -72,7 +72,10 @@ export class PublicAccessPanelComponent implements OnInit {
 	protected canDisable = computed(() => this.publicPermissions().enabled);
 
 	constructor() {
-		// Seed the local editing state whenever the server state (re)loads.
+		// Seed the local editing state whenever the server state (re)loads, so an external change
+		// to public access is picked up. Editing is blocked while a save is in flight (see the
+		// submitting() bindings in the template), which is the only window where this re-seed
+		// could otherwise discard an unsaved edit.
 		effect(() => {
 			const stored = this.publicPermissions().tables;
 			const seeded: TableSelection = new Map();
@@ -81,13 +84,13 @@ export class PublicAccessPanelComponent implements OnInit {
 			}
 			this.selection.set(seeded);
 
+			// Every already-public table needs its columns, not just the restricted ones: an
+			// unrestricted table still renders the column picker, which would otherwise be empty.
 			// Kept untracked: _loadColumns both reads and writes the column signals, so tracking it
 			// would make a finished column fetch re-run this effect and clobber edits made meanwhile.
 			untracked(() => {
 				for (const table of stored) {
-					if (table.readableColumns?.length) {
-						this._loadColumns(table.tableName);
-					}
+					this._loadColumns(table.tableName);
 				}
 			});
 		});
@@ -97,17 +100,22 @@ export class PublicAccessPanelComponent implements OnInit {
 		this.connectionID = this._connections.currentConnectionID;
 		this._usersService.loadPublicPermissions(this.connectionID);
 
+		// fetchTables does not swallow errors, so clear the loading state on failure too —
+		// otherwise the panel body is stuck on the content loader forever.
 		this._tablesService
 			.fetchTables(this.connectionID)
 			.pipe(takeUntilDestroyed(this._destroyRef))
-			.subscribe((tables) => {
-				this.tables.set(
-					tables.map((t) => ({
-						tableName: t.table,
-						displayName: t.display_name || normalizeTableName(t.table),
-					})),
-				);
-				this.tablesLoading.set(false);
+			.subscribe({
+				next: (tables) => {
+					this.tables.set(
+						tables.map((t) => ({
+							tableName: t.table,
+							displayName: t.display_name || normalizeTableName(t.table),
+						})),
+					);
+					this.tablesLoading.set(false);
+				},
+				error: () => this.tablesLoading.set(false),
 			});
 	}
 

frontend/src/app/components/users/users.component.spec.ts

@@ -6,6 +6,7 @@ import { MatSnackBarModule } from '@angular/material/snack-bar';
 import { provideRouter } from '@angular/router';
 import { Angulartics2Module } from 'angulartics2';
 import { of } from 'rxjs';
+import { PublicPermissions } from 'src/app/models/user';
 import { CedarPermissionService } from 'src/app/services/cedar-permission.service';
 import { TablesService } from 'src/app/services/tables.service';
 import { UsersService } from 'src/app/services/users.service';
@@ -56,8 +57,8 @@ describe('UsersComponent', () => {
 		fetchGroupUsers: vi.fn().mockResolvedValue([]),
 		fetchAllGroupUsers: vi.fn().mockResolvedValue(undefined),
 		fetchConnectionUsers: vi.fn(),
-		publicPermissions: signal({ enabled: false, tables: [] }).asReadonly() as any,
-		publicPermissionsLoading: signal(false).asReadonly() as any,
+		publicPermissions: signal<PublicPermissions>({ enabled: false, tables: [] }).asReadonly(),
+		publicPermissionsLoading: signal(false).asReadonly(),
 		loadPublicPermissions: vi.fn(),
 		savePublicPermissions: vi.fn().mockResolvedValue(undefined),
 	};