Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ protected override async ValueTask ExecuteAsync(ActivityExecutionContext context
else
{
// Otherwise, we can complete immediately.
context.SetResult(instanceId);
await context.CompleteActivityAsync();
Comment on lines +97 to 98

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Result type differs between modes

When WaitForCompletion = true, Result is set to the child workflow's output (context.WorkflowInput — an arbitrary object) in OnChildWorkflowCompletedAsync. When WaitForCompletion = false, Result is now a string (the instance ID). Because the property is typed as object, this compiles fine, but callers must know which mode they're in to safely cast the result. Consider adding XML-doc to the inherited Result output (or a code comment here) clarifying the two distinct return semantics.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/modules/Elsa.Workflows.Runtime/Activities/DispatchWorkflow.cs
Line: 97-98

Comment:
**Result type differs between modes**

When `WaitForCompletion = true`, `Result` is set to the child workflow's output (`context.WorkflowInput` — an arbitrary `object`) in `OnChildWorkflowCompletedAsync`. When `WaitForCompletion = false`, `Result` is now a `string` (the instance ID). Because the property is typed as `object`, this compiles fine, but callers must know which mode they're in to safely cast the result. Consider adding XML-doc to the inherited `Result` output (or a code comment here) clarifying the two distinct return semantics.

How can I resolve this? If you propose a fix, please make it concise.

}
}
Expand Down Expand Up @@ -155,4 +156,4 @@ private async ValueTask OnChildWorkflowCompletedAsync(ActivityExecutionContext c
context.Set(Result, input);
await context.CompleteActivityAsync();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
using Elsa.Common.Models;
using Elsa.Extensions;
using Elsa.Testing.Shared;
using Elsa.Workflows;
using Elsa.Workflows.Activities;
using Elsa.Workflows.Management;
using Elsa.Workflows.Models;
using Elsa.Workflows.Runtime;
using Elsa.Workflows.Runtime.Activities;
using Elsa.Workflows.Runtime.Requests;
using Elsa.Workflows.Runtime.Responses;
using Microsoft.Extensions.DependencyInjection;
using NSubstitute;

namespace Elsa.Activities.UnitTests.Composition;

public class DispatchWorkflowTests
{
private const string DefaultWorkflowDefinitionId = "test-workflow-def";
private const string DefaultGeneratedId = "generated-child-id";

[Fact]
public async Task Should_Set_Result_To_Dispatched_Workflow_Instance_Id_When_Not_Waiting()
{
var dispatchWorkflow = new DispatchWorkflow
{
WorkflowDefinitionId = new(DefaultWorkflowDefinitionId),
WaitForCompletion = new(false)
};

var (context, _) = await ExecuteAsync(dispatchWorkflow);

var result = context.GetActivityOutput(() => dispatchWorkflow.Result);
Assert.Equal(DefaultGeneratedId, result);
}

[Fact]
public async Task Should_Dispatch_Workflow_With_Generated_Instance_Id()
{
var dispatchWorkflow = new DispatchWorkflow
{
WorkflowDefinitionId = new(DefaultWorkflowDefinitionId),
WaitForCompletion = new(false)
};

var (_, workflowDispatcher) = await ExecuteAsync(dispatchWorkflow);

await workflowDispatcher.Received(1).DispatchAsync(
Arg.Is<DispatchWorkflowDefinitionRequest>(request => request.InstanceId == DefaultGeneratedId),
Arg.Any<DispatchWorkflowOptions>(),
Arg.Any<CancellationToken>());
}

private static WorkflowGraph CreateMockWorkflowGraph()
{
var writeLine = new WriteLine("Test") { Id = "test-activity" };
var workflow = Workflow.FromActivity(writeLine);
workflow.Id = "test-workflow";
var rootNode = new ActivityNode(writeLine, "Done");
return new(workflow, rootNode, [rootNode]);
}

private static IWorkflowDefinitionService CreateWorkflowDefinitionService(string workflowDefinitionId, WorkflowGraph workflowGraph)
{
var service = Substitute.For<IWorkflowDefinitionService>();
service
.FindWorkflowGraphAsync(workflowDefinitionId, Arg.Any<VersionOptions>(), Arg.Any<CancellationToken>())
.Returns(workflowGraph);
return service;
}

private static IIdentityGenerator CreateIdentityGenerator(string generatedId)
{
var generator = Substitute.For<IIdentityGenerator>();
generator.GenerateId().Returns(generatedId);
return generator;
}

private static IWorkflowDispatcher CreateWorkflowDispatcher()
{
var dispatcher = Substitute.For<IWorkflowDispatcher>();
dispatcher
.DispatchAsync(Arg.Any<DispatchWorkflowDefinitionRequest>(), Arg.Any<DispatchWorkflowOptions>(), Arg.Any<CancellationToken>())
.Returns(DispatchWorkflowResponse.Success());
return dispatcher;
}

private static async Task<(ActivityExecutionContext Context, IWorkflowDispatcher WorkflowDispatcher)> ExecuteAsync(DispatchWorkflow dispatchWorkflow)
{
var workflowGraph = CreateMockWorkflowGraph();
var workflowDefinitionService = CreateWorkflowDefinitionService(DefaultWorkflowDefinitionId, workflowGraph);
var identityGenerator = CreateIdentityGenerator(DefaultGeneratedId);
var workflowDispatcher = CreateWorkflowDispatcher();

var context = await new ActivityTestFixture(dispatchWorkflow)
.ConfigureServices(services =>
{
services.AddSingleton(workflowDefinitionService);
services.AddSingleton(identityGenerator);
services.AddSingleton(workflowDispatcher);
})
.ExecuteAsync();

return (context, workflowDispatcher);
}
}
Loading