signumsoftware / framework

Open Source framework for writing data-centric applications using the latest versions of .Net Core, C# (not-nullable), ASP.NET Web API, Typescript (strict), React, D3 and Sql Server or PostgreeSQL
https://www.signumsoftware.com/en/Framework
MIT License
222 stars 85 forks source link

fix IsStartCurrentUser() #537

Closed Faridmehr closed 2 years ago

Faridmehr commented 2 years ago

Users who starting workflows should not be based on ActorEval, because ActorEval takes a value when the object is set.

olmobrutall commented 2 years ago

We implemented this feature to allow determining users that can start the worflow by making a LDAP / AD check for users in a group.

In this case we do not use the entity, so is nul.

We could consider to make the entity nullable, but maybe it's an overkill?

Is this creating problems for you? Couldn't you just check for entity == null in your eval?

olmobrutall commented 2 years ago

One posibility: add WorkflowActivityEntity InitialActivity to WorkflowTransitionContext

then in the ActorEval we can do something like this:

(ICaseMainEntity mainEntity, WorkflowTransitionContext ctx) => {
   if(mainEntity == null)
      return ctx.InitialActivity.Lane.Actors;
    else 
       ...
}

If this sounds reasonable, feel free to make the changes and push in your branch. This will automatically update this PR.

Faridmehr commented 2 years ago

Thank you for your suggestion

Given that we used a lot of workflow for our customers, even with this method we have to modify all the ActorEval. Did you use only ActorEval to determine the users that can start the workflow by making a LDAP / AD check? If only the ActorEval is used, does it make sense to make the following change to solve this problem?

return act.Any(a =>
{
    if (a.Lane.ActorsEval != null && a.Lane.Actors.IsEmpty())
    {
        var actors = a.Lane.ActorsEval.Algorithm.GetActors(null!, new WorkflowTransitionContext(null, null, null));
        return actors.Any(a => WorkflowLogic.IsUserActor(UserEntity.Current, a));
    }
    return a.Lane.Actors.Any(a => WorkflowLogic.IsUserActor(UserEntity.Current, a));
});

In fact, if the actor is identified, determining users that can start the worflow should be done through the actor, otherwise if the actor is not identified, use the ActorEval.

olmobrutall commented 2 years ago

I understand that you have a lot of workflows, and that is also a common case letting a whole group (a Role) to start a workflow but then only allow a subset (the creator user) to continue.

On the other side, we also have workflows where we want to allow a union to continue, or we want to execute ActorsEval on the start too (for the AD thing).

My proposal;

public LaneEntity {
        public MList<Lite> Actors; 
    public ActorEvalEmbedded? ActorEval; 

    public bool UseActorEvalForStart;
    public bool CombineActorsAndActorEvalWhenContinuing; 
}

The default behaviour is false in both cases, so it will be backward compatible with your workflows.

We will implement it and close this PR when it's ready.

Faridmehr commented 2 years ago

Thanks a lot

olmobrutall commented 2 years ago

The changes are pushed in https://github.com/signumsoftware/framework/commit/d6c9c645c039cdecd78cae0cab5d673926db4c98

So I close this PR now