AAE-23358: Fix allowing to bypass required form on start event by clicking the start button before form is loaded

This commit is contained in:
Kasia Biernat 2024-07-23 10:01:26 +02:00
parent eda03ed525
commit 0c6364ef84
3 changed files with 94 additions and 88 deletions

View File

@ -1,10 +1,9 @@
<mat-card
*ngIf="(loading$ | async) === false; else spinner"
*ngIf="processDefinitionLoaded && !isFormCloudLoading; else spinner"
class="adf-start-process"
>
<mat-card-content>
<mat-card-title
*ngIf="showTitle"
class="adf-title">
@ -15,7 +14,7 @@
{{ errorMessageId | translate }}
</mat-card-subtitle>
<div *ngIf="!isProcessDefinitionsEmpty(); else emptyProcessDefinitionsList">
<div *ngIf="!isProcessDefinitionsEmpty; else emptyProcessDefinitionsList">
<form [formGroup]="processForm" class="adf-select-process-form">
<mat-form-field
class="adf-process-input-container"
@ -75,7 +74,7 @@
</adf-inplace-form-input>
</form>
<ng-container *ngIf="hasForm() else taskFormCloudButtons">
<ng-container *ngIf="hasForm else taskFormCloudButtons">
<adf-cloud-form
[appName]="appName"
[appVersion]="processDefinitionCurrent.appVersion"
@ -114,7 +113,7 @@
<button
color="primary"
mat-raised-button
[disabled]="disableStartButton() || !isProcessFormValid()"
[disabled]="disableStartButton || !isProcessFormValid"
(click)="startProcess()"
data-automation-id="btn-start"
id="button-start"

View File

