Fixed multiple edit relationship bugs

- Fixed issue making it impossible to add new relationships until the page is refreshed after deleting an existing one (only when you refreshed the page after creating the initial relationship) - Fixed NPE in DsDynamicLookupRelationModalComponent - Grouped buttons on relationship page in order to assure that they always have the same behaviour
alexandrevryghem · May 17, 2024 · f65b964 · f65b964
1 parent 479adf6
commit f65b964
Show file tree

Hide file tree

Showing 12 changed files with 73 additions and 70 deletions.
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
@@ -35,6 +35,10 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
    */
   updates$: Observable<FieldUpdates>;
 
+  hasChanges$: Observable<boolean>;
+
+  isReinstatable$: Observable<boolean>;
+
   /**
    * Route to the item's page
    */
@@ -82,6 +86,8 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
 
     this.discardTimeOut = environment.item.edit.undoTimeout;
     this.url = this.router.url;
+    this.hasChanges$ = this.hasChanges();
+    this.isReinstatable$ = this.isReinstatable();
     if (this.url.indexOf('?') > 0) {
       this.url = this.url.substr(0, this.url.indexOf('?'));
     }

diff --git a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html
@@ -5,18 +5,18 @@
             class="fas fa-upload"></i>
       <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.upload-button" | translate}}</span>
     </button>
-    <button class="btn btn-warning" *ngIf="isReinstatable() | async"
+    <button class="btn btn-warning" *ngIf="isReinstatable$ | async"
             (click)="reinstate()"><i
             class="fas fa-undo-alt"></i>
       <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.reinstate-button" | translate}}</span>
     </button>
-    <button class="btn btn-primary" [disabled]="!(hasChanges() | async) || submitting"
+    <button class="btn btn-primary" [disabled]="(hasChanges$ | async) !== true || submitting"
             (click)="submit()"><i
             class="fas fa-save"></i>
       <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.save-button" | translate}}</span>
     </button>
-    <button class="btn btn-danger" *ngIf="!(isReinstatable() | async)"
-            [disabled]="!(hasChanges() | async) || submitting"
+    <button class="btn btn-danger" *ngIf="(isReinstatable$ | async) !== true"
+            [disabled]="(hasChanges$ | async) !== true || submitting"
             (click)="discard()"><i
             class="fas fa-times"></i>
       <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.discard-button" | translate}}</span>
@@ -48,18 +48,18 @@
 
   <div class="button-row bottom">
     <div class="mt-4 float-right space-children-mr ml-gap">
-      <button class="btn btn-warning" *ngIf="isReinstatable() | async"
+      <button class="btn btn-warning" *ngIf="isReinstatable$ | async"
               (click)="reinstate()"><i
         class="fas fa-undo-alt"></i>
         <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.reinstate-button" | translate}}</span>
       </button>
-      <button class="btn btn-primary" [disabled]="!(hasChanges() | async) || submitting"
+      <button class="btn btn-primary" [disabled]="(hasChanges$ | async) !== true || submitting"
               (click)="submit()"><i
         class="fas fa-save"></i>
         <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.save-button" | translate}}</span>
       </button>
-      <button class="btn btn-danger" *ngIf="!(isReinstatable() | async)"
-              [disabled]="!(hasChanges() | async) || submitting"
+      <button class="btn btn-danger" *ngIf="(isReinstatable$ | async) !== true"
+              [disabled]="(hasChanges$ | async) !== true || submitting"
               (click)="discard()"><i
         class="fas fa-times"></i>
         <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.discard-button" | translate}}</span>

diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.spec.ts b/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.spec.ts
@@ -185,8 +185,6 @@ describe('EditItemRelationshipsService', () => {
 
       expect(itemService.invalidateByHref).toHaveBeenCalledWith(currentItem.self);
       expect(itemService.invalidateByHref).toHaveBeenCalledWith(relationshipItem1.self);
-      // TODO currently this isn't done yet
-      // expect(itemService.invalidateByHref).toHaveBeenCalledWith(relationshipItem2.self);
 
       expect(notificationsService.success).toHaveBeenCalledTimes(1);
     });

diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.html b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.html
@@ -1,23 +1,7 @@
 <div class="item-relationships">
