From 392f3d7ef2cfd7608585e778f52184b6c4bf9a7f Mon Sep 17 00:00:00 2001 From: alicia pritchett Date: Wed, 6 Oct 2021 11:38:31 -0400 Subject: [PATCH] Add Validation to 'name' of spec; update tests --- ...workflow-spec-category-dialog.component.ts | 12 ------ .../workflow-spec-dialog.component.spec.ts | 14 ++++++- .../workflow-spec-dialog.component.ts | 36 ++++++++++-------- .../workflow-spec-list.component.spec.ts | 9 ----- .../workflow-spec-list.component.ts | 37 ++++++++++--------- 5 files changed, 52 insertions(+), 56 deletions(-) diff --git a/src/app/_dialogs/workflow-spec-category-dialog/workflow-spec-category-dialog.component.ts b/src/app/_dialogs/workflow-spec-category-dialog/workflow-spec-category-dialog.component.ts index 3ac7dd3..85dfdef 100644 --- a/src/app/_dialogs/workflow-spec-category-dialog/workflow-spec-category-dialog.component.ts +++ b/src/app/_dialogs/workflow-spec-category-dialog/workflow-spec-category-dialog.component.ts @@ -27,18 +27,6 @@ export class WorkflowSpecCategoryDialogComponent { }, hideExpression: true, }, - { - key: 'name', - type: 'input', - defaultValue: this.data.name, - templateOptions: { - label: 'Name', - placeholder: 'Name of workflow spec category', - description: 'Enter a name, in lowercase letters, separated by underscores, that is easy for you to remember.' + - 'It will be converted to all_lowercase_with_underscores when you save.', - required: true, - }, - }, { key: 'display_name', type: 'input', diff --git a/src/app/_dialogs/workflow-spec-dialog/workflow-spec-dialog.component.spec.ts b/src/app/_dialogs/workflow-spec-dialog/workflow-spec-dialog.component.spec.ts index 30a9a8b..685245a 100644 --- a/src/app/_dialogs/workflow-spec-dialog/workflow-spec-dialog.component.spec.ts +++ b/src/app/_dialogs/workflow-spec-dialog/workflow-spec-dialog.component.spec.ts @@ -11,7 +11,13 @@ import {Router} from '@angular/router'; import {RouterTestingModule} from '@angular/router/testing'; import {FormlyModule} from '@ngx-formly/core'; import {FormlyMaterialModule} from '@ngx-formly/material'; -import {ApiService, MockEnvironment, mockWorkflowSpec0, mockWorkflowSpecCategories} from 'sartography-workflow-lib'; +import { + ApiService, + MockEnvironment, + mockWorkflowSpec0, + mockWorkflowSpecCategories, + mockWorkflowSpecs +} from 'sartography-workflow-lib'; import {WorkflowSpecDialogData} from '../../_interfaces/dialog-data'; import {WorkflowSpecDialogComponent} from './workflow-spec-dialog.component'; @@ -74,6 +80,12 @@ describe('WorkflowSpecDialogComponent', () => { expect(catReq.request.method).toEqual('GET'); catReq.flush(mockWorkflowSpecCategories); expect(component.categories.length).toBeGreaterThan(0); + + const specReq = httpMock.expectOne('apiRoot/workflow-specification'); + expect(specReq.request.method).toEqual('GET'); + specReq.flush(mockWorkflowSpecs); + expect(component.specs.length).toBeGreaterThan(0); + }); afterEach(() => { diff --git a/src/app/_dialogs/workflow-spec-dialog/workflow-spec-dialog.component.ts b/src/app/_dialogs/workflow-spec-dialog/workflow-spec-dialog.component.ts index ba46eea..5866dab 100644 --- a/src/app/_dialogs/workflow-spec-dialog/workflow-spec-dialog.component.ts +++ b/src/app/_dialogs/workflow-spec-dialog/workflow-spec-dialog.component.ts @@ -1,22 +1,26 @@ import {Component, Inject} from '@angular/core'; -import {FormGroup} from '@angular/forms'; +import {FormControl, FormGroup, ValidationErrors} from '@angular/forms'; import {MAT_DIALOG_DATA, MatDialogRef} from '@angular/material/dialog'; import {FormlyFieldConfig, FormlyFormOptions, FormlyTemplateOptions} from '@ngx-formly/core'; import {ApiService, toSnakeCase} from 'sartography-workflow-lib'; import {v4 as uuidv4} from 'uuid'; import {WorkflowSpecDialogData} from '../../_interfaces/dialog-data'; +import {of} from "rxjs"; + @Component({ selector: 'app-workflow-spec-dialog', templateUrl: './workflow-spec-dialog.component.html', styleUrls: ['./workflow-spec-dialog.component.scss'] }) + export class WorkflowSpecDialogComponent { form: FormGroup = new FormGroup({}); model: any = {}; options: FormlyFormOptions = {}; fields: FormlyFieldConfig[] = []; categories: any; + specs: any; constructor( private api: ApiService, @@ -29,30 +33,29 @@ export class WorkflowSpecDialogComponent { label: c.display_name, })); + this.api.getWorkflowSpecList().subscribe(wfs => { + this.specs = wfs.map(w => w.id); + this.fields = [ { key: 'id', type: 'input', defaultValue: this.data.id, templateOptions: { - label: 'ID', - placeholder: 'UUID of workflow specification', - description: 'This is an autogenerated unique ID and is not editable.', - required: true, - disabled: true, - }, - hideExpression: true, - }, - { - key: 'name', - type: 'input', - defaultValue: this.data.name, - templateOptions: { - label: 'Name', + label: 'id', placeholder: 'Name of workflow specification', - description: 'Enter a name, in lowercase letters, separated by underscores, that is easy for you to remember.' + + description: 'Enter a name to identify this spec. It cannot be changed later.' + 'It will be converted to all_lowercase_with_underscores when you save.', required: true, + disabled: this.data.id !== '', + }, + asyncValidators: { + uniqueID: { + expression: (control: FormControl) => { + return of(this.specs.indexOf(control.value) === -1); + }, + message: 'This ID name is already taken.', + }, }, }, { @@ -111,6 +114,7 @@ export class WorkflowSpecDialogComponent { }, ]; }); + }); } onNoClick() { 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 cde5f86..eee089a 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 @@ -53,7 +53,6 @@ export class MdDialogMock { const librarySpec0: WorkflowSpec = { id: 'one_thing', - name: 'one_thing', display_name: 'One thing', description: 'Do just one thing', category_id: 2, @@ -153,7 +152,6 @@ describe('WorkflowSpecListComponent', () => { it('should show a metadata dialog when editing a workflow spec', () => { let mockSpecData: WorkflowSpecDialogData = { id: '', - name: '', display_name: '', description: '', category_id: 0, @@ -261,7 +259,6 @@ describe('WorkflowSpecListComponent', () => { it('should show a metadata dialog when editing a workflow spec category', () => { let mockCatData: WorkflowSpecCategoryDialogData = { id: null, - name: '', display_name: '', admin: null, }; @@ -501,7 +498,6 @@ describe('WorkflowSpecListComponent', () => { 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, @@ -537,7 +533,6 @@ describe('WorkflowSpecListComponent', () => { it('should disallow deselecting library if being used as library', () => { let mockSpecData: WorkflowSpecDialogData = { id: '25', - name: 'name1', display_name: 'displayname', description: 'descr', category_id: 0, @@ -556,7 +551,6 @@ describe('WorkflowSpecListComponent', () => { localSelectedSpec.parents = [ { id: 1234, display_name: 'test parent', - name: 'parent1' }] component.selectedSpec = localSelectedSpec; component.editWorkflowSpec(localSelectedSpec); @@ -571,7 +565,6 @@ describe('WorkflowSpecListComponent', () => { // that fails prior to saving if any of these are blank let mockSpecData: WorkflowSpecDialogData = { id: '25', - name: 'name1', display_name: 'displayname', description: 'descr', category_id: 0, @@ -590,7 +583,6 @@ describe('WorkflowSpecListComponent', () => { localSelectedSpec.parents = [ { id: 1234, display_name: 'test parent', - name: 'parent1' }] component.selectedSpec = localSelectedSpec; component.editWorkflowSpec(localSelectedSpec); @@ -606,7 +598,6 @@ describe('WorkflowSpecListComponent', () => { badWorkflowSpec.parents=[ { id: 1234, display_name: 'test parent', - name: 'parent1' }] badWorkflowSpec.library=true; const mockConfirmDeleteData: DeleteWorkflowSpecDialogData = { 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 8093d77..0a13054 100644 --- a/src/app/workflow-spec-list/workflow-spec-list.component.ts +++ b/src/app/workflow-spec-list/workflow-spec-list.component.ts @@ -33,7 +33,6 @@ import { SettingsService } from '../settings.service'; export interface WorkflowSpecCategoryGroup { id: number; - name: string; display_name: string; workflow_specs?: WorkflowSpec[]; display_order: number; @@ -95,17 +94,17 @@ export class WorkflowSpecListComponent implements OnInit { }); } - selectCat(selectedCat?: WorkflowSpecCategory) { + selectCat(selectedCat?: WorkflowSpecCategoryGroup) { this.selectedCat = selectedCat; } - isSelected(cat: WorkflowSpecCategory) { + isSelected(cat: WorkflowSpecCategoryGroup) { return this.selectedCat && this.selectedCat.id === cat.id; } selectSpec(selectedSpec?: WorkflowSpec) { this.selectedSpec = selectedSpec; - this.location.replaceState(environment.homeRoute + '/' + selectedSpec.name); + this.location.replaceState(environment.homeRoute + '/' + selectedSpec.id); } categoryExpanded(cat: WorkflowSpecCategory) { @@ -129,7 +128,6 @@ export class WorkflowSpecListComponent implements OnInit { const hasDisplayOrder = selectedSpec && isNumberDefined(selectedSpec.display_order); const dialogData: WorkflowSpecDialogData = { id: selectedSpec ? selectedSpec.id : '', - name: selectedSpec ? selectedSpec.name || selectedSpec.id : '', display_name: selectedSpec ? selectedSpec.display_name : '', description: selectedSpec ? selectedSpec.description : '', category_id: selectedSpec ? selectedSpec.category_id : null, @@ -146,7 +144,8 @@ export class WorkflowSpecListComponent implements OnInit { }); dialogRef.afterClosed().subscribe((data: WorkflowSpecDialogData) => { - if (data && data.id && data.name && data.display_name && data.description) { + data.id = this.toLowercaseId(data.id); + if (data && data.id && data.display_name && data.description) { if (this.canSaveWorkflowSpec(data)) { this._upsertWorkflowSpecification(selectedSpec == null, data); } @@ -154,6 +153,11 @@ export class WorkflowSpecListComponent implements OnInit { }); } + // Helper function to convert strings to valid ID's. + toLowercaseId(id: String) { + return id.replace(/ /g,"_").toLowerCase(); + } + editWorkflowSpecCategory(selectedCat?: WorkflowSpecCategoryGroup) { this.selectedCat = selectedCat; @@ -163,20 +167,19 @@ export class WorkflowSpecListComponent implements OnInit { width: '50vw', data: { id: this.selectedCat ? this.selectedCat.id : null, - name: this.selectedCat ? this.selectedCat.name || this.selectedCat.id : '', display_name: this.selectedCat ? this.selectedCat.display_name : '', display_order: this.selectedCat ? this.selectedCat.display_order : null, admin: this.selectedCat ? this.selectedCat.admin : null, }, }); dialogRef.afterClosed().subscribe((data: WorkflowSpecCategoryDialogData) => { - if (data && isNumberDefined(data.id) && data.name && data.display_name) { + if (data && isNumberDefined(data.id) && data.display_name) { this._upsertWorkflowSpecCategory(data); } }); } - confirmDeleteWorkflowSpecCategory(cat: WorkflowSpecCategory) { + confirmDeleteWorkflowSpecCategory(cat: WorkflowSpecCategoryGroup) { const dialogRef = this.dialog.open(DeleteWorkflowSpecCategoryDialogComponent, { data: { confirm: false, @@ -289,11 +292,11 @@ export class WorkflowSpecListComponent implements OnInit { // Set the selected workflow to something sensible. if (!selectedSpecName && this.selectedSpec) { - selectedSpecName = this.selectedSpec.name; + selectedSpecName = this.selectedSpec.id; } if (selectedSpecName) { this.workflowSpecs.forEach(ws => { - if (selectedSpecName && selectedSpecName === ws.name) { + if (selectedSpecName && selectedSpecName === ws.id) { this.selectedSpec = ws; this.selectedCat = this.selectedSpec.category; } @@ -305,11 +308,10 @@ export class WorkflowSpecListComponent implements OnInit { } private _upsertWorkflowSpecification(isNew: boolean, data: WorkflowSpecDialogData) { - if (data.id && data.name && data.display_name && data.description) { + if (data.id && data.display_name && data.description) { const newSpec: WorkflowSpec = { id: data.id, - name: data.name, display_name: data.display_name, description: data.description, category_id: data.category_id, @@ -327,14 +329,13 @@ export class WorkflowSpecListComponent implements OnInit { } private _upsertWorkflowSpecCategory(data: WorkflowSpecCategoryDialogData) { - if (isNumberDefined(data.id) && data.name && data.display_name) { + if (isNumberDefined(data.id) && data.display_name) { // Save old workflow spec id, in case it's changed const catId = this.selectedCat ? this.selectedCat.id : undefined; const newCat: WorkflowSpecCategory = { id: data.id, - name: data.name, display_name: data.display_name, display_order: data.display_order, admin: data.admin, @@ -358,7 +359,7 @@ export class WorkflowSpecListComponent implements OnInit { private _addWorkflowSpec(newSpec: WorkflowSpec) { this.api.addWorkflowSpecification(newSpec).subscribe(_ => { - this._loadWorkflowSpecs(newSpec.name); + this._loadWorkflowSpecs(newSpec.id); this._displayMessage('Saved new workflow spec.'); }); } @@ -366,7 +367,7 @@ export class WorkflowSpecListComponent implements OnInit { private _deleteWorkflowSpec(workflowSpec: WorkflowSpec) { this.api.deleteWorkflowSpecification(workflowSpec.id).subscribe(() => { this._loadWorkflowSpecs(); - this._displayMessage(`Deleted workflow spec ${workflowSpec.name}.`); + this._displayMessage(`Deleted workflow spec ${workflowSpec.id}.`); }); } @@ -387,7 +388,7 @@ export class WorkflowSpecListComponent implements OnInit { private _deleteWorkflowSpecCategory(workflowSpecCategory: WorkflowSpecCategory) { this.api.deleteWorkflowSpecCategory(workflowSpecCategory.id).subscribe(() => { this._loadWorkflowSpecCategories(); - this._displayMessage(`Deleted workflow spec category ${workflowSpecCategory.name}.`); + this._displayMessage(`Deleted workflow spec category ${workflowSpecCategory.id}.`); }); }