From 4f5b94cb0c4d24829f500002103fba195b6d08d9 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Fri, 2 Jan 2026 11:10:08 -0500 Subject: [PATCH 01/17] NAS-139093: fix: mutating argument causing error on rsync tasks card --- .../rsync-task/rsync-task-card/rsync-task-card.component.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.ts b/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.ts index b78049944ca..3edb5efb354 100644 --- a/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.ts +++ b/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.ts @@ -229,7 +229,8 @@ export class RsyncTaskCardComponent implements OnInit { } private transformRsyncTasks(rsyncTasks: RsyncTaskUi[]): RsyncTaskUi[] { - return rsyncTasks.map((task: RsyncTaskUi) => { + return rsyncTasks.map((rsyncTask: RsyncTaskUi) => { + const task: RsyncTaskUi = { ...rsyncTask }; if (task.job === null) { task.state = { state: task.locked ? TaskState.Locked : TaskState.Pending }; } else { From ec237dd59ab048a3218e11fe4a0572e3c01ceb10 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Fri, 2 Jan 2026 15:01:44 -0500 Subject: [PATCH 02/17] NAS-139093: feat: add auto-expand to jobs page and improve details on cell state button * add a URL parameter and handling to the jobs list page. tihs allows us to navigate to the jobs page and have it immediately expand one of the rows if we want to highlight something. * add logic to the job state button we use in `ix-tables` to show a `Download Logs` button and a `View Details` button when those details are available. --- src/app/interfaces/error-report.interface.ts | 2 + .../error-dialog/error-dialog.component.ts | 2 +- .../ix-cell-state-button.component.ts | 43 ++++++++++++++++++- .../pages/jobs/jobs-list.component.spec.ts | 41 ++++++++++++++++++ src/app/pages/jobs/jobs-list.component.ts | 16 +++++++ 5 files changed, 102 insertions(+), 2 deletions(-) diff --git a/src/app/interfaces/error-report.interface.ts b/src/app/interfaces/error-report.interface.ts index 5785032a336..a00302eca98 100644 --- a/src/app/interfaces/error-report.interface.ts +++ b/src/app/interfaces/error-report.interface.ts @@ -1,3 +1,4 @@ +import { Params } from '@angular/router'; import { Job } from 'app/interfaces/job.interface'; export const traceDetailLabel = 'Trace'; @@ -5,6 +6,7 @@ export const traceDetailLabel = 'Trace'; export interface ErrorReportAction { label: string; route?: string; + params?: Params; action?: () => void; } diff --git a/src/app/modules/dialog/components/error-dialog/error-dialog.component.ts b/src/app/modules/dialog/components/error-dialog/error-dialog.component.ts index 468865d4f24..c0231c17d85 100644 --- a/src/app/modules/dialog/components/error-dialog/error-dialog.component.ts +++ b/src/app/modules/dialog/components/error-dialog/error-dialog.component.ts @@ -80,7 +80,7 @@ export class ErrorDialog { protected handleAction(action: ErrorReportAction): void { if (action.route) { - this.router.navigate([action.route]); + this.router.navigate([action.route], { queryParams: action.params }); this.dialogRef.close(); } else if (action.action) { action.action(); diff --git a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts index a7b078a55c2..839b244725a 100644 --- a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts +++ b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts @@ -5,6 +5,7 @@ import { import { MatButton } from '@angular/material/button'; import { MatDialog } from '@angular/material/dialog'; import { MatTooltip } from '@angular/material/tooltip'; +import { Router } from '@angular/router'; import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; import { Store } from '@ngrx/store'; import { TranslateService, TranslateModule } from '@ngx-translate/core'; @@ -56,6 +57,7 @@ export class IxCellStateButtonComponent extends ColumnComponent implements translate: TranslateService = inject(TranslateService); dialogService: DialogService = inject(DialogService); errorHandler: ErrorHandlerService = inject(ErrorHandlerService); + router: Router = inject(Router); private readonly rowUpdateEffect = effect(() => { this.setupRow(); @@ -142,7 +144,46 @@ export class IxCellStateButtonComponent extends ColumnComponent implements } if (state.error) { - this.dialogService.error({ title: state.state, message: `
${state.error}
` }); + // by default, we just show the error message + const message = `
${state.error}
`; + + // if there's a log excerpt though, we include the additional + // 'View Details' and 'Download Logs' buttons. + let logs; + let actions; + if (this.job()?.logs_excerpt) { + logs = this.job()?.logs_excerpt ? this.job() : null; + + // only show the 'View Details' button when we're *not* on the jobs page + // since it would imply to the user there's somewhere else they could go + // and that clicking it would take them there. + const showDetailsButton = !this.router.isActive( + '/jobs', + { + matrixParams: 'ignored', + queryParams: 'ignored', + paths: 'exact', + fragment: 'ignored', + }, + ); + + // the 'View Logs' button will take the user to the jobs page and expand the failed job's row + if (showDetailsButton) { + actions = [{ + label: this.translate.instant('View Details'), + route: '/jobs', + params: { jobId: this.job().id }, + }]; + } + } + + this.dialogService.error({ + title: state.state, + message, + // since these two may be undefined and are optional, we can just include them in the call outright + logs, + actions, + }); return; } diff --git a/src/app/pages/jobs/jobs-list.component.spec.ts b/src/app/pages/jobs/jobs-list.component.spec.ts index 2592c98e42c..237469f964b 100644 --- a/src/app/pages/jobs/jobs-list.component.spec.ts +++ b/src/app/pages/jobs/jobs-list.component.spec.ts @@ -2,6 +2,7 @@ import { HarnessLoader } from '@angular/cdk/testing'; import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed'; import { MatButtonHarness } from '@angular/material/button/testing'; import { MatSnackBar } from '@angular/material/snack-bar'; +import { ActivatedRoute } from '@angular/router'; import { createComponentFactory, mockProvider, Spectator } from '@ngneat/spectator/jest'; import { MockStore, provideMockStore } from '@ngrx/store/testing'; import { MockComponent } from 'ng-mocks'; @@ -64,6 +65,9 @@ describe('JobsListComponent', () => { }), mockProvider(DialogService), mockProvider(MatSnackBar), + mockProvider(ActivatedRoute, { + queryParams: of({}), + }), mockApi([ mockCall('core.job_download_logs', 'http://localhost/download/log'), ]), @@ -126,4 +130,41 @@ describe('JobsListComponent', () => { expect(spectator.queryAll('.expanded')).toHaveLength(1); }); + + it('should auto-expand row when jobId query parameter is provided', () => { + const mockActivatedRoute = spectator.inject(ActivatedRoute); + mockActivatedRoute.queryParams = of({ jobId: '446' }); + + store$.overrideSelector(selectJobs, fakeJobDataSource); + store$.refreshState(); + spectator.component.ngOnInit(); + spectator.detectChanges(); + + expect(spectator.queryAll('.expanded')).toHaveLength(1); + expect(spectator.query('.expanded')).toContainText('cloudsync.sync'); + }); + + it('should not expand any row when jobId query parameter does not match any job', () => { + const mockActivatedRoute = spectator.inject(ActivatedRoute); + mockActivatedRoute.queryParams = of({ jobId: '999' }); + + store$.overrideSelector(selectJobs, fakeJobDataSource); + store$.refreshState(); + spectator.component.ngOnInit(); + spectator.detectChanges(); + + expect(spectator.queryAll('.expanded')).toHaveLength(0); + }); + + it('should not expand any row when no jobId query parameter is provided', () => { + const mockActivatedRoute = spectator.inject(ActivatedRoute); + mockActivatedRoute.queryParams = of({}); + + store$.overrideSelector(selectJobs, fakeJobDataSource); + store$.refreshState(); + spectator.component.ngOnInit(); + spectator.detectChanges(); + + expect(spectator.queryAll('.expanded')).toHaveLength(0); + }); }); diff --git a/src/app/pages/jobs/jobs-list.component.ts b/src/app/pages/jobs/jobs-list.component.ts index 12da5263063..a230208e728 100644 --- a/src/app/pages/jobs/jobs-list.component.ts +++ b/src/app/pages/jobs/jobs-list.component.ts @@ -1,6 +1,7 @@ import { AsyncPipe } from '@angular/common'; import { ChangeDetectionStrategy, ChangeDetectorRef, Component, OnInit, inject, signal } from '@angular/core'; import { MatButtonToggleGroup, MatButtonToggle } from '@angular/material/button-toggle'; +import { ActivatedRoute } from '@angular/router'; import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; import { Store } from '@ngrx/store'; import { TranslateService, TranslateModule } from '@ngx-translate/core'; @@ -69,6 +70,7 @@ export class JobsListComponent implements OnInit { private translate = inject(TranslateService); private store$ = inject>(Store); private cdr = inject(ChangeDetectorRef); + private route = inject(ActivatedRoute); protected readonly searchableElements = jobsListElements; @@ -133,6 +135,13 @@ export class JobsListComponent implements OnInit { this.setDefaultSort(); this.cdr.markForCheck(); }); + + this.route.queryParams.pipe(untilDestroyed(this)).subscribe((queryParams) => { + if (queryParams.jobId) { + this.autoExpandRow(Number(queryParams.jobId)); + this.cdr.markForCheck(); + } + }); } protected onTabChange(tab: JobTab): void { @@ -156,6 +165,13 @@ export class JobsListComponent implements OnInit { this.dataProvider.setFilter({ list: this.jobs, query, columnKeys: ['method', 'description'] }); } + private autoExpandRow(jobId: number): void { + const jobToExpand = this.jobs.find((job) => job.id === jobId); + if (jobToExpand) { + this.dataProvider.expandedRow = jobToExpand; + } + } + private setDefaultSort(): void { this.dataProvider.setSorting({ active: 1, From be603b30609ef368c181da1ec0fbd22bb5ad7c0f Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Fri, 2 Jan 2026 15:12:50 -0500 Subject: [PATCH 03/17] NAS-139093: fix: race condition and old `untilDestroyed` pattern in jobs list component --- src/app/pages/jobs/jobs-list.component.ts | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/app/pages/jobs/jobs-list.component.ts b/src/app/pages/jobs/jobs-list.component.ts index a230208e728..b63fd7034a7 100644 --- a/src/app/pages/jobs/jobs-list.component.ts +++ b/src/app/pages/jobs/jobs-list.component.ts @@ -1,8 +1,8 @@ import { AsyncPipe } from '@angular/common'; -import { ChangeDetectionStrategy, ChangeDetectorRef, Component, OnInit, inject, signal } from '@angular/core'; +import { DestroyRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, OnInit, inject, signal } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { MatButtonToggleGroup, MatButtonToggle } from '@angular/material/button-toggle'; import { ActivatedRoute } from '@angular/router'; -import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; import { Store } from '@ngrx/store'; import { TranslateService, TranslateModule } from '@ngx-translate/core'; import { @@ -40,7 +40,6 @@ import { JobNameComponent } from 'app/pages/jobs/job-name/job-name.component'; import { JobTab } from 'app/pages/jobs/job-tab.enum'; import { jobsListElements } from 'app/pages/jobs/jobs-list.elements'; -@UntilDestroy() @Component({ selector: 'ix-jobs-list', templateUrl: './jobs-list.component.html', @@ -71,6 +70,7 @@ export class JobsListComponent implements OnInit { private store$ = inject>(Store); private cdr = inject(ChangeDetectorRef); private route = inject(ActivatedRoute); + private readonly destroyRef = inject(DestroyRef); protected readonly searchableElements = jobsListElements; @@ -129,18 +129,22 @@ export class JobsListComponent implements OnInit { ); ngOnInit(): void { - this.selectedJobs$.pipe(untilDestroyed(this)).subscribe((jobs) => { + combineLatest([ + this.selectedJobs$, + this.route.queryParams, + ]).pipe(takeUntilDestroyed(this.destroyRef)).subscribe(([jobs, queryParams]) => { this.jobs = jobs; this.onListFiltered(this.searchQuery()); this.setDefaultSort(); - this.cdr.markForCheck(); - }); - this.route.queryParams.pipe(untilDestroyed(this)).subscribe((queryParams) => { if (queryParams.jobId) { - this.autoExpandRow(Number(queryParams.jobId)); - this.cdr.markForCheck(); + const jobId = Number(queryParams.jobId); + if (!Number.isNaN(jobId)) { + this.autoExpandRow(jobId); + } } + + this.cdr.markForCheck(); }); } From 57a1e873602884b3e0317c9a1119d7e1ac1336fa Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Fri, 2 Jan 2026 15:20:20 -0500 Subject: [PATCH 04/17] NAS-139093: fix: don't override already expanded rows --- src/app/pages/jobs/jobs-list.component.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/pages/jobs/jobs-list.component.ts b/src/app/pages/jobs/jobs-list.component.ts index b63fd7034a7..7d51938af87 100644 --- a/src/app/pages/jobs/jobs-list.component.ts +++ b/src/app/pages/jobs/jobs-list.component.ts @@ -171,7 +171,8 @@ export class JobsListComponent implements OnInit { private autoExpandRow(jobId: number): void { const jobToExpand = this.jobs.find((job) => job.id === jobId); - if (jobToExpand) { + // if there's already a row expanded, don't override it + if (jobToExpand && !this.dataProvider.expandedRow) { this.dataProvider.expandedRow = jobToExpand; } } From 624d4a29efe8460376b9f973d89520bb27d75f70 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Fri, 2 Jan 2026 15:23:20 -0500 Subject: [PATCH 05/17] NAS-139093: fix: prevent frequent updates from re-triggering auto-expand --- src/app/pages/jobs/jobs-list.component.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/pages/jobs/jobs-list.component.ts b/src/app/pages/jobs/jobs-list.component.ts index 7d51938af87..03159bf2bec 100644 --- a/src/app/pages/jobs/jobs-list.component.ts +++ b/src/app/pages/jobs/jobs-list.component.ts @@ -8,7 +8,7 @@ import { TranslateService, TranslateModule } from '@ngx-translate/core'; import { BehaviorSubject, combineLatest, Observable, of, } from 'rxjs'; -import { map, switchMap } from 'rxjs/operators'; +import { first, map, switchMap } from 'rxjs/operators'; import { UiSearchDirective } from 'app/directives/ui-search.directive'; import { EmptyType } from 'app/enums/empty-type.enum'; import { Job } from 'app/interfaces/job.interface'; @@ -131,7 +131,7 @@ export class JobsListComponent implements OnInit { ngOnInit(): void { combineLatest([ this.selectedJobs$, - this.route.queryParams, + this.route.queryParams.pipe(first()), ]).pipe(takeUntilDestroyed(this.destroyRef)).subscribe(([jobs, queryParams]) => { this.jobs = jobs; this.onListFiltered(this.searchQuery()); From 037ba4d90641e4c40a23f75556810473a6aa3cfb Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Fri, 2 Jan 2026 16:42:40 -0500 Subject: [PATCH 06/17] NAS-139093: test: fix success/failure mismatch --- .../rsync-task-card.component.spec.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.spec.ts b/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.spec.ts index 99245bb2d49..fa7d8517343 100644 --- a/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.spec.ts +++ b/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.spec.ts @@ -13,7 +13,6 @@ import { mockAuth } from 'app/core/testing/utils/mock-auth.utils'; import { Direction } from 'app/enums/direction.enum'; import { JobState } from 'app/enums/job-state.enum'; import { RsyncMode } from 'app/enums/rsync-mode.enum'; -import { Job } from 'app/interfaces/job.interface'; import { RsyncTaskUi } from 'app/interfaces/rsync-task.interface'; import { DialogService } from 'app/modules/dialog/dialog.service'; import { IxTableHarness } from 'app/modules/ix-table/components/ix-table/ix-table.harness'; @@ -47,7 +46,7 @@ describe('RsyncTaskCardComponent', () => { delayupdates: true, job: { id: 1, - state: JobState.Success, + state: JobState.Failed, time_finished: { $date: new Date().getTime() - 50000, }, @@ -77,13 +76,7 @@ describe('RsyncTaskCardComponent', () => { selectors: [ { selector: selectJobs, - value: [{ - id: 1, - state: JobState.Success, - time_finished: { - $date: new Date().getTime() - 50000, - }, - } as unknown as Job], + value: rsyncTasks.map((task) => task.job), }, { selector: selectSystemConfigState, @@ -123,7 +116,7 @@ describe('RsyncTaskCardComponent', () => { it('should show table rows', async () => { const expectedRows = [ ['Path', 'Remote Host', 'Frequency', 'Next Run', 'Last Run', 'Enabled', 'State', ''], - ['/mnt/APPS', 'asd', 'Every hour, every day', 'Disabled', '1 min. ago', '', 'Completed', ''], + ['/mnt/APPS', 'asd', 'Every hour, every day', 'Disabled', '1 min. ago', '', 'Failed', ''], ]; const cells = await table.getCellTexts(); From 381e8b75a7b89b36e15a011d51a9551d6d780f6b Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Fri, 2 Jan 2026 16:44:53 -0500 Subject: [PATCH 07/17] NAS-139093: test: update error dialog test with new params functionality --- .../components/error-dialog/error-dialog.component.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/modules/dialog/components/error-dialog/error-dialog.component.spec.ts b/src/app/modules/dialog/components/error-dialog/error-dialog.component.spec.ts index 8cdb3cd9a62..e776e5a9a39 100644 --- a/src/app/modules/dialog/components/error-dialog/error-dialog.component.spec.ts +++ b/src/app/modules/dialog/components/error-dialog/error-dialog.component.spec.ts @@ -186,7 +186,7 @@ describe('ErrorDialog', () => { const actionButton = await loader.getHarness(MatButtonHarness.with({ text: 'Network Settings' })); await actionButton.click(); - expect(router.navigate).toHaveBeenCalledWith(['/system/network']); + expect(router.navigate).toHaveBeenCalledWith(['/system/network'], { queryParams: undefined }); expect(dialogRef.close).toHaveBeenCalled(); }); From e065c5c5c58949e5011855c7f71c1ede23014a29 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Mon, 5 Jan 2026 09:19:52 -0600 Subject: [PATCH 08/17] NAS-139093: test: improve test coverage for `ix-cell-state-button` --- .../ix-cell-state-button.component.spec.ts | 59 +++++++++++++++++++ .../ix-cell-state-button.component.ts | 2 +- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.spec.ts b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.spec.ts index 3419bdba74c..3a1f45245ab 100644 --- a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.spec.ts +++ b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.spec.ts @@ -2,6 +2,7 @@ import { HarnessLoader } from '@angular/cdk/testing'; import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed'; import { MatButtonHarness } from '@angular/material/button/testing'; import { MatDialog } from '@angular/material/dialog'; +import { Router } from '@angular/router'; import { Spectator } from '@ngneat/spectator'; import { createComponentFactory, mockProvider } from '@ngneat/spectator/jest'; import { provideMockStore } from '@ngrx/store/testing'; @@ -120,4 +121,62 @@ describe('IxCellStateButtonComponent', () => { const button = spectator.query('button')!; expect(button.getAttribute('aria-label')).toBe('Label 1 Label 2'); }); + + it('opens the dialog correctly when not on the jobs page', async () => { + const job = { + id: 123456, + logs_excerpt: 'failed', + state: JobState.Failed, + error: 'failed', + }; + + spectator.component.setRow({ + state: job.state, + job, + warnings: [{}, {}], + } as TestTableData); + + const button = await loader.getHarness(MatButtonHarness); + await button.click(); + + expect(spectator.inject(DialogService).error).toHaveBeenCalledWith({ + title: job.state, + message: '
failed
', + logs: job, + actions: [{ + label: 'View Details', + route: '/jobs', + params: { jobId: job.id }, + }], + }); + }); + + it('does not show the `View Details` button when on the jobs page', async () => { + const job = { + id: 123456, + logs_excerpt: 'failed', + state: JobState.Failed, + error: 'failed', + }; + const router = spectator.inject(Router); + + // mock the router's `isActive` call to always return true, + // so it looks like we're on the `/jobs` page. + jest.spyOn(router, 'isActive').mockReturnValue(true); + spectator.component.setRow({ + state: job.state, + job, + warnings: [{}, {}], + } as TestTableData); + + const button = await loader.getHarness(MatButtonHarness); + await button.click(); + + expect(spectator.inject(DialogService).error).toHaveBeenCalledWith({ + title: job.state, + message: '
failed
', + logs: job, + // no actions should be given, since the component thinks we're on the jobs page + }); + }); }); diff --git a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts index 839b244725a..35e1901b904 100644 --- a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts +++ b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts @@ -167,7 +167,7 @@ export class IxCellStateButtonComponent extends ColumnComponent implements }, ); - // the 'View Logs' button will take the user to the jobs page and expand the failed job's row + // the 'View Details' button will take the user to the jobs page and expand the failed job's row if (showDetailsButton) { actions = [{ label: this.translate.instant('View Details'), From c1c2a81ed4fc69bf92f6dd4f23ad39baeaffda88 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Mon, 5 Jan 2026 13:09:40 -0600 Subject: [PATCH 09/17] NAS-139093: feat: implement view/download for errors handled via `showErrorModal` --- src/app/services/errors/error-parser.service.spec.ts | 6 ++++++ src/app/services/errors/error-parser.service.ts | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/src/app/services/errors/error-parser.service.spec.ts b/src/app/services/errors/error-parser.service.spec.ts index 7a091c31a79..ec71b23181a 100644 --- a/src/app/services/errors/error-parser.service.spec.ts +++ b/src/app/services/errors/error-parser.service.spec.ts @@ -111,6 +111,12 @@ describe('ErrorParserService', () => { title: 'FAILED', message: 'DUMMY_ERROR', stackTrace: 'LOGS', + actions: [{ + label: 'View Details', + route: '/jobs', + params: { jobId: failedJob.id }, + }], + logs: failedJob, }); }); diff --git a/src/app/services/errors/error-parser.service.ts b/src/app/services/errors/error-parser.service.ts index 266aa88c897..c2e4bed2597 100644 --- a/src/app/services/errors/error-parser.service.ts +++ b/src/app/services/errors/error-parser.service.ts @@ -189,6 +189,13 @@ export class ErrorParserService { title: job.state, message: job.error || job.exception || this.translate.instant('Unknown error'), stackTrace: job.logs_excerpt || job.exception, + // display a `View Details` button and `Download Logs` button on any failed job dialogs + actions: [{ + label: this.translate.instant('View Details'), + route: '/jobs', + params: { jobId: job.id }, + }], + logs: job.logs_excerpt ? job : undefined, }; } From 5720adb73159f88dc7399e6c6b276445968050d0 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Mon, 5 Jan 2026 14:02:59 -0600 Subject: [PATCH 10/17] NAS-139093: test: fix tests for jobs panel --- .../jobs/components/jobs-panel/jobs-panel.component.spec.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/app/modules/jobs/components/jobs-panel/jobs-panel.component.spec.ts b/src/app/modules/jobs/components/jobs-panel/jobs-panel.component.spec.ts index 6da245eb53c..e80ffaca47e 100644 --- a/src/app/modules/jobs/components/jobs-panel/jobs-panel.component.spec.ts +++ b/src/app/modules/jobs/components/jobs-panel/jobs-panel.component.spec.ts @@ -194,6 +194,11 @@ describe('JobsPanelComponent', () => { message: 'Some error', title: 'FAILED', stackTrack: undefined, + actions: [{ + label: 'View Details', + params: { jobId: failedJob.id }, + route: '/jobs', + }], }); }); From e29b2cf3e496a1a51297467549e01af8ec087694 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Tue, 6 Jan 2026 10:31:39 -0600 Subject: [PATCH 11/17] NAS-139093: feat: improve UX on jobs list page * expanding rows now sets the URL parameter for easy copying and for page refresh consistency. * due to this new behavior, we don't have to hide the `View Details` button on error modals anymore, since we can be consistent no matter what. * clean up implementation of the `ix-cell-state-button`'s error showing functionality. --- .../ix-cell-state-button.component.spec.ts | 46 ++-------------- .../ix-cell-state-button.component.ts | 43 ++------------- src/app/pages/jobs/jobs-list.component.html | 1 + src/app/pages/jobs/jobs-list.component.ts | 52 +++++++++++++++---- 4 files changed, 51 insertions(+), 91 deletions(-) diff --git a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.spec.ts b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.spec.ts index 3a1f45245ab..b2373b4b1b0 100644 --- a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.spec.ts +++ b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.spec.ts @@ -2,7 +2,6 @@ import { HarnessLoader } from '@angular/cdk/testing'; import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed'; import { MatButtonHarness } from '@angular/material/button/testing'; import { MatDialog } from '@angular/material/dialog'; -import { Router } from '@angular/router'; import { Spectator } from '@ngneat/spectator'; import { createComponentFactory, mockProvider } from '@ngneat/spectator/jest'; import { provideMockStore } from '@ngrx/store/testing'; @@ -14,6 +13,8 @@ import { DialogService } from 'app/modules/dialog/dialog.service'; import { IxIconHarness } from 'app/modules/ix-icon/ix-icon.harness'; import { IxCellStateButtonComponent } from 'app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component'; import { selectJobs } from 'app/modules/jobs/store/job.selectors'; +import { ErrorHandlerService } from 'app/services/errors/error-handler.service'; +import { FailedJobError } from 'app/services/errors/error.classes'; interface TestTableData { state: JobState; @@ -122,7 +123,7 @@ describe('IxCellStateButtonComponent', () => { expect(button.getAttribute('aria-label')).toBe('Label 1 Label 2'); }); - it('opens the dialog correctly when not on the jobs page', async () => { + it('calls showErrorModal when storing a failed job', async () => { const job = { id: 123456, logs_excerpt: 'failed', @@ -139,44 +140,7 @@ describe('IxCellStateButtonComponent', () => { const button = await loader.getHarness(MatButtonHarness); await button.click(); - expect(spectator.inject(DialogService).error).toHaveBeenCalledWith({ - title: job.state, - message: '
failed
', - logs: job, - actions: [{ - label: 'View Details', - route: '/jobs', - params: { jobId: job.id }, - }], - }); - }); - - it('does not show the `View Details` button when on the jobs page', async () => { - const job = { - id: 123456, - logs_excerpt: 'failed', - state: JobState.Failed, - error: 'failed', - }; - const router = spectator.inject(Router); - - // mock the router's `isActive` call to always return true, - // so it looks like we're on the `/jobs` page. - jest.spyOn(router, 'isActive').mockReturnValue(true); - spectator.component.setRow({ - state: job.state, - job, - warnings: [{}, {}], - } as TestTableData); - - const button = await loader.getHarness(MatButtonHarness); - await button.click(); - - expect(spectator.inject(DialogService).error).toHaveBeenCalledWith({ - title: job.state, - message: '
failed
', - logs: job, - // no actions should be given, since the component thinks we're on the jobs page - }); + const expectedError = new FailedJobError(job as Job); + expect(spectator.inject(ErrorHandlerService).showErrorModal).toHaveBeenCalledWith(expectedError); }); }); diff --git a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts index 35e1901b904..b67ced36a95 100644 --- a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts +++ b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts @@ -26,6 +26,7 @@ import { JobSlice, selectJob } from 'app/modules/jobs/store/job.selectors'; import { JobStateDisplayPipe } from 'app/modules/pipes/job-state-display/job-state-display.pipe'; import { TestDirective } from 'app/modules/test-id/test.directive'; import { ErrorHandlerService } from 'app/services/errors/error-handler.service'; +import { FailedJobError } from 'app/services/errors/error.classes'; interface RowState { state: { @@ -144,46 +145,8 @@ export class IxCellStateButtonComponent extends ColumnComponent implements } if (state.error) { - // by default, we just show the error message - const message = `
${state.error}
`; - - // if there's a log excerpt though, we include the additional - // 'View Details' and 'Download Logs' buttons. - let logs; - let actions; - if (this.job()?.logs_excerpt) { - logs = this.job()?.logs_excerpt ? this.job() : null; - - // only show the 'View Details' button when we're *not* on the jobs page - // since it would imply to the user there's somewhere else they could go - // and that clicking it would take them there. - const showDetailsButton = !this.router.isActive( - '/jobs', - { - matrixParams: 'ignored', - queryParams: 'ignored', - paths: 'exact', - fragment: 'ignored', - }, - ); - - // the 'View Details' button will take the user to the jobs page and expand the failed job's row - if (showDetailsButton) { - actions = [{ - label: this.translate.instant('View Details'), - route: '/jobs', - params: { jobId: this.job().id }, - }]; - } - } - - this.dialogService.error({ - title: state.state, - message, - // since these two may be undefined and are optional, we can just include them in the call outright - logs, - actions, - }); + const error: FailedJobError = new FailedJobError(this.job()); + this.errorHandler.showErrorModal(error); return; } diff --git a/src/app/pages/jobs/jobs-list.component.html b/src/app/pages/jobs/jobs-list.component.html index 9dffd3f1610..f7a4f76f9b8 100644 --- a/src/app/pages/jobs/jobs-list.component.html +++ b/src/app/pages/jobs/jobs-list.component.html @@ -39,6 +39,7 @@ [columns]="columns" [dataProvider]="dataProvider" [isLoading]="!!(isLoading$ | async)" + (expanded)="onRowExpanded($event)" > >(Store); private cdr = inject(ChangeDetectorRef); private route = inject(ActivatedRoute); + private router = inject(Router); private readonly destroyRef = inject(DestroyRef); protected readonly searchableElements = jobsListElements; @@ -129,16 +130,30 @@ export class JobsListComponent implements OnInit { ); ngOnInit(): void { - combineLatest([ - this.selectedJobs$, - this.route.queryParams.pipe(first()), - ]).pipe(takeUntilDestroyed(this.destroyRef)).subscribe(([jobs, queryParams]) => { + const jobsTrigger$ = this.selectedJobs$.pipe( + takeUntilDestroyed(this.destroyRef), + ); + + const queryTrigger$ = this.route.queryParams.pipe( + takeUntilDestroyed(this.destroyRef), + ); + + // handle jobs changing and update our internal represetation inside `this.jobs` + jobsTrigger$.subscribe((jobs) => { this.jobs = jobs; this.onListFiltered(this.searchQuery()); this.setDefaultSort(); + this.cdr.markForCheck(); + }); - if (queryParams.jobId) { - const jobId = Number(queryParams.jobId); + // handle query updates and expand rows according to URL params. + // we combine `queryTrigger$` with `jobsTrigger$` since, if we + // were to try and run `autoExpandRow` before `this.jobs` was populated, then + // nothing would happen. `combineLatest` is a neat way to ensure that BOTH observables have + // values before doing anything. + combineLatest([jobsTrigger$, queryTrigger$]).subscribe(([_, query]) => { + if (query.jobId) { + const jobId = Number(query.jobId); if (!Number.isNaN(jobId)) { this.autoExpandRow(jobId); } @@ -148,6 +163,22 @@ export class JobsListComponent implements OnInit { }); } + protected onRowExpanded(job: Job | null): void { + if (job) { + this.router.navigate([], { + relativeTo: this.route, + queryParams: { jobId: job.id }, + queryParamsHandling: 'merge', + }); + } else { + this.router.navigate([], { + relativeTo: this.route, + queryParams: { jobId: null }, + queryParamsHandling: 'merge', + }); + } + } + protected onTabChange(tab: JobTab): void { this.selectedIndex = tab; switch (this.selectedIndex) { @@ -171,9 +202,10 @@ export class JobsListComponent implements OnInit { private autoExpandRow(jobId: number): void { const jobToExpand = this.jobs.find((job) => job.id === jobId); - // if there's already a row expanded, don't override it - if (jobToExpand && !this.dataProvider.expandedRow) { + if (jobToExpand) { + // set the expanded row and force a re-render this.dataProvider.expandedRow = jobToExpand; + this.dataProvider.currentPage$.next(this.dataProvider.currentPage$.value); } } From a16c5de76cd31a10698e43238bf70ed4a613f110 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Tue, 6 Jan 2026 11:00:55 -0600 Subject: [PATCH 12/17] NAS-139093: fix: misc. fixes * deep-copy rsync task mutation fix. * fix a typo in a comment. * add unit test to verify that the jobs list page sets URL parameters correctly. * fix a possible observable re-emission issue in the jobs list that would have randomly expanded a row without user interaction. --- .../ix-cell-state-button.component.ts | 2 -- .../rsync-task-card.component.ts | 8 ++++++- .../pages/jobs/jobs-list.component.spec.ts | 22 ++++++++++++++++++- src/app/pages/jobs/jobs-list.component.ts | 9 +++++--- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts index b67ced36a95..6a1551fdcdf 100644 --- a/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts +++ b/src/app/modules/ix-table/components/ix-table-body/cells/ix-cell-state-button/ix-cell-state-button.component.ts @@ -5,7 +5,6 @@ import { import { MatButton } from '@angular/material/button'; import { MatDialog } from '@angular/material/dialog'; import { MatTooltip } from '@angular/material/tooltip'; -import { Router } from '@angular/router'; import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; import { Store } from '@ngrx/store'; import { TranslateService, TranslateModule } from '@ngx-translate/core'; @@ -58,7 +57,6 @@ export class IxCellStateButtonComponent extends ColumnComponent implements translate: TranslateService = inject(TranslateService); dialogService: DialogService = inject(DialogService); errorHandler: ErrorHandlerService = inject(ErrorHandlerService); - router: Router = inject(Router); private readonly rowUpdateEffect = effect(() => { this.setupRow(); diff --git a/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.ts b/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.ts index 3edb5efb354..ad0a1d0c744 100644 --- a/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.ts +++ b/src/app/pages/data-protection/rsync-task/rsync-task-card/rsync-task-card.component.ts @@ -230,7 +230,13 @@ export class RsyncTaskCardComponent implements OnInit { private transformRsyncTasks(rsyncTasks: RsyncTaskUi[]): RsyncTaskUi[] { return rsyncTasks.map((rsyncTask: RsyncTaskUi) => { - const task: RsyncTaskUi = { ...rsyncTask }; + // make sure we deep-copy `state` and `job` so we aren't overriding the originals + // when we mutate `task`. + const task: RsyncTaskUi = { + ...rsyncTask, + state: { ...rsyncTask.state }, + job: { ...rsyncTask.job }, + }; if (task.job === null) { task.state = { state: task.locked ? TaskState.Locked : TaskState.Pending }; } else { diff --git a/src/app/pages/jobs/jobs-list.component.spec.ts b/src/app/pages/jobs/jobs-list.component.spec.ts index 237469f964b..db8e8849d4e 100644 --- a/src/app/pages/jobs/jobs-list.component.spec.ts +++ b/src/app/pages/jobs/jobs-list.component.spec.ts @@ -2,7 +2,7 @@ import { HarnessLoader } from '@angular/cdk/testing'; import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed'; import { MatButtonHarness } from '@angular/material/button/testing'; import { MatSnackBar } from '@angular/material/snack-bar'; -import { ActivatedRoute } from '@angular/router'; +import { Router, ActivatedRoute } from '@angular/router'; import { createComponentFactory, mockProvider, Spectator } from '@ngneat/spectator/jest'; import { MockStore, provideMockStore } from '@ngrx/store/testing'; import { MockComponent } from 'ng-mocks'; @@ -13,6 +13,7 @@ import { Job } from 'app/interfaces/job.interface'; import { DialogService } from 'app/modules/dialog/dialog.service'; import { IxEmptyRowHarness } from 'app/modules/ix-table/components/ix-empty-row/ix-empty-row.component.harness'; import { IxTableHarness } from 'app/modules/ix-table/components/ix-table/ix-table.harness'; +import { IxRowHarness } from 'app/modules/ix-table/components/ix-table/row.harness'; import { jobsInitialState, JobsState } from 'app/modules/jobs/store/job.reducer'; import { selectJobs, selectJobState } from 'app/modules/jobs/store/job.selectors'; import { LocaleService } from 'app/modules/language/locale.service'; @@ -167,4 +168,23 @@ describe('JobsListComponent', () => { expect(spectator.queryAll('.expanded')).toHaveLength(0); }); + + it('sets URL parameters when a row is expanded', async () => { + const route = spectator.inject(ActivatedRoute); + + const router = spectator.inject(Router); + const navigateSpy = jest.spyOn(router, 'navigate'); + store$.overrideSelector(selectJobs, fakeJobDataSource); + store$.refreshState(); + + const firstRow = await loader.getHarness(IxRowHarness); + const firstRowButton = await firstRow.getHarness(MatButtonHarness.with({ selector: '[ixTest="toggle-row"]' })); + await firstRowButton.click(); + + expect(navigateSpy).toHaveBeenCalledWith([], { + relativeTo: route, + queryParams: { jobId: fakeJobDataSource[0].id }, + queryParamsHandling: 'merge', + }); + }); }); diff --git a/src/app/pages/jobs/jobs-list.component.ts b/src/app/pages/jobs/jobs-list.component.ts index be35d0ce570..8b4093ebd7e 100644 --- a/src/app/pages/jobs/jobs-list.component.ts +++ b/src/app/pages/jobs/jobs-list.component.ts @@ -8,7 +8,7 @@ import { TranslateService, TranslateModule } from '@ngx-translate/core'; import { BehaviorSubject, combineLatest, Observable, of, } from 'rxjs'; -import { map, switchMap } from 'rxjs/operators'; +import { first, map, switchMap } from 'rxjs/operators'; import { UiSearchDirective } from 'app/directives/ui-search.directive'; import { EmptyType } from 'app/enums/empty-type.enum'; import { Job } from 'app/interfaces/job.interface'; @@ -138,7 +138,7 @@ export class JobsListComponent implements OnInit { takeUntilDestroyed(this.destroyRef), ); - // handle jobs changing and update our internal represetation inside `this.jobs` + // handle jobs changing and update our internal representation inside `this.jobs` jobsTrigger$.subscribe((jobs) => { this.jobs = jobs; this.onListFiltered(this.searchQuery()); @@ -151,7 +151,10 @@ export class JobsListComponent implements OnInit { // were to try and run `autoExpandRow` before `this.jobs` was populated, then // nothing would happen. `combineLatest` is a neat way to ensure that BOTH observables have // values before doing anything. - combineLatest([jobsTrigger$, queryTrigger$]).subscribe(([_, query]) => { + // + // the `first()` operator is there to ensure that `jobsTrigger$` only ever emits once, + // which will prevent job updates re-triggering row expansion. + combineLatest([jobsTrigger$.pipe(first()), queryTrigger$]).subscribe(([_, query]) => { if (query.jobId) { const jobId = Number(query.jobId); if (!Number.isNaN(jobId)) { From 067a1fde8b8e02919b00f7ee70c7f4a0429d90e7 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Tue, 6 Jan 2026 11:15:08 -0600 Subject: [PATCH 13/17] NAS-139093: fix: remove impossible branch in `onRowExpanded` --- src/app/pages/jobs/jobs-list.component.ts | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/app/pages/jobs/jobs-list.component.ts b/src/app/pages/jobs/jobs-list.component.ts index 8b4093ebd7e..b85c09c52ff 100644 --- a/src/app/pages/jobs/jobs-list.component.ts +++ b/src/app/pages/jobs/jobs-list.component.ts @@ -166,20 +166,12 @@ export class JobsListComponent implements OnInit { }); } - protected onRowExpanded(job: Job | null): void { - if (job) { - this.router.navigate([], { - relativeTo: this.route, - queryParams: { jobId: job.id }, - queryParamsHandling: 'merge', - }); - } else { - this.router.navigate([], { - relativeTo: this.route, - queryParams: { jobId: null }, - queryParamsHandling: 'merge', - }); - } + protected onRowExpanded(job: Job): void { + this.router.navigate([], { + relativeTo: this.route, + queryParams: { jobId: job.id }, + queryParamsHandling: 'merge', + }); } protected onTabChange(tab: JobTab): void { From 1bc070a933f568314aecddb8007b577b5c48c32b Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Tue, 6 Jan 2026 11:22:39 -0600 Subject: [PATCH 14/17] NAS-139093: fix: replace `first()` by `take(1)` to avoid observable completion --- src/app/pages/jobs/jobs-list.component.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/pages/jobs/jobs-list.component.ts b/src/app/pages/jobs/jobs-list.component.ts index b85c09c52ff..179fe110988 100644 --- a/src/app/pages/jobs/jobs-list.component.ts +++ b/src/app/pages/jobs/jobs-list.component.ts @@ -8,7 +8,7 @@ import { TranslateService, TranslateModule } from '@ngx-translate/core'; import { BehaviorSubject, combineLatest, Observable, of, } from 'rxjs'; -import { first, map, switchMap } from 'rxjs/operators'; +import { take, map, switchMap } from 'rxjs/operators'; import { UiSearchDirective } from 'app/directives/ui-search.directive'; import { EmptyType } from 'app/enums/empty-type.enum'; import { Job } from 'app/interfaces/job.interface'; @@ -152,9 +152,9 @@ export class JobsListComponent implements OnInit { // nothing would happen. `combineLatest` is a neat way to ensure that BOTH observables have // values before doing anything. // - // the `first()` operator is there to ensure that `jobsTrigger$` only ever emits once, + // the `take(1)` operator is there to ensure that `jobsTrigger$` only ever emits once, // which will prevent job updates re-triggering row expansion. - combineLatest([jobsTrigger$.pipe(first()), queryTrigger$]).subscribe(([_, query]) => { + combineLatest([jobsTrigger$.pipe(take(1)), queryTrigger$]).subscribe(([_, query]) => { if (query.jobId) { const jobId = Number(query.jobId); if (!Number.isNaN(jobId)) { From f3f92ae7b61fa91f3e0b2515084f1441b8cb1a4a Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Tue, 6 Jan 2026 12:45:02 -0600 Subject: [PATCH 15/17] NAS-139093: fix: format error messages correctly --- .../jobs-panel/jobs-panel.component.spec.ts | 2 +- src/app/services/errors/error-parser.service.spec.ts | 2 +- src/app/services/errors/error-parser.service.ts | 11 ++++++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/app/modules/jobs/components/jobs-panel/jobs-panel.component.spec.ts b/src/app/modules/jobs/components/jobs-panel/jobs-panel.component.spec.ts index e80ffaca47e..013fea87d5b 100644 --- a/src/app/modules/jobs/components/jobs-panel/jobs-panel.component.spec.ts +++ b/src/app/modules/jobs/components/jobs-panel/jobs-panel.component.spec.ts @@ -191,7 +191,7 @@ describe('JobsPanelComponent', () => { spectator.click(byText('replication.run')); expect(spectator.inject(DialogService).error).toHaveBeenCalledWith({ - message: 'Some error', + message: '
Some error
', title: 'FAILED', stackTrack: undefined, actions: [{ diff --git a/src/app/services/errors/error-parser.service.spec.ts b/src/app/services/errors/error-parser.service.spec.ts index ec71b23181a..7585c899153 100644 --- a/src/app/services/errors/error-parser.service.spec.ts +++ b/src/app/services/errors/error-parser.service.spec.ts @@ -109,7 +109,7 @@ describe('ErrorParserService', () => { expect(errorReport).toEqual({ title: 'FAILED', - message: 'DUMMY_ERROR', + message: '
DUMMY_ERROR
', stackTrace: 'LOGS', actions: [{ label: 'View Details', diff --git a/src/app/services/errors/error-parser.service.ts b/src/app/services/errors/error-parser.service.ts index c2e4bed2597..d6da8a69ea7 100644 --- a/src/app/services/errors/error-parser.service.ts +++ b/src/app/services/errors/error-parser.service.ts @@ -185,9 +185,18 @@ export class ErrorParserService { return this.parseJobWithArrayExtra(job); } + let message: string; + if (job.error) { + message = `
${job.error}
`; + } else if (job.exception) { + message = `
${job.exception}
`; + } else { + message = this.translate.instant('Unknown error'); + } + return { title: job.state, - message: job.error || job.exception || this.translate.instant('Unknown error'), + message, stackTrace: job.logs_excerpt || job.exception, // display a `View Details` button and `Download Logs` button on any failed job dialogs actions: [{ From 0b7f37c36c5d169445e9324a6e87bb76d87727b2 Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Tue, 6 Jan 2026 12:52:29 -0600 Subject: [PATCH 16/17] NAS-139093: fix: pipe `takeUntilDestroyed` to `combineLatest` in jobs list this resolves a potential memory leak. --- src/app/pages/jobs/jobs-list.component.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/app/pages/jobs/jobs-list.component.ts b/src/app/pages/jobs/jobs-list.component.ts index 179fe110988..ff6242f062b 100644 --- a/src/app/pages/jobs/jobs-list.component.ts +++ b/src/app/pages/jobs/jobs-list.component.ts @@ -154,16 +154,18 @@ export class JobsListComponent implements OnInit { // // the `take(1)` operator is there to ensure that `jobsTrigger$` only ever emits once, // which will prevent job updates re-triggering row expansion. - combineLatest([jobsTrigger$.pipe(take(1)), queryTrigger$]).subscribe(([_, query]) => { - if (query.jobId) { - const jobId = Number(query.jobId); - if (!Number.isNaN(jobId)) { - this.autoExpandRow(jobId); + combineLatest([jobsTrigger$.pipe(take(1)), queryTrigger$]) + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe(([_, query]) => { + if (query.jobId) { + const jobId = Number(query.jobId); + if (!Number.isNaN(jobId)) { + this.autoExpandRow(jobId); + } } - } - this.cdr.markForCheck(); - }); + this.cdr.markForCheck(); + }); } protected onRowExpanded(job: Job): void { From 87fb5c7bc197da7e7d16f2fa6309071ebc92b2ac Mon Sep 17 00:00:00 2001 From: matt wyatt Date: Tue, 6 Jan 2026 13:18:25 -0600 Subject: [PATCH 17/17] NAS-139093: test: improve coverage --- .../errors/error-parser.service.spec.ts | 39 +++++++++++++++++-- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/src/app/services/errors/error-parser.service.spec.ts b/src/app/services/errors/error-parser.service.spec.ts index 7585c899153..8b4b0fb2bed 100644 --- a/src/app/services/errors/error-parser.service.spec.ts +++ b/src/app/services/errors/error-parser.service.spec.ts @@ -105,9 +105,18 @@ describe('ErrorParserService', () => { }); it('parses a failed job', () => { - const errorReport = spectator.service.parseError(new FailedJobError(failedJob)); + const job1 = failedJob; + const job2 = { + ...failedJob, + error: undefined, + } as Job; + const job3 = { + ...failedJob, + error: undefined, + exception: undefined, + } as Job; - expect(errorReport).toEqual({ + expect(spectator.service.parseError(new FailedJobError(job1))).toEqual({ title: 'FAILED', message: '
DUMMY_ERROR
', stackTrace: 'LOGS', @@ -116,7 +125,31 @@ describe('ErrorParserService', () => { route: '/jobs', params: { jobId: failedJob.id }, }], - logs: failedJob, + logs: job1, + }); + + expect(spectator.service.parseError(new FailedJobError(job2))).toEqual({ + title: 'FAILED', + message: '
EXCEPTION
', + stackTrace: 'LOGS', + actions: [{ + label: 'View Details', + route: '/jobs', + params: { jobId: failedJob.id }, + }], + logs: job2, + }); + + expect(spectator.service.parseError(new FailedJobError(job3))).toEqual({ + title: 'FAILED', + message: 'Unknown error', + stackTrace: 'LOGS', + actions: [{ + label: 'View Details', + route: '/jobs', + params: { jobId: failedJob.id }, + }], + logs: job3, }); });