-  <ng-container *ngIf="entityType$ | async as entityType">
-    <div class="button-row top d-flex space-children-mr">
-      <button class="btn btn-danger ml-auto" *ngIf="!(isReinstatable() | async)"
-              [disabled]="!(hasChanges() | async)"
-              (click)="discard()"><i
-          class="fas fa-times"></i>
-        <span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.discard-button" | translate}}</span>
-      </button>
-      <button class="btn btn-warning ml-auto" *ngIf="isReinstatable() | async"
-              (click)="reinstate()"><i
-          class="fas fa-undo-alt"></i>
-        <span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.reinstate-button" | translate}}</span>
-      </button>
-      <button class="btn btn-primary" [disabled]="!(hasChanges() | async)"
-              (click)="submit()"><span *ngIf="isSaving$ | async" class="spinner-border spinner-border-sm" role="status"
-                                                     aria-hidden="true"></span>
-        <i *ngIf="!(isSaving$ | async)" class="fas fa-save"></i>
-        <span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.save-button" | translate}}</span>
-      </button>
+  <ng-container *ngIf="entityType$ | async as entityType; else noEntityType">
+    <div class="button-row top d-flex justify-content-end">
+      <ng-container *ngTemplateOutlet="buttons"></ng-container>
     </div>
     <div *ngIf="relationshipTypes$ | async as relationshipTypes; else loading" class="mb-4">
       <div *ngFor="let relationshipType of relationshipTypes; trackBy: trackById" class="mb-4">
@@ -26,36 +10,46 @@
           [item]="item"
           [itemType]="entityType"
           [relationshipType]="relationshipType"
-          [hasChanges]="hasChanges()"
+          [hasChanges]="hasChanges$"
         ></ds-edit-relationship-list>
       </div>
     </div>
-    <ng-template #loading>
-      <ds-themed-loading></ds-themed-loading>
-    </ng-template>
     <div class="button-row bottom">
-      <div class="float-right space-children-mr ml-gap">
-        <button class="btn btn-danger" *ngIf="!(isReinstatable() | async)"
-                [disabled]="!(hasChanges() | async)"
-                (click)="discard()"><i
-            class="fas fa-times"></i>
-          <span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.discard-button" | translate}}</span>
-        </button>
-        <button class="btn btn-warning" *ngIf="isReinstatable() | async"
-                (click)="reinstate()"><i
-            class="fas fa-undo-alt"></i>
-          <span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.reinstate-button" | translate}}</span>
-        </button>
-        <button class="btn btn-primary" [disabled]="!(hasChanges() | async)"
-                (click)="submit()"><i
-            class="fas fa-save"></i>
-          <span class="d-none d-sm-inline">&nbsp;{{"item.edit.metadata.save-button" | translate}}</span>
-        </button>
+      <div class="float-right ml-gap">
+        <ng-container *ngTemplateOutlet="buttons"></ng-container>
       </div>
     </div>
-    <div *ngIf="!entityType"
-         class="alert alert-info mt-2" role="alert">
-      {{ 'item.edit.relationships.no-entity-type' | translate }}
-    </div>
   </ng-container>
 </div>
+
+<ng-template #noEntityType>
+  <ds-alert [type]="AlertType.Info" class="d-block mt-2">
+    {{ 'item.edit.relationships.no-entity-type' | translate }}
+  </ds-alert>
+</ng-template>
+
+<ng-template #loading>
+  <ds-themed-loading></ds-themed-loading>
+</ng-template>
+
+<ng-template #buttons>
+  <div class="d-flex space-children-mr justify-content-end">
+    <button class="btn btn-danger" *ngIf="(isReinstatable$ | async) !== true"
+            [disabled]="(hasChanges$ | async) !== true"
+            (click)="discard()">
+      <i aria-hidden="true" class="fas fa-times"></i>
+      <span class="d-none d-sm-inline">&nbsp;{{ 'item.edit.metadata.discard-button' | translate }}</span>
+    </button>
+    <button class="btn btn-warning" *ngIf="isReinstatable() | async" (click)="reinstate()">
+      <i aria-hidden="true" class="fas fa-undo-alt"></i>
+      <span class="d-none d-sm-inline">&nbsp;{{ 'item.edit.metadata.reinstate-button' | translate }}</span>
+    </button>
+    <button class="btn btn-primary"
+            [disabled]="(hasChanges$ | async) !== true || (isSaving$ | async) === true"
+            (click)="submit()">
+      <span *ngIf="isSaving$ | async" aria-hidden="true" class="spinner-border spinner-border-sm" role="status"></span>
+      <i *ngIf="(isSaving$ | async) !== true" aria-hidden="true" class="fas fa-save"></i>
+      <span class="d-none d-sm-inline">&nbsp;{{ 'item.edit.metadata.save-button' | translate }}</span>
+    </button>
+  </div>
+</ng-template>
diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts
@@ -2,9 +2,7 @@ import { ChangeDetectorRef, DebugElement, NO_ERRORS_SCHEMA } from '@angular/core
 import { waitForAsync, ComponentFixture, TestBed } from '@angular/core/testing';
 import { ActivatedRoute, Router } from '@angular/router';
 import { TranslateModule } from '@ngx-translate/core';
