Refactors, fixes bugs, and adds tests for changing spec and category display orders

package-lock.json

@@ -12171,9 +12171,9 @@
       "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg=="
     },
     "sartography-workflow-lib": {
-      "version": "0.0.102",
+      "version": "0.0.104",
-      "resolved": "https://registry.npmjs.org/sartography-workflow-lib/-/sartography-workflow-lib-0.0.102.tgz",
+      "resolved": "https://registry.npmjs.org/sartography-workflow-lib/-/sartography-workflow-lib-0.0.104.tgz",
-      "integrity": "sha512-XsRht/O/WX7/d9xXlPGjpDjknpkbz6tB+VLyvgPAwquhrpYtJbEQnC0RkLiatrPbhxR5QN6m/gWxvgyPhNvb+g=="
+      "integrity": "sha512-YJpwIBa+oPvBguFDGKEPjyhulPmcYCImMyiNpY3kNPKTtiIGtGLrlebNvZgpSnhCfQzs1h939BukfF1g9b/8mQ=="
     },
     "sass": {
       "version": "1.23.3",

package.json

@@ -49,7 +49,7 @@
     "ngx-file-drop": "^8.0.8",
     "ngx-markdown": "^9.0.0",
     "rxjs": "~6.5.4",
-    "sartography-workflow-lib": "^0.0.102",
+    "sartography-workflow-lib": "^0.0.104",
     "tslib": "^1.11.1",
     "uuid": "^7.0.2",
     "zone.js": "^0.10.3"

src/app/workflow-spec-card/workflow-spec-card.component.ts

@@ -9,7 +9,6 @@ import {ApiService, WorkflowSpec} from 'sartography-workflow-lib';
 export class WorkflowSpecCardComponent implements OnInit {
   @Input() workflowSpec: WorkflowSpec;
   @Input() actionButtons: TemplateRef<any>;
-  @Output() workflowUpdated: EventEmitter<WorkflowSpec> = new EventEmitter();
 
   constructor(
     private api: ApiService
@@ -18,11 +17,4 @@ export class WorkflowSpecCardComponent implements OnInit {
 
   ngOnInit(): void {
   }
-
-  makeMasterStatus() {
-    this.workflowSpec.is_master_spec = true;
-    this.api.updateWorkflowSpecification(this.workflowSpec.id, this.workflowSpec).subscribe(spec => {
-      this.workflowUpdated.emit(spec);
-    });
-  }
 }

src/app/workflow-spec-list/workflow-spec-list.component.spec.ts

@@ -15,9 +15,13 @@ import {
   ApiService,
   MockEnvironment,
   mockWorkflowSpec0,
-  mockWorkflowSpec1, mockWorkflowSpecCategories, mockWorkflowSpecCategory0, mockWorkflowSpecCategory1,
+  mockWorkflowSpec1, mockWorkflowSpec2,
-  mockWorkflowSpecs
+  mockWorkflowSpecCategories,
+  mockWorkflowSpecCategory0,
+  mockWorkflowSpecCategory1, mockWorkflowSpecCategory2,
+  mockWorkflowSpecs, moveArrayElementUp, WorkflowSpec, WorkflowSpecCategory
 } from 'sartography-workflow-lib';
+import {ApiError} from 'sartography-workflow-lib/lib/types/api';
 import {DeleteWorkflowSpecDialogComponent} from '../_dialogs/delete-workflow-spec-dialog/delete-workflow-spec-dialog.component';
 import {
   DeleteWorkflowSpecCategoryDialogData,
@@ -28,7 +32,7 @@ import {
 import {GetIconCodePipe} from '../_pipes/get-icon-code.pipe';
 import {ApiErrorsComponent} from '../api-errors/api-errors.component';
 import {FileListComponent} from '../file-list/file-list.component';
-import {WorkflowSpecListComponent} from './workflow-spec-list.component';
+import {WorkflowSpecCategoryGroup, WorkflowSpecListComponent} from './workflow-spec-list.component';
 
 describe('WorkflowSpecListComponent', () => {
   let httpMock: HttpTestingController;
@@ -307,10 +311,167 @@ describe('WorkflowSpecListComponent', () => {
     expect(_loadWorkflowSpecCategoriesSpy).toHaveBeenCalled();
   });
 
-  it('should test validateWorkflowSpec');
+  it('should validate workflow spec', () => {
-  it('should test onWorkflowUpdated');
+    const bottomSheetSpy = spyOn((component as any).bottomSheet, 'open').and.stub();
-  it('should test editCategoryDisplayOrder');
+    const snackBarSpy = spyOn((component as any).snackBar, 'open').and.stub();
-  it('should test editSpecDisplayOrder');
+
-  it('should test _loadWorkflowSpecs with is_master_spec');
+    component.validateWorkflowSpec(mockWorkflowSpec0);
+    const validReq = httpMock.expectOne(`apiRoot/workflow-specification/${mockWorkflowSpec0.id}/validate`);
+    expect(validReq.request.method).toEqual('GET');
+    validReq.flush(null);
+    expect(bottomSheetSpy).not.toHaveBeenCalled();
+    expect(snackBarSpy).toHaveBeenCalled();
+
+    bottomSheetSpy.calls.reset();
+    snackBarSpy.calls.reset();
+
+    component.validateWorkflowSpec(mockWorkflowSpec0);
+    const invalidReq = httpMock.expectOne(`apiRoot/workflow-specification/${mockWorkflowSpec0.id}/validate`);
+    expect(invalidReq.request.method).toEqual('GET');
+
+    const mockError: ApiError = {
+      status_code: 42,
+      code: 'random_number',
+      message: 'Pick a number, any number',
+      task_id: 'abc123',
+      task_name: 'task_random_num',
+      file_name: 'random.bpmn',
+      tag: 'bpmn:definitions',
+    };
+    invalidReq.flush([mockError]);
+    expect(bottomSheetSpy).toHaveBeenCalled();
+    expect(snackBarSpy).not.toHaveBeenCalled();
+
+  });
+
+  it('should edit category display order', () => {
+    const _reorderSpy = spyOn((component as any), '_reorder').and.stub();
+    const _updateCatDisplayOrdersSpy = spyOn((component as any), '_updateCatDisplayOrders').and.stub();
+
+    component.editCategoryDisplayOrder(2, -1, mockWorkflowSpecCategories);
+    expect(_reorderSpy).toHaveBeenCalled();
+    expect(_updateCatDisplayOrdersSpy).toHaveBeenCalled();
+  });
+
+  it('should edit workflow spec display order', () => {
+    const _reorderSpy = spyOn((component as any), '_reorder').and.stub();
+    const _updateSpecDisplayOrdersSpy = spyOn((component as any), '_updateSpecDisplayOrders').and.stub();
+
+    component.editSpecDisplayOrder('few_things', -1, mockWorkflowSpecs);
+    expect(_reorderSpy).toHaveBeenCalled();
+    expect(_updateSpecDisplayOrdersSpy).toHaveBeenCalled();
+  });
+
+  it('should reorder categories', () => {
+    const snackBarSpy = spyOn((component as any).snackBar, 'open').and.stub();
+    const moveUpSpy = spyOn(component, 'moveUp').and.callThrough();
+    const moveDownSpy = spyOn(component, 'moveDown').and.callThrough();
+    const expectedCatsAfter = [ mockWorkflowSpecCategory1, mockWorkflowSpecCategory0, mockWorkflowSpecCategory2 ];
+
+    expect((component as any)._reorder(99, 1, mockWorkflowSpecCategories)).toEqual([]);
+    expect(snackBarSpy).toHaveBeenCalled();
+    expect(moveUpSpy).not.toHaveBeenCalled();
+    expect(moveDownSpy).not.toHaveBeenCalled();
+
+    snackBarSpy.calls.reset();
+    moveUpSpy.calls.reset();
+    moveDownSpy.calls.reset();
+    expect((component as any)._reorder(1, -1, mockWorkflowSpecCategories)).toEqual(expectedCatsAfter);
+    expect(snackBarSpy).not.toHaveBeenCalled();
+    expect(moveUpSpy).toHaveBeenCalled();
+    expect(moveDownSpy).not.toHaveBeenCalled();
+
+    snackBarSpy.calls.reset();
+    moveUpSpy.calls.reset();
+    moveDownSpy.calls.reset();
+    expect((component as any)._reorder(0, 1, mockWorkflowSpecCategories)).toEqual(expectedCatsAfter);
+    expect(snackBarSpy).not.toHaveBeenCalled();
+    expect(moveUpSpy).not.toHaveBeenCalled();
+    expect(moveDownSpy).toHaveBeenCalled();
+  });
+
+  it('should reorder specs', () => {
+    const snackBarSpy = spyOn((component as any).snackBar, 'open').and.stub();
+    const moveUpSpy = spyOn(component, 'moveUp').and.callThrough();
+    const moveDownSpy = spyOn(component, 'moveDown').and.callThrough();
+    const specsAfter = [
+      mockWorkflowSpec1,
+      mockWorkflowSpec0,
+      mockWorkflowSpec2,
+    ];
+
+    expect((component as any)._reorder('nonexistent_id', 1, mockWorkflowSpecs)).toEqual([]);
+    expect(snackBarSpy).toHaveBeenCalled();
+    expect(moveUpSpy).not.toHaveBeenCalled();
+    expect(moveDownSpy).not.toHaveBeenCalled();
+
+    snackBarSpy.calls.reset();
+    moveUpSpy.calls.reset();
+    moveDownSpy.calls.reset();
+    expect((component as any)._reorder(mockWorkflowSpec1.id, -1, mockWorkflowSpecs)).toEqual(specsAfter);
+    expect(snackBarSpy).not.toHaveBeenCalled();
+    expect(moveUpSpy).toHaveBeenCalled();
+    expect(moveDownSpy).not.toHaveBeenCalled();
+
+    snackBarSpy.calls.reset();
+    moveUpSpy.calls.reset();
+    moveDownSpy.calls.reset();
+    expect((component as any)._reorder(mockWorkflowSpec0.id, 1, mockWorkflowSpecs)).toEqual(specsAfter);
+    expect(snackBarSpy).not.toHaveBeenCalled();
+    expect(moveUpSpy).not.toHaveBeenCalled();
+    expect(moveDownSpy).toHaveBeenCalled();
+  });
+
+  it('should update all category display orders', () => {
+    const _loadWorkflowSpecCategoriesSpy = spyOn((component as any), '_loadWorkflowSpecCategories').and.stub();
+    (component as any)._updateCatDisplayOrders(mockWorkflowSpecCategories);
+
+    mockWorkflowSpecCategories.forEach((spec, i) => {
+      const req = httpMock.expectOne(`apiRoot/workflow-specification-category/${spec.id}`);
+      expect(req.request.method).toEqual('PUT');
+      req.flush(mockWorkflowSpecCategories[i]);
+    });
+
+    expect(_loadWorkflowSpecCategoriesSpy).toHaveBeenCalled();
+  });
+
+  it('should update all spec display orders', () => {
+    const _loadWorkflowSpecCategoriesSpy = spyOn((component as any), '_loadWorkflowSpecCategories').and.stub();
+    (component as any)._updateSpecDisplayOrders(mockWorkflowSpecs);
+
+    mockWorkflowSpecs.forEach((spec, i) => {
+      const req = httpMock.expectOne(`apiRoot/workflow-specification/${spec.id}`);
+      expect(req.request.method).toEqual('PUT');
+      req.flush(mockWorkflowSpecs[i]);
+    });
+
+    expect(_loadWorkflowSpecCategoriesSpy).toHaveBeenCalled();
+  });
+
+  it('should load master workflow spec', () => {
+    const mockMasterSpec: WorkflowSpec = {
+      id: 'master_status_spec',
+      name: 'master_status_spec',
+      display_name: 'master_status_spec',
+      description: 'master_status_spec',
+      is_master_spec: true,
+      display_order: null,
+      category_id: null,
+    };
+    (component as any)._loadWorkflowSpecs();
+    const allSpecs = createClone({circles: true})(mockWorkflowSpecs);
+    allSpecs.push(mockMasterSpec);
+
+    const req = httpMock.expectOne(`apiRoot/workflow-specification`);
+    expect(req.request.method).toEqual('GET');
+    req.flush(allSpecs);
+
+    expect(component.workflowSpecs).toEqual(allSpecs);
+    expect(component.workflowSpecsByCategory).toBeTruthy();
+    component.workflowSpecsByCategory.forEach(cat => {
+      expect(cat.workflow_specs).not.toContain(mockMasterSpec);
+    });
+    expect(component.masterStatusSpec).toEqual(mockMasterSpec);
+  });
 
 });

src/app/workflow-spec-list/workflow-spec-list.component.ts

@@ -24,7 +24,7 @@ import {
 import {ApiErrorsComponent} from '../api-errors/api-errors.component';
 
 
-interface WorkflowSpecCategoryGroup {
+export interface WorkflowSpecCategoryGroup {
   id: number;
   name: string;
   display_name: string;
@@ -44,6 +44,8 @@ export class WorkflowSpecListComponent implements OnInit {
   selectedCat: WorkflowSpecCategory;
   workflowSpecsByCategory: WorkflowSpecCategoryGroup[] = [];
   categories: WorkflowSpecCategory[];
+  moveUp = moveArrayElementUp;
+  moveDown = moveArrayElementDown;
 
   constructor(
     private api: ApiService,
@@ -145,80 +147,14 @@ export class WorkflowSpecListComponent implements OnInit {
     });
   }
 
-  onWorkflowUpdated(spec: WorkflowSpec) {
-    if (spec.is_master_spec) {
-      // Mark all other specs as not is_master_spec
-      let numUpdated = this.workflowSpecs.length - 1;
-      this.workflowSpecs.forEach(wfs => {
-        if (wfs.id !== spec.id) {
-          wfs.is_master_spec = false;
-          this.api.updateWorkflowSpecification(wfs.id, wfs).subscribe(() => {
-            numUpdated--;
-            if (numUpdated === 0) {
-              this._loadWorkflowSpecCategories();
-            }
-          });
-        }
-      });
-    }
-    this._loadWorkflowSpecCategories();
-  }
-
   editCategoryDisplayOrder(catId: number, direction: number, cats: WorkflowSpecCategoryGroup[]) {
-    // Remove the fake category with category-less specs
+    const reorderedCats = this._reorder(catId, direction, cats) as WorkflowSpecCategoryGroup[];
-    const realCats = cats.filter(cat => isNumberDefined(cat.id));
+    this._updateCatDisplayOrders(reorderedCats);
-    const i = realCats.findIndex(spec => spec.id === catId);
-    if (i !== -1) {
-      if (direction === 1) {
-        moveArrayElementDown(realCats, i);
-      } else if (direction === -1) {
-        moveArrayElementUp(realCats, i);
-      }
-    } else {
-      this.snackBar.open('Category not found. Reload the page and try again.');
-      return;
-    }
-
-    let numUpdated = 0;
-    realCats.forEach((cat, j) => {
-      if (isNumberDefined(cat.id)) {
-        const newCat: WorkflowSpecCategoryGroup = createClone()(cat);
-        delete newCat.workflow_specs;
-
-        newCat.display_order = j;
-        this.api.updateWorkflowSpecCategory(cat.id, newCat as WorkflowSpecCategory).subscribe(() => {
-          numUpdated++;
-          if (numUpdated === realCats.length) {
-            this._loadWorkflowSpecCategories();
-          }
-        });
-      }
-    });
   }
 
   editSpecDisplayOrder(specId: string, direction: number, specs: WorkflowSpec[]) {
-    const i = specs.findIndex(spec => spec.id === specId);
+    const reorderedSpecs = this._reorder(specId, direction, specs) as WorkflowSpec[];
-    if (i !== -1) {
+    this._updateSpecDisplayOrders(reorderedSpecs);
-      if (direction === 1) {
-        moveArrayElementDown(specs, i);
-      } else if (direction === -1) {
-        moveArrayElementUp(specs, i);
-      }
-    } else {
-      this.snackBar.open('Spec not found. Reload the page and try again.');
-      return;
-    }
-
-    let numUpdated = 0;
-    specs.forEach((spec, j) => {
-      spec.display_order = j;
-      this.api.updateWorkflowSpecification(spec.id, spec).subscribe(() => {
-        numUpdated++;
-        if (numUpdated === specs.length) {
-          this._loadWorkflowSpecCategories();
-        }
-      });
-    });
   }
 
   sortByDisplayOrder = (a, b) => (a.display_order < b.display_order) ? -1 : 1;
@@ -351,5 +287,58 @@ export class WorkflowSpecListComponent implements OnInit {
   private _displayMessage(message: string) {
     this.snackBar.open(message, 'Ok', {duration: 3000});
   }
+
+  private _reorder(
+    id: number|string, direction: number,
+    list: Array<WorkflowSpecCategoryGroup|WorkflowSpec>
+  ): Array<WorkflowSpecCategoryGroup|WorkflowSpec> {
+    const listClone = createClone({circles: true})(list);
+    const reorderedList = listClone.filter(item => item.id !== null && item.id !== undefined);
+    const i = reorderedList.findIndex(spec => spec.id === id);
+    if (i !== -1) {
+      if (direction === 1) {
+        this.moveDown(reorderedList, i);
+      } else if (direction === -1) {
+        this.moveUp(reorderedList, i);
+      }
+
+      return reorderedList;
+    } else {
+      this.snackBar.open('Item not found. Reload the page and try again.');
+      return [];
+    }
+  }
+
+  private _updateCatDisplayOrders(cats: WorkflowSpecCategory[]) {
+    let numUpdated = 0;
+    cats.forEach((cat, j) => {
+      if (isNumberDefined(cat.id)) {
+        const newCat: WorkflowSpecCategoryGroup = createClone({circles: true})(cat);
+        delete newCat.workflow_specs;
+
+        newCat.display_order = j;
+        this.api.updateWorkflowSpecCategory(cat.id, newCat as WorkflowSpecCategory).subscribe(updatedCat => {
+          numUpdated++;
+          if (numUpdated === cats.length) {
+            this._loadWorkflowSpecCategories();
+          }
+        });
+      }
+    });
+  }
+
+  private _updateSpecDisplayOrders(specs: WorkflowSpec[]) {
+    let numUpdated = 0;
+    specs.forEach((spec, j) => {
+      const newSpec = createClone({circles: true})(spec);
+      newSpec.display_order = j;
+      this.api.updateWorkflowSpecification(newSpec.id, newSpec).subscribe(() => {
+        numUpdated++;
+        if (numUpdated === specs.length) {
+          this._loadWorkflowSpecCategories();
+        }
+      });
+    });
+  }
 }