115046: 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 · df80c33 · df80c33
1 parent 1338712
commit df80c33
Show file tree

Hide file tree

Showing 12 changed files with 86 additions and 76 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
@@ -55,6 +55,10 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
    */
   updates$: Observable<FieldUpdates>;
 
+  hasChanges$: Observable<boolean>;
+
+  isReinstatable$: Observable<boolean>;
+
   /**
    * Route to the item's page
    */
@@ -101,10 +105,9 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
     }
 
     this.discardTimeOut = environment.item.edit.undoTimeout;
-    this.url = this.router.url;
-    if (this.url.indexOf('?') > 0) {
-      this.url = this.url.substr(0, this.url.indexOf('?'));
-    }
+    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.html b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html
@@ -6,21 +6,21 @@
             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"
             [attr.aria-label]="'item.edit.bitstreams.reinstate-button' | translate"
             (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) !== true || submitting"
+    <button class="btn btn-primary" [disabled]="(hasChanges$ | async) !== true || submitting"
             [attr.aria-label]="'item.edit.bitstreams.save-button' | translate"
             (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) !== true"
+    <button class="btn btn-danger" *ngIf="(isReinstatable$ | async) !== true"
             [attr.aria-label]="'item.edit.bitstreams.discard-button' | translate"
-            [disabled]="(hasChanges() | async) !== true || submitting"
+            [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>
@@ -52,21 +52,21 @@
 
   <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"
               [attr.aria-label]="'item.edit.bitstreams.reinstate-button' | translate"
               (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) !== true || submitting"
+      <button class="btn btn-primary" [disabled]="(hasChanges$ | async) !== true || submitting"
               [attr.aria-label]="'item.edit.bitstreams.save-button' | translate"
               (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) !== true"
+      <button class="btn btn-danger" *ngIf="(isReinstatable$ | async) !== true"
               [attr.aria-label]="'item.edit.bitstreams.discard-button' | translate"
-              [disabled]="(hasChanges() | async) !== true || submitting"
+              [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) !== true"
-              [disabled]="(hasChanges() | async) !== true"
-              (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) !== true"
-              (click)="submit()"><span *ngIf="isSaving$ | async" class="spinner-border spinner-border-sm" role="status"
-                                                     aria-hidden="true"></span>
-        <i *ngIf="(isSaving$ | async) !== true" 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-loading></ds-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) !== true"
-                [disabled]="(hasChanges() | async) !== true"
-                (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) !== true"
-                (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-loading></ds-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
@@ -13,12 +13,10 @@ import {
   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';
@@ -33,6 +31,7 @@ import { Item } from '../../../core/shared/item.model';
 import { ItemType } from '../../../core/shared/item-relationships/item-type.model';
 import { Relationship } from '../../../core/shared/item-relationships/relationship.model';
 import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model';
+import { AlertComponent } from '../../../shared/alert/alert.component';
 import { getMockThemeService } from '../../../shared/mocks/theme-service.mock';
 import {
   INotification,
@@ -78,7 +77,6 @@ let itemService: ItemDataServiceStub;
 const url = 'http://test-url.com/test-url';
 router.url = url;
 
-let scheduler: TestScheduler;
 let item;
 let author1;
 let author2;
@@ -226,7 +224,6 @@ describe('ItemRelationshipsComponent', () => {
       },
     );
 
-    scheduler = getTestScheduler();
     TestBed.configureTestingModule({
       imports: [TranslateModule.forRoot(), ItemRelationshipsComponent],
       providers: [
@@ -245,6 +242,12 @@ describe('ItemRelationshipsComponent', () => {
       ], schemas: [
         NO_ERRORS_SCHEMA,
       ],
+    }).overrideComponent(ItemRelationshipsComponent, {
+      remove: {
+        imports: [
+          AlertComponent,
+        ],
+      },
     }).compileComponents();
   }));
 

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
@@ -2,6 +2,7 @@ import {
   AsyncPipe,
   NgForOf,
   NgIf,
+  NgTemplateOutlet,
 } from '@angular/common';
 import {
   ChangeDetectorRef,
@@ -39,6 +40,8 @@ import {
   getFirstSucceededRemoteData,
   getRemoteDataPayload,
 } from '../../../core/shared/operators';
+import { AlertComponent } from '../../../shared/alert/alert.component';
+import { AlertType } from '../../../shared/alert/alert-type';
 import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
 import { followLink } from '../../../shared/utils/follow-link-config.model';
@@ -53,12 +56,14 @@ import { EditRelationshipListComponent } from './edit-relationship-list/edit-rel
   styleUrls: ['./item-relationships.component.scss'],
   templateUrl: './item-relationships.component.html',
   imports: [
-    ThemedLoadingComponent,
+    AlertComponent,
     AsyncPipe,
-    TranslateModule,
-    NgIf,
     EditRelationshipListComponent,
     NgForOf,
+    NgIf,
+    NgTemplateOutlet,
+    ThemedLoadingComponent,
+    TranslateModule,
     VarDirective,
   ],
   standalone: true,
@@ -83,6 +88,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" role="presentation">
-                <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-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
@@ -41,6 +41,7 @@ import { PaginatedList } from '../../../../../core/data/paginated-list.model';
 import { RelationshipDataService } from '../../../../../core/data/relationship-data.service';
 import { RelationshipTypeDataService } from '../../../../../core/data/relationship-type-data.service';
 import { Context } from '../../../../../core/shared/context.model';
+import { DSpaceObject } from '../../../../../core/shared/dspace-object.model';
 import { ExternalSource } from '../../../../../core/shared/external-source.model';
 import { Item } from '../../../../../core/shared/item.model';
 import { RelationshipType } from '../../../../../core/shared/item-relationships/relationship-type.model';
@@ -283,7 +284,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>) => {
@@ -326,11 +327,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
@@ -132,12 +132,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
@@ -214,7 +214,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
@@ -51,9 +51,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
@@ -24,6 +24,7 @@ import {
 import { RemoteData } from '../../../../../../core/data/remote-data';
 import { PaginationService } from '../../../../../../core/pagination/pagination.service';
 import { Context } from '../../../../../../core/shared/context.model';
+import { DSpaceObject } from '../../../../../../core/shared/dspace-object.model';
 import { PageInfo } from '../../../../../../core/shared/page-info.model';
 import { SearchConfigurationService } from '../../../../../../core/shared/search/search-configuration.service';
 import { SEARCH_CONFIG_SERVICE } from '../../../../../../my-dspace-page/my-dspace-configuration.service';
@@ -33,6 +34,7 @@ import { PageSizeSelectorComponent } from '../../../../../page-size-selector/pag
 import { PaginationComponentOptions } from '../../../../../pagination/pagination-component-options.model';
 import { createSuccessfulRemoteDataObject } from '../../../../../remote-data.utils';
 import { PaginatedSearchOptions } from '../../../../../search/models/paginated-search-options.model';
+import { SearchResult } from '../../../../../search/models/search-result.model';
 
 @Component({
   selector: 'ds-dynamic-lookup-relation-selection-tab',
@@ -91,12 +93,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
@@ -3664,6 +3664,8 @@
 
   "orgunit.page.ror": "ROR Identifier",
 
+  "orgunit.search.results.head": "Organizational Unit Search Results",
+
   "pagination.options.description": "Pagination options",
 
   "pagination.results-per-page": "Results Per Page",