From f50d6e984e3dc66c424ce4de44d6254ce2413515 Mon Sep 17 00:00:00 2001 From: Shivam Kumar <162433387+Shivamkmr8@users.noreply.github.com> Date: Tue, 8 Sep 2026 04:33:30 +0530 Subject: [PATCH] fix: preserve single latest draft when saving a new identity (#7918) Co-authored-by: Shivamkmr8 --- .../Services/WorkflowDefinitionPublisher.cs | 32 +- .../WorkflowDefinitionVersioning/Tests.cs | 372 +++++++++++++++++- 2 files changed, 393 insertions(+), 11 deletions(-) diff --git a/src/modules/Elsa.Workflows.Management/Services/WorkflowDefinitionPublisher.cs b/src/modules/Elsa.Workflows.Management/Services/WorkflowDefinitionPublisher.cs index ff9b3c394..98fe719ce 100644 --- a/src/modules/Elsa.Workflows.Management/Services/WorkflowDefinitionPublisher.cs +++ b/src/modules/Elsa.Workflows.Management/Services/WorkflowDefinitionPublisher.cs @@ -221,25 +221,37 @@ public class WorkflowDefinitionPublisher( { DefinitionId = definitionId }; - var lastVersion = await workflowDefinitionStore.FindLastVersionAsync(filter, cancellationToken); + var highestVersion = await workflowDefinitionStore.FindLastVersionAsync(filter, cancellationToken); + var latestVersion = await workflowDefinitionStore.FindAsync(new() + { + DefinitionId = definitionId, + VersionOptions = VersionOptions.Latest + }, cancellationToken); - draft.Version = draft.Id == lastVersion?.Id ? lastVersion.Version : lastVersion?.Version + 1 ?? 1; + draft.Version = draft.Id == latestVersion?.Id + ? latestVersion.Version + : draft.Id == highestVersion?.Id + ? highestVersion.Version + : highestVersion?.Version + 1 ?? 1; draft.IsLatest = true; draft = Initialize(draft); await mediator.SendAsync(new WorkflowDefinitionDraftSaving(draft), cancellationToken); - await workflowDefinitionStore.SaveAsync(draft, cancellationToken); + + if (latestVersion != null && latestVersion.Id != draft.Id) + { + latestVersion.IsLatest = false; + await workflowDefinitionStore.SaveManyAsync([latestVersion, draft], cancellationToken); + } + else + { + await workflowDefinitionStore.SaveAsync(draft, cancellationToken); + } await mediator.SendAsync(new WorkflowDefinitionDraftSaved(draft), cancellationToken); - if (lastVersion is null) + if (highestVersion is null) await mediator.SendAsync(new WorkflowDefinitionCreated(definition), cancellationToken); - if (lastVersion is { IsPublished: true, IsLatest: true }) - { - lastVersion.IsLatest = false; - await workflowDefinitionStore.SaveAsync(lastVersion, cancellationToken); - } - return draft; } diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionVersioning/Tests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionVersioning/Tests.cs index 75663612b..353b8a798 100644 --- a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionVersioning/Tests.cs +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/WorkflowDefinitionVersioning/Tests.cs @@ -1,7 +1,14 @@ +using Elsa.Common.Models; +using Elsa.Extensions; +using Elsa.Mediator.Contracts; using Elsa.Testing.Shared; using Elsa.Workflows.Management; using Elsa.Workflows.Management.Entities; +using Elsa.Workflows.Management.Filters; +using Elsa.Workflows.Management.Notifications; +using Elsa.Workflows.Management.Services; using Microsoft.Extensions.DependencyInjection; +using NSubstitute; using Xunit.Abstractions; namespace Elsa.Workflows.IntegrationTests.Scenarios.WorkflowDefinitionVersioning; @@ -15,12 +22,251 @@ public class Tests _services = new TestApplicationBuilder(testOutputHelper).Build(); } + [Fact] + public async Task SaveDraftAsync_ShouldClearLatestFlagFromPreviousUnpublishedDraft() + { + const string definitionId = "test-definition"; + var store = _services.GetRequiredService(); + var publisher = _services.GetRequiredService(); + + var existingDraft = new WorkflowDefinition + { + Id = "v1", + DefinitionId = definitionId, + Version = 1, + IsPublished = false, + IsLatest = true + }; + + var newDraft = new WorkflowDefinition + { + Id = "v2", + DefinitionId = definitionId + }; + + await store.SaveAsync(existingDraft); + + var savedDraft = await publisher.SaveDraftAsync(newDraft); + + var definitions = (await store.FindManyAsync( + new WorkflowDefinitionFilter + { + DefinitionId = definitionId + } + )).ToDictionary(x => x.Id); + + Assert.False(definitions["v1"].IsLatest); + Assert.True(definitions["v2"].IsLatest); + Assert.Single(definitions.Values, x => x.IsLatest); + Assert.Equal(2, definitions["v2"].Version); + Assert.Equal(2, savedDraft.Version); + } + + [Fact] + public async Task SaveDraftAsync_ShouldKeepExistingDraftLatestWhenResaved() + { + const string definitionId = "test-definition"; + var store = _services.GetRequiredService(); + var publisher = _services.GetRequiredService(); + + var existingDraft = new WorkflowDefinition + { + Id = "draft-1", + DefinitionId = definitionId, + Version = 1, + IsPublished = false, + IsLatest = true + }; + + await store.SaveAsync(existingDraft); + + var savedDraft = await publisher.SaveDraftAsync(existingDraft); + + var definitions = (await store.FindManyAsync( + new WorkflowDefinitionFilter + { + DefinitionId = definitionId + } + )).ToList(); + + var persistedDraft = Assert.Single(definitions); + + Assert.Equal("draft-1", persistedDraft.Id); + Assert.Equal(1, persistedDraft.Version); + Assert.True(persistedDraft.IsLatest); + Assert.Equal(1, savedDraft.Version); + Assert.True(savedDraft.IsLatest); + } + + [Fact] + public async Task SaveDraftAsync_ShouldDemoteActualLatestWhenHighestVersionIsNotLatest() + { + const string definitionId = "test-definition"; + var store = _services.GetRequiredService(); + var publisher = _services.GetRequiredService(); + await store.SaveManyAsync([ + CreateDefinition("v1", definitionId, 1, true), + CreateDefinition("v2", definitionId, 2, false) + ]); + + var savedDraft = await publisher.SaveDraftAsync(CreateDefinition("v3", definitionId, 0, false)); + + var definitions = (await store.FindManyAsync(new WorkflowDefinitionFilter { DefinitionId = definitionId })).ToList(); + Assert.False(definitions.Single(x => x.Id == "v1").IsLatest); + Assert.False(definitions.Single(x => x.Id == "v2").IsLatest); + Assert.True(definitions.Single(x => x.Id == "v3").IsLatest); + Assert.Equal(3, savedDraft.Version); + Assert.Equal(3, definitions.Single(x => x.Id == "v3").Version); + Assert.Single(definitions, x => x.IsLatest); + } + + [Fact] + public async Task SaveDraftAsync_ShouldPreserveActualLatestVersionWhenHigherVersionIsNotLatest() + { + const string definitionId = "test-definition"; + var store = _services.GetRequiredService(); + var publisher = _services.GetRequiredService(); + var latestDraft = CreateDefinition("v1", definitionId, 1, true); + await store.SaveManyAsync([ + latestDraft, + CreateDefinition("v2", definitionId, 2, false) + ]); + + var savedDraft = await publisher.SaveDraftAsync(latestDraft); + + var definitions = (await store.FindManyAsync(new WorkflowDefinitionFilter { DefinitionId = definitionId })).ToList(); + Assert.Equal(2, definitions.Count); + Assert.Equal(1, savedDraft.Version); + Assert.Equal(1, definitions.Single(x => x.Id == "v1").Version); + Assert.True(definitions.Single(x => x.Id == "v1").IsLatest); + Assert.False(definitions.Single(x => x.Id == "v2").IsLatest); + Assert.Single(definitions, x => x.IsLatest); + } + + [Fact] + public async Task SaveDraftAsync_ShouldPreserveHighestVersionWhenPromotingNonLatestDraft() + { + const string definitionId = "test-definition"; + var store = _services.GetRequiredService(); + var publisher = _services.GetRequiredService(); + await store.SaveManyAsync([ + CreateDefinition("v1", definitionId, 1, true), + CreateDefinition("v2", definitionId, 2, false) + ]); + var highestDraft = await publisher.GetDraftAsync(definitionId, VersionOptions.SpecificVersion(2)); + + var savedDraft = await publisher.SaveDraftAsync(highestDraft!); + + var definitions = (await store.FindManyAsync(new WorkflowDefinitionFilter { DefinitionId = definitionId })).ToList(); + Assert.Equal(2, definitions.Count); + Assert.Equal("v2", savedDraft.Id); + Assert.Equal(2, savedDraft.Version); + Assert.False(definitions.Single(x => x.Id == "v1").IsLatest); + Assert.True(definitions.Single(x => x.Id == "v2").IsLatest); + Assert.Single(definitions, x => x.IsLatest); + } + + [Fact] + public async Task SaveDraftAsync_ShouldPreservePublishedStateWhenCreatingDraftAfterPublishedLatest() + { + const string definitionId = "test-definition"; + var store = _services.GetRequiredService(); + var publisher = _services.GetRequiredService(); + var publishedDefinition = CreateDefinition("v1", definitionId, 1, true); + publishedDefinition.IsPublished = true; + await store.SaveAsync(publishedDefinition); + + var savedDraft = await publisher.SaveDraftAsync(CreateDefinition("v2", definitionId, 0, false)); + + var definitions = (await store.FindManyAsync(new WorkflowDefinitionFilter { DefinitionId = definitionId })).ToList(); + var persistedPublishedDefinition = definitions.Single(x => x.Id == "v1"); + Assert.True(persistedPublishedDefinition.IsPublished); + Assert.False(persistedPublishedDefinition.IsLatest); + Assert.Equal(1, persistedPublishedDefinition.Version); + Assert.False(savedDraft.IsPublished); + Assert.True(savedDraft.IsLatest); + Assert.Equal(2, savedDraft.Version); + Assert.Single(definitions, x => x.IsLatest); + } + + [Theory] + [InlineData(BatchFailurePoint.Replacement)] + [InlineData(BatchFailurePoint.PreviousLatest)] + public async Task SaveDraftAsync_WhenReplacementBatchFails_ShouldPreserveSinglePreviousLatest( + BatchFailurePoint failurePoint + ) + { + const string definitionId = "test-definition"; + + var persisted = new Dictionary + { + ["v1"] = CreateDefinition("v1", definitionId, 1, true) + }; + + var store = CreateFailingStore(persisted, failurePoint); + var publisher = ActivatorUtilities.CreateInstance( + _services, + store + ); + + var replacement = CreateDefinition("v2", definitionId, 0, false); + + await Assert.ThrowsAsync( + () => publisher.SaveDraftAsync(replacement) + ); + + var previous = Assert.Single(persisted.Values); + + Assert.Equal("v1", previous.Id); + Assert.True(previous.IsLatest); + Assert.Single(persisted.Values, x => x.IsLatest); + } + + [Fact] + public async Task SaveDraftAsync_WhenDraftSavedNotificationFails_ShouldKeepCommittedLatestStateConsistent() + { + const string definitionId = "test-definition"; + + var services = new TestApplicationBuilder( + _services.GetRequiredService() + ) + .ConfigureServices(serviceCollection => + serviceCollection.AddNotificationHandler< + FailingDraftSavedHandler, + WorkflowDefinitionDraftSaved + >() + ) + .Build(); + + var store = services.GetRequiredService(); + var publisher = services.GetRequiredService(); + + await store.SaveAsync(CreateDefinition("v1", definitionId, 1, true)); + + await Assert.ThrowsAsync( + () => publisher.SaveDraftAsync(CreateDefinition("v2", definitionId, 0, false)) + ); + + var definitions = (await store.FindManyAsync( + new WorkflowDefinitionFilter + { + DefinitionId = definitionId + } + )).ToList(); + + Assert.Equal(2, definitions.Count); + Assert.False(definitions.Single(x => x.Id == "v1").IsLatest); + Assert.True(definitions.Single(x => x.Id == "v2").IsLatest); + Assert.Single(definitions, x => x.IsLatest); + } + [Fact] public async Task RevertVersionAsync_ShouldAllocateVersionAfterHighestStoredVersion() { const string definitionId = "test-definition"; var store = _services.GetRequiredService(); var publisher = _services.GetRequiredService(); + var definitions = new WorkflowDefinition[] { new() @@ -55,4 +301,128 @@ public class Tests Assert.Equal(4, revertedDefinition.Version); } -} + + private static IWorkflowDefinitionStore CreateFailingStore( + IDictionary persisted, + BatchFailurePoint failurePoint + ) + { + var store = Substitute.For(); + + store.FindLastVersionAsync( + Arg.Any(), + Arg.Any() + ) + .Returns(_ => + Task.FromResult( + persisted.Values + .OrderByDescending(x => x.Version) + .Select(Clone) + .FirstOrDefault() + ) + ); + + store.FindAsync( + Arg.Is(x => x.VersionOptions!.Value.IsLatest), + Arg.Any() + ) + .Returns(_ => + Task.FromResult( + persisted.Values + .Where(x => x.IsLatest) + .Select(Clone) + .FirstOrDefault() + ) + ); + + store.SaveManyAsync( + Arg.Any>(), + Arg.Any() + ) + .Returns(callInfo => + { + var definitions = callInfo.Arg>().ToList(); + + var snapshot = persisted.ToDictionary( + x => x.Key, + x => Clone(x.Value) + ); + + try + { + foreach (var definition in definitions) + { + if ( + failurePoint == BatchFailurePoint.PreviousLatest + && definition.Id == "v1" + ) + { + throw new TestPersistenceException(); + } + + if ( + failurePoint == BatchFailurePoint.Replacement + && definition.Id == "v2" + ) + { + throw new TestPersistenceException(); + } + + persisted[definition.Id] = Clone(definition); + } + } + catch + { + persisted.Clear(); + + foreach (var item in snapshot) + { + persisted[item.Key] = item.Value; + } + + throw; + } + + return Task.CompletedTask; + }); + + return store; + } + + private static WorkflowDefinition CreateDefinition( + string id, + string definitionId, + int version, + bool isLatest + ) => + new() + { + Id = id, + DefinitionId = definitionId, + Version = version, + IsLatest = isLatest, + IsPublished = false + }; + + private static WorkflowDefinition Clone(WorkflowDefinition definition) => + definition.ShallowClone(); + + public enum BatchFailurePoint + { + Replacement, + PreviousLatest + } + + private class FailingDraftSavedHandler + : INotificationHandler + { + public Task HandleAsync( + WorkflowDefinitionDraftSaved notification, + CancellationToken cancellationToken + ) => throw new TestNotificationException(); + } + + private class TestPersistenceException : Exception; + + private class TestNotificationException : Exception; +} \ No newline at end of file