Merge pull request #101 from sartography/bug/resort_refactor_435

Bug/resort refactor 435

package.json

@@ -63,7 +63,7 @@
     "ngx-markdown": "^12.0.1",
     "protractor": "^7.0.0",
     "rxjs": "^6.5.3",
-    "sartography-workflow-lib": "^0.0.554",
+    "sartography-workflow-lib": "^0.0.556",
     "tslib": "^2.3.0",
     "uuid": "^8.3.2",
     "zone.js": "^0.11.4"

src/app/_dialogs/workflow-spec-category-dialog/workflow-spec-category-dialog.component.ts

@@ -3,7 +3,7 @@ import {FormGroup} from '@angular/forms';
 import {MAT_DIALOG_DATA, MatDialogRef} from '@angular/material/dialog';
 import {FormlyFieldConfig, FormlyFormOptions} from '@ngx-formly/core';
 import {toSnakeCase} from 'sartography-workflow-lib';
-import {WorkflowSpecCategoryDialogData, WorkflowSpecDialogData} from '../../_interfaces/dialog-data';
+import {WorkflowSpecCategoryDialogData} from '../../_interfaces/dialog-data';
 
 @Component({
   selector: 'app-workflow-spec-category-dialog',
@@ -50,18 +50,6 @@ export class WorkflowSpecCategoryDialogComponent {
         required: true,
       },
     },
-    {
-      key: 'display_order',
-      type: 'input',
-      defaultValue: this.data.display_order,
-      templateOptions: {
-        type: 'number',
-        label: 'Display Order',
-        placeholder: 'Order in which category will be displayed',
-        description: 'Sort order that the category should appear in. Lower numbers will appear first.',
-        required: true,
-      },
-    },
   ];
 
   constructor(

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

@@ -59,7 +59,7 @@
         <mat-divider></mat-divider>
 
       <mat-accordion class="example-headers-align" multi="false">
-      <ng-container *ngFor="let cat of workflowSpecsByCategory">
+      <ng-container *ngFor="let cat of workflowSpecsByCategory; let j = index">
 
         <div *ngIf="!(searchField.value && cat.workflow_specs.length === 0)">
         <ng-container *ngIf="!(cat.id === null && cat.workflow_specs.length === 0)">
@@ -76,24 +76,24 @@
               </button>
             </mat-expansion-panel-header>
             <mat-list>
-              <mat-list-item *ngFor="let wfs of cat.workflow_specs" class="workflow-spec" fxLayout="row" fxLayoutGap="10px" fxLayoutAlign="start center">
+              <mat-list-item *ngFor="let wfs of cat.workflow_specs; let i = index" class="workflow-spec" fxLayout="row" fxLayoutGap="10px" fxLayoutAlign="start center">
                 <span [ngClass]="{'spec_menu_item':true, 'spec-selected': selectedSpec && wfs.id === selectedSpec.id}" (click)="selectSpec(wfs)">{{wfs.display_name}}</span>
-                <span class="spec-actions" fxLayout="row" fxLayoutGap="10px" *ngIf="cat && cat.id !== null">
+                <span class="spec-actions" fxLayout="row" fxLayoutGap="10px" *ngIf="cat.id !== null && cat">
                 <button
-                  *ngIf="cat && cat.workflow_specs.length > 0 && wfs.display_order !== 0"
+                  *ngIf="i!==0 && cat.workflow_specs.length > 0"
                   mat-icon-button
                   title="Move up"
                   color="primary"
-                  (click)="editSpecDisplayOrder(wfs && wfs.id, -1, cat.workflow_specs)"
+                  (click)="editSpecDisplayOrder(cat, wfs.id, 'up')"
                 >
                   <mat-icon>arrow_upward</mat-icon>
                 </button>
                 <button
-                  *ngIf="cat && cat.workflow_specs.length > 0 && (wfs.display_order < cat.workflow_specs.length - 1)"
+                  *ngIf="i!==cat.workflow_specs.length-1 && cat.workflow_specs.length > 0"
                   mat-icon-button
                   title="Move down"
                   color="primary"
-                  (click)="editSpecDisplayOrder(wfs.id, 1, cat.workflow_specs)"
+                  (click)="editSpecDisplayOrder(cat, wfs.id, 'down')"
                 >
                   <mat-icon>arrow_downward</mat-icon>
                 </button>
@@ -110,20 +110,20 @@
                 <mat-icon>edit</mat-icon>
               </button>
               <button
-                *ngIf="cat.display_order > 0"
+                *ngIf="j!==0"
                 mat-mini-fab
                 title="Move up"
                 color="primary"
-                (click)="editCategoryDisplayOrder(cat.id, -1, workflowSpecsByCategory)"
+                (click)="editCategoryDisplayOrder(cat.id, 'up')"
               >
                 <mat-icon>arrow_upward</mat-icon>
               </button>
               <button
-                *ngIf="cat.display_order < workflowSpecsByCategory.length - 2"
+                *ngIf="j!== workflowSpecsByCategory.length-1"
                 mat-mini-fab
                 title="Move down"
                 color="primary"
-                (click)="editCategoryDisplayOrder(cat.id, 1, workflowSpecsByCategory)"
+                (click)="editCategoryDisplayOrder(cat.id, 'down')"
               >
                 <mat-icon>arrow_downward</mat-icon>
               </button>

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

@@ -15,7 +15,7 @@ import {of} from 'rxjs';
 import {
   ApiErrorsComponent,
   ApiService,
-  MockEnvironment,
+  MockEnvironment, mockWorkflowMeta1,
   mockWorkflowSpec0,
   mockWorkflowSpec1,
   mockWorkflowSpec2,
@@ -36,7 +36,7 @@ import {
 } from '../_interfaces/dialog-data';
 import {GetIconCodePipe} from '../_pipes/get-icon-code.pipe';
 import {FileListComponent} from '../file-list/file-list.component';
-import {WorkflowSpecListComponent} from './workflow-spec-list.component';
+import {WorkflowSpecCategoryGroup, WorkflowSpecListComponent} from './workflow-spec-list.component';
 
 
 export class MdDialogMock {
@@ -391,24 +391,29 @@ describe('WorkflowSpecListComponent', () => {
 
   });
 
+  /**
+   *  Deprecated - removed reorder and ability to directly edit display order
+   *
   it('should edit category display order', () => {
-    const _reorderSpy = spyOn((component as any), '_reorder').and.stub();
+    // 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(_reorderSpy).toHaveBeenCalled();
     expect(_updateCatDisplayOrdersSpy).toHaveBeenCalled();
   });
 
+
   it('should edit workflow spec display order', () => {
-    const _reorderSpy = spyOn((component as any), '_reorder').and.stub();
+    // 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(_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();
@@ -437,6 +442,7 @@ describe('WorkflowSpecListComponent', () => {
     expect(moveDownSpy).toHaveBeenCalled();
   });
 
+
   it('should reorder specs', () => {
     const snackBarSpy = spyOn((component as any).snackBar, 'open').and.stub();
     const moveUpSpy = spyOn(component, 'moveUp').and.callThrough();
@@ -468,34 +474,28 @@ describe('WorkflowSpecListComponent', () => {
     expect(moveUpSpy).not.toHaveBeenCalled();
     expect(moveDownSpy).toHaveBeenCalled();
   });
+   */
 
-  it('should update all category display orders', () => {
+  it('should update a single category display order', () => {
-    const _loadWorkflowSpecCategoriesSpy = spyOn((component as any), '_loadWorkflowSpecCategories').and.stub();
+    mockWorkflowSpecCategory1.id = 5;
-    (component as any)._updateCatDisplayOrders(mockWorkflowSpecCategories);
+    (component as any).editCategoryDisplayOrder(mockWorkflowSpecCategory1.id, 'down');
-
+    let results = { param: 'direction', value: 'down' };
-    mockWorkflowSpecCategories.forEach((spec, i) => {
+    const req = httpMock.expectOne(`apiRoot/workflow-specification-category/${mockWorkflowSpecCategory1.id}/reorder?${results.param}=${results.value}`);
-      const req = httpMock.expectOne(`apiRoot/workflow-specification-category/${spec.id}`);
     expect(req.request.method).toEqual('PUT');
-      req.flush(mockWorkflowSpecCategories[i]);
+    req.flush(mockWorkflowSpecCategory1);
   });
 
-    expect(_loadWorkflowSpecCategoriesSpy).toHaveBeenCalled();
+  it('should update a single spec display order', () => {
-  });
+    let wfs_group: WorkflowSpecCategoryGroup[] = [];
-
+    mockWorkflowSpecCategory1.workflows.push(mockWorkflowMeta1);
-  it('should update all spec display orders', () => {
+    wfs_group.push(mockWorkflowSpecCategory1);
-    const _loadWorkflowSpecCategoriesSpy = spyOn((component as any), '_loadWorkflowSpecCategories').and.stub();
+    (component as any).editSpecDisplayOrder(wfs_group[0], mockWorkflowSpec1.id, 'down');
-    (component as any)._updateSpecDisplayOrders(mockWorkflowSpecs);
+    let results = { param: 'direction', value: 'down' };
-
+    const req = httpMock.expectOne(`apiRoot/workflow-specification/${mockWorkflowSpec1.id}/reorder?${results.param}=${results.value}`);
-    mockWorkflowSpecs.forEach((spec, i) => {
-      const req = httpMock.expectOne(`apiRoot/workflow-specification/${spec.id}`);
     expect(req.request.method).toEqual('PUT');
-      req.flush(mockWorkflowSpecs[i]);
+    req.flush(mockWorkflowSpecCategory1);
   });
 
-    expect(_loadWorkflowSpecCategoriesSpy).toHaveBeenCalled();
-  });
-
-
 
   it('should load master workflow spec', () => {
     const mockMasterSpec: WorkflowSpec = {

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

@@ -7,8 +7,8 @@ import {
   ApiErrorsComponent,
   ApiService,
   isNumberDefined,
-  moveArrayElementDown,
+  // moveArrayElementDown,
-  moveArrayElementUp,
+  // moveArrayElementUp,
   WorkflowSpec,
   WorkflowSpecCategory,
 } from 'sartography-workflow-lib';
@@ -52,8 +52,8 @@ export class WorkflowSpecListComponent implements OnInit {
   selectedCat: WorkflowSpecCategory;
   workflowSpecsByCategory: WorkflowSpecCategoryGroup[] = [];
   categories: WorkflowSpecCategory[];
-  moveUp = moveArrayElementUp;
+  // moveUp = moveArrayElementUp;
-  moveDown = moveArrayElementDown;
+  // moveDown = moveArrayElementDown;
   searchField: FormControl;
 
   constructor(
@@ -137,7 +137,6 @@ export class WorkflowSpecListComponent implements OnInit {
       library: selectedSpec ? selectedSpec.library : null,
     };
 
-
     // Open new filename/workflow spec dialog
     const dialogRef = this.dialog.open(WorkflowSpecDialogComponent, {
       height: '65vh',
@@ -148,9 +147,6 @@ export class WorkflowSpecListComponent implements OnInit {
     dialogRef.afterClosed().subscribe((data: WorkflowSpecDialogData) => {
       if (data && data.id && data.name && data.display_name && data.description) {
         if (this.canSaveWorkflowSpec(data)) {
-          data.display_order = this.categories.filter(function (entry) {
-            return entry.id === data.category_id;
-          }).length;
           this._upsertWorkflowSpecification(selectedSpec == null, data);
         }
       }
@@ -222,34 +218,31 @@ export class WorkflowSpecListComponent implements OnInit {
     });
   }
 
-  editCategoryDisplayOrder(catId: number, direction: number, cats: WorkflowSpecCategoryGroup[]) {
+  editCategoryDisplayOrder(catId: number, direction: string) {
-    const reorderedCats = this._reorder(catId, direction, cats) as WorkflowSpecCategoryGroup[];
+    console.log('new wfsbycat is: ', this.workflowSpecsByCategory);
-    this._updateCatDisplayOrders(reorderedCats);
+    this.api.reorderWorkflowCategory(catId, direction).subscribe(cat_change => {
+      this.workflowSpecsByCategory = this.workflowSpecsByCategory.map(cat => {
+        let new_cat = cat_change.find(i2 => i2.id === cat.id);
+        cat.display_order = new_cat.display_order;
+        return cat;
+      });
+      this.workflowSpecsByCategory.sort((x,y) => x.display_order - y.display_order);
+    });
   }
 
-  editSpecDisplayOrder(specId: string, direction: number, specs: WorkflowSpec[]) {
+  editSpecDisplayOrder(cat: WorkflowSpecCategoryGroup, specId: string, direction: string) {
-    const reorderedSpecs = this._reorder(specId, direction, specs) as WorkflowSpec[];
+    this.api.reorderWorkflowSpecification(specId, direction).subscribe(wfs => {
-    this._updateSpecDisplayOrders(reorderedSpecs);
+      cat.workflow_specs= wfs;
+    });
   }
 
-  sortByDisplayOrder = (a, b) => (a.display_order < b.display_order) ? -1 : 1;
-
   private _loadWorkflowSpecCategories(selectedSpecName: string = null) {
     this.api.getWorkflowSpecCategoryList().subscribe(cats => {
-      this.categories = cats.sort(this.sortByDisplayOrder);
+      this.categories = cats;
-
-      // Add a container for specs without a category
-      this.workflowSpecsByCategory = [{
-        id: null,
-        name: 'none',
-        display_name: 'No category',
-        workflow_specs: [],
-        display_order: -1, // Display it at the top
-      }];
 
       this.categories.forEach((cat, i) => {
         this.workflowSpecsByCategory.push(cat);
-        this.workflowSpecsByCategory[i + 1].workflow_specs = [];
+        this.workflowSpecsByCategory[i].workflow_specs = [];
       });
       this._loadWorkflowSpecs(selectedSpecName);
       this._loadWorkflowLibraries();
@@ -257,13 +250,11 @@ export class WorkflowSpecListComponent implements OnInit {
   }
 
   private _loadWorkflowLibraries() {
-
     this.api.getWorkflowSpecificationLibraries().subscribe(wfs => {
       this.workflowLibraries = wfs;
     });
   }
 
-
   private _loadWorkflowSpecs(selectedSpecName: string = null, searchSpecName: string = null) {
 
     this.api.getWorkflowSpecList().subscribe(wfs => {
@@ -271,6 +262,7 @@ export class WorkflowSpecListComponent implements OnInit {
       this.workflowSpecsByCategory.forEach(cat => {
         cat.workflow_specs = this.workflowSpecs
           .filter(wf => {
+            // Find and set master spec
             if (wf.is_master_spec) {
               this.masterStatusSpec = wf;
             } else {
@@ -281,7 +273,7 @@ export class WorkflowSpecListComponent implements OnInit {
               }
             }
           })
-          .sort(this.sortByDisplayOrder);
+        cat.workflow_specs.sort((x,y) => x.display_order - y.display_order);
       });
 
       // Set the selected workflow to something sensible.
@@ -309,10 +301,11 @@ export class WorkflowSpecListComponent implements OnInit {
         display_name: data.display_name,
         description: data.description,
         category_id: data.category_id,
-        display_order: data.display_order,
+        // display_order: data.display_order,
         standalone: data.standalone,
         library: data.library,
       };
+      console.log('DO: ', data.display_order);
 
       if (isNew) {
         this._addWorkflowSpec(newSpec);
@@ -391,6 +384,9 @@ export class WorkflowSpecListComponent implements OnInit {
     this.snackBar.open(message, 'Ok', {duration: 3000});
   }
 
+  /**
+   *  Deprecated - backend now reorders
+   *
   private _reorder(
     id: number | string, direction: number,
     list: Array<WorkflowSpecCategoryGroup | WorkflowSpec>,
@@ -411,7 +407,11 @@ export class WorkflowSpecListComponent implements OnInit {
       return [];
     }
   }
+   **/
 
+  /**
+   * Deprecated - backend updates display_order
+   *
   private _updateCatDisplayOrders(cats: WorkflowSpecCategory[]) {
     let numUpdated = 0;
     cats.forEach((cat, j) => {
@@ -430,6 +430,7 @@ export class WorkflowSpecListComponent implements OnInit {
     });
   }
 
+
   private _updateSpecDisplayOrders(specs: WorkflowSpec[]) {
     if (this.selectedCat && this.selectedSpec.category) {
       if (this.selectedCat.id !== this.selectedSpec.category.id) {
@@ -448,7 +449,7 @@ export class WorkflowSpecListComponent implements OnInit {
       });
     });
   }
-
+   */
 
 }