Refactor license contract page and improve collection display (ufal/clarin-dspace#114) (DSpace#1260)

* Display contract even if no collectionId given

lists all authorized collections and their licenses

* Proper cleanup

replaced the nested manual subscriptions in
license-contract-page.component.ts with a single RxJS pipeline using
tap(...), switchMap(...), and takeUntil(this.destroy$), then completed
cleanup in ngOnDestroy().

* fix non unique ids



* Guard license stream with EMPTY and stricter check

Guard license stream with EMPTY and stricter check

* Refactor license contract page and improve collection display (#115)

* Initial plan

* Add error/loading handling for bogus collectionId in single-collection mode



---------




---------





(cherry picked from commit 0ca1429)

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: amadulhaxxani <hassan@ufal.mff.cuni.cz>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kosarko <1842385+kosarko@users.noreply.github.com>

Author: 4 people

src/app/license-contract-page/license-contract-page.component.html

@@ -1,13 +1,48 @@
 <div class="container">
-  <div class="card" *ngVar="(collectionRD$ | async)?.payload as collection">
+  <div class="card">
     <h5 class="card-header">{{'contract.message.distribution-license-agreement' | translate}}</h5>
-    <div class="card-body">
-      <div class=" well" id="cz_cuni_mff_ufal_ContractPage_div_licenses">
-          <h3>{{collection?.name}}</h3>
+    <ng-container *ngIf="!isListMode(); else authorizedCollectionsTemplate">
+      <ng-container *ngVar="(collectionRD$ | async) as collectionRD">
+        <ds-error *ngIf="collectionRD?.hasFailed" message="{{'error.collection' | translate}}"></ds-error>
+        <ds-themed-loading *ngIf="!collectionRD || collectionRD?.isLoading" message="{{'loading.collection' | translate}}"></ds-themed-loading>
+        <div class="card-body" *ngIf="collectionRD?.payload">
+          <div class=" well" id="cz_cuni_mff_ufal_ContractPage_div_licenses">
+            <h3>{{collectionRD?.payload?.name}}</h3>
             <textarea class="form-control" cols="0" id="cz_cuni_mff_ufal_ContractPage_field_license_text_18"
                       name="license_text_18" readonly
                       rows="34">{{(licenseRD$ | async)?.payload?.text}}</textarea>
-      </div>
-    </div>
+          </div>
+        </div>
+      </ng-container>
+    </ng-container>
   </div>
 </div>
+
+<ng-template #authorizedCollectionsTemplate>
+  <ng-container *ngVar="(collectionsRD$ | async) as collectionsRD">
+    <ds-pagination *ngIf="collectionsRD?.payload?.totalElements > 0 || collectionsRD?.payload?.page?.length > 0"
+                   [paginationOptions]="pageConfig"
+                   [collectionSize]="collectionsRD?.payload?.totalElements"
+                   [hideGear]="true"
+                   [hidePagerWhenSinglePage]="true">
+      <div class="card-body" *ngFor="let collection of collectionsRD?.payload?.page; let i = index">
+        <div class=" well" id="cz_cuni_mff_ufal_ContractPage_div_licenses_{{i}}">
+          <h3>{{collection?.name}}</h3>
+          <textarea class="form-control"
+                    cols="0"
+                    readonly
+                    rows="34">{{(collection?.license | async)?.payload?.text}}</textarea>
+        </div>
+      </div>
+    </ds-pagination>
+
+    <div *ngIf="collectionsRD?.payload?.totalElements === 0 || collectionsRD?.payload?.page?.length === 0"
+         class="alert alert-info mt-3"
+         role="alert">
+      {{'collection.select.empty' | translate}}
+    </div>
+
+    <ds-error *ngIf="collectionsRD?.hasFailed" message="{{'error.collections' | translate}}"></ds-error>
+    <ds-themed-loading *ngIf="!collectionsRD || collectionsRD?.isLoading" message="{{'loading.collections' | translate}}"></ds-themed-loading>
+  </ng-container>
+</ng-template>

src/app/license-contract-page/license-contract-page.component.spec.ts

@@ -5,45 +5,107 @@ import { CommonModule } from '@angular/common';
 import { FormsModule, ReactiveFormsModule } from '@angular/forms';
 import { TranslateModule } from '@ngx-translate/core';
 import { ActivatedRoute, Params } from '@angular/router';
-import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
+import { Directive, Input, NO_ERRORS_SCHEMA } from '@angular/core';
+import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
 import { CollectionDataService } from '../core/data/collection-data.service';
 import { Collection } from '../core/shared/collection.model';
-import { mockLicenseRD$ } from '../shared/testing/clarin-license-mock';
-import { take } from 'rxjs/operators';
-import { getFirstCompletedRemoteData } from '../core/shared/operators';
+import { License } from '../core/shared/license.model';
+import { PaginationService } from '../core/pagination/pagination.service';
+import { buildPaginatedList } from '../core/data/paginated-list.model';
+import { PageInfo } from '../core/shared/page-info.model';
+import { of as observableOf } from 'rxjs';
+import { FindListOptions } from '../core/data/find-list-options.model';
+import { PaginationComponentOptions } from '../shared/pagination/pagination-component-options.model';
+import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model';
+
+/* eslint-disable @angular-eslint/directive-selector */
+@Directive({
+  selector: '[ngVar]'
+})
+class MockNgVarDirective {
+  @Input() ngVar: unknown;
+}
 
 describe('LicenseContractPageComponent', () => {
   let component: LicenseContractPageComponent;
   let fixture: ComponentFixture<LicenseContractPageComponent>;
 
-  let collection: Collection;
-
   let routeStub: any;
-  let collectionService: CollectionDataService;
+  let collectionService: jasmine.SpyObj<CollectionDataService>;
+  let paginationService: jasmine.SpyObj<PaginationService>;
 
   const paramCollectionId = 'collectionId';
   const paramCollectionIdValue = '1';
 
   const paramObject: Params = {};
   paramObject[paramCollectionId] = paramCollectionIdValue;
 
-  collection = Object.assign(new Collection(), {
+  const singleCollectionLicense = Object.assign(new License(), {
+    text: 'Single collection license text'
+  });
+
+  const collection = Object.assign(new Collection(), {
     uuid: 'fake-collection-id',
+    name: 'Single collection',
     _links: {
       self: {href: 'collection-selflink'},
       license: {href: 'license-link'}
     },
-    license: mockLicenseRD$
+    license: createSuccessfulRemoteDataObject$(singleCollectionLicense)
   });
 
+  const secondCollectionLicense = Object.assign(new License(), {
+    text: 'Second collection license text'
+  });
+
+  const authorizedCollections = [
+    collection,
+    Object.assign(new Collection(), {
+      uuid: 'second-collection-id',
+      name: 'Second collection',
+      _links: {
+        self: { href: 'second-collection-selflink' },
+        license: { href: 'second-license-link' }
+      },
+      license: createSuccessfulRemoteDataObject$(secondCollectionLicense)
+    })
+  ];
+
+  const authorizedCollectionsRD$ = createSuccessfulRemoteDataObject$(
+    buildPaginatedList(
+      new PageInfo({
+        currentPage: 1,
+        elementsPerPage: 10,
+        totalElements: authorizedCollections.length,
+        totalPages: 1
+      }),
+      authorizedCollections
+    )
+  );
+
   routeStub = {
     snapshot: {
-      queryParams: paramObject,
+      queryParams: { ...paramObject },
     }
   };
 
-  collectionService = jasmine.createSpyObj('collectionService', {
-    findById: createSuccessfulRemoteDataObject$(collection)
+  collectionService = jasmine.createSpyObj<CollectionDataService>('collectionService', {
+    findById: createSuccessfulRemoteDataObject$(collection),
+    getAuthorizedCollection: authorizedCollectionsRD$
+  });
+
+  paginationService = jasmine.createSpyObj<PaginationService>('paginationService', {
+    getFindListOptions: observableOf(Object.assign(new FindListOptions(), {
+      currentPage: 1,
+      elementsPerPage: 10
+    })),
+    getCurrentPagination: observableOf(Object.assign(new PaginationComponentOptions(), {
+      currentPage: 1,
+      pageSize: 10,
+      pageSizeOptions: [1, 5, 10, 20, 40, 60, 80, 100],
+    })),
+    getCurrentSort: observableOf(new SortOptions('name', SortDirection.ASC)),
+    clearPagination: undefined
   });
 
   beforeEach(async () => {
@@ -53,20 +115,29 @@ describe('LicenseContractPageComponent', () => {
         CommonModule,
         FormsModule,
         ReactiveFormsModule,
-        TranslateModule.forRoot()
+        TranslateModule.forRoot(),
       ],
       declarations: [
-        LicenseContractPageComponent
+        LicenseContractPageComponent,
+        MockNgVarDirective
       ],
       providers: [
         { provide: ActivatedRoute, useValue: routeStub },
         { provide: CollectionDataService, useValue: collectionService },
-      ]
+        { provide: PaginationService, useValue: paginationService },
+      ],
+      schemas: [NO_ERRORS_SCHEMA]
     })
     .compileComponents();
   });
 
   beforeEach(() => {
+    routeStub.snapshot.queryParams = { ...paramObject };
+    collectionService.findById.and.returnValue(createSuccessfulRemoteDataObject$(collection));
+    collectionService.findById.calls.reset();
+    collectionService.getAuthorizedCollection.calls.reset();
+    paginationService.getFindListOptions.calls.reset();
+    paginationService.clearPagination.calls.reset();
     fixture = TestBed.createComponent(LicenseContractPageComponent);
     component = fixture.componentInstance;
     fixture.detectChanges();
@@ -77,19 +148,52 @@ describe('LicenseContractPageComponent', () => {
   });
 
   it('should load collectionRD$', () => {
-    collectionService.findById(collection.uuid)
-      .pipe(getFirstCompletedRemoteData())
-      .subscribe(collectionRD$ => {
-        expect(component.collectionRD$.value).toEqual(collectionRD$);
-      });
+    expect(component.collectionRD$.value.payload).toEqual(collection);
   });
 
   it('should load licenseRD$', () => {
-    collection.license
-      .pipe(take(1))
-      .subscribe(licenseRD$ => {
-        expect(component.licenseRD$.value).toEqual(licenseRD$);
-      });
+    expect(component.licenseRD$.value.payload).toEqual(singleCollectionLicense);
+  });
+
+  it('should set hasFailed on collectionRD$ when collectionId is bogus', () => {
+    collectionService.findById.and.returnValue(createFailedRemoteDataObject$('Not Found', 404));
+    collectionService.findById.calls.reset();
+
+    const failFixture = TestBed.createComponent(LicenseContractPageComponent);
+    const failComponent = failFixture.componentInstance;
+    failFixture.detectChanges();
+
+    expect(collectionService.findById).toHaveBeenCalled();
+    expect(failComponent.collectionRD$.value.hasFailed).toBeTrue();
+  });
+
+  it('should load authorized collections when collectionId is missing', () => {
+    routeStub.snapshot.queryParams = {};
+    collectionService.findById.calls.reset();
+    collectionService.getAuthorizedCollection.calls.reset();
+
+    const listFixture = TestBed.createComponent(LicenseContractPageComponent);
+    const listComponent = listFixture.componentInstance;
+    listFixture.detectChanges();
+
+    expect(collectionService.findById).not.toHaveBeenCalled();
+    expect(collectionService.getAuthorizedCollection).toHaveBeenCalled();
+
+    listComponent.collectionsRD$.subscribe((collectionsRD) => {
+      expect(collectionsRD.payload.page).toEqual(authorizedCollections);
+    });
+  });
+
+  it('should clear pagination state on destroy in list mode', () => {
+    routeStub.snapshot.queryParams = {};
+    paginationService.clearPagination.calls.reset();
+
+    const listFixture = TestBed.createComponent(LicenseContractPageComponent);
+    const listComponent = listFixture.componentInstance;
+    listFixture.detectChanges();
+    listComponent.ngOnDestroy();
+
+    expect(paginationService.clearPagination).toHaveBeenCalledWith(listComponent.paginationId);
   });
 
 });

src/app/license-contract-page/license-contract-page.component.ts

@@ -1,13 +1,17 @@
-import { Component, OnInit } from '@angular/core';
+import { Component, OnDestroy, OnInit } from '@angular/core';
 import { ActivatedRoute } from '@angular/router';
-import { BehaviorSubject } from 'rxjs';
+import { BehaviorSubject, EMPTY, Observable, Subject } from 'rxjs';
+import { filter, switchMap, takeUntil, tap } from 'rxjs/operators';
 import { RemoteData } from '../core/data/remote-data';
 import { Collection } from '../core/shared/collection.model';
 import { CollectionDataService } from '../core/data/collection-data.service';
 import { License } from '../core/shared/license.model';
 import { followLink } from '../shared/utils/follow-link-config.model';
-import { filter } from 'rxjs/operators';
-import { isNotUndefined } from '../shared/empty.util';
+import { isNotEmpty } from '../shared/empty.util';
+import { PaginatedList } from '../core/data/paginated-list.model';
+import { PaginationComponentOptions } from '../shared/pagination/pagination-component-options.model';
+import { FindListOptions } from '../core/data/find-list-options.model';
+import { PaginationService } from '../core/pagination/pagination.service';
 
 /**
  * The component load and show distribution license based on the collection.
@@ -17,10 +21,14 @@ import { isNotUndefined } from '../shared/empty.util';
   templateUrl: './license-contract-page.component.html',
   styleUrls: ['./license-contract-page.component.scss']
 })
-export class LicenseContractPageComponent implements OnInit {
+export class LicenseContractPageComponent implements OnInit, OnDestroy {
+
+  readonly paginationId = 'contract-collections';
+  private readonly destroy$ = new Subject<void>();
 
   constructor(private route: ActivatedRoute,
-              protected collectionDataService: CollectionDataService,) {
+              protected collectionDataService: CollectionDataService,
+              protected paginationService: PaginationService,) {
   }
 
   /**
@@ -38,18 +46,58 @@ export class LicenseContractPageComponent implements OnInit {
    */
   licenseRD$: BehaviorSubject<RemoteData<License>> = new BehaviorSubject<RemoteData<License>>(null);
 
+  /**
+   * Collection list RemoteData object loaded from the API.
+   */
+  collectionsRD$: Observable<RemoteData<PaginatedList<Collection>>>;
+
+  /**
+   * The current pagination configuration for the page used by the authorized collection request.
+   */
+  config: FindListOptions = Object.assign(new FindListOptions(), {
+    elementsPerPage: 10
+  });
+
+  /**
+   * The current pagination configuration for the page.
+   */
+  pageConfig: PaginationComponentOptions = Object.assign(new PaginationComponentOptions(), {
+    id: this.paginationId,
+    pageSize: 10
+  });
+
   ngOnInit(): void {
     this.collectionId = this.route.snapshot.queryParams.collectionId;
-    this.collectionDataService.findById(this.collectionId, false, true, followLink('license'))
-      .pipe(
-        filter((collectionData: RemoteData<Collection>) => isNotUndefined((collectionData.payload))))
-      .subscribe(res => {
-        // load collection
-        this.collectionRD$.next(res);
-        res.payload.license.subscribe(licenseRD$ => {
-          // load license of the collection
-          this.licenseRD$.next(licenseRD$);
-        });
-      });
+    if (isNotEmpty(this.collectionId)) {
+      this.collectionDataService.findById(this.collectionId, false, true, followLink('license'))
+        .pipe(
+          tap((collectionData: RemoteData<Collection>) => this.collectionRD$.next(collectionData)),
+          filter((collectionData: RemoteData<Collection>) => isNotEmpty(collectionData.payload)),
+          switchMap((collectionData: RemoteData<Collection>) => collectionData.payload.license ?? EMPTY),
+          tap((licenseRD: RemoteData<License>) => this.licenseRD$.next(licenseRD)),
+          takeUntil(this.destroy$)
+        )
+        .subscribe();
+    } else {
+      this.loadAuthorizedCollections();
+    }
+  }
+
+  ngOnDestroy(): void {
+    this.destroy$.next();
+    this.destroy$.complete();
+    if (this.isListMode()) {
+      this.paginationService.clearPagination(this.paginationId);
+    }
+  }
+
+  isListMode(): boolean {
+    return !isNotEmpty(this.collectionId);
+  }
+
+  private loadAuthorizedCollections(): void {
+    this.collectionsRD$ = this.paginationService.getFindListOptions(this.paginationId, this.config).pipe(
+      switchMap((config: FindListOptions) => this.collectionDataService.getAuthorizedCollection('', config, true, true, followLink('license')))
+    );
   }
 }