Skip to content

Commit 1444271

Browse files
alexandrevryghemalexandrevryghem
committed
115046: Fixed edit item bitstream tab buttons not updating correctly & removed method calls retuning observables in edit collection's content source tab
1 parent 813db7c commit 1444271

6 files changed

Lines changed: 58 additions & 54 deletions

File tree

‎src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
<div class="container-fluid">
22
<div class="d-inline-block float-right space-children-mr">
3-
<button class=" btn btn-danger" *ngIf="!(isReinstatable() | async)"
4-
[disabled]="!(hasChanges() | async)"
3+
<button class=" btn btn-danger" *ngIf="(isReinstatable$ | async) !== true"
4+
[disabled]="(hasChanges$ | async) !== true"
55
(click)="discard()"><i
66
class="fas fa-times"></i>
77
<span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.discard-button" | translate}}</span>
88
</button>
9-
<button class="btn btn-warning" *ngIf="isReinstatable() | async"
9+
<button class="btn btn-warning" *ngIf="isReinstatable$ | async"
1010
(click)="reinstate()"><i
1111
class="fas fa-undo-alt"></i>
1212
<span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.reinstate-button" | translate}}</span>
1313
</button>
1414
<button class="btn btn-primary"
15-
[disabled]="!(hasChanges() | async) || !isValid() || (initialHarvestType === harvestTypeNone && contentSource.harvestType === initialHarvestType)"
15+
[disabled]="(hasChanges$ | async) !== true || !isValid() || (initialHarvestType === harvestTypeNone && contentSource.harvestType === initialHarvestType)"
1616
(click)="onSubmit()"><i
1717
class="fas fa-save"></i>
1818
<span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.save-button" | translate}}</span>
@@ -44,19 +44,19 @@ <h4 *ngIf="contentSource && (contentSource?.harvestType !== harvestTypeNone)">{{
4444
<div class="row">
4545
<div class="col-12">
4646
<div class="d-inline-block float-right ml-1 space-children-mr">
47-
<button class=" btn btn-danger" *ngIf="!(isReinstatable() | async)"
48-
[disabled]="!(hasChanges() | async)"
47+
<button class=" btn btn-danger" *ngIf="(isReinstatable$ | async) !== true"
48+
[disabled]="(hasChanges$ | async) !== true"
4949
(click)="discard()"><i
5050
class="fas fa-times"></i>
5151
<span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.discard-button" | translate}}</span>
5252
</button>
53-
<button class="btn btn-warning" *ngIf="isReinstatable() | async"
53+
<button class="btn btn-warning" *ngIf="isReinstatable$ | async"
5454
(click)="reinstate()"><i
5555
class="fas fa-undo-alt"></i>
5656
<span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.reinstate-button" | translate}}</span>
5757
</button>
5858
<button class="btn btn-primary"
59-
[disabled]="!(hasChanges() | async) || !isValid() || (initialHarvestType === harvestTypeNone && contentSource.harvestType === initialHarvestType)"
59+
[disabled]="(hasChanges$ | async) !== true || !isValid() || (initialHarvestType === harvestTypeNone && contentSource.harvestType === initialHarvestType)"
6060
(click)="onSubmit()"><i
6161
class="fas fa-save"></i>
6262
<span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.save-button" | translate}}</span>
@@ -66,7 +66,7 @@ <h4 *ngIf="contentSource && (contentSource?.harvestType !== harvestTypeNone)">{{
6666
</div>
6767
</div>
6868
<ds-collection-source-controls
69-
[isEnabled]="!(hasChanges()|async)"
69+
[isEnabled]="(hasChanges$ | async) !== true"
7070
[shouldShow]="contentSource?.harvestType !== harvestTypeNone"
7171
[collection]="(collectionRD$ |async)?.payload"
7272
>

‎src/app/collection-page/edit-collection-page/collection-source/collection-source.component.ts‎

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,6 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem
204204
*/
205205
formGroup: UntypedFormGroup;
206206

207-
/**
208-
* Subscription to update the current form
209-
*/
210-
updateSub: Subscription;
211-
212207
/**
213208
* The content harvesting type used when harvesting is disabled
214209
*/
@@ -228,28 +223,29 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem
228223
*/
229224
displayedNotifications: INotification[] = [];
230225

231-
public constructor(public objectUpdatesService: ObjectUpdatesService,
232-
public notificationsService: NotificationsService,
233-
protected location: Location,
234-
protected formService: DynamicFormService,
235-
protected translate: TranslateService,
236-
protected route: ActivatedRoute,
237-
protected router: Router,
238-
protected collectionService: CollectionDataService,
239-
protected requestService: RequestService) {
240-
super(objectUpdatesService, notificationsService, translate);
226+
subs: Subscription[] = [];
227+
228+
public constructor(
229+
public objectUpdatesService: ObjectUpdatesService,
230+
public notificationsService: NotificationsService,
231+
public translateService: TranslateService,
232+
public router: Router,
233+
protected location: Location,
234+
protected formService: DynamicFormService,
235+
protected route: ActivatedRoute,
236+
protected collectionService: CollectionDataService,
237+
protected requestService: RequestService,
238+
) {
239+
super(objectUpdatesService, notificationsService, translateService, router);
241240
}
242241

243242
/**
244243
* Initialize properties to setup the Field Update and Form
245244
*/
246245
ngOnInit(): void {
246+
super.ngOnInit();
247247
this.notificationsPrefix = 'collection.edit.tabs.source.notifications.';
248248
this.discardTimeOut = environment.collection.edit.undoTimeout;
249-
this.url = this.router.url;
250-
if (this.url.indexOf('?') > 0) {
251-
this.url = this.url.substr(0, this.url.indexOf('?'));
252-
}
253249
this.formGroup = this.formService.createFormGroup(this.formModel);
254250
this.collectionRD$ = this.route.parent.data.pipe(first(), map((data) => data.dso));
255251

@@ -263,10 +259,9 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem
263259
});
264260

265261
this.updateFieldTranslations();
266-
this.translate.onLangChange
267-
.subscribe(() => {
268-
this.updateFieldTranslations();
269-
});
262+
this.subs.push(this.translateService.onLangChange.subscribe(() => {
263+
this.updateFieldTranslations();
264+
}));
270265
}
271266

272267
/**
@@ -281,7 +276,7 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem
281276
this.update$ = this.objectUpdatesService.getFieldUpdates(this.url, [initialContentSource]).pipe(
282277
map((updates: FieldUpdates) => updates[initialContentSource.uuid])
283278
);
284-
this.updateSub = this.update$.subscribe((update: FieldUpdate) => {
279+
this.subs.push(this.update$.subscribe((update: FieldUpdate) => {
285280
if (update) {
286281
const field = update.field as ContentSource;
287282
let configId;
@@ -308,7 +303,7 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem
308303
}
309304
this.contentSource.metadataConfigId = configId;
310305
}
311-
});
306+
}));
312307
}
313308

314309
/**
@@ -342,18 +337,18 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem
342337
* @param fieldModel
343338
*/
344339
private updateFieldTranslation(fieldModel: DynamicFormControlModel) {
345-
fieldModel.label = this.translate.instant(this.LABEL_KEY_PREFIX + fieldModel.id);
340+
fieldModel.label = this.translateService.instant(this.LABEL_KEY_PREFIX + fieldModel.id);
346341
if (isNotEmpty(fieldModel.validators)) {
347342
fieldModel.errorMessages = {};
348343
Object.keys(fieldModel.validators).forEach((key) => {
349-
fieldModel.errorMessages[key] = this.translate.instant(this.ERROR_KEY_PREFIX + fieldModel.id + '.' + key);
344+
fieldModel.errorMessages[key] = this.translateService.instant(this.ERROR_KEY_PREFIX + fieldModel.id + '.' + key);
350345
});
351346
}
352347
if (fieldModel instanceof DynamicOptionControlModel) {
353348
if (isNotEmpty(fieldModel.options)) {
354349
fieldModel.options.forEach((option) => {
355350
if (hasNoValue(option.label)) {
356-
option.label = this.translate.instant(this.OPTIONS_KEY_PREFIX + fieldModel.id + '.' + option.value);
351+
option.label = this.translateService.instant(this.OPTIONS_KEY_PREFIX + fieldModel.id + '.' + option.value);
357352
}
358353
});
359354
}
@@ -470,8 +465,6 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem
470465
* Make sure open subscriptions are closed
471466
*/
472467
ngOnDestroy(): void {
473-
if (this.updateSub) {
474-
this.updateSub.unsubscribe();
475-
}
468+
this.subs.forEach((sub: Subscription) => sub.unsubscribe());
476469
}
477470
}

‎src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,6 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
3535
*/
3636
updates$: Observable<FieldUpdates>;
3737

38-
hasChanges$: Observable<boolean>;
39-
40-
isReinstatable$: Observable<boolean>;
41-
4238
/**
4339
* Route to the item's page
4440
*/
@@ -58,7 +54,7 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
5854
public translateService: TranslateService,
5955
public route: ActivatedRoute
6056
) {
61-
super(objectUpdatesService, notificationsService, translateService);
57+
super(objectUpdatesService, notificationsService, translateService, router);
6258
}
6359

6460
/**
@@ -83,11 +79,9 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
8379
this.setItem(rd.payload);
8480
});
8581
}
82+
super.ngOnInit();
8683

8784
this.discardTimeOut = environment.item.edit.undoTimeout;
88-
this.url = this.router.url.split('?')[0];
89-
this.hasChanges$ = this.hasChanges();
90-
this.isReinstatable$ = this.isReinstatable();
9185
this.hasChanges().pipe(first()).subscribe((hasChanges) => {
9286
if (!hasChanges) {
9387
this.initializeOriginalFields();

‎src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { ChangeDetectorRef, Component, NgZone, OnDestroy } from '@angular/core';
22
import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component';
33
import { filter, map, switchMap, take } from 'rxjs/operators';
4-
import { Observable, Subscription, zip as observableZip } from 'rxjs';
4+
import { Observable, Subscription, zip as observableZip, combineLatest } from 'rxjs';
55
import { ItemDataService } from '../../../core/data/item-data.service';
66
import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service';
77
import { ActivatedRoute, Router } from '@angular/router';
@@ -222,7 +222,7 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
222222
*/
223223
isReinstatable(): Observable<boolean> {
224224
return this.bundles$.pipe(
225-
switchMap((bundles: Bundle[]) => observableZip(...bundles.map((bundle: Bundle) => this.objectUpdatesService.isReinstatable(bundle.self)))),
225+
switchMap((bundles: Bundle[]) => combineLatest(bundles.map((bundle: Bundle) => this.objectUpdatesService.isReinstatable(bundle.self)))),
226226
map((reinstatable: boolean[]) => reinstatable.includes(true))
227227
);
228228
}
@@ -232,7 +232,7 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
232232
*/
233233
hasChanges(): Observable<boolean> {
234234
return this.bundles$.pipe(
235-
switchMap((bundles: Bundle[]) => observableZip(...bundles.map((bundle: Bundle) => this.objectUpdatesService.hasUpdates(bundle.self)))),
235+
switchMap((bundles: Bundle[]) => combineLatest(bundles.map((bundle: Bundle) => this.objectUpdatesService.hasUpdates(bundle.self)))),
236236
map((hasChanges: boolean[]) => hasChanges.includes(true))
237237
);
238238
}

‎src/app/shared/trackable/abstract-trackable.component.spec.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { NotificationsService } from '../notifications/notifications.service';
99
import { NO_ERRORS_SCHEMA } from '@angular/core';
1010
import { TestScheduler } from 'rxjs/testing';
1111
import { getTestScheduler } from 'jasmine-marbles';
12+
import { RouterStub } from '../testing/router.stub';
13+
import { Router } from '@angular/router';
1214

1315
describe('AbstractTrackableComponent', () => {
1416
let comp: AbstractTrackableComponent;
@@ -27,6 +29,7 @@ describe('AbstractTrackableComponent', () => {
2729
success: successNotification
2830
}
2931
);
32+
let router: RouterStub;
3033

3134
const url = 'http://test-url.com/test-url';
3235

@@ -42,6 +45,8 @@ describe('AbstractTrackableComponent', () => {
4245
isValidPage: observableOf(true)
4346
}
4447
);
48+
router = new RouterStub();
49+
router.url = url;
4550

4651
scheduler = getTestScheduler();
4752

@@ -51,6 +56,7 @@ describe('AbstractTrackableComponent', () => {
5156
providers: [
5257
{ provide: ObjectUpdatesService, useValue: objectUpdatesService },
5358
{ provide: NotificationsService, useValue: notificationsService },
59+
{ provide: Router, useValue: router },
5460
], schemas: [
5561
NO_ERRORS_SCHEMA
5662
]
@@ -60,7 +66,6 @@ describe('AbstractTrackableComponent', () => {
6066
beforeEach(() => {
6167
fixture = TestBed.createComponent(AbstractTrackableComponent);
6268
comp = fixture.componentInstance;
63-
comp.url = url;
6469

6570
fixture.detectChanges();
6671
});

‎src/app/shared/trackable/abstract-trackable.component.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import { ObjectUpdatesService } from '../../core/data/object-updates/object-upda
22
import { NotificationsService } from '../notifications/notifications.service';
33
import { TranslateService } from '@ngx-translate/core';
44
import { Observable } from 'rxjs';
5-
import { Component } from '@angular/core';
5+
import { Component, OnInit } from '@angular/core';
6+
import { Router } from '@angular/router';
67

78
/**
89
* Abstract Component that is able to track changes made in the inheriting component using the ObjectUpdateService
@@ -11,7 +12,7 @@ import { Component } from '@angular/core';
1112
selector: 'ds-abstract-trackable',
1213
template: ''
1314
})
14-
export class AbstractTrackableComponent {
15+
export class AbstractTrackableComponent implements OnInit {
1516

1617
/**
1718
* The time span for being able to undo discarding changes
@@ -21,14 +22,25 @@ export class AbstractTrackableComponent {
2122
public url: string;
2223
public notificationsPrefix = 'static-pages.form.notification';
2324

25+
hasChanges$: Observable<boolean>;
26+
27+
isReinstatable$: Observable<boolean>;
28+
2429
constructor(
2530
public objectUpdatesService: ObjectUpdatesService,
2631
public notificationsService: NotificationsService,
2732
public translateService: TranslateService,
33+
public router: Router,
2834
) {
2935

3036
}
3137

38+
ngOnInit(): void {
39+
this.url = this.router.url.split('?')[0];
40+
this.hasChanges$ = this.hasChanges();
41+
this.isReinstatable$ = this.isReinstatable();
42+
}
43+
3244
/**
3345
* Request the object updates service to discard all current changes to this item
3446
* Shows a notification to remind the user that they can undo this

0 commit comments

Comments
 (0)