Skip to content

Conversation

@notfahmed
Copy link

@notfahmed notfahmed commented Jun 21, 2025

Several get endpoints didn't explicitly set their return type on the method and ElsaEndpoint<TReq,TResp>. Changes were made to use the ExecuteAsync rather than the HandleAsync method, explicitly setting the return type on the method, to use ElsaEndpoint<TRequest, TResponse>, and a couple smaller refactors to align with the coding standards.


This change is Reviewable

@notfahmed
Copy link
Author

@dotnet-policy-service agree

@sfmskywalker sfmskywalker requested a review from Copilot June 23, 2025 08:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors several GET endpoints to explicitly specify their return types by switching from HandleAsync to ExecuteAsync and updating ElsaEndpoint<TRequest> to ElsaEndpoint<TRequest, TResponse>.

  • Endpoints now return strongly typed response objects via ExecuteAsync instead of manually calling SendOkAsync/SendCreatedAtAsync.
  • Signature changes across workflow‐definition and graph endpoints align with new method patterns.
  • A couple of model classes were added or moved to support the new response/request types.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Refresh/Endpoint.cs Changed to ElsaEndpoint<Request, Response> and ExecuteAsync
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/IsNameUnique/Endpoint.cs Changed to ElsaEndpoint<Request, Response> and ExecuteAsync
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportFiles/Models.cs Added Response model for imported file count
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportFiles/Endpoint.cs Switched to ExecuteAsync and return Response
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Import/Endpoint.cs Changed to ElsaEndpoint<WorkflowDefinitionModel, LinkedWorkflowDefinitionModel> and ExecuteAsync
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Graph/Segments/Endpoint.cs Updated signature to ElsaEndpoint<Request, Response>
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Graph/Endpoint.cs Updated signature to ElsaEndpoint<Request, ActivityNode>
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/GetManyById/Endpoint.cs Changed to ExecuteAsync returning ListResponse<...>
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/GetById/Endpoint.cs Changed to ExecuteAsync returning LinkedWorkflowDefinitionModel
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/GetByDefinitionId/Endpoint.cs Changed to ExecuteAsync returning LinkedWorkflowDefinitionModel
src/modules/Elsa.Expressions.JavaScript/Endpoints/TypeDefinitions/Models.cs Extracted Request record into its own file
src/modules/Elsa.Expressions.JavaScript/Endpoints/TypeDefinitions/Endpoint.cs Removed duplicate Request record from endpoint file
Comments suppressed due to low confidence (3)

src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Graph/Segments/Endpoint.cs:15

  • You updated the endpoint signature to use ElsaEndpoint<Request, Response> but didn't rename the override from HandleAsync to ExecuteAsync. Update the method signature and return type accordingly.
internal class Nodes(IWorkflowDefinitionService workflowDefinitionService, IApiSerializer apiSerializer, ActivityWriter activityWriter) : ElsaEndpoint<Request, Response>

src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Graph/Endpoint.cs:13

  • The class signature was updated for a generic response type but the handler method still overrides HandleAsync. It should be renamed to ExecuteAsync(Request, CancellationToken) returning ActivityNode.
internal class Graph(IWorkflowDefinitionService workflowDefinitionService, IApiSerializer apiSerializer, ActivityWriter activityWriter) : ElsaEndpoint<Request, ActivityNode>

src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Import/Endpoint.cs:63

  • The return updatedModel; is inside the if (isNew) block, making the ValidationFailed branch unreachable and omitting a return path for non-new, non-validation cases. Refactor so that validation is handled first and updatedModel is returned on all successful paths.
            return updatedModel;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant