Skip to content

Commit

Permalink
frontend: Fix 500 error after detail page refresh (kubeflow#1967)
Browse files Browse the repository at this point in the history
Fix 500 error when refreshing KWA's detail page by also considering the
namespace variable as a query param.

Signed-off-by: Elena Zioga <elena@arrikto.com>
  • Loading branch information
elenzio9 committed Nov 7, 2022
1 parent 54b020b commit 9b49362
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 33 deletions.
5 changes: 4 additions & 1 deletion pkg/new-ui/v1beta1/frontend/src/app/app-routing.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import { TrialModalComponent } from './pages/experiment-details/trials-table/tri

const routes: Routes = [
{ path: '', component: ExperimentsComponent },
{ path: 'experiment/:experimentName', component: ExperimentDetailsComponent },
{
path: 'experiment/:namespace/:experimentName',
component: ExperimentDetailsComponent,
},
{ path: 'new', component: ExperimentCreationComponent },
{
path: 'experiment/:experimentName/trial/:trialName',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,34 +26,32 @@ import { MatSnackBarModule } from '@angular/material/snack-bar';

let KWABackendServiceStub: Partial<KWABackendService>;
let NamespaceServiceStub: Partial<NamespaceService>;
let ActivatedRouteStub: Partial<ActivatedRoute>;

KWABackendServiceStub = {
getExperimentTrialsInfo: () => of([]),
getExperimentTrialsInfo: () =>
of(
'Status,trialName,Validation-accuracy,Train-accuracy,lr,num-layers,optimizer\nSucceeded,tpe-05daf02d,0.977807,0.993104,0.023222418198803642,3,sgd',
),
getExperiment: () => of(),
deleteExperiment: () => of(),
};

NamespaceServiceStub = {
getSelectedNamespace: () => of(),
updateSelectedNamespace: () => {},
};

ActivatedRouteStub = {
params: of({ namespace: 'kubeflow-user', experimentName: '' }),
queryParams: of({}),
};

describe('ExperimentDetailsComponent', () => {
let component: ExperimentDetailsComponent;
let fixture: ComponentFixture<ExperimentDetailsComponent>;
let activatedRouteSpy;

beforeEach(
waitForAsync(() => {
activatedRouteSpy = {
snapshot: {
params: {
experimentName: '',
},
queryParams: {
tab: '',
},
},
};
TestBed.configureTestingModule({
imports: [
CommonModule,
Expand All @@ -75,7 +73,7 @@ describe('ExperimentDetailsComponent', () => {
],
declarations: [ExperimentDetailsComponent],
providers: [
{ provide: ActivatedRoute, useValue: activatedRouteSpy },
{ provide: ActivatedRoute, useValue: ActivatedRouteStub },
{ provide: Router, useValue: {} },
{ provide: KWABackendService, useValue: KWABackendServiceStub },
{ provide: ConfirmDialogService, useValue: {} },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export class ExperimentDetailsComponent implements OnInit, OnDestroy {
private router: Router,
private backendService: KWABackendService,
private confirmDialog: ConfirmDialogService,
private backend: KWABackendService,
private namespaceService: NamespaceService,
) {}

Expand All @@ -68,20 +67,18 @@ export class ExperimentDetailsComponent implements OnInit, OnDestroy {
private subs = new Subscription();

ngOnInit() {
this.name = this.activatedRoute.snapshot.params.experimentName;
this.activatedRoute.params.subscribe(params => {
this.namespaceService.updateSelectedNamespace(params.namespace);

if (this.activatedRoute.snapshot.queryParams['tab']) {
this.selectedTab = this.tabs.get(
this.activatedRoute.snapshot.queryParams['tab'],
);
}
this.name = params.experimentName;
this.namespace = params.namespace;

this.subs.add(
this.namespaceService.getSelectedNamespace().subscribe(namespace => {
this.namespace = namespace;
this.updateExperimentInfo();
}),
);
this.updateExperimentInfo();
});

this.activatedRoute.queryParams.subscribe(queryParams => {
this.selectedTab = this.tabs.get(queryParams.tab);
});
}

tabChanged(event: MatTabChangeEvent) {
Expand Down Expand Up @@ -146,7 +143,7 @@ export class ExperimentDetailsComponent implements OnInit, OnDestroy {
}

// Close the open dialog only if the DELETE request succeeded
this.backend.deleteExperiment(name, namespace).subscribe({
this.backendService.deleteExperiment(name, namespace).subscribe({
next: _ => {
ref.close(DIALOG_RESP.ACCEPT);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,11 @@ export class TrialModalComponent implements OnInit {
}

returnToExperimentDetails() {
this.router.navigate([`/experiment/${this.experimentName}`], {
queryParams: { tab: 'trials' },
});
this.router.navigate(
[`/experiment/${this.namespace}/${this.experimentName}`],
{
queryParams: { tab: 'trials' },
},
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export class ExperimentsComponent implements OnInit, OnDestroy {
const exp = a.data as Experiment;
switch (a.action) {
case 'name:link':
this.router.navigate([`/experiment/${exp.name}`]);
this.router.navigate([`/experiment/${exp.namespace}/${exp.name}`]);
break;
case 'delete':
this.onDeleteExperiment(exp.name);
Expand Down

0 comments on commit 9b49362

Please sign in to comment.