Skip to content

[NAE-2231] Unable to change behavior of taskRef on finish event witho… #371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: release/6.4.2
Choose a base branch
from
Open

[NAE-2231] Unable to change behavior of taskRef on finish event witho… #371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions ...application/engine/workflow/domain/eventoutcomes/taskoutcomes/FinishTaskEventOutcome.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,27 @@
@Data
public class FinishTaskEventOutcome extends TaskEventOutcome {

/**
* Outcome flag, which is true if the task is still executable after the finish task event
*/
protected boolean isTaskStillExecutable;

public FinishTaskEventOutcome() {
super();
}

public FinishTaskEventOutcome(Case useCase, Task task) {
super(useCase, task);
this.isTaskStillExecutable = isTaskStillExecutable(useCase, task);
}

public FinishTaskEventOutcome(Case useCase, Task task, List<EventOutcome> outcomes) {
this(useCase, task);
this.setOutcomes(outcomes);
}

protected boolean isTaskStillExecutable(Case useCase, Task task) {
return useCase.getTasks().stream()
.anyMatch(taskPair -> taskPair.getTask().equals(task.getStringId()));
}
Comment on lines +32 to +35
Copy link

@coderabbitai coderabbitai bot Oct 14, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard against potential NPE and consider method naming.

The method has two concerns:

  1. NPE risk: If taskPair.getTask() returns null, the equals call will throw a NullPointerException.
  2. Naming clarity: The method name isTaskStillExecutable suggests checking executability, but the logic only verifies existence in the case's tasks collection. Consider a name like isTaskStillPresent or doesTaskStillExist for clarity.

Apply this diff to add null safety:

 protected boolean isTaskStillExecutable(Case useCase, Task task) {
     return useCase.getTasks().stream()
-            .anyMatch(taskPair -> taskPair.getTask().equals(task.getStringId()));
+            .anyMatch(taskPair -> task.getStringId().equals(taskPair.getTask()));
 }

Or with explicit null check:

 protected boolean isTaskStillExecutable(Case useCase, Task task) {
     return useCase.getTasks().stream()
-            .anyMatch(taskPair -> taskPair.getTask().equals(task.getStringId()));
+            .anyMatch(taskPair -> {
+                String taskId = taskPair.getTask();
+                return taskId != null && taskId.equals(task.getStringId());
+            });
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected boolean isTaskStillExecutable(Case useCase, Task task) {
return useCase.getTasks().stream()
.anyMatch(taskPair -> taskPair.getTask().equals(task.getStringId()));
}
protected boolean isTaskStillExecutable(Case useCase, Task task) {
return useCase.getTasks().stream()
.anyMatch(taskPair -> {
String taskId = taskPair.getTask();
return taskId != null && taskId.equals(task.getStringId());
});
}
🤖 Prompt for AI Agents 
In
src/main/java/com/netgrif/application/engine/workflow/domain/eventoutcomes/taskoutcomes/FinishTaskEventOutcome.java
around lines 32 to 35, the method risks an NPE by calling
taskPair.getTask().equals(... ) and the name misleads about intent; change the
name to something like doesTaskStillExist or isTaskStillPresent, make the
existence check null-safe (e.g., use Objects.equals(task.getStringId(),
taskPair.getTask()) or invert the equals to compare
task.getStringId().equals(taskPair.getTask()) only after null-check of
task.getStringId()), and update all call sites and imports accordingly so the
method reflects its purpose and no longer throws on null task IDs.

}
7 changes: 7 additions & 0 deletions ...ion/engine/workflow/web/responsebodies/eventoutcomes/LocalisedFinishTaskEventOutcome.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,19 @@

import com.netgrif.application.engine.workflow.domain.eventoutcomes.taskoutcomes.FinishTaskEventOutcome;
import com.netgrif.application.engine.workflow.web.responsebodies.eventoutcomes.base.LocalisedTaskEventOutcome;
import lombok.Getter;

import java.util.Locale;

public class LocalisedFinishTaskEventOutcome extends LocalisedTaskEventOutcome {

@Getter
protected Boolean isTaskStillExecutable;
Comment on lines +11 to +12
Copy link

@coderabbitai coderabbitai bot Oct 14, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider field naming for cleaner getter.

The @Getter annotation generates getIsTaskStillExecutable(), which deviates from the standard JavaBean convention of isXxx() for boolean properties. While not incorrect, renaming the field to taskStillExecutable would result in a more conventional getter getTaskStillExecutable().

🤖 Prompt for AI Agents 
In
src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/eventoutcomes/LocalisedFinishTaskEventOutcome.java
around lines 11-12, the protected Boolean field named isTaskStillExecutable
yields a non-idiomatic getter getIsTaskStillExecutable(); rename the field to
taskStillExecutable to produce a conventional getter getTaskStillExecutable(),
update all references/usages (including tests, serialization names or builders)
to the new field name, and ensure any JSON/property mappings or reflection
usages are adjusted (or add @JsonProperty if name must remain externally) so
behavior stays identical.


public LocalisedFinishTaskEventOutcome(FinishTaskEventOutcome outcome, Locale locale) {
super(outcome, locale);
if (outcome != null) {
this.isTaskStillExecutable = outcome.isTaskStillExecutable() ;
}
Comment on lines +16 to +18
Copy link

@coderabbitai coderabbitai bot Oct 14, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Remove extra whitespace.

Minor formatting issue: there's an extra space before the semicolon on line 17.

Apply this diff:

 if (outcome != null) {
-    this.isTaskStillExecutable = outcome.isTaskStillExecutable() ;
+    this.isTaskStillExecutable = outcome.isTaskStillExecutable();
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (outcome != null) {
this.isTaskStillExecutable = outcome.isTaskStillExecutable() ;
}
if (outcome != null) {
this.isTaskStillExecutable = outcome.isTaskStillExecutable();
}
🤖 Prompt for AI Agents 
In
src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/eventoutcomes/LocalisedFinishTaskEventOutcome.java
around lines 16 to 18, there's an extra space before the semicolon on line 17;
remove the stray space so the assignment reads without a space before the
semicolon (i.e., ensure there is no space between the method call and the
semicolon), and save formatting so no other whitespace changes are introduced.

}
}
Loading