From e0e3cba53862d3907a79328800151bfd3a87f82c Mon Sep 17 00:00:00 2001 From: Aaron Louie Date: Mon, 13 Apr 2020 12:13:44 -0400 Subject: [PATCH] Refactors, fixes bugs, and adds tests for changing spec and category display orders --- package-lock.json | 6 +- package.json | 2 +- .../workflow-spec-card.component.ts | 8 - .../workflow-spec-list.component.spec.ts | 177 +++++++++++++++++- .../workflow-spec-list.component.ts | 131 ++++++------- 5 files changed, 233 insertions(+), 91 deletions(-) diff --git a/package-lock.json b/package-lock.json index affc570..3af2401 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12171,9 +12171,9 @@ "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==" }, "sartography-workflow-lib": { - "version": "0.0.102", - "resolved": "https://registry.npmjs.org/sartography-workflow-lib/-/sartography-workflow-lib-0.0.102.tgz", - "integrity": "sha512-XsRht/O/WX7/d9xXlPGjpDjknpkbz6tB+VLyvgPAwquhrpYtJbEQnC0RkLiatrPbhxR5QN6m/gWxvgyPhNvb+g==" + "version": "0.0.104", + "resolved": "https://registry.npmjs.org/sartography-workflow-lib/-/sartography-workflow-lib-0.0.104.tgz", + "integrity": "sha512-YJpwIBa+oPvBguFDGKEPjyhulPmcYCImMyiNpY3kNPKTtiIGtGLrlebNvZgpSnhCfQzs1h939BukfF1g9b/8mQ==" }, "sass": { "version": "1.23.3", diff --git a/package.json b/package.json index 5e99920..f7c7823 100644 --- a/package.json +++ b/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" diff --git a/src/app/workflow-spec-card/workflow-spec-card.component.ts b/src/app/workflow-spec-card/workflow-spec-card.component.ts index adb4284..2c0cc1d 100644 --- a/src/app/workflow-spec-card/workflow-spec-card.component.ts +++ b/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; - @Output() workflowUpdated: EventEmitter = 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); - }); - } } diff --git a/src/app/workflow-spec-list/workflow-spec-list.component.spec.ts b/src/app/workflow-spec-list/workflow-spec-list.component.spec.ts index e2d93e8..0424769 100644 --- a/src/app/workflow-spec-list/workflow-spec-list.component.spec.ts +++ b/src/app/workflow-spec-list/workflow-spec-list.component.spec.ts @@ -15,9 +15,13 @@ import { ApiService, MockEnvironment, mockWorkflowSpec0, - mockWorkflowSpec1, mockWorkflowSpecCategories, mockWorkflowSpecCategory0, mockWorkflowSpecCategory1, - mockWorkflowSpecs + mockWorkflowSpec1, mockWorkflowSpec2, + 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 test onWorkflowUpdated'); - it('should test editCategoryDisplayOrder'); - it('should test editSpecDisplayOrder'); - it('should test _loadWorkflowSpecs with is_master_spec'); + it('should validate workflow spec', () => { + const bottomSheetSpy = spyOn((component as any).bottomSheet, 'open').and.stub(); + const snackBarSpy = spyOn((component as any).snackBar, 'open').and.stub(); + + 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); + }); }); diff --git a/src/app/workflow-spec-list/workflow-spec-list.component.ts b/src/app/workflow-spec-list/workflow-spec-list.component.ts index 9e23629..0f5770b 100644 --- a/src/app/workflow-spec-list/workflow-spec-list.component.ts +++ b/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 realCats = cats.filter(cat => isNumberDefined(cat.id)); - 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(); - } - }); - } - }); + const reorderedCats = this._reorder(catId, direction, cats) as WorkflowSpecCategoryGroup[]; + this._updateCatDisplayOrders(reorderedCats); } editSpecDisplayOrder(specId: string, direction: number, specs: WorkflowSpec[]) { - const i = specs.findIndex(spec => spec.id === specId); - if (i !== -1) { - 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(); - } - }); - }); + const reorderedSpecs = this._reorder(specId, direction, specs) as WorkflowSpec[]; + this._updateSpecDisplayOrders(reorderedSpecs); } 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 + ): Array { + 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(); + } + }); + }); + } }