From cc4bf76c13e38e9065d49161b6e0485657424577 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 29 Dec 2021 15:33:24 +0100 Subject: [PATCH] Handle async validators --- .../e2e/src/suites-local/plugins.e2e-spec.ts | 3 +- .../plugin-search.component.scss | 4 ++ .../shared/video-edit.component.ts | 12 ++++-- .../video-go-live.component.ts | 6 +-- .../video-import-torrent.component.ts | 6 +-- .../video-import-url.component.ts | 6 +-- .../video-add-components/video-send.ts | 13 +++--- .../video-upload.component.ts | 9 ++-- .../+video-edit/video-update.component.ts | 13 +++--- .../comment/video-comment-add.component.ts | 2 +- .../form-validators/form-validator.model.ts | 4 +- .../app/shared/shared-forms/form-reactive.ts | 42 ++++++++++++------- .../shared-forms/form-validator.service.ts | 16 ++++++- .../remote-subscribe.component.ts | 2 +- .../register-client-form-field.model.ts | 2 +- 15 files changed, 82 insertions(+), 58 deletions(-) diff --git a/client/e2e/src/suites-local/plugins.e2e-spec.ts b/client/e2e/src/suites-local/plugins.e2e-spec.ts index 14802c1ca..55f020c44 100644 --- a/client/e2e/src/suites-local/plugins.e2e-spec.ts +++ b/client/e2e/src/suites-local/plugins.e2e-spec.ts @@ -63,11 +63,10 @@ describe('Plugins', () => { const checkbox = await getPluginCheckbox() await checkbox.click() - await browserSleep(5000) - await expectSubmitState({ disabled: true }) const error = await $('.form-error*=Should be enabled') + expect(await error.isDisplayed()).toBeTruthy() }) diff --git a/client/src/app/+admin/plugins/plugin-search/plugin-search.component.scss b/client/src/app/+admin/plugins/plugin-search/plugin-search.component.scss index 8bb710fc2..10401e9df 100644 --- a/client/src/app/+admin/plugins/plugin-search/plugin-search.component.scss +++ b/client/src/app/+admin/plugins/plugin-search/plugin-search.component.scss @@ -28,3 +28,7 @@ font-size: 13px; font-weight: $font-semibold; } + +.alert { + margin-top: 15px; +} diff --git a/client/src/app/+videos/+video-edit/shared/video-edit.component.ts b/client/src/app/+videos/+video-edit/shared/video-edit.component.ts index 8ce36121d..be3bbe9be 100644 --- a/client/src/app/+videos/+video-edit/shared/video-edit.component.ts +++ b/client/src/app/+videos/+video-edit/shared/video-edit.component.ts @@ -2,7 +2,7 @@ import { forkJoin } from 'rxjs' import { map } from 'rxjs/operators' import { SelectChannelItem } from 'src/types/select-options-item.model' import { ChangeDetectorRef, Component, EventEmitter, Input, NgZone, OnDestroy, OnInit, Output, ViewChild } from '@angular/core' -import { AbstractControl, FormArray, FormControl, FormGroup, ValidationErrors, Validators } from '@angular/forms' +import { AbstractControl, FormArray, FormControl, FormGroup, Validators } from '@angular/forms' import { HooksService, PluginService, ServerService } from '@app/core' import { removeElementFromArray } from '@app/helpers' import { BuildFormValidator } from '@app/shared/form-validators' @@ -309,10 +309,10 @@ export class VideoEditComponent implements OnInit, OnDestroy { for (const setting of this.pluginFields) { await this.pluginService.translateSetting(setting.pluginInfo.plugin.npmName, setting.commonOptions) - const validator = (control: AbstractControl): ValidationErrors | null => { + const validator = async (control: AbstractControl) => { if (!setting.commonOptions.error) return null - const error = setting.commonOptions.error({ formValues: this.form.value, value: control.value }) + const error = await setting.commonOptions.error({ formValues: this.form.value, value: control.value }) return error?.error ? { [setting.commonOptions.name]: error.text } : null } @@ -320,7 +320,8 @@ export class VideoEditComponent implements OnInit, OnDestroy { const name = setting.commonOptions.name pluginObj[name] = { - VALIDATORS: [ validator ], + ASYNC_VALIDATORS: [ validator ], + VALIDATORS: [], MESSAGES: {} } @@ -342,6 +343,9 @@ export class VideoEditComponent implements OnInit, OnDestroy { this.cd.detectChanges() this.pluginFieldsAdded.emit() + + // Plugins may need other control values to calculate potential errors + this.form.valueChanges.subscribe(() => this.formValidatorService.updateTreeValidity(this.pluginDataFormGroup)) } private trackPrivacyChange () { diff --git a/client/src/app/+videos/+video-edit/video-add-components/video-go-live.component.ts b/client/src/app/+videos/+video-edit/video-add-components/video-go-live.component.ts index 46a7ebb0b..fde8c884b 100644 --- a/client/src/app/+videos/+video-edit/video-add-components/video-go-live.component.ts +++ b/client/src/app/+videos/+video-edit/video-add-components/video-go-live.component.ts @@ -110,10 +110,8 @@ export class VideoGoLiveComponent extends VideoSend implements OnInit, AfterView }) } - updateSecondStep () { - if (this.checkForm() === false) { - return - } + async updateSecondStep () { + if (!await this.isFormValid()) return const video = new VideoEdit() video.patch(this.form.value) diff --git a/client/src/app/+videos/+video-edit/video-add-components/video-import-torrent.component.ts b/client/src/app/+videos/+video-edit/video-add-components/video-import-torrent.component.ts index 5e758910e..c369ba2b7 100644 --- a/client/src/app/+videos/+video-edit/video-add-components/video-import-torrent.component.ts +++ b/client/src/app/+videos/+video-edit/video-add-components/video-import-torrent.component.ts @@ -123,10 +123,8 @@ export class VideoImportTorrentComponent extends VideoSend implements OnInit, Af }) } - updateSecondStep () { - if (this.checkForm() === false) { - return - } + async updateSecondStep () { + if (!await this.isFormValid()) return this.video.patch(this.form.value) diff --git a/client/src/app/+videos/+video-edit/video-add-components/video-import-url.component.ts b/client/src/app/+videos/+video-edit/video-add-components/video-import-url.component.ts index 2ea70ed55..0c78669c1 100644 --- a/client/src/app/+videos/+video-edit/video-add-components/video-import-url.component.ts +++ b/client/src/app/+videos/+video-edit/video-add-components/video-import-url.component.ts @@ -124,10 +124,8 @@ export class VideoImportUrlComponent extends VideoSend implements OnInit, AfterV }) } - updateSecondStep () { - if (this.checkForm() === false) { - return - } + async updateSecondStep () { + if (!await this.isFormValid()) return this.video.patch(this.form.value) diff --git a/client/src/app/+videos/+video-edit/video-add-components/video-send.ts b/client/src/app/+videos/+video-edit/video-add-components/video-send.ts index 5e086ef42..3d0e1bf2a 100644 --- a/client/src/app/+videos/+video-edit/video-add-components/video-send.ts +++ b/client/src/app/+videos/+video-edit/video-add-components/video-send.ts @@ -60,12 +60,6 @@ export abstract class VideoSend extends FormReactive implements OnInit { }) } - checkForm () { - this.forceCheck() - - return this.form.valid - } - protected updateVideoAndCaptions (video: VideoEdit) { this.loadingBar.useRef().start() @@ -80,4 +74,11 @@ export abstract class VideoSend extends FormReactive implements OnInit { }) ) } + + protected async isFormValid () { + await this.waitPendingCheck() + this.forceCheck() + + return this.form.valid + } } diff --git a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts index fa5800897..2251b0511 100644 --- a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts +++ b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts @@ -226,7 +226,7 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy } isPublishingButtonDisabled () { - return !this.checkForm() || + return !this.form.valid || this.isUpdatingVideo === true || this.videoUploaded !== true || !this.videoUploadedIds.id @@ -239,10 +239,9 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy return $localize`Upload ${videofile.name}` } - updateSecondStep () { - if (this.isPublishingButtonDisabled()) { - return - } + async updateSecondStep () { + if (!await this.isFormValid()) return + if (this.isPublishingButtonDisabled()) return const video = new VideoEdit() video.patch(this.form.value) diff --git a/client/src/app/+videos/+video-edit/video-update.component.ts b/client/src/app/+videos/+video-edit/video-update.component.ts index e44aea10a..5e4955f6a 100644 --- a/client/src/app/+videos/+video-edit/video-update.component.ts +++ b/client/src/app/+videos/+video-edit/video-update.component.ts @@ -91,12 +91,6 @@ export class VideoUpdateComponent extends FormReactive implements OnInit { return { canDeactivate: this.formChanged === false, text } } - checkForm () { - this.forceCheck() - - return this.form.valid - } - isWaitTranscodingEnabled () { if (this.videoDetails.getFiles().length > 1) { // Already transcoded return false @@ -109,8 +103,11 @@ export class VideoUpdateComponent extends FormReactive implements OnInit { return true } - update () { - if (this.checkForm() === false || this.isUpdatingVideo === true) { + async update () { + await this.waitPendingCheck() + this.forceCheck() + + if (!this.form.valid || this.isUpdatingVideo === true) { return } diff --git a/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.ts b/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.ts index 71fb127f6..85da83a4c 100644 --- a/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.ts +++ b/client/src/app/+videos/+video-watch/shared/comment/video-comment-add.component.ts @@ -97,7 +97,7 @@ export class VideoCommentAddComponent extends FormReactive implements OnChanges, } onValidKey () { - this.check() + this.forceCheck() if (!this.form.valid) return this.formValidated() diff --git a/client/src/app/shared/form-validators/form-validator.model.ts b/client/src/app/shared/form-validators/form-validator.model.ts index 6f2472ccd..31c253b9b 100644 --- a/client/src/app/shared/form-validators/form-validator.model.ts +++ b/client/src/app/shared/form-validators/form-validator.model.ts @@ -1,7 +1,9 @@ -import { ValidatorFn } from '@angular/forms' +import { AsyncValidatorFn, ValidatorFn } from '@angular/forms' export type BuildFormValidator = { VALIDATORS: ValidatorFn[] + ASYNC_VALIDATORS?: AsyncValidatorFn[] + MESSAGES: { [ name: string ]: string } } diff --git a/client/src/app/shared/shared-forms/form-reactive.ts b/client/src/app/shared/shared-forms/form-reactive.ts index 30b59c141..07a12c6f6 100644 --- a/client/src/app/shared/shared-forms/form-reactive.ts +++ b/client/src/app/shared/shared-forms/form-reactive.ts @@ -1,4 +1,6 @@ + import { FormGroup } from '@angular/forms' +import { wait } from '@root-helpers/utils' import { BuildFormArgument, BuildFormDefaultValues } from '../form-validators/form-validator.model' import { FormValidatorService } from './form-validator.service' @@ -22,30 +24,42 @@ export abstract class FormReactive { this.formErrors = formErrors this.validationMessages = validationMessages - this.form.valueChanges.subscribe(() => this.onValueChanged(this.form, this.formErrors, this.validationMessages, false)) + this.form.statusChanges.subscribe(async status => { + // FIXME: remove when https://github.com/angular/angular/issues/41519 is fixed + await this.waitPendingCheck() + + this.onStatusChanged(this.form, this.formErrors, this.validationMessages) + }) + } + + protected async waitPendingCheck () { + if (this.form.status !== 'PENDING') return + + // FIXME: the following line does not work: https://github.com/angular/angular/issues/41519 + // return firstValueFrom(this.form.statusChanges.pipe(filter(status => status !== 'PENDING'))) + // So we have to fallback to active wait :/ + + do { + await wait(10) + } while (this.form.status === 'PENDING') } protected forceCheck () { - return this.onValueChanged(this.form, this.formErrors, this.validationMessages, true) + this.onStatusChanged(this.form, this.formErrors, this.validationMessages, false) } - protected check () { - return this.onValueChanged(this.form, this.formErrors, this.validationMessages, false) - } - - private onValueChanged ( + private onStatusChanged ( form: FormGroup, formErrors: FormReactiveErrors, validationMessages: FormReactiveValidationMessages, - forceCheck = false + onlyDirty = true ) { for (const field of Object.keys(formErrors)) { if (formErrors[field] && typeof formErrors[field] === 'object') { - this.onValueChanged( + this.onStatusChanged( form.controls[field] as FormGroup, formErrors[field] as FormReactiveErrors, - validationMessages[field] as FormReactiveValidationMessages, - forceCheck + validationMessages[field] as FormReactiveValidationMessages ) continue } @@ -56,8 +70,7 @@ export abstract class FormReactive { if (control.dirty) this.formChanged = true - if (forceCheck) control.updateValueAndValidity({ emitEvent: false }) - if (!control || !control.dirty || !control.enabled || control.valid) continue + if (!control || (onlyDirty && !control.dirty) || !control.enabled || !control.errors) continue const staticMessages = validationMessages[field] for (const key of Object.keys(control.errors)) { @@ -65,11 +78,10 @@ export abstract class FormReactive { // Try to find error message in static validation messages first // Then check if the validator returns a string that is the error - if (typeof formErrorValue === 'boolean') formErrors[field] += staticMessages[key] + ' ' + if (staticMessages[key]) formErrors[field] += staticMessages[key] + ' ' else if (typeof formErrorValue === 'string') formErrors[field] += control.errors[key] else throw new Error('Form error value of ' + field + ' is invalid') } } } - } diff --git a/client/src/app/shared/shared-forms/form-validator.service.ts b/client/src/app/shared/shared-forms/form-validator.service.ts index 055fbb2d9..0fe50ac9b 100644 --- a/client/src/app/shared/shared-forms/form-validator.service.ts +++ b/client/src/app/shared/shared-forms/form-validator.service.ts @@ -1,5 +1,5 @@ import { Injectable } from '@angular/core' -import { FormBuilder, FormControl, FormGroup, ValidatorFn } from '@angular/forms' +import { AsyncValidatorFn, FormArray, FormBuilder, FormControl, FormGroup, ValidatorFn } from '@angular/forms' import { BuildFormArgument, BuildFormDefaultValues } from '../form-validators/form-validator.model' import { FormReactiveErrors, FormReactiveValidationMessages } from './form-reactive' @@ -68,11 +68,23 @@ export class FormValidatorService { form.addControl( name, - new FormControl(defaultValue, field?.VALIDATORS as ValidatorFn[]) + new FormControl(defaultValue, field?.VALIDATORS as ValidatorFn[], field?.ASYNC_VALIDATORS as AsyncValidatorFn[]) ) } } + updateTreeValidity (group: FormGroup | FormArray): void { + for (const key of Object.keys(group.controls)) { + const abstractControl = group.controls[key] as FormControl + + if (abstractControl instanceof FormGroup || abstractControl instanceof FormArray) { + this.updateTreeValidity(abstractControl) + } else { + abstractControl.updateValueAndValidity({ emitEvent: false }) + } + } + } + private isRecursiveField (field: any) { return field && typeof field === 'object' && !field.MESSAGES && !field.VALIDATORS } diff --git a/client/src/app/shared/shared-user-subscription/remote-subscribe.component.ts b/client/src/app/shared/shared-user-subscription/remote-subscribe.component.ts index a951134eb..369692715 100644 --- a/client/src/app/shared/shared-user-subscription/remote-subscribe.component.ts +++ b/client/src/app/shared/shared-user-subscription/remote-subscribe.component.ts @@ -27,7 +27,7 @@ export class RemoteSubscribeComponent extends FormReactive implements OnInit { } onValidKey () { - this.check() + this.forceCheck() if (!this.form.valid) return this.formValidated() diff --git a/shared/models/plugins/client/register-client-form-field.model.ts b/shared/models/plugins/client/register-client-form-field.model.ts index 30fd63266..153c4a6ea 100644 --- a/shared/models/plugins/client/register-client-form-field.model.ts +++ b/shared/models/plugins/client/register-client-form-field.model.ts @@ -19,7 +19,7 @@ export type RegisterClientFormFieldOptions = { // Return undefined | null if there is no error or return a string with the detailed error // Not supported by plugin setting registration - error?: (options: any) => { error: boolean, text?: string } + error?: (options: any) => Promise<{ error: boolean, text?: string }> } export interface RegisterClientVideoFieldOptions {