-import { getTestScheduler } from 'jasmine-marbles';
 import { combineLatest as observableCombineLatest, of as observableOf } from 'rxjs';
-import { TestScheduler } from 'rxjs/testing';
 import { ObjectCacheService } from '../../../core/cache/object-cache.service';
 import { RestResponse } from '../../../core/cache/response.models';
 import { EntityTypeDataService } from '../../../core/data/entity-type-data.service';
@@ -30,6 +28,7 @@ import { relationshipTypes } from '../../../shared/testing/relationship-types.mo
 import { ThemeService } from '../../../shared/theme-support/theme.service';
 import { getMockThemeService } from '../../../shared/mocks/theme-service.mock';
 import { ItemDataServiceStub } from '../../../shared/testing/item-data.service.stub';
+import { NoopAnimationsModule } from '@angular/platform-browser/animations';
 
 let comp: any;
 let fixture: ComponentFixture<ItemRelationshipsComponent>;
@@ -58,7 +57,6 @@ let itemService: ItemDataServiceStub;
 const url = 'http://test-url.com/test-url';
 router.url = url;
 
-let scheduler: TestScheduler;
 let item;
 let author1;
 let author2;
@@ -206,9 +204,8 @@ describe('ItemRelationshipsComponent', () => {
       }
     );
 
