diff --git a/Directory.Packages.props b/Directory.Packages.props index a870fec07..4e459b4ec 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -7,7 +7,7 @@ - + diff --git a/Elsa.Studio.sln.DotSettings b/Elsa.Studio.sln.DotSettings index 2c114d9cc..6df5f691b 100644 --- a/Elsa.Studio.sln.DotSettings +++ b/Elsa.Studio.sln.DotSettings @@ -6,6 +6,7 @@ True True True + True True True True diff --git a/src/framework/Elsa.Studio.Core/Elsa.Studio.Core.csproj b/src/framework/Elsa.Studio.Core/Elsa.Studio.Core.csproj index 5ec710313..3b6dd3dab 100644 --- a/src/framework/Elsa.Studio.Core/Elsa.Studio.Core.csproj +++ b/src/framework/Elsa.Studio.Core/Elsa.Studio.Core.csproj @@ -15,7 +15,7 @@ - + diff --git a/src/modules/Elsa.Studio.Labels/UI/Components/CreateLabelDialog.razor b/src/modules/Elsa.Studio.Labels/UI/Components/CreateLabelDialog.razor index ea5d2e5dd..1c42ea96e 100644 --- a/src/modules/Elsa.Studio.Labels/UI/Components/CreateLabelDialog.razor +++ b/src/modules/Elsa.Studio.Labels/UI/Components/CreateLabelDialog.razor @@ -5,16 +5,21 @@ - - - - - - - + @* Blazilla's FluentValidator captures the Validator instance during OnInitialized and ignores later reassignment, + so the form must not render until _validator (assigned after an awaited API lookup) is available. *@ + @if (_validator is not null) + { + + + + + + + + } @Localizer["Cancel"] - @Localizer["Ok"] + @Localizer["Ok"] \ No newline at end of file diff --git a/src/modules/Elsa.Studio.Labels/UI/Components/CreateLabelDialog.razor.cs b/src/modules/Elsa.Studio.Labels/UI/Components/CreateLabelDialog.razor.cs index aeeca8079..64973be75 100644 --- a/src/modules/Elsa.Studio.Labels/UI/Components/CreateLabelDialog.razor.cs +++ b/src/modules/Elsa.Studio.Labels/UI/Components/CreateLabelDialog.razor.cs @@ -1,4 +1,4 @@ -using Blazored.FluentValidation; +using Blazilla; using Elsa.Studio.Contracts; using Elsa.Studio.Labels.Client; using Elsa.Studio.Labels.Models; @@ -14,8 +14,7 @@ public partial class CreateLabelDialog { private readonly LabelInputModel _inputModel = new() { Name = string.Empty }; private EditContext _editContext = null!; - private FluentValidationValidator _fluentValidationValidator = null!; - private LabelInputModelValidator _validator = null!; + private LabelInputModelValidator? _validator; /// The default name of the agent to create. [Parameter] public string LabelName { get; set; } = ""; @@ -40,7 +39,12 @@ private Task OnCancelClicked() private async Task OnSubmitClicked() { - if(!await _fluentValidationValidator.ValidateAsync()) + // Guard against submitting while the validator is still loading: no FluentValidator is attached to + // _editContext yet, so ValidateAsync() would find no messages and return true, letting an empty label through. + if (_validator is null) + return; + + if(!await _editContext.ValidateAsync()) return; await OnValidSubmit(); diff --git a/src/modules/Elsa.Studio.Labels/_Imports.razor b/src/modules/Elsa.Studio.Labels/_Imports.razor index 029320166..87ac067ba 100644 --- a/src/modules/Elsa.Studio.Labels/_Imports.razor +++ b/src/modules/Elsa.Studio.Labels/_Imports.razor @@ -1,6 +1,6 @@ @using Elsa.Studio.Components @using Elsa.Studio.Localization -@using Blazored.FluentValidation +@using Blazilla @using Microsoft.AspNetCore.Authorization @using Microsoft.AspNetCore.Components.Web @using Microsoft.AspNetCore.Components.Routing diff --git a/src/modules/Elsa.Studio.Workflows.Tests/Support/ControlledWorkflowDefinitionService.cs b/src/modules/Elsa.Studio.Workflows.Tests/Support/ControlledWorkflowDefinitionService.cs new file mode 100644 index 000000000..cf59166a0 --- /dev/null +++ b/src/modules/Elsa.Studio.Workflows.Tests/Support/ControlledWorkflowDefinitionService.cs @@ -0,0 +1,81 @@ +using Elsa.Api.Client.Resources.WorkflowDefinitions.Models; +using Elsa.Api.Client.Resources.WorkflowDefinitions.Requests; +using Elsa.Api.Client.Resources.WorkflowDefinitions.Responses; +using Elsa.Api.Client.Shared.Models; +using Elsa.Studio.Models; +using Elsa.Studio.Workflows.Domain.Contracts; +using Elsa.Studio.Workflows.Domain.Models; + +namespace Elsa.Studio.Workflows.Tests.Support; + +/// +/// An stub that lets a test control when an asynchronous name +/// uniqueness check completes and records how the create dialog invoked it. Every member other than +/// and +/// is unused by the tests that consume this stub and throws to flag an unexpected call. +/// +internal sealed class ControlledWorkflowDefinitionService : IWorkflowDefinitionService +{ + private readonly Queue _pendingValidations = new(); + + public int CreateCallCount { get; private set; } + public string? CreatedName { get; private set; } + + public PendingValidation EnqueueValidation() + { + var validation = new PendingValidation(); + lock (_pendingValidations) + _pendingValidations.Enqueue(validation); + return validation; + } + + public async Task GetIsNameUniqueAsync(string name, string? definitionId = null, CancellationToken cancellationToken = default) + { + PendingValidation validation; + lock (_pendingValidations) + validation = _pendingValidations.Dequeue(); + + validation.Name = name; + validation.Started.TrySetResult(true); + return await validation.Result.Task.WaitAsync(cancellationToken); + } + + public Task> CreateNewDefinitionAsync(string name, string? description, string? rootActivityTemplateKey, Action? configureRequest = null, CancellationToken cancellationToken = default) + { + CreateCallCount++; + CreatedName = name; + return Task.FromResult(new Result(new WorkflowDefinition + { + Name = name, + Description = description + })); + } + + public sealed class PendingValidation + { + public string? Name { get; set; } + public TaskCompletionSource Started { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + public TaskCompletionSource Result { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + } + + public Task> ListAsync(ListWorkflowDefinitionsRequest request, VersionOptions? versionOptions = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task FindByDefinitionIdAsync(string definitionId, VersionOptions? versionOptions = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task FindByIdAsync(string id, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> FindManyByIdAsync(IEnumerable ids, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task FindSubgraphAsync(string id, string? parentNodeId = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetPathSegmentsAsync(string id, string? childNodeId = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task DeleteAsync(string definitionId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task DeleteVersionAsync(WorkflowDefinitionVersion workflowDefinitionVersion, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task PublishAsync(string definitionId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> RetractAsync(string definitionId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task BulkDeleteAsync(IEnumerable definitionIds, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task BulkDeleteVersionsAsync(IEnumerable workflowDefinitionVersions, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task BulkPublishAsync(IEnumerable definitionIds, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task BulkRetractAsync(IEnumerable definitionIds, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GenerateUniqueNameAsync(CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> CreateNewDefinitionAsync(string name, string? description = null, Action? configureRequest = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task ExportDefinitionAsync(string definitionId, VersionOptions? versionOptions = null, bool includeConsumingWorkflows = false, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task BulkExportDefinitionsAsync(IEnumerable ids, bool includeConsumingWorkflows = false, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task UpdateReferencesAsync(string definitionId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task ExecuteAsync(string definitionId, ExecuteWorkflowDefinitionRequest? request, CancellationToken cancellationToken = default) => throw new NotSupportedException(); +} diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowDefinitionMetadataTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowDefinitionMetadataTests.cs index 64903a35e..c97838bfa 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowDefinitionMetadataTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowDefinitionMetadataTests.cs @@ -9,6 +9,7 @@ using Elsa.Studio.Workflows.Domain.Contracts; using Elsa.Studio.Workflows.Domain.Models; using Elsa.Studio.Workflows.Domain.Notifications; +using Elsa.Studio.Workflows.Tests.Support; using Bunit; using Microsoft.AspNetCore.Components; using Microsoft.AspNetCore.Components.Rendering; @@ -230,6 +231,58 @@ public async Task FormSubmissionDoesNotCommitWhileValidationIsPendingOrWhenItFai Assert.Contains("A workflow with this name already exists.", cut.Markup); } + [Fact] + public async Task FormSubmissionDoesNotCommitAnEmptyName() + { + var definition = CreateDefinition(); + var callbackValues = new List<(string? Name, string? Description)>(); + var cut = RenderMetadata(definition, callbackValues); + var nameInput = cut.FindComponents>()[0].Find("input"); + + await nameInput.InputAsync(string.Empty); + var validation = _workflowDefinitionService.EnqueueValidation(); + var submitTask = cut.Find("form").SubmitAsync(); + await validation.Started.Task.WaitAsync(TimeSpan.FromSeconds(5)); + validation.Result.SetResult(true); + await submitTask; + + Assert.Equal("initial name", definition.Name); + Assert.Empty(callbackValues); + Assert.Contains("Please enter a name for the workflow.", cut.Markup); + } + + [Fact] + public async Task AParentRerenderAfterACommitKeepsTheFormValidating() + { + var definition = CreateDefinition(); + var callbackValues = new List<(string? Name, string? Description)>(); + var cut = RenderMetadata(definition, callbackValues); + + await cut.FindComponents>()[0].Find("input").InputAsync("committed name"); + var commit = _workflowDefinitionService.EnqueueValidation(); + var blurTask = cut.FindComponents>()[0].Find("input").BlurAsync(); + await commit.Started.Task.WaitAsync(TimeSpan.FromSeconds(5)); + commit.Result.SetResult(true); + await blurTask; + + Assert.Single(callbackValues); + + // The parent rerenders this component while handling WorkflowDefinitionUpdated, which must not + // disturb the form's EditContext or the validation wiring hanging off it. + cut.Render(parameters => parameters.Add(x => x.WorkflowDefinition, definition)); + + await cut.FindComponents>()[0].Find("input").InputAsync(string.Empty); + var rejected = _workflowDefinitionService.EnqueueValidation(); + var submitTask = cut.Find("form").SubmitAsync(); + await rejected.Started.Task.WaitAsync(TimeSpan.FromSeconds(5)); + rejected.Result.SetResult(true); + await submitTask; + + Assert.Equal("committed name", definition.Name); + Assert.Single(callbackValues); + Assert.Contains("Please enter a name for the workflow.", cut.Markup); + } + [Fact] public async Task FormSubmissionDoesNotCommitANameChangedWhileItsValidationIsPending() { @@ -630,59 +683,6 @@ private IRenderedComponent RenderMetadata(WorkflowDefinition Description = "initial description" }; - private sealed class ControlledWorkflowDefinitionService : IWorkflowDefinitionService - { - private readonly Queue _pendingValidations = new(); - - public PendingValidation EnqueueValidation() - { - var validation = new PendingValidation(); - lock (_pendingValidations) - _pendingValidations.Enqueue(validation); - return validation; - } - - public async Task GetIsNameUniqueAsync(string name, string? definitionId = null, CancellationToken cancellationToken = default) - { - PendingValidation validation; - lock (_pendingValidations) - validation = _pendingValidations.Dequeue(); - - validation.Name = name; - validation.Started.TrySetResult(true); - return await validation.Result.Task.WaitAsync(cancellationToken); - } - - public sealed class PendingValidation - { - public string? Name { get; set; } - public TaskCompletionSource Started { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); - public TaskCompletionSource Result { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); - } - - public Task> ListAsync(ListWorkflowDefinitionsRequest request, VersionOptions? versionOptions = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task FindByDefinitionIdAsync(string definitionId, VersionOptions? versionOptions = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task FindByIdAsync(string id, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task> FindManyByIdAsync(IEnumerable ids, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task FindSubgraphAsync(string id, string? parentNodeId = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task GetPathSegmentsAsync(string id, string? childNodeId = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task DeleteAsync(string definitionId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task DeleteVersionAsync(WorkflowDefinitionVersion workflowDefinitionVersion, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task PublishAsync(string definitionId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task> RetractAsync(string definitionId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task BulkDeleteAsync(IEnumerable definitionIds, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task BulkDeleteVersionsAsync(IEnumerable workflowDefinitionVersions, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task BulkPublishAsync(IEnumerable definitionIds, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task BulkRetractAsync(IEnumerable definitionIds, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task GenerateUniqueNameAsync(CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task> CreateNewDefinitionAsync(string name, string? description = null, Action? configureRequest = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task> CreateNewDefinitionAsync(string name, string? description, string? rootActivityTemplateKey, Action? configureRequest = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task ExportDefinitionAsync(string definitionId, VersionOptions? versionOptions = null, bool includeConsumingWorkflows = false, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task BulkExportDefinitionsAsync(IEnumerable ids, bool includeConsumingWorkflows = false, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task UpdateReferencesAsync(string definitionId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - public Task ExecuteAsync(string definitionId, ExecuteWorkflowDefinitionRequest? request, CancellationToken cancellationToken = default) => throw new NotSupportedException(); - } - private sealed class RecordingWorkflowDefinitionEditorService : IWorkflowDefinitionEditorService { public int SaveCallCount { get; private set; } diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowDialogValidationTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowDialogValidationTests.cs new file mode 100644 index 000000000..11deb1725 --- /dev/null +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowDialogValidationTests.cs @@ -0,0 +1,631 @@ +using Bunit; +using Elsa.Api.Client.Resources.StorageDrivers.Models; +using Elsa.Api.Client.Resources.VariableTypes.Models; +using Elsa.Api.Client.Resources.WorkflowDefinitions.Models; +using Elsa.Api.Client.Resources.WorkflowDefinitions.Requests; +using Elsa.Api.Client.Resources.WorkflowDefinitions.Responses; +using Elsa.Api.Client.Shared.Models; +using Elsa.Studio.Localization; +using Elsa.Studio.Models; +using Elsa.Studio.Workflows.Components.WorkflowDefinitionEditor.Components.WorkflowProperties.Tabs.InputOutput.Components.Inputs; +using Elsa.Studio.Workflows.Components.WorkflowDefinitionEditor.Components.WorkflowProperties.Tabs.InputOutput.Components.Outputs; +using Elsa.Studio.Workflows.Components.WorkflowDefinitionList; +using Elsa.Studio.Workflows.Domain.Contracts; +using Elsa.Studio.Workflows.Domain.Models; +using Elsa.Studio.Workflows.Domain.Services; +using Elsa.Studio.Workflows.Models; +using Elsa.Studio.Workflows.Tests.Support; +using Microsoft.AspNetCore.Components; +using Microsoft.AspNetCore.Components.Web; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Localization; +using MudBlazor; +using MudBlazor.Services; +using MudExtensions.Services; +using Xunit; + +namespace Elsa.Studio.Workflows.Tests; + +/// +/// Covers the FluentValidation wiring of the workflow dialogs. +/// +/// The create and clone dialogs validate the workflow name with an asynchronous uniqueness rule. Blazor +/// decides whether to raise OnValidSubmit from the synchronous part of EditContext.Validate(), +/// so both dialogs route their form through OnSubmit and await the validation before acting on it. +/// Every test drives the native form submission as well as the dialog's Ok button, because only the button +/// path ever awaited the asynchronous rule. +/// +/// +/// The input and output dialogs populate their default field values (e.g. "Input1"/"Output1") only after +/// awaiting the storage driver and variable type services, so the stubs below deliberately yield to +/// reproduce that delay and the tests wait for the resulting render before interacting with the form. +/// +/// +public sealed class WorkflowDialogValidationTests : BunitContext, IAsyncLifetime +{ + private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(5); + private const string WorkflowName = "New workflow"; + private const string DuplicateNameMessage = "A workflow with this name already exists."; + + private readonly ControlledWorkflowDefinitionService _workflowDefinitionService = new(); + private readonly DeferredStorageDriverService _storageDriverService = new(); + private readonly DeferredVariableTypeService _variableTypeService = new(); + private readonly IRenderedComponent _dialogProvider; + + public WorkflowDialogValidationTests() + { + JSInterop.Mode = JSRuntimeMode.Loose; + Services.AddMudServices(); + Services.AddSingleton(); + Services.AddSingleton(_workflowDefinitionService); + Services.AddSingleton(new DefaultWorkflowRootActivityTemplateProvider()); + Services.AddMudExtensions(); + Services.AddSingleton(_storageDriverService); + Services.AddSingleton(_variableTypeService); + Render(); + _dialogProvider = Render(); + } + + Task IAsyncLifetime.InitializeAsync() => Task.CompletedTask; + async Task IAsyncLifetime.DisposeAsync() => await base.DisposeAsync(); + + /// Identifies how the dialog is submitted, since both routes must obey the same validation. + public enum SubmitPath + { + /// A native form submission, which is what pressing Enter in the name field produces. + Form, + + /// The dialog's Ok button, which lives outside the form and calls the component directly. + OkButton + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CreateDialogDoesNotCreateAWorkflowWhenTheNameIsNotUnique(SubmitPath submitPath) + { + var dialog = await ShowWorkflowDialogAsync(); + var validation = _workflowDefinitionService.EnqueueValidation(); + + var submitTask = Submit(submitPath); + await validation.Started.Task.WaitAsync(Timeout); + + Assert.Equal(0, _workflowDefinitionService.CreateCallCount); + Assert.False(dialog.Result.IsCompleted); + + validation.Result.SetResult(false); + await submitTask; + + Assert.Equal(0, _workflowDefinitionService.CreateCallCount); + Assert.False(dialog.Result.IsCompleted); + _dialogProvider.WaitForAssertion(() => Assert.Contains(DuplicateNameMessage, _dialogProvider.Markup)); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CreateDialogCreatesTheWorkflowWhenTheNameIsUnique(SubmitPath submitPath) + { + var dialog = await ShowWorkflowDialogAsync(); + var validation = _workflowDefinitionService.EnqueueValidation(); + + var submitTask = Submit(submitPath); + await validation.Started.Task.WaitAsync(Timeout); + validation.Result.SetResult(true); + await submitTask; + + var result = await dialog.Result.WaitAsync(Timeout); + + Assert.Equal(WorkflowName, validation.Name); + Assert.Equal(1, _workflowDefinitionService.CreateCallCount); + Assert.Equal(WorkflowName, _workflowDefinitionService.CreatedName); + Assert.False(result?.Canceled); + Assert.Equal(WorkflowName, Assert.IsType>(result?.Data).Success?.Name); + Assert.DoesNotContain(DuplicateNameMessage, _dialogProvider.Markup); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CreateDialogDoesNotCreateAWorkflowWhenTheNameChangesWhileTheUniquenessCheckIsPending(SubmitPath submitPath) + { + const string changedName = "Renamed workflow"; + var dialog = await ShowWorkflowDialogAsync(); + var submitValidation = _workflowDefinitionService.EnqueueValidation(); + + var submitTask = Submit(submitPath); + await submitValidation.Started.Task.WaitAsync(Timeout); + + // Changing the name while the submission's uniqueness check is still pending triggers Blazilla's + // own field-changed validation, which runs a second (unrelated) uniqueness check for the new value. + var fieldChangeValidation = _workflowDefinitionService.EnqueueValidation(); + await SetNameAsync(changedName); + await fieldChangeValidation.Started.Task.WaitAsync(Timeout); + fieldChangeValidation.Result.SetResult(true); + + submitValidation.Result.SetResult(true); + await submitTask; + + Assert.Equal(WorkflowName, submitValidation.Name); + Assert.Equal(0, _workflowDefinitionService.CreateCallCount); + Assert.False(dialog.Result.IsCompleted); + + var retryValidation = _workflowDefinitionService.EnqueueValidation(); + var retrySubmitTask = Submit(submitPath); + await retryValidation.Started.Task.WaitAsync(Timeout); + retryValidation.Result.SetResult(true); + await retrySubmitTask; + + var result = await dialog.Result.WaitAsync(Timeout); + + Assert.Equal(changedName, retryValidation.Name); + Assert.Equal(1, _workflowDefinitionService.CreateCallCount); + Assert.Equal(changedName, _workflowDefinitionService.CreatedName); + Assert.False(result?.Canceled); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CreateDialogDoesNotCreateAWorkflowWhenAStaleFieldCheckResolvesAfterTheSubmitCheckReportsADuplicate(SubmitPath submitPath) + { + const string duplicateName = "Duplicate workflow"; + var dialog = await ShowWorkflowDialogAsync(); + + // A field change starts its own (unrelated) uniqueness check for the name. + var fieldChangeValidation = _workflowDefinitionService.EnqueueValidation(); + await SetNameAsync(duplicateName); + await fieldChangeValidation.Started.Task.WaitAsync(Timeout); + + // Submitting the same name starts a second, component-owned uniqueness check. + var submitValidation = _workflowDefinitionService.EnqueueValidation(); + var submitTask = Submit(submitPath); + await submitValidation.Started.Task.WaitAsync(Timeout); + + // The submit's own check reports the name as a duplicate first. + submitValidation.Result.SetResult(false); + await submitTask; + + Assert.Equal(0, _workflowDefinitionService.CreateCallCount); + Assert.False(dialog.Result.IsCompleted); + + // The stale field-change check resolves afterwards with an outdated "unique" answer. It must not + // resurrect a submission that the component already rejected. + fieldChangeValidation.Result.SetResult(true); + + Assert.Equal(0, _workflowDefinitionService.CreateCallCount); + Assert.False(dialog.Result.IsCompleted); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CreateDialogClearsTheDuplicateNameMessageWhenTheNameFieldChanges(SubmitPath submitPath) + { + var dialog = await ShowWorkflowDialogAsync(); + var submitValidation = _workflowDefinitionService.EnqueueValidation(); + + var submitTask = Submit(submitPath); + await submitValidation.Started.Task.WaitAsync(Timeout); + submitValidation.Result.SetResult(false); + await submitTask; + + _dialogProvider.WaitForAssertion(() => Assert.Contains(DuplicateNameMessage, _dialogProvider.Markup)); + + // Blazilla's own field-changed validation clears only its own message store, so the duplicate-name + // message the submit published to its dedicated store must be cleared independently. + var fieldChangeValidation = _workflowDefinitionService.EnqueueValidation(); + await SetNameAsync("Another workflow"); + + _dialogProvider.WaitForAssertion(() => Assert.DoesNotContain(DuplicateNameMessage, _dialogProvider.Markup)); + Assert.False(dialog.Result.IsCompleted); + + fieldChangeValidation.Result.SetResult(true); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CreateDialogDoesNotShowADuplicateNameMessageWhenTheStaleSubmitCheckReportsADuplicateForANameTheUserHasSinceChanged(SubmitPath submitPath) + { + const string changedName = "Renamed workflow"; + var dialog = await ShowWorkflowDialogAsync(); + var submitValidation = _workflowDefinitionService.EnqueueValidation(); + + var submitTask = Submit(submitPath); + await submitValidation.Started.Task.WaitAsync(Timeout); + + // Changing the name while the submission's uniqueness check is still pending triggers Blazilla's + // own field-changed validation, which runs a second (unrelated) uniqueness check for the new value. + var fieldChangeValidation = _workflowDefinitionService.EnqueueValidation(); + await SetNameAsync(changedName); + await fieldChangeValidation.Started.Task.WaitAsync(Timeout); + fieldChangeValidation.Result.SetResult(true); + + // The submit's own check resolves after the name changed, reporting the superseded value as a + // duplicate. Because it no longer describes the current name, it must not be published as a + // duplicate-name message against the current field value. + submitValidation.Result.SetResult(false); + await submitTask; + + Assert.Equal(0, _workflowDefinitionService.CreateCallCount); + Assert.False(dialog.Result.IsCompleted); + Assert.DoesNotContain(DuplicateNameMessage, _dialogProvider.Markup); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CloneDialogDoesNotCloseWhenTheNameIsNotUnique(SubmitPath submitPath) + { + var dialog = await ShowWorkflowDialogAsync(); + var validation = _workflowDefinitionService.EnqueueValidation(); + + var submitTask = Submit(submitPath); + await validation.Started.Task.WaitAsync(Timeout); + + Assert.False(dialog.Result.IsCompleted); + + validation.Result.SetResult(false); + await submitTask; + + Assert.False(dialog.Result.IsCompleted); + _dialogProvider.WaitForAssertion(() => Assert.Contains(DuplicateNameMessage, _dialogProvider.Markup)); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CloneDialogReturnsTheMetadataWhenTheNameIsUnique(SubmitPath submitPath) + { + var dialog = await ShowWorkflowDialogAsync(); + var validation = _workflowDefinitionService.EnqueueValidation(); + + var submitTask = Submit(submitPath); + await validation.Started.Task.WaitAsync(Timeout); + validation.Result.SetResult(true); + await submitTask; + + var result = await dialog.Result.WaitAsync(Timeout); + + Assert.Equal(WorkflowName, validation.Name); + Assert.False(result?.Canceled); + Assert.Equal(WorkflowName, Assert.IsType(result?.Data).Name); + Assert.DoesNotContain(DuplicateNameMessage, _dialogProvider.Markup); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CloneDialogDoesNotCloseWhenTheNameChangesWhileTheUniquenessCheckIsPending(SubmitPath submitPath) + { + const string changedName = "Renamed clone"; + var dialog = await ShowWorkflowDialogAsync(); + var submitValidation = _workflowDefinitionService.EnqueueValidation(); + + var submitTask = Submit(submitPath); + await submitValidation.Started.Task.WaitAsync(Timeout); + + // Changing the name while the submission's uniqueness check is still pending triggers Blazilla's + // own field-changed validation, which runs a second (unrelated) uniqueness check for the new value. + var fieldChangeValidation = _workflowDefinitionService.EnqueueValidation(); + await SetNameAsync(changedName); + await fieldChangeValidation.Started.Task.WaitAsync(Timeout); + fieldChangeValidation.Result.SetResult(true); + + submitValidation.Result.SetResult(true); + await submitTask; + + Assert.Equal(WorkflowName, submitValidation.Name); + Assert.False(dialog.Result.IsCompleted); + + var retryValidation = _workflowDefinitionService.EnqueueValidation(); + var retrySubmitTask = Submit(submitPath); + await retryValidation.Started.Task.WaitAsync(Timeout); + retryValidation.Result.SetResult(true); + await retrySubmitTask; + + var result = await dialog.Result.WaitAsync(Timeout); + + Assert.Equal(changedName, retryValidation.Name); + Assert.False(result?.Canceled); + Assert.Equal(changedName, Assert.IsType(result?.Data).Name); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CloneDialogDoesNotCloseWhenAStaleFieldCheckResolvesAfterTheSubmitCheckReportsADuplicate(SubmitPath submitPath) + { + const string duplicateName = "Duplicate clone"; + var dialog = await ShowWorkflowDialogAsync(); + + // A field change starts its own (unrelated) uniqueness check for the name. + var fieldChangeValidation = _workflowDefinitionService.EnqueueValidation(); + await SetNameAsync(duplicateName); + await fieldChangeValidation.Started.Task.WaitAsync(Timeout); + + // Submitting the same name starts a second, component-owned uniqueness check. + var submitValidation = _workflowDefinitionService.EnqueueValidation(); + var submitTask = Submit(submitPath); + await submitValidation.Started.Task.WaitAsync(Timeout); + + // The submit's own check reports the name as a duplicate first. + submitValidation.Result.SetResult(false); + await submitTask; + + Assert.False(dialog.Result.IsCompleted); + + // The stale field-change check resolves afterwards with an outdated "unique" answer. It must not + // resurrect a submission that the component already rejected. + fieldChangeValidation.Result.SetResult(true); + + Assert.False(dialog.Result.IsCompleted); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CloneDialogClearsTheDuplicateNameMessageWhenTheNameFieldChanges(SubmitPath submitPath) + { + var dialog = await ShowWorkflowDialogAsync(); + var submitValidation = _workflowDefinitionService.EnqueueValidation(); + + var submitTask = Submit(submitPath); + await submitValidation.Started.Task.WaitAsync(Timeout); + submitValidation.Result.SetResult(false); + await submitTask; + + _dialogProvider.WaitForAssertion(() => Assert.Contains(DuplicateNameMessage, _dialogProvider.Markup)); + + // Blazilla's own field-changed validation clears only its own message store, so the duplicate-name + // message the submit published to its dedicated store must be cleared independently. + var fieldChangeValidation = _workflowDefinitionService.EnqueueValidation(); + await SetNameAsync("Another clone"); + + _dialogProvider.WaitForAssertion(() => Assert.DoesNotContain(DuplicateNameMessage, _dialogProvider.Markup)); + Assert.False(dialog.Result.IsCompleted); + + fieldChangeValidation.Result.SetResult(true); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task CloneDialogDoesNotShowADuplicateNameMessageWhenTheStaleSubmitCheckReportsADuplicateForANameTheUserHasSinceChanged(SubmitPath submitPath) + { + const string changedName = "Renamed clone"; + var dialog = await ShowWorkflowDialogAsync(); + var submitValidation = _workflowDefinitionService.EnqueueValidation(); + + var submitTask = Submit(submitPath); + await submitValidation.Started.Task.WaitAsync(Timeout); + + // Changing the name while the submission's uniqueness check is still pending triggers Blazilla's + // own field-changed validation, which runs a second (unrelated) uniqueness check for the new value. + var fieldChangeValidation = _workflowDefinitionService.EnqueueValidation(); + await SetNameAsync(changedName); + await fieldChangeValidation.Started.Task.WaitAsync(Timeout); + fieldChangeValidation.Result.SetResult(true); + + // The submit's own check resolves after the name changed, reporting the superseded value as a + // duplicate. Because it no longer describes the current name, it must not be published as a + // duplicate-name message against the current field value. + submitValidation.Result.SetResult(false); + await submitTask; + + Assert.False(dialog.Result.IsCompleted); + Assert.DoesNotContain(DuplicateNameMessage, _dialogProvider.Markup); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task InputDialogDoesNotCloseWhenTheNameIsEmpty(SubmitPath submitPath) + { + var dialog = await ShowInputDialogAsync(); + await SetNameAsync(string.Empty); + + await Submit(submitPath); + + Assert.False(dialog.Result.IsCompleted); + _dialogProvider.WaitForAssertion(() => Assert.Contains("Please enter a name for the input.", _dialogProvider.Markup)); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task InputDialogClosesWithTheInputWhenTheNameIsValid(SubmitPath submitPath) + { + var dialog = await ShowInputDialogAsync(); + + await Submit(submitPath); + + var result = await dialog.Result.WaitAsync(Timeout); + + Assert.False(result?.Canceled); + Assert.Equal("Input1", Assert.IsType(result?.Data).Name); + Assert.DoesNotContain("Please enter a name for the input.", _dialogProvider.Markup); + } + + [Fact] + public async Task InputDialogDoesNotSubmitBeforeDescriptorLookupsHaveCompleted() + { + var storageDriverGate = new TaskCompletionSource(); + var variableTypeGate = new TaskCompletionSource(); + _storageDriverService.Gate = storageDriverGate; + _variableTypeService.Gate = variableTypeGate; + + var dialog = await ShowDialogAsync( + new DialogParameters { { x => x.WorkflowDefinition, new WorkflowDefinition() } }); + + // The Ok button stays disabled until the descriptor lookups (storage drivers, variable types, UI + // hints) complete and the model is populated. + Assert.Contains("disabled", OkButton().OuterHtml); + + // A non-empty name satisfies the synchronous "required" rule, so submitting via the form (e.g. + // pressing Enter) would reach OnValidSubmit while the descriptor-dependent fields are still unset. + await SetNameAsync("MyInput"); + await Submit(SubmitPath.Form); + + Assert.False(dialog.Result.IsCompleted); + + storageDriverGate.SetResult(true); + variableTypeGate.SetResult(true); + + _dialogProvider.WaitForAssertion(() => Assert.Equal("Input1", NameField().Find("input").GetAttribute("value"))); + Assert.DoesNotContain("disabled", OkButton().OuterHtml); + + await Submit(SubmitPath.Form); + + var result = await dialog.Result.WaitAsync(Timeout); + + Assert.False(result?.Canceled); + Assert.Equal("Input1", Assert.IsType(result?.Data).Name); + } + + [Fact] + public async Task InputDialogLoadsDescriptorsOnceAcrossInitialRenderAndParentReRender() + { + await ShowInputDialogAsync(); + + Assert.Equal(1, _storageDriverService.CallCount); + Assert.Equal(1, _variableTypeService.CallCount); + + var editInputDialog = _dialogProvider.FindComponent(); + editInputDialog.Render(parameters => parameters + .Add(x => x.WorkflowDefinition, new WorkflowDefinition())); + + Assert.Equal(1, _storageDriverService.CallCount); + Assert.Equal(1, _variableTypeService.CallCount); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task OutputDialogDoesNotCloseWhenTheNameIsEmpty(SubmitPath submitPath) + { + var dialog = await ShowOutputDialogAsync(); + await SetNameAsync(string.Empty); + + await Submit(submitPath); + + Assert.False(dialog.Result.IsCompleted); + _dialogProvider.WaitForAssertion(() => Assert.Contains("Please enter a name for the output.", _dialogProvider.Markup)); + } + + [Theory] + [InlineData(SubmitPath.Form)] + [InlineData(SubmitPath.OkButton)] + public async Task OutputDialogClosesWithTheOutputWhenTheNameIsValid(SubmitPath submitPath) + { + var dialog = await ShowOutputDialogAsync(); + + await Submit(submitPath); + + var result = await dialog.Result.WaitAsync(Timeout); + + Assert.False(result?.Canceled); + Assert.Equal("Output1", Assert.IsType(result?.Data).Name); + Assert.DoesNotContain("Please enter a name for the output.", _dialogProvider.Markup); + } + + private async Task ShowDialogAsync(DialogParameters parameters) where TDialog : ComponentBase + { + var dialogService = Services.GetRequiredService(); + var dialog = await _dialogProvider.InvokeAsync(() => dialogService.ShowAsync("Workflow", parameters)); + _dialogProvider.WaitForElement("form"); + return dialog; + } + + private Task ShowWorkflowDialogAsync() where TDialog : ComponentBase => + ShowDialogAsync(new() { { nameof(CloneWorkflowDialog.WorkflowName), WorkflowName } }); + + // The find and the trigger run on the renderer's dispatcher so that a render cannot invalidate the + // element's event handler in between. The returned task still completes only when the handler does. + private Task Submit(SubmitPath submitPath) => _dialogProvider.InvokeAsync(() => submitPath switch + { + SubmitPath.Form => _dialogProvider.Find("form").SubmitAsync(), + SubmitPath.OkButton => OkButton().ClickAsync(new MouseEventArgs()), + _ => throw new ArgumentOutOfRangeException(nameof(submitPath), submitPath, null) + }); + + private async Task ShowInputDialogAsync() + { + var dialog = await ShowDialogAsync( + new DialogParameters { { x => x.WorkflowDefinition, new WorkflowDefinition() } }); + _dialogProvider.WaitForAssertion(() => Assert.Equal("Input1", NameField().Find("input").GetAttribute("value"))); + return dialog; + } + + private async Task ShowOutputDialogAsync() + { + var dialog = await ShowDialogAsync( + new DialogParameters { { x => x.WorkflowDefinition, new WorkflowDefinition() } }); + _dialogProvider.WaitForAssertion(() => Assert.Equal("Output1", NameField().Find("input").GetAttribute("value"))); + return dialog; + } + + private Task SetNameAsync(string name) => NameField().Find("input").ChangeAsync(new ChangeEventArgs { Value = name }); + + private IRenderedComponent> NameField() => _dialogProvider.FindComponents>()[0]; + + private AngleSharp.Dom.IElement OkButton() => _dialogProvider.FindAll("button").Single(x => x.TextContent.Trim() == "Ok"); + + /// + /// Yields before returning so that the input and output dialogs render once with their default field + /// values still unset, reproducing the delay the tests wait out before interacting with the form. + /// Also counts calls so tests can assert the input dialog loads its descriptors once, in + /// OnInitializedAsync, rather than on every parent re-render. + /// + private sealed class DeferredStorageDriverService : IStorageDriverService + { + public int CallCount { get; private set; } + + /// + /// When set, held instead of the default single yield, so a test can control exactly when the + /// lookup completes. + /// + public TaskCompletionSource? Gate { get; set; } + + public async Task> GetStorageDriversAsync(CancellationToken cancellationToken = default) + { + CallCount++; + + if (Gate is not null) + await Gate.Task; + else + await Task.Yield(); + + return [new("Workflow", "Workflow", 0, false)]; + } + } + + /// + private sealed class DeferredVariableTypeService : IVariableTypeService + { + public int CallCount { get; private set; } + + /// + public TaskCompletionSource? Gate { get; set; } + + public async Task> GetVariableTypesAsync(CancellationToken cancellationToken = default) + { + CallCount++; + + if (Gate is not null) + await Gate.Task; + else + await Task.Yield(); + + return [new("System.String", "String", "Text", null)]; + } + } + + private sealed class TestLocalizer : ILocalizer + { + public LocalizedString this[string? key] => new(key ?? string.Empty, key ?? string.Empty); + public LocalizedString this[string? key, params object[] arguments] => + new(key ?? string.Empty, string.Format(key ?? string.Empty, arguments)); + } +} diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Inputs/EditInputDialog.razor b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Inputs/EditInputDialog.razor index ab722b284..86ec887dd 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Inputs/EditInputDialog.razor +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Inputs/EditInputDialog.razor @@ -8,8 +8,8 @@ - - + + @@ -46,6 +46,6 @@ @Localizer["Cancel"] - @Localizer["Ok"] + @Localizer["Ok"] \ No newline at end of file diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Inputs/EditInputDialog.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Inputs/EditInputDialog.razor.cs index 14fb95be8..55cfdb62f 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Inputs/EditInputDialog.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Inputs/EditInputDialog.razor.cs @@ -1,4 +1,4 @@ -using Blazored.FluentValidation; +using Blazilla; using Elsa.Api.Client.Resources.StorageDrivers.Models; using Elsa.Api.Client.Resources.VariableTypes.Models; using Elsa.Api.Client.Resources.WorkflowDefinitions.Models; @@ -21,7 +21,7 @@ public partial class EditInputDialog private readonly InputDefinitionModel _model = new(); private EditContext _editContext = default!; private InputModelValidator _validator = default!; - private FluentValidationValidator _fluentValidationValidator = default!; + private bool _initialized; private ICollection _storageDriverDescriptors = new List(); private ICollection _variableTypes = new List(); private ICollection _uiHints = new List(); @@ -36,11 +36,15 @@ public partial class EditInputDialog /// protected override async Task OnInitializedAsync() { + // Instantiate the edit context first so that it is available when rendering (which happens as soon as we call an async method on the next line). _editContext = new(_model); + _validator = new(WorkflowDefinition, Localizer); + _storageDriverDescriptors = (await StorageDriverService.GetStorageDriversAsync()).ToList(); _variableTypes = (await VariableTypeService.GetVariableTypesAsync()).ToList(); _uiHints = (await GetUIHintsAsync()).ToList(); _groupedVariableTypes = _variableTypes.GroupBy(x => x.Category).ToList(); + _initialized = true; } /// @@ -48,7 +52,7 @@ protected override void OnParametersSet() { _editContext = new(_model); _validator = new(WorkflowDefinition, Localizer); - + if (Input == null) { _model.Name = GetNewInputName(WorkflowDefinition.Inputs); @@ -114,7 +118,10 @@ private Task OnCancelClicked() private async Task OnSubmitClicked() { - if (!await _fluentValidationValidator.ValidateAsync()) + if (!_initialized) + return; + + if (!await _editContext.ValidateAsync()) return; await OnValidSubmit(); @@ -122,6 +129,9 @@ private async Task OnSubmitClicked() private Task OnValidSubmit() { + if (!_initialized) + return Task.CompletedTask; + var input = Input ?? new InputDefinition(); input.Name = _model.Name; diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Outputs/EditOutputDialog.razor b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Outputs/EditOutputDialog.razor index 37090d36c..d376a8b01 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Outputs/EditOutputDialog.razor +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Outputs/EditOutputDialog.razor @@ -6,8 +6,8 @@ - - + + diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Outputs/EditOutputDialog.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Outputs/EditOutputDialog.razor.cs index 2723b2413..665cc881e 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Outputs/EditOutputDialog.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/InputOutput/Components/Outputs/EditOutputDialog.razor.cs @@ -1,4 +1,4 @@ -using Blazored.FluentValidation; +using Blazilla; using Elsa.Api.Client.Resources.StorageDrivers.Models; using Elsa.Api.Client.Resources.VariableTypes.Models; using Elsa.Api.Client.Resources.WorkflowDefinitions.Models; @@ -20,7 +20,6 @@ public partial class EditOutputDialog private readonly OutputDefinitionModel _model = new(); private EditContext _editContext = null!; private OutputModelValidator _validator = null!; - private FluentValidationValidator _fluentValidationValidator = null!; private ICollection _storageDriverDescriptors = new List(); private ICollection _variableTypes = new List(); private ICollection> _groupedVariableTypes = new List>(); @@ -87,7 +86,7 @@ private Task OnCancelClicked() private async Task OnSubmitClicked() { - if (!await _fluentValidationValidator.ValidateAsync()) + if (!await _editContext.ValidateAsync()) return; await OnValidSubmit(); diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/Variables/Components/EditVariableDialog.razor b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/Variables/Components/EditVariableDialog.razor index 5aaed4cdc..a74296a09 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/Variables/Components/EditVariableDialog.razor +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/Variables/Components/EditVariableDialog.razor @@ -7,8 +7,8 @@ - - + + @_model.Id diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/Variables/Components/EditVariableDialog.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/Variables/Components/EditVariableDialog.razor.cs index fbbfca59e..7fb4dee8a 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/Variables/Components/EditVariableDialog.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionEditor/Components/WorkflowProperties/Tabs/Variables/Components/EditVariableDialog.razor.cs @@ -1,4 +1,4 @@ -using Blazored.FluentValidation; +using Blazilla; using Elsa.Api.Client.Resources.StorageDrivers.Models; using Elsa.Api.Client.Resources.VariableTypes.Models; using Elsa.Api.Client.Resources.WorkflowDefinitions.Models; @@ -19,7 +19,6 @@ public partial class EditVariableDialog private readonly VariableModel _model = new(); private EditContext _editContext = null!; private VariableModelValidator _validator = null!; - private FluentValidationValidator _fluentValidationValidator = null!; private ICollection _storageDriverDescriptors = new List(); private ICollection _variableTypes = new List(); private ICollection> _groupedVariableTypes = new List>(); @@ -87,7 +86,7 @@ private Task OnCancelClicked() private async Task OnSubmitClicked() { - if (!await _fluentValidationValidator.ValidateAsync()) + if (!await _editContext.ValidateAsync()) return; await OnValidSubmit(); diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CloneWorkflowDialog.razor b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CloneWorkflowDialog.razor index c44763774..82a3768bc 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CloneWorkflowDialog.razor +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CloneWorkflowDialog.razor @@ -5,8 +5,8 @@ - - + + diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CloneWorkflowDialog.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CloneWorkflowDialog.razor.cs index d5176b5fb..154c225b5 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CloneWorkflowDialog.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CloneWorkflowDialog.razor.cs @@ -1,4 +1,4 @@ -using Blazored.FluentValidation; +using Blazilla; using Elsa.Studio.Workflows.Domain.Contracts; using Elsa.Studio.Workflows.Models; using Elsa.Studio.Workflows.Validators; @@ -11,20 +11,20 @@ namespace Elsa.Studio.Workflows.Components.WorkflowDefinitionList; /// /// A dialog that allows the user to clone a workflow. /// -public partial class CloneWorkflowDialog +public partial class CloneWorkflowDialog : IDisposable { private readonly WorkflowMetadataModel _metadataModel = new(); private EditContext _editContext = null!; private WorkflowPropertiesModelValidator _validator = null!; - private FluentValidationValidator _fluentValidationValidator = null!; - + private WorkflowMetadataSubmitValidator _submitValidator = null!; + /// /// The name of the workflow to create. /// [Parameter] public string WorkflowName { get; set; } = null!; [Parameter] public string? WorkflowDescription { get; set; } [CascadingParameter] private IMudDialogInstance MudDialog { get; set; } = null!; - [Inject] private IWorkflowDefinitionService WorkflowDefinitionService { get; set; } = null!; + [Inject] private IWorkflowDefinitionService WorkflowDefinitionService { get; set; } = null!; /// protected override void OnParametersSet() @@ -33,6 +33,11 @@ protected override void OnParametersSet() _metadataModel.Description = WorkflowDescription; _editContext = new(_metadataModel); _validator = new(WorkflowDefinitionService, Localizer); + + // Discard the previous instance's field-changed subscription before it is replaced, so it does not + // keep running against an edit context this component no longer uses. + _submitValidator?.Dispose(); + _submitValidator = new(_validator, _metadataModel, _editContext); } private Task OnCancelClicked() @@ -41,17 +46,28 @@ private Task OnCancelClicked() return Task.CompletedTask; } - private async Task OnSubmitClicked() - { - if(!await _fluentValidationValidator.ValidateAsync()) - return; + private Task OnSubmitClicked() => ValidateAndSubmitAsync(); - await OnValidSubmit(); - } + // Blazilla runs the async uniqueness rule outside the synchronous validation pass, so the form is + // routed through OnSubmit: OnValidSubmit would fire before that rule completed and could clone a + // workflow under a name that turns out to be taken. + private Task OnSubmit(EditContext _) => ValidateAndSubmitAsync(); - private Task OnValidSubmit() + private async Task ValidateAndSubmitAsync() { + // Validate directly against the component-owned validator rather than EditContext.ValidateAsync(), + // whose shared, unversioned message store can be overwritten by Blazilla's own asynchronous + // field-change validation resolving after this check does. The validator also discards a result + // that no longer describes the current name, so a bound-name change while the check was pending + // never submits a stale value; the user's next submission re-validates the current value. + var isValid = await _submitValidator.ValidateAndPublishAsync(); + + if (!isValid) + return; + MudDialog.Close(_metadataModel); - return Task.CompletedTask; } -} \ No newline at end of file + + /// + public void Dispose() => _submitValidator?.Dispose(); +} diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CreateWorkflowDialog.razor b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CreateWorkflowDialog.razor index 000d89d4a..fcdf7e3d4 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CreateWorkflowDialog.razor +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CreateWorkflowDialog.razor @@ -7,8 +7,8 @@ - - + + diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CreateWorkflowDialog.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CreateWorkflowDialog.razor.cs index 5ac7b3668..af68b7066 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CreateWorkflowDialog.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/CreateWorkflowDialog.razor.cs @@ -1,4 +1,4 @@ -using Blazored.FluentValidation; +using Blazilla; using Elsa.Studio.Localization; using Elsa.Studio.Workflows.Domain.Contracts; using Elsa.Studio.Workflows.Domain.Models; @@ -13,15 +13,15 @@ namespace Elsa.Studio.Workflows.Components.WorkflowDefinitionList; /// /// A dialog that allows the user to create a new workflow. /// -public partial class CreateWorkflowDialog +public partial class CreateWorkflowDialog : IDisposable { private readonly WorkflowMetadataModel _metadataModel = new(); private IReadOnlyCollection _rootActivityTemplates = []; private string? _selectedRootActivityTemplateKey; private EditContext _editContext = null!; private WorkflowPropertiesModelValidator _validator = null!; - private FluentValidationValidator _fluentValidationValidator = null!; - + private WorkflowMetadataSubmitValidator _submitValidator = null!; + /// /// The name of the workflow to create. /// @@ -36,6 +36,12 @@ protected override void OnParametersSet() _metadataModel.Name = WorkflowName; _editContext = new(_metadataModel); _validator = new(WorkflowDefinitionService, Localizer); + + // Discard the previous instance's field-changed subscription before it is replaced, so it does not + // keep running against an edit context this component no longer uses. + _submitValidator?.Dispose(); + _submitValidator = new(_validator, _metadataModel, _editContext); + _rootActivityTemplates = WorkflowRootActivityTemplateProvider.List(); _selectedRootActivityTemplateKey ??= WorkflowRootActivityTemplateProvider.GetDefault().Key; } @@ -46,18 +52,29 @@ private Task OnCancelClicked() return Task.CompletedTask; } - private async Task OnSubmitClicked() - { - if(!await _fluentValidationValidator.ValidateAsync()) - return; + private Task OnSubmitClicked() => ValidateAndSubmitAsync(); - await OnValidSubmit(); - } + // Blazilla runs the async uniqueness rule outside the synchronous validation pass, so the form is + // routed through OnSubmit: OnValidSubmit would fire before that rule completed and could create a + // workflow whose name turns out to be taken. + private Task OnSubmit(EditContext _) => ValidateAndSubmitAsync(); - private async Task OnValidSubmit() + private async Task ValidateAndSubmitAsync() { + // Validate directly against the component-owned validator rather than EditContext.ValidateAsync(), + // whose shared, unversioned message store can be overwritten by Blazilla's own asynchronous + // field-change validation resolving after this check does. The validator also discards a result + // that no longer describes the current name, so a bound-name change while the check was pending + // never submits a stale value; the user's next submission re-validates the current value. + var isValid = await _submitValidator.ValidateAndPublishAsync(); + + if (!isValid) + return; + var result = await WorkflowDefinitionService.CreateNewDefinitionAsync(_metadataModel.Name!, _metadataModel.Description!, _selectedRootActivityTemplateKey); MudDialog.Close(result); } + /// + public void Dispose() => _submitValidator?.Dispose(); } diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/WorkflowMetadataValidation.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/WorkflowMetadataValidation.cs new file mode 100644 index 000000000..4e6374e9b --- /dev/null +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowDefinitionList/WorkflowMetadataValidation.cs @@ -0,0 +1,71 @@ +using Elsa.Studio.Workflows.Models; +using FluentValidation; +using Microsoft.AspNetCore.Components.Forms; + +namespace Elsa.Studio.Workflows.Components.WorkflowDefinitionList; + +/// +/// Validates a directly against its validator and publishes the result to +/// its own message store. This keeps the submit decision independent of Blazilla's shared, unversioned +/// field-change validation, which can otherwise overwrite the message store with a stale answer from an +/// earlier asynchronous uniqueness check. +/// +/// +/// Owns the message store's subscription to the edit context's +/// event, so that a duplicate-name message this instance published does not linger once the user edits the +/// field it was published against. Dispose the instance when its edit context is discarded (e.g. because the +/// owning dialog rebuilds it in OnParametersSet) so its handler does not keep running against a +/// replaced context. +/// +internal sealed class WorkflowMetadataSubmitValidator : IDisposable +{ + private readonly IValidator _validator; + private readonly WorkflowMetadataModel _model; + private readonly EditContext _editContext; + private readonly ValidationMessageStore _validationMessages; + + public WorkflowMetadataSubmitValidator(IValidator validator, WorkflowMetadataModel model, EditContext editContext) + { + _validator = validator; + _model = model; + _editContext = editContext; + _validationMessages = new(editContext); + _editContext.OnFieldChanged += OnFieldChanged; + } + + /// + /// Validates the model and publishes the result to this instance's message store, unless the bound name + /// changed while the (asynchronous) validation was in flight. In that case the result no longer describes + /// the current value, so it is discarded without being published or acted on; the caller must treat this + /// as "not submitted" and let the user's next submission re-validate the current value. + /// + public async Task ValidateAndPublishAsync() + { + var submittedName = _model.Name; + + var result = await _validator.ValidateAsync(_model); + + if (!string.Equals(_model.Name, submittedName, StringComparison.Ordinal)) + return false; + + _validationMessages.Clear(); + + foreach (var error in result.Errors) + _validationMessages.Add(new FieldIdentifier(_model, error.PropertyName), error.ErrorMessage); + + _editContext.NotifyValidationStateChanged(); + + return result.IsValid; + } + + // Blazilla's field-change validation clears only its own message store, so a duplicate-name message + // this instance published would otherwise remain after the user corrects the field. + private void OnFieldChanged(object? sender, FieldChangedEventArgs e) + { + _validationMessages.Clear(e.FieldIdentifier); + _editContext.NotifyValidationStateChanged(); + } + + /// + public void Dispose() => _editContext.OnFieldChanged -= OnFieldChanged; +} diff --git a/src/modules/Elsa.Studio.Workflows/_Imports.razor b/src/modules/Elsa.Studio.Workflows/_Imports.razor index a63d01328..1fccdfe2b 100644 --- a/src/modules/Elsa.Studio.Workflows/_Imports.razor +++ b/src/modules/Elsa.Studio.Workflows/_Imports.razor @@ -3,7 +3,7 @@ @using Microsoft.AspNetCore.Components.Web @using Microsoft.AspNetCore.Components.Web.Virtualization @using Microsoft.AspNetCore.Components.Forms -@using Blazored.FluentValidation +@using Blazilla @using Elsa.Studio.Localization @using Elsa.Studio.Components @using Elsa.Studio.Workflows.Components