From a94d731b76dabb70fab09ea93627f1515b0cdc25 Mon Sep 17 00:00:00 2001 From: Tobias Kohr Date: Wed, 11 Dec 2024 17:28:10 +0100 Subject: [PATCH 1/9] feat(notifications-service): log errors in console when displaying error notifications --- .../src/app/duplicate-record.resolver.spec.ts | 16 +++-- .../src/app/duplicate-record.resolver.ts | 28 +++++---- .../src/app/edit-record.resolver.spec.ts | 16 +++-- .../src/app/edit-record.resolver.ts | 28 +++++---- .../src/app/edit/edit-page.component.spec.ts | 32 ++++++---- .../src/app/edit/edit-page.component.ts | 58 +++++++++++-------- .../import-record.component.spec.ts | 4 +- .../import-record/import-record.component.ts | 3 +- ...ld-online-link-resources.component.spec.ts | 16 +++-- ...m-field-online-link-resources.component.ts | 28 +++++---- .../form-field-online-resources.component.ts | 28 +++++---- .../form-field-overviews.component.spec.ts | 16 +++-- .../form-field-overviews.component.ts | 26 +++++---- .../src/lib/notifications.service.ts | 7 ++- .../results-table-container.component.spec.ts | 16 +++-- .../results-table-container.component.ts | 28 +++++---- 16 files changed, 210 insertions(+), 140 deletions(-) diff --git a/apps/metadata-editor/src/app/duplicate-record.resolver.spec.ts b/apps/metadata-editor/src/app/duplicate-record.resolver.spec.ts index 2ec7eafc53..9290157ac4 100644 --- a/apps/metadata-editor/src/app/duplicate-record.resolver.spec.ts +++ b/apps/metadata-editor/src/app/duplicate-record.resolver.spec.ts @@ -75,12 +75,16 @@ describe('DuplicateRecordResolver', () => { expect(resolvedData).toBeUndefined() }) it('should show error notification', () => { - expect(notificationsService.showNotification).toHaveBeenCalledWith({ - type: 'error', - title: 'editor.record.loadError.title', - text: 'editor.record.loadError.body oopsie', - closeMessage: 'editor.record.loadError.closeMessage', - }) + expect(notificationsService.showNotification).toHaveBeenCalledWith( + { + type: 'error', + title: 'editor.record.loadError.title', + text: 'editor.record.loadError.body oopsie', + closeMessage: 'editor.record.loadError.closeMessage', + }, + undefined, + expect.any(Error) + ) }) }) }) diff --git a/apps/metadata-editor/src/app/duplicate-record.resolver.ts b/apps/metadata-editor/src/app/duplicate-record.resolver.ts index 89df9a5699..78f95c4890 100644 --- a/apps/metadata-editor/src/app/duplicate-record.resolver.ts +++ b/apps/metadata-editor/src/app/duplicate-record.resolver.ts @@ -23,18 +23,22 @@ export class DuplicateRecordResolver { .openRecordForDuplication(route.paramMap.get('uuid')) .pipe( catchError((error) => { - this.notificationsService.showNotification({ - type: 'error', - title: this.translateService.instant( - 'editor.record.loadError.title' - ), - text: `${this.translateService.instant( - 'editor.record.loadError.body' - )} ${error.message}`, - closeMessage: this.translateService.instant( - 'editor.record.loadError.closeMessage' - ), - }) + this.notificationsService.showNotification( + { + type: 'error', + title: this.translateService.instant( + 'editor.record.loadError.title' + ), + text: `${this.translateService.instant( + 'editor.record.loadError.body' + )} ${error.message}`, + closeMessage: this.translateService.instant( + 'editor.record.loadError.closeMessage' + ), + }, + undefined, + error + ) return EMPTY }) ) diff --git a/apps/metadata-editor/src/app/edit-record.resolver.spec.ts b/apps/metadata-editor/src/app/edit-record.resolver.spec.ts index ffe79371a8..28bb067edb 100644 --- a/apps/metadata-editor/src/app/edit-record.resolver.spec.ts +++ b/apps/metadata-editor/src/app/edit-record.resolver.spec.ts @@ -75,12 +75,16 @@ describe('EditRecordResolver', () => { expect(resolvedData).toBeUndefined() }) it('should show error notification', () => { - expect(notificationsService.showNotification).toHaveBeenCalledWith({ - type: 'error', - title: 'editor.record.loadError.title', - text: 'editor.record.loadError.body oopsie', - closeMessage: 'editor.record.loadError.closeMessage', - }) + expect(notificationsService.showNotification).toHaveBeenCalledWith( + { + type: 'error', + title: 'editor.record.loadError.title', + text: 'editor.record.loadError.body oopsie', + closeMessage: 'editor.record.loadError.closeMessage', + }, + undefined, + expect.any(Error) + ) }) }) }) diff --git a/apps/metadata-editor/src/app/edit-record.resolver.ts b/apps/metadata-editor/src/app/edit-record.resolver.ts index 337a1bf858..fe4c9d7486 100644 --- a/apps/metadata-editor/src/app/edit-record.resolver.ts +++ b/apps/metadata-editor/src/app/edit-record.resolver.ts @@ -23,18 +23,22 @@ export class EditRecordResolver { .openRecordForEdition(route.paramMap.get('uuid')) .pipe( catchError((error) => { - this.notificationsService.showNotification({ - type: 'error', - title: this.translateService.instant( - 'editor.record.loadError.title' - ), - text: `${this.translateService.instant( - 'editor.record.loadError.body' - )} ${error.message}`, - closeMessage: this.translateService.instant( - 'editor.record.loadError.closeMessage' - ), - }) + this.notificationsService.showNotification( + { + type: 'error', + title: this.translateService.instant( + 'editor.record.loadError.title' + ), + text: `${this.translateService.instant( + 'editor.record.loadError.body' + )} ${error.message}`, + closeMessage: this.translateService.instant( + 'editor.record.loadError.closeMessage' + ), + }, + undefined, + error + ) return EMPTY }) ) diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts index 1885913f8e..e06c44c5d4 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts @@ -121,24 +121,32 @@ describe('EditPageComponent', () => { describe('publish version error', () => { it('shows notification', () => { ;(facade.saveError$ as any).next(new PublicationVersionError('1.0.0')) - expect(notificationsService.showNotification).toHaveBeenCalledWith({ - type: 'error', - title: 'editor.record.publishVersionError.title', - text: 'editor.record.publishVersionError.body', - closeMessage: 'editor.record.publishVersionError.closeMessage', - }) + expect(notificationsService.showNotification).toHaveBeenCalledWith( + { + type: 'error', + title: 'editor.record.publishVersionError.title', + text: 'editor.record.publishVersionError.body', + closeMessage: 'editor.record.publishVersionError.closeMessage', + }, + undefined, + expect.any(PublicationVersionError) + ) }) }) describe('publish error', () => { it('shows notification', () => { ;(facade.saveError$ as any).next(new Error('oopsie')) - expect(notificationsService.showNotification).toHaveBeenCalledWith({ - type: 'error', - title: 'editor.record.publishError.title', - text: 'editor.record.publishError.body oopsie', - closeMessage: 'editor.record.publishError.closeMessage', - }) + expect(notificationsService.showNotification).toHaveBeenCalledWith( + { + type: 'error', + title: 'editor.record.publishError.title', + text: 'editor.record.publishError.body oopsie', + closeMessage: 'editor.record.publishError.closeMessage', + }, + undefined, + expect.any(Error) + ) }) }) diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.ts b/apps/metadata-editor/src/app/edit/edit-page.component.ts index fe630e18ba..6e6753275e 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.ts @@ -83,32 +83,40 @@ export class EditPageComponent implements OnInit, OnDestroy { this.subscription.add( this.facade.saveError$.subscribe((error) => { if (error instanceof PublicationVersionError) { - this.notificationsService.showNotification({ - type: 'error', - title: this.translateService.instant( - 'editor.record.publishVersionError.title' - ), - text: this.translateService.instant( - 'editor.record.publishVersionError.body', - { currentVersion: error.detectedApiVersion } - ), - closeMessage: this.translateService.instant( - 'editor.record.publishVersionError.closeMessage' - ), - }) + this.notificationsService.showNotification( + { + type: 'error', + title: this.translateService.instant( + 'editor.record.publishVersionError.title' + ), + text: this.translateService.instant( + 'editor.record.publishVersionError.body', + { currentVersion: error.detectedApiVersion } + ), + closeMessage: this.translateService.instant( + 'editor.record.publishVersionError.closeMessage' + ), + }, + undefined, + error + ) } else { - this.notificationsService.showNotification({ - type: 'error', - title: this.translateService.instant( - 'editor.record.publishError.title' - ), - text: `${this.translateService.instant( - 'editor.record.publishError.body' - )} ${error.message}`, - closeMessage: this.translateService.instant( - 'editor.record.publishError.closeMessage' - ), - }) + this.notificationsService.showNotification( + { + type: 'error', + title: this.translateService.instant( + 'editor.record.publishError.title' + ), + text: `${this.translateService.instant( + 'editor.record.publishError.body' + )} ${error.message}`, + closeMessage: this.translateService.instant( + 'editor.record.publishError.closeMessage' + ), + }, + undefined, + error + ) } }) ) diff --git a/libs/feature/editor/src/lib/components/import-record/import-record.component.spec.ts b/libs/feature/editor/src/lib/components/import-record/import-record.component.spec.ts index 09430bde77..934309aea4 100644 --- a/libs/feature/editor/src/lib/components/import-record/import-record.component.spec.ts +++ b/libs/feature/editor/src/lib/components/import-record/import-record.component.spec.ts @@ -7,6 +7,7 @@ import { NotificationsService } from '@geonetwork-ui/feature/notifications' import { RecordsRepositoryInterface } from '@geonetwork-ui/common/domain/repository/records-repository.interface' import { of, throwError } from 'rxjs' import { MockBuilder, MockComponent, MockModule, MockProviders } from 'ng-mocks' +import exp from 'constants' describe('ImportRecordComponent', () => { let component: ImportRecordComponent @@ -112,7 +113,8 @@ describe('ImportRecordComponent', () => { title: 'editor.record.importFromExternalFile.failure.title', text: `editor.record.importFromExternalFile.failure.body `, }), - 2500 + 2500, + mockError ) expect(component.isRecordImportInProgress).toBe(false) diff --git a/libs/feature/editor/src/lib/components/import-record/import-record.component.ts b/libs/feature/editor/src/lib/components/import-record/import-record.component.ts index 2bb72582c4..02e470ebb0 100644 --- a/libs/feature/editor/src/lib/components/import-record/import-record.component.ts +++ b/libs/feature/editor/src/lib/components/import-record/import-record.component.ts @@ -142,7 +142,8 @@ export class ImportRecordComponent { 'editor.record.importFromExternalFile.failure.body' )} ${error.message ?? ''}`, }, - 2500 + 2500, + error ) this.isRecordImportInProgress = false this.cdr.markForCheck() diff --git a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.spec.ts b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.spec.ts index ce65f266c4..0ad7fa1c75 100644 --- a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.spec.ts +++ b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.spec.ts @@ -143,12 +143,16 @@ describe('FormFieldOnlineLinkResourcesComponent', () => { expect(component.uploadProgress).toBeUndefined() component.handleFileChange(file) uploadSubject.error(new Error('something went wrong')) - expect(notificationsService.showNotification).toHaveBeenCalledWith({ - type: 'error', - closeMessage: 'editor.record.onlineResourceError.closeMessage', - text: 'editor.record.onlineResourceError.body something went wrong', - title: 'editor.record.onlineResourceError.title', - }) + expect(notificationsService.showNotification).toHaveBeenCalledWith( + { + type: 'error', + closeMessage: 'editor.record.onlineResourceError.closeMessage', + text: 'editor.record.onlineResourceError.body something went wrong', + title: 'editor.record.onlineResourceError.title', + }, + undefined, + expect.any(Error) + ) }) }) describe('handleUploadCancel', () => { diff --git a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.ts b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.ts index 3000324226..97cc3c138a 100644 --- a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.ts +++ b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.ts @@ -137,18 +137,22 @@ export class FormFieldOnlineLinkResourcesComponent { private handleError(error: Error) { this.uploadProgress = undefined this.cd.detectChanges() - this.notificationsService.showNotification({ - type: 'error', - title: this.translateService.instant( - 'editor.record.onlineResourceError.title' - ), - text: `${this.translateService.instant( - 'editor.record.onlineResourceError.body' - )} ${error.message}`, - closeMessage: this.translateService.instant( - 'editor.record.onlineResourceError.closeMessage' - ), - }) + this.notificationsService.showNotification( + { + type: 'error', + title: this.translateService.instant( + 'editor.record.onlineResourceError.title' + ), + text: `${this.translateService.instant( + 'editor.record.onlineResourceError.body' + )} ${error.message}`, + closeMessage: this.translateService.instant( + 'editor.record.onlineResourceError.closeMessage' + ), + }, + undefined, + error + ) } private openEditDialog(resource: OnlineLinkResource, index: number) { diff --git a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-resources/form-field-online-resources.component.ts b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-resources/form-field-online-resources.component.ts index 6079095018..8aaca6802d 100644 --- a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-resources/form-field-online-resources.component.ts +++ b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-resources/form-field-online-resources.component.ts @@ -192,18 +192,22 @@ export class FormFieldOnlineResourcesComponent { private handleError(error: Error) { this.uploadProgress = undefined - this.notificationsService.showNotification({ - type: 'error', - title: this.translateService.instant( - 'editor.record.onlineResourceError.title' - ), - text: `${this.translateService.instant( - 'editor.record.onlineResourceError.body' - )} ${error.message}`, - closeMessage: this.translateService.instant( - 'editor.record.onlineResourceError.closeMessage' - ), - }) + this.notificationsService.showNotification( + { + type: 'error', + title: this.translateService.instant( + 'editor.record.onlineResourceError.title' + ), + text: `${this.translateService.instant( + 'editor.record.onlineResourceError.body' + )} ${error.message}`, + closeMessage: this.translateService.instant( + 'editor.record.onlineResourceError.closeMessage' + ), + }, + undefined, + error + ) } private openEditDialog(resource: OnlineNotLinkResource, index: number) { diff --git a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.spec.ts b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.spec.ts index a18dab7012..d3555ed6d7 100644 --- a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.spec.ts +++ b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.spec.ts @@ -119,12 +119,16 @@ describe('FormFieldOverviewsComponent', () => { expect(component.uploadProgress).toBeUndefined() component.handleFileChange(file) uploadSubject.error(new Error('something went wrong')) - expect(notificationsService.showNotification).toHaveBeenCalledWith({ - type: 'error', - closeMessage: 'editor.record.resourceError.closeMessage', - text: 'editor.record.resourceError.body something went wrong', - title: 'editor.record.resourceError.title', - }) + expect(notificationsService.showNotification).toHaveBeenCalledWith( + { + type: 'error', + closeMessage: 'editor.record.resourceError.closeMessage', + text: 'editor.record.resourceError.body something went wrong', + title: 'editor.record.resourceError.title', + }, + undefined, + expect.any(Error) + ) }) }) diff --git a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.ts b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.ts index ae56250a87..3e4c25db48 100644 --- a/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.ts +++ b/libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.ts @@ -109,15 +109,21 @@ export class FormFieldOverviewsComponent { private handleError = (error: Error) => { this.uploadProgress = undefined this.cd.markForCheck() - this.notificationsService.showNotification({ - type: 'error', - title: this.translateService.instant('editor.record.resourceError.title'), - text: `${this.translateService.instant( - 'editor.record.resourceError.body' - )} ${error.message}`, - closeMessage: this.translateService.instant( - 'editor.record.resourceError.closeMessage' - ), - }) + this.notificationsService.showNotification( + { + type: 'error', + title: this.translateService.instant( + 'editor.record.resourceError.title' + ), + text: `${this.translateService.instant( + 'editor.record.resourceError.body' + )} ${error.message}`, + closeMessage: this.translateService.instant( + 'editor.record.resourceError.closeMessage' + ), + }, + undefined, + error + ) } } diff --git a/libs/feature/notifications/src/lib/notifications.service.ts b/libs/feature/notifications/src/lib/notifications.service.ts index cbf38e52c2..b7281989ff 100644 --- a/libs/feature/notifications/src/lib/notifications.service.ts +++ b/libs/feature/notifications/src/lib/notifications.service.ts @@ -10,7 +10,12 @@ type NotificationWithIdentity = NotificationContent & { id: number } export class NotificationsService { notifications$ = new BehaviorSubject([]) - showNotification(content: NotificationContent, timeoutMs?: number) { + showNotification( + content: NotificationContent, + timeoutMs?: number, + error?: Error + ) { + error && console.error(error) const id = Math.floor(Math.random() * 1000000) this.notifications$.next([...this.notifications$.value, { ...content, id }]) if (typeof timeoutMs === 'undefined') return diff --git a/libs/feature/search/src/lib/results-table/results-table-container.component.spec.ts b/libs/feature/search/src/lib/results-table/results-table-container.component.spec.ts index 85e63b4d85..6504bea923 100644 --- a/libs/feature/search/src/lib/results-table/results-table-container.component.spec.ts +++ b/libs/feature/search/src/lib/results-table/results-table-container.component.spec.ts @@ -120,12 +120,16 @@ describe('ResultsTableContainerComponent', () => { throwError(() => 'oopsie') ) component.handleDeleteRecord(datasetRecordsFixture()[0]) - expect(notificationsService.showNotification).toHaveBeenCalledWith({ - type: 'error', - title: 'editor.record.deleteError.title', - text: 'editor.record.deleteError.body oopsie', - closeMessage: 'editor.record.deleteError.closeMessage', - }) + expect(notificationsService.showNotification).toHaveBeenCalledWith( + { + type: 'error', + title: 'editor.record.deleteError.title', + text: 'editor.record.deleteError.body oopsie', + closeMessage: 'editor.record.deleteError.closeMessage', + }, + undefined, + 'oopsie' + ) }) }) diff --git a/libs/feature/search/src/lib/results-table/results-table-container.component.ts b/libs/feature/search/src/lib/results-table/results-table-container.component.ts index 43afbe95bc..b2f75f483b 100644 --- a/libs/feature/search/src/lib/results-table/results-table-container.component.ts +++ b/libs/feature/search/src/lib/results-table/results-table-container.component.ts @@ -79,18 +79,22 @@ export class ResultsTableContainerComponent implements OnDestroy { ) }, error: (error) => { - this.notificationsService.showNotification({ - type: 'error', - title: this.translateService.instant( - 'editor.record.deleteError.title' - ), - text: `${this.translateService.instant( - 'editor.record.deleteError.body' - )} ${error}`, - closeMessage: this.translateService.instant( - 'editor.record.deleteError.closeMessage' - ), - }) + this.notificationsService.showNotification( + { + type: 'error', + title: this.translateService.instant( + 'editor.record.deleteError.title' + ), + text: `${this.translateService.instant( + 'editor.record.deleteError.body' + )} ${error}`, + closeMessage: this.translateService.instant( + 'editor.record.deleteError.closeMessage' + ), + }, + undefined, + error + ) }, }) ) From 07dcd58b154bf47f0bf0d5e76f001e703123cbef Mon Sep 17 00:00:00 2001 From: Tobias Kohr Date: Thu, 12 Dec 2024 16:27:48 +0100 Subject: [PATCH 2/9] fix(edit-page): do not navigate to edit during create since this provokes calling the edit-record.resolver which needs a metadata xml too early and results in an exception note: bug ccould be reproduced from any second access to /create --- .../src/app/edit/edit-page.component.spec.ts | 16 ---------------- .../src/app/edit/edit-page.component.ts | 10 ---------- 2 files changed, 26 deletions(-) diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts index e06c44c5d4..e9fd2794ae 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts @@ -165,22 +165,6 @@ describe('EditPageComponent', () => { }) }) - describe('new record', () => { - beforeEach(() => { - const activatedRoute = TestBed.inject(ActivatedRoute) - activatedRoute.snapshot.routeConfig.path = '/create' - fixture.detectChanges() - }) - it('navigate from /create to /edit/uuid on first change', () => { - const router = TestBed.inject(Router) - const navigateSpy = jest.spyOn(router, 'navigate') - ;(facade.draftSaveSuccess$ as any).next() - expect(navigateSpy).toHaveBeenCalledWith(['edit', 'my-dataset-001'], { - replaceUrl: true, - }) - }) - }) - describe('unique identifier of the current record changes', () => { beforeEach(() => { fixture.detectChanges() diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.ts b/apps/metadata-editor/src/app/edit/edit-page.component.ts index 6e6753275e..9babe0aa39 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.ts @@ -137,16 +137,6 @@ export class EditPageComponent implements OnInit, OnDestroy { ) }) ) - - // if we're on the /create route, go to /edit/{uuid} on first change - if (this.route.snapshot.routeConfig?.path.includes('create')) { - this.facade.draftSaveSuccess$.pipe(take(1)).subscribe(() => { - this.router.navigate(['edit', currentRecord.uniqueIdentifier], { - replaceUrl: true, - }) - }) - } - // if the record unique identifier changes, navigate to /edit/newUuid this.facade.record$ .pipe( From a97b566ab19a4a98b0ef3831aa8da821642a4a49 Mon Sep 17 00:00:00 2001 From: Tobias Kohr Date: Thu, 12 Dec 2024 16:31:54 +0100 Subject: [PATCH 3/9] fix(edit-page): do not navigate to edit with new id when uniqueIdentifier changes this causes exceptions when modifying fields in a draft. it does not seem to be used anymore, since the uniqueIdentifier has been replaced by the resourceIdentifier --- .../src/app/edit/edit-page.component.spec.ts | 15 --------------- .../src/app/edit/edit-page.component.ts | 12 ------------ 2 files changed, 27 deletions(-) diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts index e9fd2794ae..bd9ba22fd9 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts @@ -165,21 +165,6 @@ describe('EditPageComponent', () => { }) }) - describe('unique identifier of the current record changes', () => { - beforeEach(() => { - fixture.detectChanges() - }) - it('navigates to /edit/newUuid', () => { - const router = TestBed.inject(Router) - const navigateSpy = jest.spyOn(router, 'navigate') - ;(facade.record$ as any).next({ - ...datasetRecordsFixture()[0], - uniqueIdentifier: 'new-uuid', - }) - expect(navigateSpy).toHaveBeenCalledWith(['edit', 'new-uuid']) - }) - }) - describe('isLastPage$', () => { let editorFacade: EditorFacadeMock beforeEach(() => { diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.ts b/apps/metadata-editor/src/app/edit/edit-page.component.ts index 9babe0aa39..456da41f47 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.ts @@ -137,18 +137,6 @@ export class EditPageComponent implements OnInit, OnDestroy { ) }) ) - // if the record unique identifier changes, navigate to /edit/newUuid - this.facade.record$ - .pipe( - filter( - (record) => - record?.uniqueIdentifier !== currentRecord.uniqueIdentifier - ), - take(1) - ) - .subscribe((savedRecord) => { - this.router.navigate(['edit', savedRecord.uniqueIdentifier]) - }) } ngOnDestroy() { From 5050de293da227e068117cf45061746e9b17fcc0 Mon Sep 17 00:00:00 2001 From: Olivia Guyot Date: Tue, 17 Dec 2024 13:16:59 +0100 Subject: [PATCH 4/9] Revert "fix(edit-page): do not navigate to edit with new id when uniqueIdentifier changes" This reverts commit a97b566ab19a4a98b0ef3831aa8da821642a4a49. --- .../src/app/edit/edit-page.component.spec.ts | 15 +++++++++++++++ .../src/app/edit/edit-page.component.ts | 12 ++++++++++++ 2 files changed, 27 insertions(+) diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts index bd9ba22fd9..e9fd2794ae 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts @@ -165,6 +165,21 @@ describe('EditPageComponent', () => { }) }) + describe('unique identifier of the current record changes', () => { + beforeEach(() => { + fixture.detectChanges() + }) + it('navigates to /edit/newUuid', () => { + const router = TestBed.inject(Router) + const navigateSpy = jest.spyOn(router, 'navigate') + ;(facade.record$ as any).next({ + ...datasetRecordsFixture()[0], + uniqueIdentifier: 'new-uuid', + }) + expect(navigateSpy).toHaveBeenCalledWith(['edit', 'new-uuid']) + }) + }) + describe('isLastPage$', () => { let editorFacade: EditorFacadeMock beforeEach(() => { diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.ts b/apps/metadata-editor/src/app/edit/edit-page.component.ts index 456da41f47..9babe0aa39 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.ts @@ -137,6 +137,18 @@ export class EditPageComponent implements OnInit, OnDestroy { ) }) ) + // if the record unique identifier changes, navigate to /edit/newUuid + this.facade.record$ + .pipe( + filter( + (record) => + record?.uniqueIdentifier !== currentRecord.uniqueIdentifier + ), + take(1) + ) + .subscribe((savedRecord) => { + this.router.navigate(['edit', savedRecord.uniqueIdentifier]) + }) } ngOnDestroy() { From cdba4703a354c64c0ccdef93c88db71d809a3a9c Mon Sep 17 00:00:00 2001 From: Olivia Guyot Date: Tue, 17 Dec 2024 13:17:08 +0100 Subject: [PATCH 5/9] Revert "fix(edit-page): do not navigate to edit during create" This reverts commit 07dcd58b154bf47f0bf0d5e76f001e703123cbef. --- .../src/app/edit/edit-page.component.spec.ts | 16 ++++++++++++++++ .../src/app/edit/edit-page.component.ts | 10 ++++++++++ 2 files changed, 26 insertions(+) diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts index e9fd2794ae..e06c44c5d4 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts @@ -165,6 +165,22 @@ describe('EditPageComponent', () => { }) }) + describe('new record', () => { + beforeEach(() => { + const activatedRoute = TestBed.inject(ActivatedRoute) + activatedRoute.snapshot.routeConfig.path = '/create' + fixture.detectChanges() + }) + it('navigate from /create to /edit/uuid on first change', () => { + const router = TestBed.inject(Router) + const navigateSpy = jest.spyOn(router, 'navigate') + ;(facade.draftSaveSuccess$ as any).next() + expect(navigateSpy).toHaveBeenCalledWith(['edit', 'my-dataset-001'], { + replaceUrl: true, + }) + }) + }) + describe('unique identifier of the current record changes', () => { beforeEach(() => { fixture.detectChanges() diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.ts b/apps/metadata-editor/src/app/edit/edit-page.component.ts index 9babe0aa39..6e6753275e 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.ts @@ -137,6 +137,16 @@ export class EditPageComponent implements OnInit, OnDestroy { ) }) ) + + // if we're on the /create route, go to /edit/{uuid} on first change + if (this.route.snapshot.routeConfig?.path.includes('create')) { + this.facade.draftSaveSuccess$.pipe(take(1)).subscribe(() => { + this.router.navigate(['edit', currentRecord.uniqueIdentifier], { + replaceUrl: true, + }) + }) + } + // if the record unique identifier changes, navigate to /edit/newUuid this.facade.record$ .pipe( From 286e34ed50bb6adac7ca2c8829e77d809a825e22 Mon Sep 17 00:00:00 2001 From: Olivia Guyot Date: Tue, 17 Dec 2024 13:26:45 +0100 Subject: [PATCH 6/9] fix(editor): add missing unsubscribe on edit-page component teardown This avoid having unwanted navigations to /edit/uuid when loading a new record for instance. --- .../src/app/edit/edit-page.component.ts | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.ts b/apps/metadata-editor/src/app/edit/edit-page.component.ts index 6e6753275e..9930b6f78f 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.ts @@ -24,7 +24,6 @@ import { combineLatest, filter, firstValueFrom, Subscription, take } from 'rxjs' import { map } from 'rxjs/operators' import { SidebarComponent } from '../dashboard/sidebar/sidebar.component' import { PageSelectorComponent } from './components/page-selector/page-selector.component' -import { PublishButtonComponent } from './components/publish-button/publish-button.component' import { TopToolbarComponent } from './components/top-toolbar/top-toolbar.component' marker('editor.record.form.bottomButtons.comeBackLater') @@ -41,7 +40,6 @@ marker('editor.record.form.bottomButtons.next') CommonModule, ButtonComponent, MatProgressSpinnerModule, - PublishButtonComponent, TopToolbarComponent, NotificationsContainerComponent, PageSelectorComponent, @@ -148,17 +146,19 @@ export class EditPageComponent implements OnInit, OnDestroy { } // if the record unique identifier changes, navigate to /edit/newUuid - this.facade.record$ - .pipe( - filter( - (record) => - record?.uniqueIdentifier !== currentRecord.uniqueIdentifier - ), - take(1) - ) - .subscribe((savedRecord) => { - this.router.navigate(['edit', savedRecord.uniqueIdentifier]) - }) + this.subscription.add( + this.facade.record$ + .pipe( + filter( + (record) => + record?.uniqueIdentifier !== currentRecord.uniqueIdentifier + ), + take(1) + ) + .subscribe((savedRecord) => { + this.router.navigate(['edit', savedRecord.uniqueIdentifier]) + }) + ) } ngOnDestroy() { From ace948a2249ab1b50423fc153af2fe39667b5353 Mon Sep 17 00:00:00 2001 From: Tobias Kohr Date: Tue, 17 Dec 2024 15:15:24 +0100 Subject: [PATCH 7/9] fix(edit-page): add missing unsubscribe when on /create --- .../src/app/edit/edit-page.component.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.ts b/apps/metadata-editor/src/app/edit/edit-page.component.ts index 9930b6f78f..bead8d580e 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.ts @@ -138,11 +138,13 @@ export class EditPageComponent implements OnInit, OnDestroy { // if we're on the /create route, go to /edit/{uuid} on first change if (this.route.snapshot.routeConfig?.path.includes('create')) { - this.facade.draftSaveSuccess$.pipe(take(1)).subscribe(() => { - this.router.navigate(['edit', currentRecord.uniqueIdentifier], { - replaceUrl: true, + this.subscription.add( + this.facade.draftSaveSuccess$.pipe(take(1)).subscribe(() => { + this.router.navigate(['edit', currentRecord.uniqueIdentifier], { + replaceUrl: true, + }) }) - }) + ) } // if the record unique identifier changes, navigate to /edit/newUuid From f329c1314a0d0631a76e6a597136ff1526693853 Mon Sep 17 00:00:00 2001 From: Tobias Kohr Date: Tue, 17 Dec 2024 15:16:30 +0100 Subject: [PATCH 8/9] test(edit-page): test component's subscriptions --- .../src/app/edit/edit-page.component.spec.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts index e06c44c5d4..cee46e3c67 100644 --- a/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts +++ b/apps/metadata-editor/src/app/edit/edit-page.component.spec.ts @@ -212,4 +212,25 @@ describe('EditPageComponent', () => { expect(await firstValueFrom(component.isLastPage$)).toBe(false) }) }) + + describe('subscriptions', () => { + it('should add 3 subscriptions to component.subscription', () => { + const addSpy = jest.spyOn(component.subscription, 'add') + component.ngOnInit() + expect(addSpy).toHaveBeenCalledTimes(3) + }) + it('should add 4 subscriptions to component.subscription when on /create route', () => { + const activatedRoute = TestBed.inject(ActivatedRoute) + activatedRoute.snapshot.routeConfig.path = '/create' + fixture.detectChanges() + const addSpy = jest.spyOn(component.subscription, 'add') + component.ngOnInit() + expect(addSpy).toHaveBeenCalledTimes(4) + }) + it('unsubscribes', () => { + const unsubscribeSpy = jest.spyOn(component.subscription, 'unsubscribe') + component.ngOnDestroy() + expect(unsubscribeSpy).toHaveBeenCalled() + }) + }) }) From ace56df74f4da51ead9b89af38bcf11a9cfe0060 Mon Sep 17 00:00:00 2001 From: Tobias Kohr Date: Thu, 19 Dec 2024 09:23:07 +0100 Subject: [PATCH 9/9] chore(import-record): clean unneeded import --- .../lib/components/import-record/import-record.component.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/feature/editor/src/lib/components/import-record/import-record.component.spec.ts b/libs/feature/editor/src/lib/components/import-record/import-record.component.spec.ts index 934309aea4..d55480972d 100644 --- a/libs/feature/editor/src/lib/components/import-record/import-record.component.spec.ts +++ b/libs/feature/editor/src/lib/components/import-record/import-record.component.spec.ts @@ -7,7 +7,6 @@ import { NotificationsService } from '@geonetwork-ui/feature/notifications' import { RecordsRepositoryInterface } from '@geonetwork-ui/common/domain/repository/records-repository.interface' import { of, throwError } from 'rxjs' import { MockBuilder, MockComponent, MockModule, MockProviders } from 'ng-mocks' -import exp from 'constants' describe('ImportRecordComponent', () => { let component: ImportRecordComponent