-    scheduler = getTestScheduler();
     TestBed.configureTestingModule({
-      imports: [SharedModule, TranslateModule.forRoot()],
+      imports: [NoopAnimationsModule, SharedModule, TranslateModule.forRoot()],
       declarations: [ItemRelationshipsComponent],
       providers: [
         { provide: ThemeService, useValue: getMockThemeService() },

diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts
@@ -28,6 +28,7 @@ import { PaginatedList } from '../../../core/data/paginated-list.model';
 import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
 import { EditItemRelationshipsService } from './edit-item-relationships.service';
 import { compareArraysUsingIds } from '../../simple/item-types/shared/item-relationships-utils';
+import { AlertType } from '../../../shared/alert/aletr-type';
 
 @Component({
   selector: 'ds-item-relationships',
@@ -54,6 +55,8 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent {
     return this.editItemRelationshipsService.isSaving$;
   }
 
+  readonly AlertType = AlertType;
+
   constructor(
     public itemService: ItemDataService,
     public objectUpdatesService: ObjectUpdatesService,

diff --git a/...der/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html b/...der/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html
@@ -32,7 +32,7 @@ <h4 class="modal-title" id="modal-title">{{ ('submission.sections.describe.relat
                 </ng-template>
             </li>
             <li ngbNavItem *ngFor="let source of (externalSourcesRD$ | async); let idx = index">
-                <a ngbNavLink>{{'submission.sections.describe.relationship-lookup.search-tab.tab-title.' + source.id | translate  : { count: (totalExternal$ | async)[idx] } }}</a>
+                <a ngbNavLink>{{'submission.sections.describe.relationship-lookup.search-tab.tab-title.' + source.id | translate  : { count: (totalExternal$ | async)?.[idx] } }}</a>
                 <ng-template ngbNavContent>
                     <ds-themed-dynamic-lookup-relation-external-source-tab
                             [label]="label"

diff --git a/...ilder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts b/...ilder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts
@@ -30,6 +30,7 @@ import { getAllSucceededRemoteDataPayload } from '../../../../../core/shared/ope
 import { followLink } from '../../../../utils/follow-link-config.model';
 import { RelationshipType } from '../../../../../core/shared/item-relationships/relationship-type.model';
 import { ItemSearchResult } from '../../../../object-collection/shared/item-search-result.model';
+import { DSpaceObject } from '../../../../../core/shared/dspace-object.model';
 
 @Component({
   selector: 'ds-dynamic-lookup-relation-modal',
@@ -217,7 +218,7 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy
    * Select (a list of) objects and add them to the store
    * @param selectableObjects
    */
-  select(...selectableObjects: SearchResult<Item>[]) {
+  select(...selectableObjects: SearchResult<DSpaceObject>[]) {
     this.zone.runOutsideAngular(
       () => {
         const obs: Observable<any[]> = observableCombineLatest([...selectableObjects.map((sri: SearchResult<Item>) => {
@@ -260,11 +261,11 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy
    * Deselect (a list of) objects and remove them from the store
    * @param selectableObjects
    */
-  deselect(...selectableObjects: SearchResult<Item>[]) {
+  deselect(...selectableObjects: SearchResult<DSpaceObject>[]) {
     this.zone.runOutsideAngular(
       () => selectableObjects.forEach((object) => {
         this.subMap[object.indexableObject.uuid].unsubscribe();
-        this.store.dispatch(new RemoveRelationshipAction(this.item, object.indexableObject, this.relationshipOptions.relationshipType, this.submissionId));
+        this.store.dispatch(new RemoveRelationshipAction(this.item, object.indexableObject as Item, this.relationshipOptions.relationshipType, this.submissionId));
       })
     );
   }

diff --git a/...-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.ts b/...-form-ui/relation-lookup-modal/search-tab/dynamic-lookup-relation-search-tab.component.ts
@@ -96,12 +96,12 @@ export class DsDynamicLookupRelationSearchTabComponent implements OnInit, OnDest
   /**
    * Send an event to deselect an object from the list
    */
-  @Output() deselectObject: EventEmitter<ListableObject> = new EventEmitter<ListableObject>();
+  @Output() deselectObject: EventEmitter<SearchResult<DSpaceObject>> = new EventEmitter();
 
   /**
    * Send an event to select an object from the list
    */
-  @Output() selectObject: EventEmitter<ListableObject> = new EventEmitter<ListableObject>();
+  @Output() selectObject: EventEmitter<SearchResult<DSpaceObject>> = new EventEmitter();
 
   /**
    * Search results
@@ -178,7 +178,7 @@ export class DsDynamicLookupRelationSearchTabComponent implements OnInit, OnDest
     this.selection$
       .pipe(take(1))
       .subscribe((selection: SearchResult<Item>[]) => {
-        const filteredPage = page.filter((pageItem) => selection.findIndex((selected) => selected.equals(pageItem)) < 0);
+        const filteredPage: SearchResult<DSpaceObject>[] = page.filter((pageItem: SearchResult<DSpaceObject>) => selection.findIndex((selected: SearchResult<Item>) => selected.equals(pageItem)) < 0);
         this.selectObject.emit(...filteredPage);
       });
     this.selectableListService.select(this.listId, page);

diff --git a/...i/relation-lookup-modal/search-tab/themed-dynamic-lookup-relation-search-tab.component.ts b/...i/relation-lookup-modal/search-tab/themed-dynamic-lookup-relation-search-tab.component.ts
@@ -43,9 +43,9 @@ export class ThemedDynamicLookupRelationSearchTabComponent extends ThemedCompone
 
   @Input() isEditRelationship: boolean;
 
-  @Output() deselectObject: EventEmitter<ListableObject> = new EventEmitter();
+  @Output() deselectObject: EventEmitter<SearchResult<DSpaceObject>> = new EventEmitter();
 
-  @Output() selectObject: EventEmitter<ListableObject> = new EventEmitter();
+  @Output() selectObject: EventEmitter<SearchResult<DSpaceObject>> = new EventEmitter();
 
   @Output() resultFound: EventEmitter<SearchObjects<DSpaceObject>> = new EventEmitter();
 

diff --git a/...ui/relation-lookup-modal/selection-tab/dynamic-lookup-relation-selection-tab.component.ts b/...ui/relation-lookup-modal/selection-tab/dynamic-lookup-relation-selection-tab.component.ts
@@ -13,6 +13,8 @@ import { PageInfo } from '../../../../../../core/shared/page-info.model';
 import { Context } from '../../../../../../core/shared/context.model';
 import { createSuccessfulRemoteDataObject } from '../../../../../remote-data.utils';
 import { PaginationService } from '../../../../../../core/pagination/pagination.service';
+import { SearchResult } from '../../../../../search/models/search-result.model';
+import { DSpaceObject } from '../../../../../../core/shared/dspace-object.model';
 
 @Component({
   selector: 'ds-dynamic-lookup-relation-selection-tab',
@@ -63,12 +65,12 @@ export class DsDynamicLookupRelationSelectionTabComponent {
   /**
    * Send an event to deselect an object from the list
    */
-  @Output() deselectObject: EventEmitter<ListableObject> = new EventEmitter<ListableObject>();
+  @Output() deselectObject: EventEmitter<SearchResult<DSpaceObject>> = new EventEmitter();
 
   /**
    * Send an event to select an object from the list
    */
-  @Output() selectObject: EventEmitter<ListableObject> = new EventEmitter<ListableObject>();
+  @Output() selectObject: EventEmitter<SearchResult<DSpaceObject>> = new EventEmitter();
 
   /**
    * The initial pagination to use

diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5
@@ -3088,6 +3088,8 @@
 
   "orgunit.page.titleprefix": "Organizational Unit: ",
 
+  "orgunit.search.results.head": "Organizational Unit Search Results",
+
   "pagination.options.description": "Pagination options",
 
   "pagination.results-per-page": "Results Per Page",