Skip to content

Commit c260554

Browse files
authored
Fix InferenceService row click details navigation (#194)
## Problem Clicking an `InferenceService` name in the KServe UI Endpoints table can move the browser URL to the details route without rendering the details page. In the reported flow, the Central Dashboard parent URL and the KServe iframe URL both changed to: ```text /kserve-endpoints/details/<namespace>/<name> ``` However, the iframe still displayed the Endpoints list. The details page rendered only after refreshing the browser on the same details URL or opening the details URL directly. That behavior showed the route, Istio path, Dashboard iframe wrapper, and backend API path were valid. The failing boundary was the row-click navigation path in the KServe UI: the URL changed, but the details component was not activated and the detail SSE request was not opened. ## Root Cause The Endpoints table name column is an internal link. In the embedded Dashboard flow, that link can leave the KServe UI in a stale list view after the URL changes to the details route. When the page is loaded directly on the details route, the details component initializes correctly and opens the detail SSE stream: ```text /kserve-endpoints/api/sse/namespaces/<namespace>/inferenceservices/<name> ``` So the fix makes the embedded Dashboard name-click path use the same reliable page-load behavior as direct details navigation, while preserving normal Angular router navigation outside the Dashboard wrapper. ## Changes - Intercept `name:link` actions for non-terminating `InferenceService` rows only for normal primary-button clicks. - Leave modified clicks and non-primary mouse-button clicks to the browser so open-in-new-tab/window behavior remains intact. - Track the existing `NamespaceService.dashboardConnected$` state instead of applying the page-load workaround to every deployment mode. - In Dashboard-connected mode, stop the table link event, build the details URL through Angular router serialization and `LocationStrategy`, then move the parent Dashboard shell to `/_/kserve-endpoints/details/<namespace>/<name>` while preserving the selected namespace query parameter. - In disconnected standalone mode, preserve Angular `router.navigate()` behavior for row-name navigation. - If the parent Dashboard URL cannot be read, fall back to loading the KServe application details URL directly in the current frame. - Keep the existing terminating-resource guard, including the snackbar message. - Add focused Jest coverage for: - the name column remaining an internal anchor, - Dashboard-wrapper details navigation, - standalone router navigation, - browser-managed modified clicks, - parent-location read failure fallback, - blocked navigation for terminating `InferenceService` rows. ## Validation Automated checks: ```text npm run format:check --prefix frontend npm run lint-check --prefix frontend npm run test:jest --prefix frontend -- --runInBand npm run build --prefix frontend git diff --check ``` Results: - Format check: passed. - Angular lint: all files passed. - Full Jest: 29 suites and 142 tests passed. - Production frontend build: passed. - Whitespace check: passed. Notes: - Jest still emits existing shallow-test Angular unknown-element warnings for mocked shared UI components. - The production build still emits existing CommonJS optimization and bundle-budget warnings. Live reduced Kind verification: - Original image `ghcr.io/kserve/models-web-app:1.0.0`: reproduced the bug. The first row click changed the parent and iframe URLs to the details route, but the iframe stayed on the Endpoints list until refresh. - Fixed image `local/models-web-app:row-click-fix-v5`: verified the embedded Dashboard page-load approach. The first row click rendered `Endpoint details` immediately without refresh, and the detail SSE request returned HTTP 200. - The later review follow-up preserves that embedded Dashboard path and adds automated coverage for standalone navigation and browser-managed click behavior. --------- Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
1 parent fd0cddb commit c260554

2 files changed

Lines changed: 278 additions & 4 deletions

File tree

frontend/src/app/pages/index/index.component.spec.ts

Lines changed: 182 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,31 @@ import {
1212
PollerService,
1313
KubeflowModule,
1414
DashboardState,
15+
STATUS_TYPE,
16+
LinkType,
1517
} from 'kubeflow';
1618
import { CommonModule } from '@angular/common';
1719
import { IndexComponent } from './index.component';
18-
import { Observable, Observer, of, Subject } from 'rxjs';
20+
import { defaultConfig } from './config';
21+
import { BehaviorSubject, Observable, Observer, of, Subject } from 'rxjs';
1922
import { SSEService, WatchEvent } from 'src/app/services/sse.service';
2023
import { InferenceServiceK8s } from 'src/app/types/kfserving/v1beta1';
24+
import { Router } from '@angular/router';
25+
import { LocationStrategy } from '@angular/common';
26+
import { BROWSER_WINDOW } from './index.component';
2127

2228
let MWABackendServiceStub: Partial<MWABackendService>;
2329
let NamespaceServiceStub: Partial<NamespaceService>;
2430
let MWANamespaceServiceStub: Partial<MWANamespaceService>;
2531
let SSEServiceStub: Partial<SSEService>;
2632
let sseEvents: Subject<WatchEvent<InferenceServiceK8s>>;
2733
let sseTeardown: jest.Mock;
34+
let snackBarOpen: jest.Mock;
35+
let locationAssign: jest.Mock;
36+
let parentLocationAssign: jest.Mock;
37+
let prepareExternalUrl: jest.Mock;
38+
let browserWindowMock: any;
39+
let dashboardConnectionState: BehaviorSubject<DashboardState>;
2840

2941
MWABackendServiceStub = {
3042
getInferenceServices: () => of(),
@@ -52,6 +64,7 @@ MWANamespaceServiceStub = {
5264
describe('IndexComponent', () => {
5365
let component: IndexComponent;
5466
let fixture: ComponentFixture<IndexComponent>;
67+
let router: Router;
5568

5669
const inferenceService = (
5770
name: string,
@@ -83,9 +96,45 @@ describe('IndexComponent', () => {
8396
} as any,
8497
});
8598

99+
const nameLinkAction = (phase: STATUS_TYPE, event: any) =>
100+
({
101+
action: 'name:link',
102+
data: {
103+
metadata: {
104+
name: 'model-a',
105+
namespace: 'kubeflow-user',
106+
},
107+
ui: {
108+
status: {
109+
phase,
110+
},
111+
},
112+
},
113+
event,
114+
} as any);
115+
86116
beforeEach(waitForAsync(() => {
117+
dashboardConnectionState = new BehaviorSubject<DashboardState>(
118+
DashboardState.Disconnected,
119+
);
87120
sseEvents = new Subject<WatchEvent<InferenceServiceK8s>>();
88121
sseTeardown = jest.fn();
122+
snackBarOpen = jest.fn();
123+
locationAssign = jest.fn();
124+
parentLocationAssign = jest.fn();
125+
prepareExternalUrl = jest.fn(path => `/kserve-endpoints${path}`);
126+
browserWindowMock = {
127+
location: {
128+
href: 'http://localhost:8081/kserve-endpoints/',
129+
assign: locationAssign,
130+
},
131+
parent: {
132+
location: {
133+
href: 'http://localhost:8081/_/kserve-endpoints/?ns=kubeflow-user',
134+
assign: parentLocationAssign,
135+
},
136+
},
137+
};
89138
SSEServiceStub = {
90139
watchInferenceServices: <T>() =>
91140
new Observable<WatchEvent<T>>(observer => {
@@ -99,6 +148,12 @@ describe('IndexComponent', () => {
99148
}),
100149
};
101150

151+
NamespaceServiceStub = {
152+
getSelectedNamespace: () => of(),
153+
getSelectedNamespace2: () => of(),
154+
dashboardConnected$: dashboardConnectionState.asObservable(),
155+
};
156+
102157
TestBed.configureTestingModule({
103158
declarations: [IndexComponent],
104159
imports: [
@@ -114,16 +169,27 @@ describe('IndexComponent', () => {
114169
{ provide: NamespaceService, useValue: NamespaceServiceStub },
115170
{ provide: MWANamespaceService, useValue: MWANamespaceServiceStub },
116171
{ provide: SSEService, useValue: SSEServiceStub },
117-
{ provide: SnackBarService, useValue: {} },
172+
{ provide: SnackBarService, useValue: { open: snackBarOpen } },
118173
{ provide: Clipboard, useValue: {} },
119174
{ provide: PollerService, useValue: { exponential: () => of() } },
175+
{
176+
provide: LocationStrategy,
177+
useValue: {
178+
prepareExternalUrl,
179+
},
180+
},
181+
{
182+
provide: BROWSER_WINDOW,
183+
useValue: browserWindowMock,
184+
},
120185
],
121186
}).compileComponents();
122187
}));
123188

124189
beforeEach(() => {
125190
fixture = TestBed.createComponent(IndexComponent);
126191
component = fixture.componentInstance;
192+
router = TestBed.inject(Router);
127193
fixture.detectChanges();
128194
});
129195

@@ -198,4 +264,118 @@ describe('IndexComponent', () => {
198264
'model-b',
199265
]);
200266
});
267+
268+
it('should render name links as internal anchors', () => {
269+
const nameColumn = defaultConfig.columns.find(
270+
column => column.matColumnDef === 'name',
271+
);
272+
273+
expect(nameColumn?.value.linkType).toBe(LinkType.Internal);
274+
});
275+
276+
it('should reload the parent dashboard to the details route for name link actions', () => {
277+
dashboardConnectionState.next(DashboardState.Connected);
278+
const event = {
279+
preventDefault: jest.fn(),
280+
stopPropagation: jest.fn(),
281+
};
282+
const navigateSpy = jest.spyOn(router, 'navigate').mockResolvedValue(true);
283+
284+
component.reactToAction(nameLinkAction(STATUS_TYPE.READY, event));
285+
286+
expect(event.preventDefault).toHaveBeenCalled();
287+
expect(event.stopPropagation).toHaveBeenCalled();
288+
expect(prepareExternalUrl).toHaveBeenCalledWith(
289+
'/details/kubeflow-user/model-a',
290+
);
291+
expect(parentLocationAssign).toHaveBeenCalledWith(
292+
'/_/kserve-endpoints/details/kubeflow-user/model-a?ns=kubeflow-user',
293+
);
294+
expect(locationAssign).not.toHaveBeenCalled();
295+
expect(navigateSpy).not.toHaveBeenCalled();
296+
});
297+
298+
it('should use router navigation when disconnected from the dashboard', () => {
299+
browserWindowMock.parent = browserWindowMock;
300+
const event = {
301+
preventDefault: jest.fn(),
302+
stopPropagation: jest.fn(),
303+
};
304+
const navigateSpy = jest.spyOn(router, 'navigate').mockResolvedValue(true);
305+
306+
component.reactToAction(nameLinkAction(STATUS_TYPE.READY, event));
307+
308+
expect(navigateSpy).toHaveBeenCalledWith([
309+
'/details',
310+
'kubeflow-user',
311+
'model-a',
312+
]);
313+
expect(locationAssign).not.toHaveBeenCalled();
314+
expect(parentLocationAssign).not.toHaveBeenCalled();
315+
});
316+
317+
it('should let the browser handle modified name link clicks', () => {
318+
dashboardConnectionState.next(DashboardState.Connected);
319+
const event = {
320+
button: 0,
321+
ctrlKey: true,
322+
preventDefault: jest.fn(),
323+
stopPropagation: jest.fn(),
324+
};
325+
const navigateSpy = jest.spyOn(router, 'navigate').mockResolvedValue(true);
326+
327+
component.reactToAction(nameLinkAction(STATUS_TYPE.READY, event));
328+
329+
expect(event.preventDefault).not.toHaveBeenCalled();
330+
expect(event.stopPropagation).not.toHaveBeenCalled();
331+
expect(navigateSpy).not.toHaveBeenCalled();
332+
expect(locationAssign).not.toHaveBeenCalled();
333+
expect(parentLocationAssign).not.toHaveBeenCalled();
334+
});
335+
336+
it('should reload the frame directly when parent dashboard location cannot be read', () => {
337+
dashboardConnectionState.next(DashboardState.Connected);
338+
const event = {
339+
preventDefault: jest.fn(),
340+
stopPropagation: jest.fn(),
341+
};
342+
Object.defineProperty(browserWindowMock.parent.location, 'href', {
343+
get: () => {
344+
throw new DOMException(
345+
'Blocked parent location access',
346+
'SecurityError',
347+
);
348+
},
349+
});
350+
351+
component.reactToAction(nameLinkAction(STATUS_TYPE.READY, event));
352+
353+
expect(locationAssign).toHaveBeenCalledWith(
354+
'/kserve-endpoints/details/kubeflow-user/model-a',
355+
);
356+
expect(parentLocationAssign).not.toHaveBeenCalled();
357+
});
358+
359+
it('should block navigation for terminating inference service name link actions', () => {
360+
const event = {
361+
preventDefault: jest.fn(),
362+
stopPropagation: jest.fn(),
363+
};
364+
const navigateSpy = jest.spyOn(router, 'navigate').mockResolvedValue(true);
365+
366+
component.reactToAction(nameLinkAction(STATUS_TYPE.TERMINATING, event));
367+
368+
expect(event.preventDefault).toHaveBeenCalled();
369+
expect(event.stopPropagation).toHaveBeenCalled();
370+
expect(navigateSpy).not.toHaveBeenCalled();
371+
expect(locationAssign).not.toHaveBeenCalled();
372+
expect(parentLocationAssign).not.toHaveBeenCalled();
373+
expect(snackBarOpen).toHaveBeenCalledWith(
374+
expect.objectContaining({
375+
data: expect.objectContaining({
376+
msg: 'Endpoint is being deleted, cannot show details.',
377+
}),
378+
}),
379+
);
380+
});
201381
});

frontend/src/app/pages/index/index.component.ts

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import { Component, OnInit, OnDestroy, ChangeDetectorRef } from '@angular/core';
1+
import {
2+
Component,
3+
OnInit,
4+
OnDestroy,
5+
ChangeDetectorRef,
6+
Inject,
7+
InjectionToken,
8+
} from '@angular/core';
9+
import { LocationStrategy } from '@angular/common';
210
import { MWABackendService } from 'src/app/services/backend.service';
311
import { MWANamespaceService } from 'src/app/services/mwa-namespace.service';
412
import { SSEService } from 'src/app/services/sse.service';
@@ -30,6 +38,11 @@ import {
3038
getPredictorExtensionSpec,
3139
} from 'src/app/shared/utils';
3240

41+
export const BROWSER_WINDOW = new InjectionToken<Window>('Browser Window', {
42+
providedIn: 'root',
43+
factory: () => window,
44+
});
45+
3346
@Component({
3447
selector: 'app-index',
3548
templateUrl: './index.component.html',
@@ -47,6 +60,7 @@ export class IndexComponent implements OnInit, OnDestroy {
4760
inferenceServices: InferenceServiceIR[] = [];
4861

4962
dashboardDisconnectedState = DashboardState.Disconnected;
63+
private dashboardState = DashboardState.Disconnected;
5064

5165
private newEndpointButton = new ToolbarButton({
5266
text: $localize`New Endpoint`,
@@ -79,11 +93,14 @@ export class IndexComponent implements OnInit, OnDestroy {
7993
public mwaNamespace: MWANamespaceService,
8094
public poller: PollerService,
8195
private cdr: ChangeDetectorRef,
96+
private locationStrategy: LocationStrategy,
97+
@Inject(BROWSER_WINDOW) private browserWindow: Window,
8298
) {}
8399

84100
ngOnInit(): void {
85101
this.dashboardSubscription = this.ns.dashboardConnected$.subscribe(
86102
dashboardState => {
103+
this.dashboardState = dashboardState;
87104
this.namespaceSubscription.unsubscribe();
88105

89106
if (dashboardState === DashboardState.Disconnected) {
@@ -231,7 +248,7 @@ export class IndexComponent implements OnInit, OnDestroy {
231248
break;
232249
case 'name:link':
233250
/*
234-
* don't allow the user to navigate to the details page of a server
251+
* do not allow the user to navigate to the details page of a server
235252
* that is being deleted
236253
*/
237254
if (inferenceService.ui?.status?.phase === STATUS_TYPE.TERMINATING) {
@@ -246,10 +263,87 @@ export class IndexComponent implements OnInit, OnDestroy {
246263
this.snack.open(snackConfiguration);
247264
return;
248265
}
266+
if (this.isBrowserManagedLinkClick(a.event)) {
267+
return;
268+
}
269+
270+
a.event?.stopPropagation();
271+
a.event?.preventDefault();
272+
this.navigateToDetails(inferenceService);
249273
break;
250274
}
251275
}
252276

277+
private navigateToDetails(inferenceService: InferenceServiceIR) {
278+
const namespace = inferenceService.metadata?.namespace || '';
279+
const name = inferenceService.metadata?.name || '';
280+
const detailsRoute = ['/details', namespace, name];
281+
282+
if (this.dashboardState !== DashboardState.Connected) {
283+
this.router.navigate(detailsRoute);
284+
return;
285+
}
286+
287+
const detailsUrl = this.router.serializeUrl(
288+
this.router.createUrlTree(detailsRoute),
289+
);
290+
const applicationDetailsUrl =
291+
this.locationStrategy.prepareExternalUrl(detailsUrl);
292+
293+
if (
294+
this.navigateParentDashboardToDetails(applicationDetailsUrl, namespace)
295+
) {
296+
return;
297+
}
298+
299+
this.browserWindow.location.assign(applicationDetailsUrl);
300+
}
301+
302+
private navigateParentDashboardToDetails(
303+
applicationDetailsUrl: string,
304+
namespace: string,
305+
): boolean {
306+
const parentWindow = this.browserWindow.parent;
307+
308+
if (!parentWindow || parentWindow === this.browserWindow) {
309+
return false;
310+
}
311+
312+
try {
313+
const parentUrl = new URL(parentWindow.location.href);
314+
if (!parentUrl.pathname.startsWith('/_/')) {
315+
return false;
316+
}
317+
318+
parentUrl.pathname = `/_${applicationDetailsUrl}`;
319+
if (namespace) {
320+
parentUrl.searchParams.set('ns', namespace);
321+
}
322+
323+
parentWindow.location.assign(
324+
`${parentUrl.pathname}${parentUrl.search}${parentUrl.hash}`,
325+
);
326+
return true;
327+
} catch {
328+
return false;
329+
}
330+
}
331+
332+
private isBrowserManagedLinkClick(event?: Event): boolean {
333+
if (!event) {
334+
return false;
335+
}
336+
337+
const mouseEvent = event as MouseEvent;
338+
return (
339+
mouseEvent.ctrlKey ||
340+
mouseEvent.metaKey ||
341+
mouseEvent.shiftKey ||
342+
mouseEvent.altKey ||
343+
(typeof mouseEvent.button === 'number' && mouseEvent.button !== 0)
344+
);
345+
}
346+
253347
private deleteClicked(inferenceService: InferenceServiceIR) {
254348
const dialogConfiguration = generateDeleteConfig(inferenceService);
255349

0 commit comments

Comments
 (0)