144398: fix bundle removal state and visual regressions

Author: Ali Jaber

src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html

@@ -1,4 +1,5 @@
-<div class="item-bitstreams" *ngVar="(bundles$ | async) as bundles">
+@let bundles = (bundles$ | async);
+<div class="item-bitstreams">
   <div class="mt-2" id="reorder-description">
     <ds-alert [content]="'item.edit.bitstreams.info-alert'" [type]="AlertType.Info"></ds-alert>
   </div>
@@ -37,19 +38,18 @@
 
 @if (item && bundles?.length > 0) {
   <div class="mt-4 table-border scrollable-table" [ngClass]="{'disabled-overlay': (isProcessingMoveRequest | async)}">
-    <ng-container *ngVar="(bundleFieldUpdates$ | async) as bundleUpdates">
-      @for (bundle of bundles; track bundle; let isFirst = $first) {
-        <ds-item-edit-bitstream-bundle
-          [bundle]="bundle"
-          [item]="item"
-          [columnSizes]="columnSizes"
-          [isFirstTable]="isFirst"
-          [bundleUpdate]="bundleUpdates?.[bundle.uuid]"
-          [bundleUpdatesUrl]="bundleUpdatesUrl"
-          aria-describedby="reorder-description">
-        </ds-item-edit-bitstream-bundle>
-      }
-    </ng-container>
+    @let bundleUpdates = (bundleFieldUpdates$ | async);
+    @for (bundle of bundles; track bundle.uuid; let isFirst = $first) {
+      <ds-item-edit-bitstream-bundle
+        [bundle]="bundle"
+        [item]="item"
+        [columnSizes]="columnSizes"
+        [isFirstTable]="isFirst"
+        [bundleUpdate]="bundleUpdates?.[bundle.uuid]"
+        [bundleUpdatesUrl]="bundleUpdatesUrl"
+        aria-describedby="reorder-description">
+      </ds-item-edit-bitstream-bundle>
+    }
   </div>
 }
 @if (bundles?.length === 0) {

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

@@ -1,12 +1,15 @@
 import {
   ChangeDetectorRef,
+  Component,
+  Input,
   NO_ERRORS_SCHEMA,
 } from '@angular/core';
 import {
   ComponentFixture,
   TestBed,
   waitForAsync,
 } from '@angular/core/testing';
+import { By } from '@angular/platform-browser';
 import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
 import {
   ActivatedRoute,
@@ -40,12 +43,14 @@ import {
 } from '@dspace/core/utilities/remote-data.utils';
 import { hasValue } from '@dspace/shared/utils/empty.util';
 import { TranslateModule } from '@ngx-translate/core';
-import { of } from 'rxjs';
+import {
+  BehaviorSubject,
+  of,
+} from 'rxjs';
 
 import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component';
 import { SearchConfigurationService } from '../../../shared/search/search-configuration.service';
 import { ObjectValuesPipe } from '../../../shared/utils/object-values-pipe';
-import { VarDirective } from '../../../shared/utils/var.directive';
 import { ItemBitstreamsComponent } from './item-bitstreams.component';
 import { ItemBitstreamsService } from './item-bitstreams.service';
 import {
@@ -105,6 +110,21 @@ let requestService: RequestService;
 let searchConfig: SearchConfigurationService;
 let bundleService: BundleDataService;
 let itemBitstreamsService: ItemBitstreamsServiceStub;
+let bundlesRD$: BehaviorSubject<any>;
+let bundleFieldUpdates$: BehaviorSubject<any>;
+
+@Component({
+  selector: 'ds-item-edit-bitstream-bundle',
+  template: '',
+})
+class ItemEditBitstreamBundleTestComponent {
+  @Input() bundle: Bundle;
+  @Input() item: Item;
+  @Input() columnSizes;
+  @Input() isFirstTable: boolean;
+  @Input() bundleUpdate;
+  @Input() bundleUpdatesUrl: string;
+}
 
 describe('ItemBitstreamsComponent', () => {
   beforeEach(waitForAsync(() => {
@@ -131,11 +151,12 @@ describe('ItemBitstreamsComponent', () => {
         getMoveOperations: of(moveOperations),
       },
     );
+    bundleFieldUpdates$ = new BehaviorSubject({
+      [bundle.uuid]: { field: bundle, changeType: undefined },
+    });
     (objectUpdatesService.getFieldUpdatesExclusive as jasmine.Spy).and.callFake((bundleListUrl: string) => {
       if (hasValue(bundleListUrl) && bundleListUrl.endsWith('/bundles')) {
-        return of({
-          [bundle.uuid]: { field: bundle, changeType: undefined },
-        });
+        return bundleFieldUpdates$.asObservable();
       }
       return of({
         [bitstream1.uuid]: fieldUpdate1,
@@ -183,10 +204,11 @@ describe('ItemBitstreamsComponent', () => {
       data: of({}),
       url: url,
     });
+    bundlesRD$ = new BehaviorSubject(createSuccessfulRemoteDataObject(createPaginatedList([bundle])));
     bundleService = jasmine.createSpyObj('bundleService', {
       patch: createSuccessfulRemoteDataObject$({}),
       removeMultiple: createSuccessfulRemoteDataObject$({} as NoContent),
-      findAllByItem: createSuccessfulRemoteDataObject$(createPaginatedList([bundle])),
+      findAllByItem: bundlesRD$.asObservable(),
     });
 
     itemBitstreamsService = getItemBitstreamsServiceStub();
@@ -196,7 +218,6 @@ describe('ItemBitstreamsComponent', () => {
         TranslateModule.forRoot(),
         ItemBitstreamsComponent,
         ObjectValuesPipe,
-        VarDirective,
         BrowserAnimationsModule,
       ],
       providers: [
@@ -221,6 +242,9 @@ describe('ItemBitstreamsComponent', () => {
           imports: [ItemEditBitstreamBundleComponent,
             ThemedLoadingComponent],
         },
+        add: {
+          imports: [ItemEditBitstreamBundleTestComponent],
+        },
       })
       .compileComponents();
   }));
@@ -232,6 +256,27 @@ describe('ItemBitstreamsComponent', () => {
     fixture.detectChanges();
   });
 
+  it('should preserve bundle components and staged updates when bundle observables emit again', () => {
+    const bundleDebugElement = fixture.debugElement.query(By.directive(ItemEditBitstreamBundleTestComponent));
+    const bundleComponent = bundleDebugElement.componentInstance as ItemEditBitstreamBundleTestComponent;
+
+    bundleFieldUpdates$.next({
+      [bundle.uuid]: { field: bundle, changeType: FieldChangeType.REMOVE },
+    });
+    fixture.detectChanges();
+
+    expect(fixture.debugElement.query(By.directive(ItemEditBitstreamBundleTestComponent)).componentInstance)
+      .toBe(bundleComponent);
+    expect(bundleComponent.bundleUpdate.changeType).toBe(FieldChangeType.REMOVE);
+
+    bundlesRD$.next(createSuccessfulRemoteDataObject(createPaginatedList([bundle])));
+    fixture.detectChanges();
+
+    expect(fixture.debugElement.query(By.directive(ItemEditBitstreamBundleTestComponent)).componentInstance)
+      .toBe(bundleComponent);
+    expect(objectUpdatesService.initialize).toHaveBeenCalledTimes(1);
+  });
+
   describe('when submit is called', () => {
     beforeEach(() => {
       spyOn(bitstreamService, 'removeMultiple').and.callThrough();

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

@@ -61,7 +61,6 @@ import { BtnDisabledDirective } from '../../../shared/btn-disabled.directive';
 import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component';
 import { ResponsiveTableSizes } from '../../../shared/responsive-table-sizes/responsive-table-sizes';
 import { ObjectValuesPipe } from '../../../shared/utils/object-values-pipe';
-import { VarDirective } from '../../../shared/utils/var.directive';
 import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component';
 import { ItemBitstreamsService } from './item-bitstreams.service';
 import { ItemEditBitstreamBundleComponent } from './item-edit-bitstream-bundle/item-edit-bitstream-bundle.component';
@@ -79,7 +78,6 @@ import { ItemEditBitstreamBundleComponent } from './item-edit-bitstream-bundle/i
     RouterLink,
     ThemedLoadingComponent,
     TranslateModule,
-    VarDirective,
   ],
   providers: [ObjectValuesPipe],
 })
@@ -173,7 +171,7 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
       shareReplay({ bufferSize: 1, refCount: true }),
     );
 
-    this.subs.push(this.bundles$.subscribe((bundles: Bundle[]) => {
+    this.subs.push(this.bundles$.pipe(take(1)).subscribe((bundles: Bundle[]) => {
       this.objectUpdatesService.initialize(this.bundleUpdatesUrl, bundles, new Date());
     }));
 

src/app/item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/item-edit-bitstream-bundle.component.html

@@ -42,8 +42,7 @@
                   }
                 </div>
               </th>
-              <td class="text-end row-element bundle-row-actions {{ columnSizes.columns[3].buildClasses() }}">
-                <div class="d-flex justify-content-end w-100">
+              <td class="text-center row-element bundle-row-actions {{ columnSizes.columns[3].buildClasses() }}">
                 <div class="btn-group">
                   <button [routerLink]="[itemPageRoute, 'bitstreams', 'new']"
                     [queryParams]="{bundle: bundle.id}"
@@ -53,12 +52,12 @@
                     title="{{'item.edit.bitstreams.bundle.edit.buttons.upload' | translate}}">
                     <i class="fas fa-upload fa-fw"></i>
                   </button>
-                  <button [dsBtnDisabled]="!canRemoveBundle()" (click)="removeBundle()"
+                  <button type="button" [dsBtnDisabled]="!canRemoveBundle()" (click)="removeBundle()"
                     class="btn btn-outline-danger btn-sm"
                     title="{{'item.edit.bitstreams.bundle.edit.buttons.remove' | translate}}">
                     <i class="fas fa-trash-alt fa-fw"></i>
                   </button>
-                  <button [dsBtnDisabled]="!canUndoBundleRemove()" (click)="undoBundleRemove()"
+                  <button type="button" [dsBtnDisabled]="!canUndoBundleRemove()" (click)="undoBundleRemove()"
                     class="btn btn-outline-warning btn-sm"
                     title="{{'item.edit.bitstreams.bundle.edit.buttons.undo' | translate}}">
                     <i class="fas fa-undo-alt fa-fw"></i>
@@ -91,7 +90,6 @@
                     </ul>
                   </div>
                 </div>
-                </div>
               </td>
             </tr>
             @for (entry of (tableEntries$ | async); track entry) {
@@ -101,11 +99,13 @@
                   (cdkDragStarted)="dragStart()" (cdkDragEnded)="dragEnd()">
                   <th class="bitstream-name row-element {{ columnSizes.columns[0].buildClasses() }}"
                     scope="row" id="{{ entry.nameStripped }}" headers="{{ bundleName }} name">
-                    <div class="drag-handle text-muted float-start p-1 me-2 d-inline" tabindex="0" cdkDragHandle
-                      (keydown.enter)="select($event, entry)" (keydown.space)="select($event, entry)" (click)="select($event, entry)">
-                      <i class="drag-icon" [title]="'item.edit.bitstreams.edit.buttons.drag' | translate"></i>
+                    <div class="d-flex align-items-center">
+                      <div class="drag-handle text-muted me-2" tabindex="0" cdkDragHandle
+                        (keydown.enter)="select($event, entry)" (keydown.space)="select($event, entry)" (click)="select($event, entry)">
+                        <i class="drag-icon" [title]="'item.edit.bitstreams.edit.buttons.drag' | translate"></i>
+                      </div>
+                      <span class="dont-break-out">{{ entry.name }}</span>
                     </div>
-                    <span class="dont-break-out">{{ entry.name }}</span>
                   </th>
                   <td class="row-element {{ columnSizes.columns[1].buildClasses() }}"
                     headers="{{ entry.nameStripped }} {{ bundleName }} description">

src/app/item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/item-edit-bitstream-bundle.component.scss

@@ -7,26 +7,20 @@
   --bs-table-color-state: var(--bs-light);
 }
 
-.bundle-row {
+.bundle-row:not(.table-danger) {
   --bs-table-color-state: var(--bs-dark);
   --bs-table-bg: var(--bs-light-bg-subtle);
   --bs-table-bg-state: var(--bs-light-bg-subtle);
+}
 
+.bundle-row {
   .bundle-row-name.row-element {
     padding-right: 1.25rem;
 
     @media (min-width: 768px) {
       padding-right: 1.75rem;
     }
   }
-
-  .bundle-row-actions.row-element {
-    padding-left: 1rem;
-
-    @media (min-width: 768px) {
-      padding-left: 1.25rem;
-    }
-  }
 }
 
 .row-element {

src/app/item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/item-edit-bitstream-bundle.component.spec.ts

@@ -8,20 +8,24 @@ import {
   TestBed,
   waitForAsync,
 } from '@angular/core/testing';
+import { By } from '@angular/platform-browser';
+import { provideRouter } from '@angular/router';
 import { BundleDataService } from '@dspace/core/data/bundle-data.service';
 import { AuthorizationDataService } from '@dspace/core/data/feature-authorization/authorization-data.service';
 import { FieldChangeType } from '@dspace/core/data/object-updates/field-change-type.model';
 import { FieldUpdate } from '@dspace/core/data/object-updates/field-update.model';
 import { ObjectUpdatesService } from '@dspace/core/data/object-updates/object-updates.service';
 import { RequestService } from '@dspace/core/data/request.service';
 import { PaginationService } from '@dspace/core/pagination/pagination.service';
+import { PaginationComponentOptions } from '@dspace/core/pagination/pagination-component-options.model';
 import { Bundle } from '@dspace/core/shared/bundle.model';
 import { Item } from '@dspace/core/shared/item.model';
 import { AuthorizationDataServiceStub } from '@dspace/core/testing/authorization-service.stub';
 import { PaginationServiceStub } from '@dspace/core/testing/pagination-service.stub';
 import { getMockRequestService } from '@dspace/core/testing/request.service.mock';
 import { createPaginatedList } from '@dspace/core/testing/utils.test';
 import { createSuccessfulRemoteDataObject$ } from '@dspace/core/utilities/remote-data.utils';
+import { provideMockStore } from '@ngrx/store/testing';
 import { TranslateModule } from '@ngx-translate/core';
 import {
   of,
@@ -77,12 +81,20 @@ describe('ItemEditBitstreamBundleComponent', () => {
   beforeEach(waitForAsync(() => {
     objectUpdatesService = jasmine.createSpyObj('objectUpdatesService', {
       initialize: undefined,
-      getFieldUpdatesExclusive: of(null),
+      getFieldUpdatesExclusive: of({}),
       saveRemoveFieldUpdate: undefined,
       removeSingleFieldUpdate: undefined,
     });
 
     itemBitstreamsService = getItemBitstreamsServiceStub();
+    itemBitstreamsService.getInitialBitstreamsPaginationOptions.and.returnValue(
+      Object.assign(new PaginationComponentOptions(), {
+        id: 'bundle-bitstreams-test',
+        currentPage: 1,
+        pageSize: 10,
+        pageSizeOptions: [5, 10],
+      }),
+    );
 
     TestBed.configureTestingModule({
       imports: [TranslateModule.forRoot(), ItemEditBitstreamBundleComponent],
@@ -93,6 +105,8 @@ describe('ItemEditBitstreamBundleComponent', () => {
         { provide: RequestService, useValue: getMockRequestService() },
         { provide: ItemBitstreamsService, useValue: itemBitstreamsService },
         { provide: AuthorizationDataService, useValue: new AuthorizationDataServiceStub() },
+        provideMockStore(),
+        provideRouter([]),
       ],
       schemas: [
         NO_ERRORS_SCHEMA,
@@ -108,14 +122,20 @@ describe('ItemEditBitstreamBundleComponent', () => {
     comp.columnSizes = columnSizes;
     comp.bundleUpdatesUrl = 'https://rest/api/core/items/item/bundles';
     viewContainerRef = (comp as any).viewContainerRef;
-    spyOn(viewContainerRef, 'createEmbeddedView');
+    spyOn(viewContainerRef, 'createEmbeddedView').and.callThrough();
     fixture.detectChanges();
   });
 
   it('should create an embedded view of the component', () => {
     expect(viewContainerRef.createEmbeddedView).toHaveBeenCalled();
   });
 
+  it('should center the bundle action buttons', () => {
+    const actionCell = fixture.debugElement.query(By.css('.bundle-row-actions'));
+    expect(actionCell.classes['text-center']).toBeTrue();
+    expect(actionCell.classes['text-end']).not.toBeTrue();
+  });
+
   describe('bundle removal', () => {
     it('removeBundle should register removal with object updates', () => {
       comp.removeBundle();
@@ -202,6 +222,15 @@ describe('ItemEditBitstreamBundleComponent', () => {
   });
 
   describe('getRowClass', () => {
+    it('should return \'table-danger\' when the bundle is marked for removal', () => {
+      comp.bundleUpdate = {
+        field: bundle,
+        changeType: FieldChangeType.REMOVE,
+      };
+
+      expect(comp.getRowClass(undefined, undefined)).toEqual('table-danger');
+    });
+
     it('should return \'table-info\' when the bitstream is the selected bitstream', () => {
       itemBitstreamsService.getSelectedBitstream.and.returnValue({
         bitstream: { id: 'bitstream-id' },

src/app/item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/item-edit-bitstream-bundle.component.ts

@@ -444,7 +444,7 @@ export class ItemEditBitstreamBundleComponent implements OnInit, OnDestroy {
    */
   getRowClass(update: FieldUpdate, bitstream: BitstreamTableEntry): string {
     if (this.isMarkedForRemoval()) {
-      return 'table-secondary';
+      return 'table-danger';
     }
 
     const selected = this.itemBitstreamsService.getSelectedBitstream();