Rewrite of process engine to support actions in tasks resolves #205 and #207 #237
Conversation
Needs more tests and verification
tjololo
added
the
breaking-change
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| catch (Exception exception) | ||
| { | ||
| _logger.LogWarning(exception, "Exception when sending event with the Events component"); | ||
| } |
Check notice
Code scanning / CodeQL
Generic catch clause
Added logic to dispatch abandon event if action is reject
tjololo
changed the title
src/Altinn.App.Core/Infrastructure/Clients/Authorization/AuthorizationClient.cs
Outdated
Show resolved
Hide resolved
src/Altinn.App.Core/Infrastructure/Clients/Authorization/AuthorizationClient.cs
Outdated
Show resolved
Hide resolved
| return await _processChangeHandler.HandleStart(processChange); | ||
| if (string.IsNullOrEmpty(instanceEvent.User.OrgId) && userId != null) | ||
| { | ||
| UserProfile up = await _profileService.GetUserProfile((int)userId); |
There was a problem hiding this comment.
Seems like a http call that should have been done outside of this method, ideally done once for the entire user request and just passed in as a dependency.
| /// <param name="startRequest"></param> | ||
| /// <param name="events"></param> | ||
| /// <returns></returns> | ||
| Task<Instance> UpdateInstanceAndRerunEvents(ProcessStartRequest startRequest, List<InstanceEvent>? events); |
There was a problem hiding this comment.
This seems a bit misplaced compared to the other two and the name causes me to think of side effects and why we need to re-run events.
There was a problem hiding this comment.
Added an issue for tracking this: #240
| /// <summary> | ||
| /// Interface used to descipt the process navigator | ||
| /// </summary> | ||
| public interface IProcessNavigator |
There was a problem hiding this comment.
Is IProcessEngine considered as a home for this method since this has process navigation methods already?
There was a problem hiding this comment.
This is whats left of the IFlowHydration interface. I didn't want to merge it with the processengine yet as that code really still needs alot of cleanup.
Leaving it for now until the IProcessEngine is more clean
| /// <returns></returns> | ||
| public Task<List<SequenceFlow>> FilterAsync(List<SequenceFlow> outgoingFlows, Instance instance); | ||
| public Task<List<SequenceFlow>> FilterAsync(List<SequenceFlow> outgoingFlows, Instance instance, string? action); |
There was a problem hiding this comment.
Is it correct that an exclusive gateway returns a list of sequence flows?
There was a problem hiding this comment.
Had the same thought. We need to decide if it should be possible to return more than one flow from a gateway and then choose the default (if defined), or if a gateway always needs to return one flow
| foreach (InstanceEvent instanceEvent in events) | ||
| { | ||
| if (Enum.TryParse<InstanceEventType>(instanceEvent.EventType, true, out InstanceEventType eventType)) | ||
| { | ||
| string? elementId = instanceEvent.ProcessInfo?.CurrentTask?.ElementId; | ||
| ITask task = GetProcessTask(instanceEvent.ProcessInfo?.CurrentTask?.AltinnTaskType); | ||
| switch (eventType) | ||
| { | ||
| case InstanceEventType.process_StartEvent: | ||
| break; | ||
| case InstanceEventType.process_StartTask: | ||
| await task.HandleTaskStart(elementId, instance, prefill); | ||
| break; | ||
| case InstanceEventType.process_EndTask: | ||
| await task.HandleTaskComplete(elementId, instance); | ||
| break; | ||
| case InstanceEventType.process_AbandonTask: | ||
| await task.HandleTaskAbandon(elementId, instance); | ||
| await _instanceService.UpdateProcess(instance); | ||
| break; | ||
| case InstanceEventType.process_EndEvent: | ||
| await _appEvents.OnEndAppEvent(instanceEvent.ProcessInfo?.EndEvent, instance); | ||
| break; | ||
| } | ||
| } | ||
| } |
Check notice
Code scanning / CodeQL
Missed opportunity to use Where
| /// <summary> | ||
| /// The process change was not allowed due to the current state of the process | ||
| /// </summary> | ||
| Conflict |
There was a problem hiding this comment.
Only possible return today is Conflict so not really used. If error results emerges we would need to use it
Co-authored-by: Ronny Birkeli <ronny.birkeli@gmail.com>
|
SonarCloud Quality Gate failed. |
Description
Resolves #205
Resolves #207
Rewrite of process engine and process definition (BPMN). Process navigation is now decided based on actions defined on a process task.
This requires a small rewrite of the proces.bpmn file in apps as we are moving altinn specific properties to bpmn:extensionProperties.
This also introduces additional properties in the GET /process endpoint listing available actions for the current task
POST /process/next has an additional query parameter
actionto make migration to v8 easierelementIdparam is still available and act as a alias foraction.Applications leveraging
elementIdto perform gateway navigation would need to change some parts of the frontend to support v8.Related Issue(s)
Verification
Documentation