elsa-workflows / elsa-core

A .NET workflows library
https://v3.elsaworkflows.io/
MIT License
6.29k stars 1.15k forks source link

[ENH] allow Trigger name to be configured independently from Activity name, to allow creation of custom Event Triggers #5731

Open gstsv4 opened 3 months ago

gstsv4 commented 3 months ago

Enhancement Overview

Currently it is problematic to create custom activities inherited from Elsa.Workflows.Runtime.Activities.Event, because events (ex-signals) raised by PublishEvent activity will not get those custom activities triggered by default.

This happens because when the trigger for a custom activity is registered in the database, the "Name" column will be assigned the name of a class itself, e.g., "My.CustomActivity", which technically comes from IActivity.Type. At the same time, PublishEvent mechanism queries database for registered triggers with the name "Elsa.Event".

Although IActivity.Type for a custom activity could be changed to "Elsa.Event", this results in severe side effects, and generally feels like a hack.

Proposed Enhancement

The proposal is to make changes which would allow extension of Elsa.Workflows.Runtime.Activities.Event activity, while at the same time allowing such extended activities to be triggered by "standard" PublishEvent.

Suggested solution is to extend ITrigger interface with TriggerName field, which would default to IActivity.Type, but could be customized if needed. Then ITrigger.TriggerName could be used instead of IActivity.Type by TriggerIndexer when registering trigger in the DB.

That way it would be possible make custom activities to use the name "Elsa.Event" during trigger registration (hence PublishEvent could trigger them), while maintaining "proper" IActivity.Type for all other purposes.

In other words, the solution might look something like this:


public interface ITrigger : IActivity
{
    ...
    string TriggerName => this.Type;
}

public class TriggerIndexer : ITriggerIndexer
{
   ...
   private async Task<ICollection<StoredTrigger>> CreateWorkflowTriggersAsync(WorkflowIndexingContext context, ITrigger trigger)
   {
      ...
      // used to be: var triggerTypeName = trigger.Type;
      var triggerTypeName = trigger.TriggerName;
      ...
   }
}

public class MyCustomEvent : Trigger<object?>, ITrigger
{
   ...
   // Allow MyCustomEvent to be triggered by PublishEvent
   string ITrigger.TriggerName => "Elsa.Event";
   ...
}

Background

Our scenario involves providing Elsa Studio's interface to our customers, where they can create workflows for their business processes. To make things user-friendly for non-technical users, we plan to hide some more technical aspects of the Studio, and to provide numerous custom activities which our customers can easily understand. Among these custom activities there will be user-friendly versions for Event activity, which will allow to build event (signal) name by selecting values from dropdowns and other means, instead of manually typing in the event (signal) name.

Alternative Solutions

None that I can think of given my current understanding of Elsa's codebase.

glime-ai[bot] commented 3 months ago

To address the issue of creating custom activities inherited from Elsa.Workflows.Runtime.Activities.Event and having them triggered by the PublishEvent activity, your proposed enhancement seems like a viable solution. Extending the ITrigger interface with a TriggerName property that defaults to IActivity.Type but can be overridden allows for the flexibility needed to register custom triggers under a common name that the PublishEvent mechanism can recognize.

Your understanding of the problem and the proposed solution aligns with the extensibility principles of Elsa Workflows. By allowing the TriggerName to be customized, developers can create user-friendly versions of the Event activity without affecting the underlying activity type, thus maintaining the integrity of the system while providing a better experience for non-technical users.

Implementing this enhancement would involve modifying the ITrigger interface and the TriggerIndexer class as you've outlined. It would also require ensuring that all parts of the Elsa Workflows engine that interact with triggers are updated to use the TriggerName property instead of directly accessing the Type property.

Before proceeding with this change, it would be advisable to discuss it with the Elsa Workflows community, perhaps by opening an issue or a pull request on the official Elsa Workflows GitHub repository. This would allow for feedback from the maintainers and other users, ensuring that the change is in line with the project's roadmap and that it doesn't introduce any unintended side effects.

If you decide to contribute this enhancement to the Elsa Workflows project, make sure to follow the project's contribution guidelines, provide thorough unit tests, and update any relevant documentation to reflect the new functionality.

This comment was generated by Glime.