@ -140,7 +140,7 @@ describe('StartProcessCloudComponent', () => {
fixture.detectChanges();
const startBtn = fixture.nativeElement.querySelector('#button-start');
expect(component.isProcessFormValid()).toBe(true);
expect(component.isProcessFormValid).toBe(true);
expect(startBtn.disabled).toBe(false);
}));
@ -154,7 +154,7 @@ describe('StartProcessCloudComponent', () => {
expect(component.processDefinitionCurrent.name).toBe(JSON.parse(JSON.stringify(fakeProcessDefinitions[1])).name);
const startBtn = fixture.nativeElement.querySelector('#button-start');
expect(component.isProcessFormValid()).toBe(true);
expect(component.isProcessFormValid).toBe(true);
expect(startBtn.disabled).toBe(false);
});
@ -167,7 +167,7 @@ describe('StartProcessCloudComponent', () => {
const startBtn = fixture.nativeElement.querySelector('#button-start');
expect(startBtn.disabled).toBe(true);
expect(component.isProcessFormValid()).toBe(false);
expect(component.isProcessFormValid).toBe(false);
});
it('should have start button disabled when name not filled out', async () => {
@ -179,7 +179,7 @@ describe('StartProcessCloudComponent', () => {
const startBtn = fixture.nativeElement.querySelector('#button-start');
expect(startBtn.disabled).toBe(true);
expect(component.isProcessFormValid()).toBe(false);
expect(component.isProcessFormValid).toBe(false);
});
it('should include the static input mappings in the resolved values', fakeAsync(() => {
@ -334,7 +334,7 @@ describe('StartProcessCloudComponent', () => {
const startBtn = fixture.nativeElement.querySelector('#button-start');
expect(startBtn.disabled).toBe(false);
expect(component.isProcessFormValid()).toBe(true);
expect(component.isProcessFormValid).toBe(true);
});
it('should have start button enabled when default values are set', async () => {
@ -444,7 +444,7 @@ describe('StartProcessCloudComponent', () => {
await fixture.whenStable();
const processForm = fixture.nativeElement.querySelector('adf-cloud-form');
expect(component.hasForm()).toBeTruthy();
expect(component.hasForm).toBeTruthy();
expect(processForm).not.toBeNull();
});
@ -713,7 +713,7 @@ describe('StartProcessCloudComponent', () => {
await fixture.whenStable();
const processForm = fixture.nativeElement.querySelector('adf-cloud-form');
expect(component.hasForm()).toBeTruthy();
expect(component.hasForm).toBeTruthy();
expect(processForm).not.toBeNull();
const payload: ProcessPayloadCloud = new ProcessPayloadCloud({
@ -896,7 +896,6 @@ describe('StartProcessCloudComponent', () => {
});
it('should hide title', () => {
component.loading$.next(false);
component.showTitle = false;
fixture.detectChanges();
@ -906,7 +905,7 @@ describe('StartProcessCloudComponent', () => {
});
it('should show title', () => {
component.loading$.next(false);
component.processDefinitionLoaded = true;
fixture.detectChanges();
const title = fixture.debugElement.query(By.css('.adf-title'));
@ -915,7 +914,7 @@ describe('StartProcessCloudComponent', () => {
});
it('should show process definition dropdown', () => {
component.loading$.next(false);
component.processDefinitionLoaded = true;
component.processDefinitionList = fakeProcessDefinitions;
fixture.detectChanges();
@ -925,7 +924,7 @@ describe('StartProcessCloudComponent', () => {
});
it('should hide process definition dropdown', () => {
component.loading$.next(false);
component.processDefinitionLoaded = true;
component.processDefinitionList = fakeProcessDefinitions;
component.showSelectProcessDropdown = false;
fixture.detectChanges();
@ -936,7 +935,17 @@ describe('StartProcessCloudComponent', () => {
});
it('should show the loading spinner before process definitions loaded', () => {
component.loading$.next(true);
component.processDefinitionLoaded = false;
fixture.detectChanges();
const spinner = fixture.debugElement.query(By.css('.adf-loading'));
expect(spinner).toBeTruthy();
});
it('should show the loading spinner when cloud form is loading', () => {
component.processDefinitionLoaded = true;
component.isFormCloudLoading = true;
fixture.detectChanges();
const spinner = fixture.debugElement.query(By.css('.adf-loading'));
@ -945,7 +954,7 @@ describe('StartProcessCloudComponent', () => {
});
it('should show the process card after process definitions loaded', () => {
component.loading$.next(false);
component.processDefinitionLoaded = true;
fixture.detectChanges();
const card = fixture.debugElement.query(By.css('.adf-start-process'));
@ -968,13 +977,13 @@ describe('StartProcessCloudComponent', () => {
fixture.detectChanges();
component.processForm.controls['processInstanceName'].setValue(fakeProcessDefinitions[0].id);
component.appName = 'test app name';
component.isLoading = false;
component.isProcessStarting = false;
fixture.detectChanges();
await fixture.whenStable();
const startButton = fixture.debugElement.query(By.css('#button-start'));
expect(startButton).not.toBeNull();
expect(component.disableStartButton()).toBeFalse();
expect(component.disableStartButton).toBeFalse();
expect((startButton.nativeElement as HTMLButtonElement).disabled).toBeFalse();
});
@ -982,13 +991,13 @@ describe('StartProcessCloudComponent', () => {
fixture.detectChanges();
component.processForm.controls['processInstanceName'].setValue(fakeProcessDefinitions[0].id);
component.appName = 'test app name';
component.isLoading = true;
component.isProcessStarting = true;
fixture.detectChanges();
await fixture.whenStable();
const startButton = fixture.debugElement.query(By.css('#button-start'));
expect(startButton).not.toBeNull();
expect(component.disableStartButton()).toBeTrue();
expect(component.disableStartButton).toBeTrue();
expect((startButton.nativeElement as HTMLButtonElement).disabled).toBeTrue();
});
});

View File

@ -19,6 +19,7 @@ import {
Component,
EventEmitter,
HostListener,
inject,
Input,
OnChanges,
OnDestroy,
@ -30,13 +31,13 @@ import {
} from '@angular/core';
import { ContentLinkModel, FORM_FIELD_VALIDATORS, FormFieldValidator, FormModel } from '@alfresco/adf-core';
import { AbstractControl, UntypedFormBuilder, UntypedFormControl, UntypedFormGroup, ValidatorFn, Validators } from '@angular/forms';
import { AbstractControl, FormControl, FormGroup, ValidatorFn, Validators } from '@angular/forms';
import { MatAutocompleteTrigger } from '@angular/material/autocomplete';
import { debounceTime, takeUntil, tap } from 'rxjs/operators';
import { debounceTime, takeUntil } from 'rxjs/operators';
import { ProcessInstanceCloud } from '../models/process-instance-cloud.model';
import { ProcessPayloadCloud } from '../models/process-payload-cloud.model';
import { StartProcessCloudService } from '../services/start-process-cloud.service';
import { BehaviorSubject, Subject } from 'rxjs';
import { Subject } from 'rxjs';
import { ProcessDefinitionCloud } from '../../../models/process-definition-cloud.model';
import { TaskVariableCloud } from '../../../form/models/task-variable-cloud.model';
import { ProcessNameCloudPipe } from '../../../pipes/process-name-cloud.pipe';
@ -115,39 +116,65 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
processDefinitionSelection: EventEmitter<ProcessDefinitionCloud> = new EventEmitter<ProcessDefinitionCloud>();
processDefinitionList: ProcessDefinitionCloud[] = [];
processDefinitionCurrent: ProcessDefinitionCloud;
processDefinitionCurrent?: ProcessDefinitionCloud;
errorMessageId: string = '';
processForm: UntypedFormGroup;
processPayloadCloud = new ProcessPayloadCloud();
filteredProcesses: ProcessDefinitionCloud[] = [];
isLoading = false;
isFormCloudLoaded = false;
formCloud: FormModel;
staticMappings: TaskVariableCloud[] = [];
resolvedValues: TaskVariableCloud[];
resolvedValues?: TaskVariableCloud[];
protected onDestroy$ = new Subject<boolean>();
processDefinitionLoaded = false;
loading$ = new BehaviorSubject<boolean>(!this.processDefinitionLoaded);
constructor(
private startProcessCloudService: StartProcessCloudService,
private formBuilder: UntypedFormBuilder,
private processNameCloudPipe: ProcessNameCloudPipe
) {}
isProcessStarting = false;
isFormCloudLoaded = false;
isFormCloudLoading = false;
processDefinitionLoaded = false;
formCloud?: FormModel;
processForm = new FormGroup({
processInstanceName: new FormControl('', [
Validators.required,
Validators.maxLength(this.getMaxNameLength()),
Validators.pattern('^[^\\s]+(\\s+[^\\s]+)*$')
]),
processDefinition: new FormControl('', [Validators.required, this.processDefinitionNameValidator()])
});
private readonly startProcessCloudService = inject(StartProcessCloudService);
private readonly processNameCloudPipe = inject(ProcessNameCloudPipe);
get isProcessFormValid(): boolean {
if (this.hasForm && this.isFormCloudLoaded) {
return (this.formCloud ? !Object.keys(this.formCloud.values).length : false) || this.formCloud?.isValid || this.isProcessStarting;
} else {
return this.processForm.valid || this.isProcessStarting;
}
}
get disableStartButton(): boolean {
return !this.appName || !this.processDefinition.valid || this.isProcessStarting;
}
get isProcessDefinitionsEmpty(): boolean {
return !this.processDefinitionList.length;
}
get processInstanceName(): FormControl<string> {
return this.processForm.controls.processInstanceName;
}
get processDefinition(): FormControl<string> {
return this.processForm.controls.processDefinition;
}
get hasForm(): boolean {
return !!this.processDefinitionCurrent?.formKey;
}
ngOnInit() {
this.initFieldValidators();
this.processForm = this.formBuilder.group({
processInstanceName: new UntypedFormControl('', [
Validators.required,
Validators.maxLength(this.getMaxNameLength()),
Validators.pattern('^[^\\s]+(\\s+[^\\s]+)*$')
]),
processDefinition: new UntypedFormControl(this.processDefinitionName, [Validators.required, this.processDefinitionNameValidator()])
});
this.processDefinition.setValue(this.processDefinitionName);
this.processDefinition.valueChanges
.pipe(debounceTime(PROCESS_DEFINITION_DEBOUNCE))
.pipe(takeUntil(this.onDestroy$))
@ -175,10 +202,6 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
event.stopPropagation();
}
hasForm(): boolean {
return this.processDefinitionCurrent && !!this.processDefinitionCurrent.formKey;
}
onFormLoaded(form: FormModel) {
this.isFormCloudLoaded = true;
this.formCloud = form;
@ -195,12 +218,13 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
private selectProcessDefinitionByProcessDefinitionName(processDefinitionName: string): void {
this.filteredProcesses = this.getProcessDefinitionListByNameOrKey(processDefinitionName);
if (this.isProcessFormValid() && this.filteredProcesses && this.filteredProcesses.length === 1) {
if (this.isProcessFormValid && this.filteredProcesses && this.filteredProcesses.length === 1) {
this.setProcessDefinitionOnForm(this.filteredProcesses[0].name);
}
}
setProcessDefinitionOnForm(selectedProcessDefinitionName: string) {
this.isFormCloudLoading = true;
const processDefinitionCurrent = this.filteredProcesses.find(
(process: ProcessDefinitionCloud) => process.name === selectedProcessDefinitionName || process.key === selectedProcessDefinitionName
);
@ -210,10 +234,12 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
this.staticMappings = staticMappings;
this.resolvedValues = this.staticMappings.concat(this.values || []);
this.processDefinitionCurrent = processDefinitionCurrent;
this.isFormCloudLoading = false;
},
() => {
this.resolvedValues = this.values;
this.processDefinitionCurrent = processDefinitionCurrent;
this.isFormCloudLoading = false;
}
);
@ -254,13 +280,7 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
this.startProcessCloudService
.getProcessDefinitions(this.appName)
.pipe(
tap(() => {
this.processDefinitionLoaded = true;
this.loading$.next(false);
}),
takeUntil(this.onDestroy$)
)
.pipe(takeUntil(this.onDestroy$))
.subscribe(
(processDefinitionRepresentations: ProcessDefinitionCloud[]) => {
this.processDefinitionList = processDefinitionRepresentations;
@ -276,6 +296,8 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
this.processDefinitionSelectionChanged(processDefinition);
}
}
this.processDefinitionLoaded = true;
},
() => {
this.errorMessageId = 'ADF_CLOUD_PROCESS_LIST.ADF_CLOUD_START_PROCESS.ERROR.LOAD_PROCESS_DEFS';
@ -287,14 +309,6 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
return !!name;
}
isProcessFormValid(): boolean {
if (this.hasForm() && this.isFormCloudLoaded) {
return (this.formCloud ? !Object.keys(this.formCloud.values).length : false) || this.formCloud?.isValid || this.isLoading;
} else {
return this.processForm.valid || this.isLoading;
}
}
private getProcessDefinition(processDefinitionCloud: ProcessDefinitionCloud, processDefinitionName: string): boolean {
return (
(this.isValidName(processDefinitionCloud.name) &&
@ -303,28 +317,24 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
);
}
isProcessDefinitionsEmpty(): boolean {
return this.processDefinitionList.length === 0;
}
buildProcessCloudPayload() {
this.processPayloadCloud.name = this.processInstanceName.value;
if (this.variables) {
this.processPayloadCloud.variables = this.variables;
}
if (this.hasForm()) {
if (this.hasForm) {
this.processPayloadCloud.variables = Object.assign(this.processPayloadCloud.variables, this.formCloud.values);
}
}
startProcess() {
this.isLoading = true;
this.isProcessStarting = true;
let payloadVariables = {};
if (this.variables) {
payloadVariables = this.variables;
}
if (this.hasForm()) {
if (this.hasForm) {
payloadVariables = Object.assign(payloadVariables, this.formCloud.values);
}
const createPayload = new ProcessPayloadCloud({
@ -335,12 +345,12 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
this.startProcessCloudService.startProcess(this.appName, createPayload).subscribe(
(res) => {
this.success.emit(res);
this.isLoading = false;
this.isProcessStarting = false;
},
(err) => {
this.errorMessageId = 'ADF_CLOUD_PROCESS_LIST.ADF_CLOUD_START_PROCESS.ERROR.START';
this.error.emit(err);
this.isLoading = false;
this.isProcessStarting = false;
}
);
}
@ -398,14 +408,6 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
return process.name ? process.name : process.key;
}
get processInstanceName(): UntypedFormControl {
return this.processForm.get('processInstanceName') as UntypedFormControl;
}
get processDefinition(): AbstractControl {
return this.processForm.get('processDefinition');
}
onFormContentClicked(content: ContentLinkModel) {
this.formContentClicked.emit(content);
}
@ -429,8 +431,4 @@ export class StartProcessCloudComponent implements OnChanges, OnInit, OnDestroy
this.onDestroy$.next(true);
this.onDestroy$.complete();
}
disableStartButton(): boolean {
return !this.appName || !this.processDefinition.valid || this.isLoading;
}
}