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.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(); }); 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.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..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 @@ -13,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; @@ -120,4 +122,25 @@ describe('IxCellStateButtonComponent', () => { const button = spectator.query('button')!; expect(button.getAttribute('aria-label')).toBe('Label 1 Label 2'); }); + + it('calls showErrorModal when storing a failed job', 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(); + + 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 a7b078a55c2..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 @@ -25,6 +25,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: { @@ -142,7 +143,8 @@ export class IxCellStateButtonComponent extends ColumnComponent implements } if (state.error) { - this.dialogService.error({ title: state.state, message: `
${state.error}
` }); + const error: FailedJobError = new FailedJobError(this.job()); + this.errorHandler.showErrorModal(error); return; } 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..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,9 +191,14 @@ describe('JobsPanelComponent', () => { spectator.click(byText('replication.run')); expect(spectator.inject(DialogService).error).toHaveBeenCalledWith({ - message: 'Some error', + message: '
Some error
', title: 'FAILED', stackTrack: undefined, + actions: [{ + label: 'View Details', + params: { jobId: failedJob.id }, + route: '/jobs', + }], }); }); 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(); 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..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 @@ -229,7 +229,14 @@ export class RsyncTaskCardComponent implements OnInit { } private transformRsyncTasks(rsyncTasks: RsyncTaskUi[]): RsyncTaskUi[] { - return rsyncTasks.map((task: RsyncTaskUi) => { + return rsyncTasks.map((rsyncTask: RsyncTaskUi) => { + // 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.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)" > { }), mockProvider(DialogService), mockProvider(MatSnackBar), + mockProvider(ActivatedRoute, { + queryParams: of({}), + }), mockApi([ mockCall('core.job_download_logs', 'http://localhost/download/log'), ]), @@ -126,4 +131,60 @@ 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); + }); + + 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 12da5263063..ff6242f062b 100644 --- a/src/app/pages/jobs/jobs-list.component.ts +++ b/src/app/pages/jobs/jobs-list.component.ts @@ -1,13 +1,14 @@ 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 { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; +import { Router, ActivatedRoute } from '@angular/router'; import { Store } from '@ngrx/store'; import { TranslateService, TranslateModule } from '@ngx-translate/core'; import { BehaviorSubject, combineLatest, Observable, of, } from 'rxjs'; -import { 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'; @@ -39,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', @@ -69,6 +69,9 @@ export class JobsListComponent implements OnInit { private translate = inject(TranslateService); private store$ = inject>(Store); private cdr = inject(ChangeDetectorRef); + private route = inject(ActivatedRoute); + private router = inject(Router); + private readonly destroyRef = inject(DestroyRef); protected readonly searchableElements = jobsListElements; @@ -127,12 +130,50 @@ export class JobsListComponent implements OnInit { ); ngOnInit(): void { - this.selectedJobs$.pipe(untilDestroyed(this)).subscribe((jobs) => { + const jobsTrigger$ = this.selectedJobs$.pipe( + takeUntilDestroyed(this.destroyRef), + ); + + const queryTrigger$ = this.route.queryParams.pipe( + takeUntilDestroyed(this.destroyRef), + ); + + // handle jobs changing and update our internal representation inside `this.jobs` + jobsTrigger$.subscribe((jobs) => { this.jobs = jobs; this.onListFiltered(this.searchQuery()); this.setDefaultSort(); this.cdr.markForCheck(); }); + + // 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. + // + // 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$]) + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe(([_, query]) => { + if (query.jobId) { + const jobId = Number(query.jobId); + if (!Number.isNaN(jobId)) { + this.autoExpandRow(jobId); + } + } + + this.cdr.markForCheck(); + }); + } + + protected onRowExpanded(job: Job): void { + this.router.navigate([], { + relativeTo: this.route, + queryParams: { jobId: job.id }, + queryParamsHandling: 'merge', + }); } protected onTabChange(tab: JobTab): void { @@ -156,6 +197,15 @@ 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) { + // set the expanded row and force a re-render + this.dataProvider.expandedRow = jobToExpand; + this.dataProvider.currentPage$.next(this.dataProvider.currentPage$.value); + } + } + private setDefaultSort(): void { this.dataProvider.setSorting({ active: 1, diff --git a/src/app/services/errors/error-parser.service.spec.ts b/src/app/services/errors/error-parser.service.spec.ts index 7a091c31a79..8b4b0fb2bed 100644 --- a/src/app/services/errors/error-parser.service.spec.ts +++ b/src/app/services/errors/error-parser.service.spec.ts @@ -105,12 +105,51 @@ 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', + message: '
DUMMY_ERROR
', + stackTrace: 'LOGS', + actions: [{ + label: 'View Details', + route: '/jobs', + params: { jobId: failedJob.id }, + }], + 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, }); }); diff --git a/src/app/services/errors/error-parser.service.ts b/src/app/services/errors/error-parser.service.ts index 266aa88c897..d6da8a69ea7 100644 --- a/src/app/services/errors/error-parser.service.ts +++ b/src/app/services/errors/error-parser.service.ts @@ -185,10 +185,26 @@ 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: [{ + label: this.translate.instant('View Details'), + route: '/jobs', + params: { jobId: job.id }, + }], + logs: job.logs_excerpt ? job : undefined, }; }