68346: Review - TypeDocs and Test cases

src/app/+bitstream-page/bitstream-page-routing.module.ts

@@ -6,6 +6,9 @@ import { BitstreamPageResolver } from './bitstream-page.resolver';
 
 const EDIT_BITSTREAM_PATH = ':id/edit';
 
+/**
+ * Routing module to help navigate Bitstream pages
+ */
 @NgModule({
   imports: [
     RouterModule.forChild([

src/app/+bitstream-page/bitstream-page.module.ts

@@ -4,6 +4,9 @@ import { SharedModule } from '../shared/shared.module';
 import { EditBitstreamPageComponent } from './edit-bitstream-page/edit-bitstream-page.component';
 import { BitstreamPageRoutingModule } from './bitstream-page-routing.module';
 
+/**
+ * This module handles all components that are necessary for Bitstream related pages
+ */
 @NgModule({
   imports: [
     CommonModule,

src/app/+item-page/bitstreams/upload/upload-bitstream.component.ts

@@ -69,6 +69,14 @@ export class UploadBitstreamComponent implements OnInit, OnDestroy {
               protected translate: TranslateService) {
   }
 
+  /**
+   * Initialize component properties:
+   * itemRD$          Fetched from the current route data (populated by BitstreamPageResolver)
+   * bundlesRD$       List of bundles on the item
+   * selectedBundleId Starts off by checking if the route's queryParams contain a "bundle" parameter. If none is found,
+   *                  the ID of the first bundle in the list is selected.
+   * Calls setUploadUrl after setting the selected bundle
+   */
   ngOnInit(): void {
     this.itemRD$ = this.route.data.pipe(map((data) => data.item));
     this.bundlesRD$ = this.itemRD$.pipe(

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

@@ -11,6 +11,8 @@ import { ResponsiveTableSizes } from '../../../../shared/responsive-table-sizes/
 })
 /**
  * Component that displays a single bundle of an item on the item bitstreams edit page
+ * Creates an embedded view of the contents. This is to ensure the table structure won't break.
+ * (which means it'll be added to the parents html without a wrapping ds-item-edit-bitstream-bundle element)
  */
 export class ItemEditBitstreamBundleComponent implements OnInit {
 

src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-drag-handle/item-edit-bitstream-drag-handle.component.ts

@@ -5,6 +5,11 @@ import { Component, OnInit, TemplateRef, ViewChild, ViewContainerRef } from '@an
   styleUrls: ['../item-bitstreams.component.scss'],
   templateUrl: './item-edit-bitstream-drag-handle.component.html',
 })
+/**
+ * Component displaying a drag handle for the item-edit-bitstream page
+ * Creates an embedded view of the contents
+ * (which means it'll be added to the parents html without a wrapping ds-item-edit-bitstream-drag-handle element)
+ */
 export class ItemEditBitstreamDragHandleComponent implements OnInit {
   /**
    * The view on the drag-handle

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

@@ -17,6 +17,8 @@ import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
 })
 /**
  * Component that displays a single bitstream of an item on the edit page
+ * Creates an embedded view of the contents
+ * (which means it'll be added to the parents html without a wrapping ds-item-edit-bitstream element)
  */
 export class ItemEditBitstreamComponent implements OnChanges, OnInit {
 

src/app/core/data/array-move-change-analyzer.service.spec.ts

@@ -0,0 +1,107 @@
+import { ArrayMoveChangeAnalyzer } from './array-move-change-analyzer.service';
+import { moveItemInArray } from '@angular/cdk/drag-drop';
+import { Operation } from 'fast-json-patch';
+
+/**
+ * Helper class for creating move tests
+ * Define a "from" and "to" index to move objects within the array before comparing
+ */
+class MoveTest {
+  from: number;
+  to: number;
+
+  constructor(from: number, to: number) {
+    this.from = from;
+    this.to = to;
+  }
+}
+
+describe('ArrayMoveChangeAnalyzer', () => {
+  const comparator = new ArrayMoveChangeAnalyzer<string>();
+
+  let originalArray = [];
+
+  describe('when all values are defined', () => {
+    beforeEach(() => {
+      originalArray = [
+        '98700118-d65d-4636-b1d0-dba83fc932e1',
+        '4d7d0798-a8fa-45b8-b4fc-deb2819606c8',
+        'e56eb99e-2f7c-4bee-9b3f-d3dcc83386b1',
+        '0f608168-cdfc-46b0-92ce-889f7d3ac684',
+        '546f9f5c-15dc-4eec-86fe-648007ac9e1c'
+      ];
+    });
+
+    testMove([
+      { op: 'move', from: '/2', path: '/4' },
+    ], new MoveTest(2, 4));
+
+    testMove([
+      { op: 'move', from: '/0', path: '/3' },
+    ], new MoveTest(0, 3));
+
+    testMove([
+      { op: 'move', from: '/0', path: '/3' },
+      { op: 'move', from: '/2', path: '/1' }
+    ], new MoveTest(0, 3), new MoveTest(1, 2));
+
+    testMove([
+      { op: 'move', from: '/0', path: '/1' },
+      { op: 'move', from: '/3', path: '/4' }
+    ], new MoveTest(0, 1), new MoveTest(3, 4));
+
+    testMove([], new MoveTest(0, 4), new MoveTest(4, 0));
+
+    testMove([
+      { op: 'move', from: '/0', path: '/3' },
+      { op: 'move', from: '/2', path: '/1' }
+    ], new MoveTest(0, 4), new MoveTest(1, 3), new MoveTest(2, 4));
+  });
+
+  describe('when some values are undefined (index 2 and 3)', () => {
+    beforeEach(() => {
+      originalArray = [
+        '98700118-d65d-4636-b1d0-dba83fc932e1',
+        '4d7d0798-a8fa-45b8-b4fc-deb2819606c8',
+        undefined,
+        undefined,
+        '546f9f5c-15dc-4eec-86fe-648007ac9e1c'
+      ];
+    });
+
+    // It can't create a move operation for undefined values, so it should create move operations for the defined values instead
+    testMove([
+      { op: 'move', from: '/4', path: '/3' },
+    ], new MoveTest(2, 4));
+
+    // Moving a defined value should result in the same operations
+    testMove([
+      { op: 'move', from: '/0', path: '/3' },
+    ], new MoveTest(0, 3));
+  });
+
+  /**
+   * Helper function for creating a move test
+   *
+   * @param expectedOperations  An array of expected operations after comparing the original array with the array
+   *                            created using the provided MoveTests
+   * @param moves               An array of MoveTest objects telling the test where to move objects before comparing
+   */
+  function testMove(expectedOperations: Operation[], ...moves: MoveTest[]) {
+    describe(`move ${moves.map((move) => `${move.from} to ${move.to}`).join(' and ')}`, () => {
+      let result;
+
+      beforeEach(() => {
+        const movedArray = [...originalArray];
+        moves.forEach((move) => {
+          moveItemInArray(movedArray, move.from, move.to);
+        });
+        result = comparator.diff(originalArray, movedArray);
+      });
+
+      it('should create the expected move operations', () => {
+        expect(result).toEqual(expectedOperations);
+      });
+    });
+  }
+});

src/app/core/data/data.service.spec.ts

@@ -302,7 +302,7 @@ describe('DataService', () => {
     });
   });
 
-  describe('immediatePatch', () => {
+  describe('patch', () => {
     const dso = {
       uuid: 'dso-uuid'
     };
@@ -315,7 +315,7 @@ describe('DataService', () => {
     ];
 
     beforeEach(() => {
-      service.immediatePatch(dso, operations);
+      service.patch(dso, operations);
     });
 
     it('should configure a PatchRequest', () => {

src/app/core/data/object-updates/object-updates.service.spec.ts

@@ -13,6 +13,8 @@ import { Notification } from '../../../shared/notifications/models/notification.
 import { NotificationType } from '../../../shared/notifications/models/notification-type';
 import { OBJECT_UPDATES_TRASH_PATH } from './object-updates.reducer';
 import {Relationship} from '../../shared/item-relationships/relationship.model';
+import { MoveOperation } from 'fast-json-patch/lib/core';
+import { ArrayMoveChangeAnalyzer } from '../array-move-change-analyzer.service';
 
 describe('ObjectUpdatesService', () => {
   let service: ObjectUpdatesService;
@@ -45,7 +47,7 @@ describe('ObjectUpdatesService', () => {
     };
     store = new Store<CoreState>(undefined, undefined, undefined);
     spyOn(store, 'dispatch');
-    service = new ObjectUpdatesService(store, undefined);
+    service = new ObjectUpdatesService(store, new ArrayMoveChangeAnalyzer<string>());
 
     spyOn(service as any, 'getObjectEntry').and.returnValue(observableOf(objectEntry));
     spyOn(service as any, 'getFieldState').and.callFake((uuid) => {
@@ -97,6 +99,66 @@ describe('ObjectUpdatesService', () => {
     });
   });
 
+  describe('getFieldUpdatesExclusive', () => {
+    it('should return the list of all fields, including their update if there is one, excluding updates that aren\'t part of the initial values provided', (done) => {
+      const result$ = service.getFieldUpdatesExclusive(url, identifiables);
+      expect((service as any).getObjectEntry).toHaveBeenCalledWith(url);
+
+      const expectedResult = {
+        [identifiable1.uuid]: { field: identifiable1Updated, changeType: FieldChangeType.UPDATE },
+        [identifiable2.uuid]: { field: identifiable2, changeType: undefined }
+      };
+
+      result$.subscribe((result) => {
+        expect(result).toEqual(expectedResult);
+        done();
+      });
+    });
+  });
+
+  describe('getFieldUpdatesByCustomOrder', () => {
+    beforeEach(() => {
+      const fieldStates = {
+        [identifiable1.uuid]: { editable: false, isNew: false, isValid: true },
+        [identifiable2.uuid]: { editable: true, isNew: false, isValid: false },
+        [identifiable3.uuid]: { editable: true, isNew: true, isValid: true },
+      };
+
+      const customOrder = {
+        initialOrderPages: [{
+          order: [identifiable1.uuid, identifiable2.uuid, identifiable3.uuid]
+        }],
+        newOrderPages: [{
+          order: [identifiable2.uuid, identifiable3.uuid, identifiable1.uuid]
+        }],
+        pageSize: 20,
+        changed: true
+      };
+
+      const objectEntry = {
+        fieldStates, fieldUpdates, lastModified: modDate, virtualMetadataSources: {}, customOrder
+      };
+
+      (service as any).getObjectEntry.and.returnValue(observableOf(objectEntry))
+    });
+
+    it('should return the list of all fields, including their update if there is one, ordered by their custom order', (done) => {
+      const result$ = service.getFieldUpdatesByCustomOrder(url, identifiables);
+      expect((service as any).getObjectEntry).toHaveBeenCalledWith(url);
+
+      const expectedResult = {
+        [identifiable2.uuid]: { field: identifiable2, changeType: undefined },
+        [identifiable3.uuid]: { field: identifiable3, changeType: FieldChangeType.ADD },
+        [identifiable1.uuid]: { field: identifiable1Updated, changeType: FieldChangeType.UPDATE }
+      };
+
+      result$.subscribe((result) => {
+        expect(result).toEqual(expectedResult);
+        done();
+      });
+    });
+  });
+
   describe('isEditable', () => {
     it('should return false if this identifiable is currently not editable in the store', () => {
       const result$ = service.isEditable(url, identifiable1.uuid);
@@ -283,4 +345,45 @@ describe('ObjectUpdatesService', () => {
       expect(store.dispatch).toHaveBeenCalledWith(new SelectVirtualMetadataAction(url, relationship.uuid, identifiable1.uuid, true));
     });
   });
+
+  describe('getMoveOperations', () => {
+    beforeEach(() => {
+      const fieldStates = {
+        [identifiable1.uuid]: { editable: false, isNew: false, isValid: true },
+        [identifiable2.uuid]: { editable: true, isNew: false, isValid: false },
+        [identifiable3.uuid]: { editable: true, isNew: true, isValid: true },
+      };
+
+      const customOrder = {
+        initialOrderPages: [{
+          order: [identifiable1.uuid, identifiable2.uuid, identifiable3.uuid]
+        }],
+        newOrderPages: [{
+          order: [identifiable2.uuid, identifiable3.uuid, identifiable1.uuid]
+        }],
+        pageSize: 20,
+        changed: true
+      };
+
+      const objectEntry = {
+        fieldStates, fieldUpdates, lastModified: modDate, virtualMetadataSources: {}, customOrder
+      };
+
+      (service as any).getObjectEntry.and.returnValue(observableOf(objectEntry))
+    });
+
+    it('should return the expected move operations', (done) => {
+      const result$ = service.getMoveOperations(url);
+
+      const expectedResult = [
+        { op: 'move', from: '/0', path: '/2' }
+      ] as MoveOperation[];
+
+      result$.subscribe((result) => {
+        expect(result).toEqual(expectedResult);
+        done();
+      });
+    });
+  });
+
 });

src/app/core/shared/item-bitstreams-utils.ts

@@ -1,16 +0,0 @@
-import { Observable } from 'rxjs/internal/Observable';
-import { RemoteData } from '../data/remote-data';
-import { PaginatedList } from '../data/paginated-list';
-import { Bitstream } from './bitstream.model';
-import { map } from 'rxjs/operators';
-import { hasValue, hasValueOperator } from '../../shared/empty.util';
-
-/**
- * Operator for turning the current page of bitstreams into an array
- */
-export const toBitstreamsArray = () =>
-  (source: Observable<RemoteData<PaginatedList<Bitstream>>>): Observable<Bitstream[]> =>
-    source.pipe(
-      hasValueOperator(),
-      map((bitstreamRD: RemoteData<PaginatedList<Bitstream>>) => bitstreamRD.payload.page.filter((bitstream: Bitstream) => hasValue(bitstream)))
-    );

src/app/shared/form/builder/ds-dynamic-form-ui/models/custom-switch/custom-switch.component.spec.ts

@@ -0,0 +1,99 @@
+import { DynamicFormsCoreModule, DynamicFormService } from '@ng-dynamic-forms/core';
+import { FormGroup, ReactiveFormsModule } from '@angular/forms';
+import { async, ComponentFixture, inject, TestBed } from '@angular/core/testing';
+import { DebugElement } from '@angular/core';
+import { NoopAnimationsModule } from '@angular/platform-browser/animations';
+import { TextMaskModule } from 'angular2-text-mask';
+import { By } from '@angular/platform-browser';
+import { DynamicCustomSwitchModel } from './custom-switch.model';
+import { CustomSwitchComponent } from './custom-switch.component';
+
+describe('CustomSwitchComponent', () => {
+
+  const testModel = new DynamicCustomSwitchModel({id: 'switch'});
+  const formModel = [testModel];
+  let formGroup: FormGroup;
+  let fixture: ComponentFixture<CustomSwitchComponent>;
+  let component: CustomSwitchComponent;
+  let debugElement: DebugElement;
+  let testElement: DebugElement;
+
+  beforeEach(async(() => {
+    TestBed.configureTestingModule({
+      imports: [
+        ReactiveFormsModule,
+        NoopAnimationsModule,
+        TextMaskModule,
+        DynamicFormsCoreModule.forRoot()
+      ],
+      declarations: [CustomSwitchComponent]
+
+    }).compileComponents().then(() => {
+      fixture = TestBed.createComponent(CustomSwitchComponent);
+
+      component = fixture.componentInstance;
+      debugElement = fixture.debugElement;
+    });
+  }));
+
+  beforeEach(inject([DynamicFormService], (service: DynamicFormService) => {
+    formGroup = service.createFormGroup(formModel);
+
+    component.group = formGroup;
+    component.model = testModel;
+
+    fixture.detectChanges();
+
+    testElement = debugElement.query(By.css(`input[id='${testModel.id}']`));
+  }));
+
+  it('should initialize correctly', () => {
+    expect(component.bindId).toBe(true);
+    expect(component.group instanceof FormGroup).toBe(true);
+    expect(component.model instanceof DynamicCustomSwitchModel).toBe(true);
+
+    expect(component.blur).toBeDefined();
+    expect(component.change).toBeDefined();
+    expect(component.focus).toBeDefined();
+
+    expect(component.onBlur).toBeDefined();
+    expect(component.onChange).toBeDefined();
+    expect(component.onFocus).toBeDefined();
+
+    expect(component.hasFocus).toBe(false);
+    expect(component.isValid).toBe(true);
+    expect(component.isInvalid).toBe(false);
+  });
+
+  it('should have an input element', () => {
+    expect(testElement instanceof DebugElement).toBe(true);
+  });
+
+  it('should have an input element of type checkbox', () => {
+    expect(testElement.nativeElement.getAttribute('type')).toEqual('checkbox');
+  });
+
+  it('should emit blur event', () => {
+    spyOn(component.blur, 'emit');
+
+    component.onBlur(null);
+
+    expect(component.blur.emit).toHaveBeenCalled();
+  });
+
+  it('should emit change event', () => {
+    spyOn(component.change, 'emit');
+
+    component.onChange(null);
+
+    expect(component.change.emit).toHaveBeenCalled();
+  });
+
+  it('should emit focus event', () => {
+    spyOn(component.focus, 'emit');
+
+    component.onFocus(null);
+
+    expect(component.focus.emit).toHaveBeenCalled();
+  });
+});

src/app/shared/form/builder/ds-dynamic-form-ui/models/custom-switch/custom-switch.component.ts

@@ -8,13 +8,48 @@ import { DynamicCustomSwitchModel } from './custom-switch.model';
   styleUrls: ['./custom-switch.component.scss'],
   templateUrl: './custom-switch.component.html',
 })
+/**
+ * Component displaying a custom switch usable in dynamic forms
+ * Extends from bootstrap's checkbox component but displays a switch instead
+ */
 export class CustomSwitchComponent extends DynamicNGBootstrapCheckboxComponent {
+  /**
+   * Use the model's ID for the input element
+   */
   @Input() bindId = true;
+
+  /**
+   * The formgroup containing this component
+   */
   @Input() group: FormGroup;
+
+  /**
+   * The model used for displaying the switch
+   */
   @Input() model: DynamicCustomSwitchModel;
+
+  /**
+   * Emit an event when the input is selected
+   */
   @Output() selected = new EventEmitter<number>();
+
+  /**
+   * Emit an event when the input value is removed
+   */
   @Output() remove = new EventEmitter<number>();
+
+  /**
+   * Emit an event when the input is blurred out
+   */
   @Output() blur = new EventEmitter<any>();
+
+  /**
+   * Emit an event when the input value changes
+   */
   @Output() change = new EventEmitter<any>();
+
+  /**
+   * Emit an event when the input is focused
+   */
   @Output() focus = new EventEmitter<any>();
 }

src/app/shared/form/builder/ds-dynamic-form-ui/models/custom-switch/custom-switch.model.ts

@@ -7,6 +7,10 @@ import {
 
 export const DYNAMIC_FORM_CONTROL_TYPE_CUSTOM_SWITCH = 'CUSTOM_SWITCH';
 
+/**
+ * Model class for displaying a custom switch input in a form
+ * Functions like a checkbox, but displays a switch instead
+ */
 export class DynamicCustomSwitchModel extends DynamicCheckboxModel {
   @serializable() readonly type: string = DYNAMIC_FORM_CONTROL_TYPE_CUSTOM_SWITCH;