From a70817559c305af6a00f8e6d3eb5e759259e1cb9 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Fri, 14 Jun 2024 22:56:57 +0200 Subject: [PATCH] 115046: Fixed edit item bitstream tab buttons not updating correctly & removed method calls retuning observables in edit collection's content source tab --- .../bitstream-page/bitstream-page-routes.ts | 2 + .../collection-source.component.html | 18 +++--- .../collection-source.component.ts | 55 ++++++++----------- .../abstract-item-update.component.ts | 10 +--- .../item-bitstreams.component.ts | 5 +- .../abstract-trackable.component.spec.ts | 7 ++- .../trackable/abstract-trackable.component.ts | 19 ++++++- 7 files changed, 63 insertions(+), 53 deletions(-) diff --git a/src/app/bitstream-page/bitstream-page-routes.ts b/src/app/bitstream-page/bitstream-page-routes.ts index 6d5e312db5a..05de91d5003 100644 --- a/src/app/bitstream-page/bitstream-page-routes.ts +++ b/src/app/bitstream-page/bitstream-page-routes.ts @@ -24,11 +24,13 @@ export const ROUTES: Route[] = [ { // Resolve XMLUI bitstream download URLs path: 'handle/:prefix/:suffix/:filename', + component: BitstreamDownloadPageComponent, canActivate: [legacyBitstreamURLRedirectGuard], }, { // Resolve JSPUI bitstream download URLs path: ':prefix/:suffix/:sequence_id/:filename', + component: BitstreamDownloadPageComponent, canActivate: [legacyBitstreamURLRedirectGuard], }, { diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html index 22940850963..8f807c1aee8 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html @@ -1,18 +1,18 @@
- - -
diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.ts index 2d3de1c8a8d..43614e43a6c 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.ts @@ -249,11 +249,6 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem */ formGroup: UntypedFormGroup; - /** - * Subscription to update the current form - */ - updateSub: Subscription; - /** * The content harvesting type used when harvesting is disabled */ @@ -273,28 +268,29 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem */ displayedNotifications: INotification[] = []; - public constructor(public objectUpdatesService: ObjectUpdatesService, - public notificationsService: NotificationsService, - protected location: Location, - protected formService: DynamicFormService, - protected translate: TranslateService, - protected route: ActivatedRoute, - protected router: Router, - protected collectionService: CollectionDataService, - protected requestService: RequestService) { - super(objectUpdatesService, notificationsService, translate); + subs: Subscription[] = []; + + public constructor( + public objectUpdatesService: ObjectUpdatesService, + public notificationsService: NotificationsService, + public translateService: TranslateService, + public router: Router, + protected location: Location, + protected formService: DynamicFormService, + protected route: ActivatedRoute, + protected collectionService: CollectionDataService, + protected requestService: RequestService, + ) { + super(objectUpdatesService, notificationsService, translateService, router); } /** * Initialize properties to setup the Field Update and Form */ ngOnInit(): void { + super.ngOnInit(); this.notificationsPrefix = 'collection.edit.tabs.source.notifications.'; this.discardTimeOut = environment.collection.edit.undoTimeout; - this.url = this.router.url; - if (this.url.indexOf('?') > 0) { - this.url = this.url.substr(0, this.url.indexOf('?')); - } this.formGroup = this.formService.createFormGroup(this.formModel); this.collectionRD$ = this.route.parent.data.pipe(first(), map((data) => data.dso)); @@ -308,10 +304,9 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem }); this.updateFieldTranslations(); - this.translate.onLangChange - .subscribe(() => { - this.updateFieldTranslations(); - }); + this.subs.push(this.translateService.onLangChange.subscribe(() => { + this.updateFieldTranslations(); + })); } /** @@ -326,7 +321,7 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem this.update$ = this.objectUpdatesService.getFieldUpdates(this.url, [initialContentSource]).pipe( map((updates: FieldUpdates) => updates[initialContentSource.uuid]), ); - this.updateSub = this.update$.subscribe((update: FieldUpdate) => { + this.subs.push(this.update$.subscribe((update: FieldUpdate) => { if (update) { const field = update.field as ContentSource; let configId; @@ -353,7 +348,7 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem } this.contentSource.metadataConfigId = configId; } - }); + })); } /** @@ -387,18 +382,18 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem * @param fieldModel */ private updateFieldTranslation(fieldModel: DynamicFormControlModel) { - fieldModel.label = this.translate.instant(this.LABEL_KEY_PREFIX + fieldModel.id); + fieldModel.label = this.translateService.instant(this.LABEL_KEY_PREFIX + fieldModel.id); if (isNotEmpty(fieldModel.validators)) { fieldModel.errorMessages = {}; Object.keys(fieldModel.validators).forEach((key) => { - fieldModel.errorMessages[key] = this.translate.instant(this.ERROR_KEY_PREFIX + fieldModel.id + '.' + key); + fieldModel.errorMessages[key] = this.translateService.instant(this.ERROR_KEY_PREFIX + fieldModel.id + '.' + key); }); } if (fieldModel instanceof DynamicOptionControlModel) { if (isNotEmpty(fieldModel.options)) { fieldModel.options.forEach((option) => { if (hasNoValue(option.label)) { - option.label = this.translate.instant(this.OPTIONS_KEY_PREFIX + fieldModel.id + '.' + option.value); + option.label = this.translateService.instant(this.OPTIONS_KEY_PREFIX + fieldModel.id + '.' + option.value); } }); } @@ -515,8 +510,6 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem * Make sure open subscriptions are closed */ ngOnDestroy(): void { - if (this.updateSub) { - this.updateSub.unsubscribe(); - } + this.subs.forEach((sub: Subscription) => sub.unsubscribe()); } } diff --git a/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts b/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts index 69b234fbaf7..3829b3286c0 100644 --- a/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts +++ b/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts @@ -55,10 +55,6 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl */ updates$: Observable; - hasChanges$: Observable; - - isReinstatable$: Observable; - /** * Route to the item's page */ @@ -78,7 +74,7 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl public translateService: TranslateService, public route: ActivatedRoute, ) { - super(objectUpdatesService, notificationsService, translateService); + super(objectUpdatesService, notificationsService, translateService, router); } /** @@ -103,11 +99,9 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl this.setItem(rd.payload); }); } + super.ngOnInit(); this.discardTimeOut = environment.item.edit.undoTimeout; - this.url = this.router.url.split('?')[0]; - this.hasChanges$ = this.hasChanges(); - this.isReinstatable$ = this.isReinstatable(); this.hasChanges().pipe(first()).subscribe((hasChanges) => { if (!hasChanges) { this.initializeOriginalFields(); diff --git a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts index d96699d3c21..685ad006e03 100644 --- a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts +++ b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts @@ -20,6 +20,7 @@ import { } from '@ngx-translate/core'; import { Operation } from 'fast-json-patch'; import { + combineLatest, Observable, Subscription, zip as observableZip, @@ -273,7 +274,7 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme */ isReinstatable(): Observable { return this.bundles$.pipe( - switchMap((bundles: Bundle[]) => observableZip(...bundles.map((bundle: Bundle) => this.objectUpdatesService.isReinstatable(bundle.self)))), + switchMap((bundles: Bundle[]) => combineLatest(bundles.map((bundle: Bundle) => this.objectUpdatesService.isReinstatable(bundle.self)))), map((reinstatable: boolean[]) => reinstatable.includes(true)), ); } @@ -283,7 +284,7 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme */ hasChanges(): Observable { return this.bundles$.pipe( - switchMap((bundles: Bundle[]) => observableZip(...bundles.map((bundle: Bundle) => this.objectUpdatesService.hasUpdates(bundle.self)))), + switchMap((bundles: Bundle[]) => combineLatest(bundles.map((bundle: Bundle) => this.objectUpdatesService.hasUpdates(bundle.self)))), map((hasChanges: boolean[]) => hasChanges.includes(true)), ); } diff --git a/src/app/shared/trackable/abstract-trackable.component.spec.ts b/src/app/shared/trackable/abstract-trackable.component.spec.ts index bfd88e733ef..5667bc4b8bb 100644 --- a/src/app/shared/trackable/abstract-trackable.component.spec.ts +++ b/src/app/shared/trackable/abstract-trackable.component.spec.ts @@ -4,6 +4,7 @@ import { TestBed, waitForAsync, } from '@angular/core/testing'; +import { Router } from '@angular/router'; import { TranslateModule } from '@ngx-translate/core'; import { getTestScheduler } from 'jasmine-marbles'; import { of as observableOf } from 'rxjs'; @@ -16,6 +17,7 @@ import { } from '../notifications/models/notification.model'; import { NotificationType } from '../notifications/models/notification-type'; import { NotificationsService } from '../notifications/notifications.service'; +import { RouterStub } from '../testing/router.stub'; import { AbstractTrackableComponent } from './abstract-trackable.component'; describe('AbstractTrackableComponent', () => { @@ -35,6 +37,7 @@ describe('AbstractTrackableComponent', () => { success: successNotification, }, ); + let router: RouterStub; const url = 'http://test-url.com/test-url'; @@ -50,6 +53,8 @@ describe('AbstractTrackableComponent', () => { isValidPage: observableOf(true), }, ); + router = new RouterStub(); + router.url = url; scheduler = getTestScheduler(); @@ -58,6 +63,7 @@ describe('AbstractTrackableComponent', () => { providers: [ { provide: ObjectUpdatesService, useValue: objectUpdatesService }, { provide: NotificationsService, useValue: notificationsService }, + { provide: Router, useValue: router }, ], schemas: [ NO_ERRORS_SCHEMA, ], @@ -67,7 +73,6 @@ describe('AbstractTrackableComponent', () => { beforeEach(() => { fixture = TestBed.createComponent(AbstractTrackableComponent); comp = fixture.componentInstance; - comp.url = url; fixture.detectChanges(); }); diff --git a/src/app/shared/trackable/abstract-trackable.component.ts b/src/app/shared/trackable/abstract-trackable.component.ts index 3f6a92b95b1..ec886ccd47c 100644 --- a/src/app/shared/trackable/abstract-trackable.component.ts +++ b/src/app/shared/trackable/abstract-trackable.component.ts @@ -1,4 +1,8 @@ -import { Component } from '@angular/core'; +import { + Component, + OnInit, +} from '@angular/core'; +import { Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; import { Observable } from 'rxjs'; @@ -13,7 +17,7 @@ import { NotificationsService } from '../notifications/notifications.service'; template: '', standalone: true, }) -export class AbstractTrackableComponent { +export class AbstractTrackableComponent implements OnInit { /** * The time span for being able to undo discarding changes @@ -23,14 +27,25 @@ export class AbstractTrackableComponent { public url: string; public notificationsPrefix = 'static-pages.form.notification'; + hasChanges$: Observable; + + isReinstatable$: Observable; + constructor( public objectUpdatesService: ObjectUpdatesService, public notificationsService: NotificationsService, public translateService: TranslateService, + public router: Router, ) { } + ngOnInit(): void { + this.url = this.router.url.split('?')[0]; + this.hasChanges$ = this.hasChanges(); + this.isReinstatable$ = this.isReinstatable(); + } + /** * Request the object updates service to discard all current changes to this item * Shows a notification to remind the user that they can undo this