[NAE-2285] Group as a value of userlists#397
Conversation
- change user field types to actor field types in process files - update initialization of ActorListField in ActionDelegate and DataService - introduce new way of initialization of ActorField in ActionDelegate and DataService - refactor attributes of elastic data fields - update ElasticCaseMappingService according to refactored fields - update Importer to work also with actor field types - refactor ActorField and ActorListField (renamed user fields) - apply polymorphism on actor field values (user field values) - introduce GroupFieldValue
- resolve todos
- add todos - use actorRef instead of userRef in processes - update process model validations - remove unnecessary transactional annotations - update process importer - use actorRef keyword instead of userRef in the project - update initialization of actorRef attributes in Case and Task
- update authorization implementation to handle groups - fix authorization bug - update elastic document attributes
- fix tests - add todos
- update DataSearchRequestTest
- update DataSearchRequestTest
- add todos
- update permission queries for Elasticsearch - update tests
- update permission queries for MongoDB
- update permission queries for Elasticsearch - update tests
- update permission queries for MongoDB
- update permissions.md
- update documentation - rename user and userList fields to actor and actorList
- update ActorListTest
- update QueryDSLViewPermissionTest - update TaskAuthorizationServiceTest - update WorkflowAuthorizationServiceTest
- update documentation
WalkthroughReplaces user-centric types/flows with actor-centric ones across models, services, import/export, Elasticsearch adapters, XML resources, tests and docs; adds Group resolution so actor IDs resolve to either users or groups during runtime mapping, permission evaluation, export and action handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant AuthSvc as AuthorizationService
participant UserSvc as UserService
participant GroupSvc as GroupService
participant Index as Elastic/Mongo
Client->>AuthSvc: request permission/action
AuthSvc->>UserSvc: get logged user id
UserSvc-->>AuthSvc: LoggedUser (id, groupIds)
AuthSvc->>GroupSvc: (optional) fetch group details for groupIds
AuthSvc->>AuthSvc: build actorIdSet = {userId} ∪ groupIds
loop for each actorId in actorIdSet
AuthSvc->>Index: query actorIdValue
Index-->>AuthSvc: permission/data for actorId
end
AuthSvc->>AuthSvc: merge permission maps (preserve negatives)
AuthSvc-->>Client: allow/deny decision
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 50
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (22)
application-engine/src/main/java/com/netgrif/application/engine/importer/service/IModelValidator.java (1)
10-19: Parameterized logging change is correct; consider avoiding per-call logger creation.Using
log.warn("Data attribute [{}] is deprecated.", attrName);is the right SLF4J pattern and preserves behavior. If this method is called frequently, consider hoisting the logger (e.g., astatic final Loggerin implementors, or adefault Logger logger()accessor) to avoid repeatedLoggerFactory.getLogger(...)calls.application-engine/src/main/resources/petriNets/engine-processes/org_group.xml (1)
57-76: Undefined variables in action block will cause runtime failures.The
user_selectionfield contains critical bugs in its action handlers:
Line 71:
groupis undefined when used on the right-hand side of the assignment. The code attempts to callgroupService.addUser()while referencinggroup.stringId, butgrouphasn't been defined yet. You likely need to fetch or initialize the group before this call.Line 73: The variable
useris undefined. Based on context, this should beuserField.value:options.put(userField.value.getStringId(), new com.netgrif.application.engine.objects.petrinet.domain.I18nString(userField.value.getUsername()))Apply this diff to fix both issues:
<action trigger="set"> selection: f.members, userField: f.user_selection, id: f.group_id; - def group = groupService.addUser(userField.value.id, group.stringId, userField.value.realmId) + def group = groupService.findById(id.value) + groupService.addUser(userField.value.id, group.stringId, userField.value.realmId) def options = selection.options - options.put(user.getStringId(), new com.netgrif.application.engine.objects.petrinet.domain.I18nString(user.getUsername())) + options.put(userField.value.getStringId(), new com.netgrif.application.engine.objects.petrinet.domain.I18nString(userField.value.getUsername())) change selection options { options } </action>nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java (1)
10-15: Consider avoiding@Datahere (or at least avoid generating a public setter forallowedNets).
Even thoughallowedNetsisprotected(Line 15), Lombok@Datawill generate setters/getters, which re-opens uncontrolled mutation from outside the class hierarchy.If you want
allowedNetsto be immutable after construction, prefer@Getterand omit@Setter(or use@Setter(AccessLevel.PROTECTED)), and/or make the fieldfinalif feasible.nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java (1)
162-196: Copy constructor sharesactorRefsmap reference (mutations will leak across copies)
this.actorRefs = petriNet.getActorRefs();means the “copy” and the source share the sameactorRefs(and inner maps). If either instance callsaddActorPermission, it can mutate the other unintentionally.- this.actorRefs = petriNet.getActorRefs(); + this.actorRefs = petriNet.getActorRefs() == null + ? new HashMap<>() + : petriNet.getActorRefs().entrySet().stream() + .collect(Collectors.toMap( + Map.Entry::getKey, + e -> e.getValue() == null ? null : new HashMap<>(e.getValue()), + (x, y) -> y, + HashMap::new + ));Also applies to: 187-187
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/Transition.java (2)
92-106: Constructor init is fine, but don’t rely on it foractorRefsnon-nullness.
Some serializers/deserializers can bypass constructors;addActorRef()should still tolerateactorRefs == null.
253-273: Copy-constructor can NPE if anyactorRefsvalue map is null (allowed by currentaddActorRef).
new HashMap<>(e.getValue())throws ife.getValue()is null; guard or normalize null to empty.- this.setActorRefs(transition.actorRefs == null ? null : transition.actorRefs.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, e -> new HashMap<>(e.getValue())))); + this.setActorRefs(transition.actorRefs == null ? null : + transition.actorRefs.entrySet().stream().collect(Collectors.toMap( + Map.Entry::getKey, + e -> e.getValue() == null ? new HashMap<>() : new HashMap<>(e.getValue()) + )));nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java (2)
82-108: Constructor: null-safety onnew HashSet<>(...)+ verifyidsource matches current ES/Mongo ID strategy
- If
useCase.getViewActorRefs()/getViewActors()/getNegativeViewActors()can returnnull, Lines 100/102/103 will throw NPE—preferOptional.ofNullable(...).orElseGet(Collections::emptySet).- Line 83 still uses
useCase.getStringId(). Based on prior ES migration learnings (stringId removed /idholds Mongo object id), please confirm this is still correct for this module and index, otherwise ES joins/searches can silently break. Based on learnings, stringId consolidation happened earlier.
110-127:update()assigns view sets by reference (possible aliasing); consider defensive copies for ES documents
Lines 119/121/122 assign sets directly; if the sourceElasticCaseinstance is reused/mutated, this can leak state across indexed documents. Prefernew HashSet<>(...)(and same for roles/tags if they’re mutable).nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticTask.java (2)
109-125:update()doesn’t propagateactorRefs/actorsand assigns sets by reference
If assignees/permissions can change,actorRefs/actorsshould likely be updated too (not only view* sets). Also, Lines 119–124 assign collections directly—consider defensive copies to avoid shared mutable state in the indexed representation.
80-107: Constructor uses inconsistent identifiers: bothidandtaskIdincorrectly set fromtask.getStringId()Lines 81–84 set
idandtaskIdto the same value viatask.getStringId(). Per prior refactoring,stringIdshould no longer exist andidshould contain the MongoDB object ID directly. Additionally,taskIdshould betask.getTaskId()to maintain the semantic distinction between document identifier and business task identifier used throughout queue and index lifecycles. This mismatch will cause tracking and lifecycle bugs downstream. Also add null-safety checks on lines 99, 101, 102 (new HashSet<>(...)).application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FieldTest.groovy (1)
182-187: Rename helper methods to reflect Actor terminology (avoids confusion in future edits).
assertUserField()/assertUserList()now validateActorField/ActorListField, but the method names still encode the old type. Consider renaming toassertActorField()/assertActorList()(and update call sites intestImport).Also applies to: 205-210
application-engine/src/test/resources/pdf_test_3.xml (1)
157-163: Test fixture IDs look accidentally generated ([object Object]_0)—consider normalizing.
Even though the change totype="actor"is consistent, the IDs in this area appear UI-serialized and reduce test readability/maintainability. If feasible, replace them with stable IDs (and update matching<dataRef>entries).application-engine/src/test/resources/pdf_test_1.xml (1)
109-117: Update inline comment to match new type (actor).
The block is nowtype="actor"but still labeled<!-- USER -->, which is confusing when scanning fixtures. Consider renaming the comment to<!-- ACTOR -->(keeping<id>user</id>is fine if you need compatibility).docs/search/elastic_mapping.md (1)
1-167: Fix markdownlint MD047: add a trailing newline at EOF.application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy (1)
225-255: Misleading dataset field ids (pos_user_list/neg_user_list) now carryactorList.
If the XML dataset ids are intentionally kept for backwards compatibility, fine; otherwise consider renaming in the PetriNet resources too (and adapting tests) to avoid future confusion when debugging permissions.Also applies to: 257-282
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java (1)
53-70: Guard and source-of-truth mismatch:actorRefsvsactorscan yield incorrectnullpermissions.
You short-circuit ontask.getActorRefs()but then read fromtask.getActors(). If these diverge, authorization becomes inconsistent. Alsotask.getActors()could be null → potential NPE depending onfindUserPermissions(Map, user)implementation upstream.@@ public Boolean userHasUserListPermission(AbstractUser user, Task task, RolePermission... permissions) { - if (task.getActorRefs() == null || task.getActorRefs().isEmpty()) { + if (task.getActors() == null || task.getActors().isEmpty()) { return null; } Map<String, Boolean> userPermissions = findUserPermissions(task, user); if (userPermissions == null) { return null; } @@ private Map<String, Boolean> findUserPermissions(Task task, AbstractUser user) { - return findUserPermissions(task.getActors(), user); + if (task.getActors() == null || task.getActors().isEmpty()) { + return null; + } + return findUserPermissions(task.getActors(), user); }Also applies to: 165-168
application-engine/src/main/resources/petriNets/engine-processes/impersonation_config.xml (2)
80-101:impersonatedswitched toactorbut runtime logic still assumes a user (username + userService lookup).
If a group can be selected,impersonated.value.usernamemay be null anduserService.findById(...)may fail or return wrong semantics. Either constrain the field to user-only selection or add explicit guarding/error handling for non-user actors.Also applies to: 150-216
12-19: Update validation naming/messaging for actor semantics (and decide policy for group owners).
validateUsernow effectively validates actor ownership; ifconfig_ownercan contain groups, decide whether group members/admins can manipulate configs and reflect that in logic + error message.Also applies to: 145-146
application-engine/src/test/resources/data_test.xml (1)
104-110: Update fixture comments/titles to actor terminology (currently still “USER” / “Empty user list”). Not functional, but reduces migration confusion and keeps resources aligned with actor-centric docs.Also applies to: 138-143
docs/roles/permissions.md (1)
292-351: Address markdownlint violations (language fences + tabs) and a grammar typo (“whose”).
Static analysis reports MD040 + MD010 in the newly added “Actor list / Actor reference” section; also “who’s ID” should be “whose ID”.@@ -In NAE, actor list is a type of data field, that is used for managing access of a set of actors (who's ID is in the +In NAE, actor list is a type of data field, that is used for managing access of a set of actors (whose ID is in the @@ -``` +```xml <document xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://petriflow.com/petriflow.schema.xsd"> - ... - <data type="actorList"> - <id>actor_list_1</id> - <title>Actor list 1</title> - </data> - ... + ... + <data type="actorList"> + <id>actor_list_1</id> + <title>Actor list 1</title> + </data> + ... </document>@@
-Permission documentation can be found here. Actor list can be referenced as follows:
+Permission documentation can be found here. Actor list can be referenced as follows:
@@
-+xml
@@
-+xml</blockquote></details> <details> <summary>docs/_media/roles/actorRef_net.xml (1)</summary><blockquote> `41-58`: **Rename remaining “user” titles to “actor” for consistency with `actorRef`/`actorList`.** E.g., “Assign to user ref” / “Remove from user ref” (Line 45-50) and “User list 1/2” (Line 53-58). </blockquote></details> <details> <summary>application-engine/src/test/groovy/com/netgrif/application/engine/permissions/QueryDSLViewPermissionTest.groovy (1)</summary><blockquote> `78-93`: **Close `FileInputStream`s in `init()` (resource leak).** `new FileInputStream(...)` (Line 81, 85) isn’t closed; switch to `withCloseable {}` or `Files.newInputStream(...)`. ```diff @BeforeEach void init() { testHelper.truncateDbs() - ImportPetriNetEventOutcome net = petriNetService.importPetriNet(new FileInputStream("src/test/resources/view_permission_test.xml"), VersionType.MAJOR, superCreator.getLoggedSuper()) + def net = new File("src/test/resources/view_permission_test.xml").newInputStream().withCloseable { is -> + petriNetService.importPetriNet(is, VersionType.MAJOR, superCreator.getLoggedSuper()) + } assert net.getNet() != null this.net = net.getNet() - ImportPetriNetEventOutcome netWithUserRefs = petriNetService.importPetriNet(new FileInputStream("src/test/resources/view_permission_with_userRefs_test.xml"), VersionType.MAJOR, superCreator.getLoggedSuper()) + def netWithUserRefs = new File("src/test/resources/view_permission_with_userRefs_test.xml").newInputStream().withCloseable { is -> + petriNetService.importPetriNet(is, VersionType.MAJOR, superCreator.getLoggedSuper()) + } assert netWithUserRefs.getNet() != null this.netWithUserRefs = netWithUserRefs.getNet() @@ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (107)
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java(11 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticViewPermissionService.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/export/service/ExportService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/impersonation/service/ImpersonationAuthorizationService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/importer/service/DataValidator.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/importer/service/DocumentValidator.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/importer/service/FieldFactory.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/importer/service/IModelValidator.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/importer/service/Importer.java(11 hunks)application-engine/src/main/java/com/netgrif/application/engine/importer/service/TransitionValidator.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/pdf/generator/service/fieldbuilder/TextFieldBuilder.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java(5 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java(7 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/MongoSearchService.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskSearchService.java(7 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskService.java(6 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/ITaskService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IWorkflowService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/Task.java(1 hunks)application-engine/src/main/resources/petriNets/engine-processes/impersonation_config.xml(10 hunks)application-engine/src/main/resources/petriNets/engine-processes/org_group.xml(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy(5 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy(6 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/elastic/DataSearchRequestTest.groovy(6 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/impersonation/ImpersonationServiceTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/importer/ActorListTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/permissions/ElasticSearchViewPermissionTest.groovy(7 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/permissions/QueryDSLViewPermissionTest.groovy(6 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FieldTest.groovy(3 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/workflow/UserRefsTest.groovy(2 hunks)application-engine/src/test/resources/actor_list.xml(3 hunks)application-engine/src/test/resources/all_data.xml(2 hunks)application-engine/src/test/resources/all_data_pdf.xml(1 hunks)application-engine/src/test/resources/data_test.xml(2 hunks)application-engine/src/test/resources/initial_behavior.xml(1 hunks)application-engine/src/test/resources/mapping_test.xml(1 hunks)application-engine/src/test/resources/org_group.xml(2 hunks)application-engine/src/test/resources/pdf_run_action.xml(1 hunks)application-engine/src/test/resources/pdf_test_1.xml(1 hunks)application-engine/src/test/resources/pdf_test_3.xml(1 hunks)application-engine/src/test/resources/petriNets/FM_v0_2.xml(5 hunks)application-engine/src/test/resources/petriNets/all_data.xml(1 hunks)application-engine/src/test/resources/petriNets/all_data_refs.xml(1 hunks)application-engine/src/test/resources/petriNets/dynamic_init.xml(1 hunks)application-engine/src/test/resources/petriNets/impersonation_test.xml(2 hunks)application-engine/src/test/resources/petriNets/test_model_immediate_data.xml(1 hunks)application-engine/src/test/resources/poistenie.xml(1 hunks)application-engine/src/test/resources/poistenie_rozsirene.xml(1 hunks)application-engine/src/test/resources/poistenie_transaction.xml(1 hunks)application-engine/src/test/resources/poistenie_triggers.xml(1 hunks)application-engine/src/test/resources/predefinedPermissions/role_permissions_anonymous_role_shadowed_userref.xml(2 hunks)application-engine/src/test/resources/predefinedPermissions/role_permissions_anonymous_role_shadowed_usersref.xml(2 hunks)application-engine/src/test/resources/predefinedPermissions/role_permissions_default_role_shadowed_userref.xml(2 hunks)application-engine/src/test/resources/predefinedPermissions/role_permissions_default_role_shadowed_usersref.xml(2 hunks)application-engine/src/test/resources/prikladFM.xml(5 hunks)application-engine/src/test/resources/prikladFM_import.xml(1 hunks)application-engine/src/test/resources/prikladFM_test.xml(1 hunks)application-engine/src/test/resources/role_all_data.xml(1 hunks)application-engine/src/test/resources/set_data_test.xml(1 hunks)application-engine/src/test/resources/task_authorization_service_test_with_userRefs.xml(2 hunks)application-engine/src/test/resources/userrefs_test.xml(3 hunks)application-engine/src/test/resources/view_permission_with_userRefs_test.xml(3 hunks)application-engine/src/test/resources/workflow_authorization_service_test_with_userRefs.xml(2 hunks)application-engine/src/test/resources/zaverecna_praca.xml(2 hunks)docs/_media/roles/actorRef_net.xml(6 hunks)docs/_sidebar.md(1 hunks)docs/roles/actorlist.md(1 hunks)docs/roles/permissions.md(8 hunks)docs/roles/userlist.md(0 hunks)docs/search/elastic_mapping.md(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ActorField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ActorListField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ActorMappingData.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/BooleanField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DateField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java(3 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticTask.java(3 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FilterField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/NumberField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/StringCollectionField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TextField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java(0 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserListField.java(0 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java(4 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/Transition.java(4 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorFieldValue.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorListField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorListFieldValue.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/FieldType.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/GroupFieldValue.java(1 hunks)
⛔ Files not processed due to max files limit (27)
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/UserField.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/UserFieldValue.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/UserListFieldValue.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/localised/LocalisedFieldFactory.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/localised/LocalisedUserField.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/localised/LocalisedUserListField.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Task.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ActorField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ActorListField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/BooleanField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ButtonField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/DateField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticTask.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FileField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/I18nField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/MapField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/NumberField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/TextField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/UserField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/UserListField.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/workflow/domain/Case.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/workflow/domain/Task.java
💤 Files with no reviewable changes (3)
- docs/roles/userlist.md
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserListField.java
🧰 Additional context used
🧠 Learnings (14)
📚 Learning: 2025-08-19T20:07:43.748Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 339
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java:16-16
Timestamp: 2025-08-19T20:07:43.748Z
Learning: In CaseField.java, the separate caseValue field (List<String>) is intentionally maintained alongside fulltextValue for specific Elasticsearch query requirements, rather than being derived on-the-fly from fulltextValue.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/StringCollectionField.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TextField.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.javaapplication-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
📚 Learning: 2025-06-23T13:30:13.096Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 318
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemConstants.java:60-62
Timestamp: 2025-06-23T13:30:13.096Z
Learning: In MenuItemConstants enum in nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemConstants.java, the field `attributeId` will be renamed to `value` to make it more generic and appropriate for both dataset attribute identifiers and technical constants like PATH_SEPARATOR.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/FieldType.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java
📚 Learning: 2025-08-22T12:49:43.219Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 343
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/FieldType.java:39-47
Timestamp: 2025-08-22T12:49:43.219Z
Learning: In FieldType enum, both fromString() and fromName() methods should coexist as they serve different purposes. The fromName() method is specifically needed for other libraries, while fromString() serves existing use cases.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/FieldType.java
📚 Learning: 2025-08-20T07:27:02.660Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 339
File: application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticTaskQueueManager.java:38-45
Timestamp: 2025-08-20T07:27:02.660Z
Learning: When reviewing ElasticTaskQueueManager changes, task.getTask().getId() returns the document identifier while task.getTaskId() returns the business task identifier. The queue operations should use consistent identifiers throughout the lifecycle (scheduling, processing, cleanup).
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/ITaskService.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskSearchService.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticTask.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskService.java
📚 Learning: 2025-09-05T10:21:54.893Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 350
File: application-engine/src/test/groovy/com/netgrif/application/engine/export/service/ExportServiceTest.groovy:135-135
Timestamp: 2025-09-05T10:21:54.893Z
Learning: In ExportServiceTest.groovy, writing to src/test/resources is intentional to simulate production behavior where the working tree is mutated during file exports. This mirrors how the system works in production.
Applied to files:
application-engine/src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovyapplication-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovyapplication-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy
📚 Learning: 2025-09-29T10:31:31.469Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 362
File: application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/ProcessRoleController.java:43-45
Timestamp: 2025-09-29T10:31:31.469Z
Learning: In the Netgrif application engine, when Authentication objects reach controller endpoints, auth.getPrincipal() always returns a LoggedUser instance and is never null, so defensive casting checks are not necessary in controller methods.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/service/MongoSearchService.javaapplication-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy
📚 Learning: 2025-07-31T08:05:21.587Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 329
File: nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/RealmServiceImpl.java:39-39
Timestamp: 2025-07-31T08:05:21.587Z
Learning: In the Netgrif Application Engine, the domain Realm class (com.netgrif.application.engine.objects.auth.domain.Realm) is abstract and cannot be instantiated. The concrete implementation is in the adapter package (com.netgrif.application.engine.adapter.spring.auth.domain.Realm) which extends the abstract domain class. Services must instantiate the concrete adapter class and cast it when saving to repositories that accept the abstract domain type.
Applied to files:
application-engine/src/test/groovy/com/netgrif/application/engine/permissions/QueryDSLViewPermissionTest.groovyapplication-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovyapplication-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy
📚 Learning: 2025-09-05T10:17:28.577Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 350
File: application-engine/src/test/groovy/com/netgrif/application/engine/export/service/ExportServiceTest.groovy:82-82
Timestamp: 2025-09-05T10:17:28.577Z
Learning: In the application-engine test suite, testHelper.truncateDbs() follows a two-phase approach: first it clears the databases, then it executes superCreator to create the admin user. This means Optional.get() is safe when retrieving the admin user after truncateDbs() because the admin user is guaranteed to exist.
Applied to files:
application-engine/src/test/groovy/com/netgrif/application/engine/permissions/QueryDSLViewPermissionTest.groovy
📚 Learning: 2025-08-19T20:13:40.087Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 339
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java:16-16
Timestamp: 2025-08-19T20:13:40.087Z
Learning: In CaseField.java, fulltextValue is mapped as a keyword field type in Elasticsearch (for exact matches, filtering, aggregations), while the separate caseValue field serves different Elasticsearch query requirements, allowing the system to support multiple query patterns on the same data through different field mappings.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.javaapplication-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
📚 Learning: 2025-07-31T23:40:46.499Z
Learnt from: tuplle
Repo: netgrif/application-engine PR: 334
File: application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java:204-214
Timestamp: 2025-07-31T23:40:46.499Z
Learning: In the PetriNetService.importPetriNet method, existingNet.getVersion() cannot be null because all existing nets in the system were deployed through processes that ensure every net always has a version assigned.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/importer/service/Importer.javaapplication-engine/src/test/groovy/com/netgrif/application/engine/importer/ActorListTest.groovy
📚 Learning: 2025-09-29T10:31:57.325Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 362
File: application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java:513-529
Timestamp: 2025-09-29T10:31:57.325Z
Learning: PetriNet.getStringId() returns a simple ObjectId string representation (_id.toString()), not a composite Netgrif ID format, so new ObjectId(petriNetId) works correctly when petriNetId comes from PetriNet.getStringId().
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/importer/service/Importer.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java
📚 Learning: 2025-08-19T19:38:19.471Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 338
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/FileFieldValue.java:40-51
Timestamp: 2025-08-19T19:38:19.471Z
Learning: In FileFieldValue.getPath() method in the NAE codebase, the deprecated method returns an empty string instead of throwing an exception because it's still used in tests across multiple projects. Tests will be refactored in later snapshots, making the gradual deprecation approach necessary for backward compatibility.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java
📚 Learning: 2025-10-20T11:44:44.907Z
Learnt from: machacjozef
Repo: netgrif/application-engine PR: 367
File: application-engine/src/main/resources/application.yaml:24-24
Timestamp: 2025-10-20T11:44:44.907Z
Learning: In the netgrif/application-engine project, the correction of the Elasticsearch task index name from "_taks" to "_task" in application.yaml was approved by maintainer machacjozef, indicating that any data migration concerns for this typo fix are handled separately or not applicable to their deployment scenario.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskSearchService.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticTask.java
📚 Learning: 2025-08-19T20:07:15.621Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 339
File: application-engine/src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovy:341-341
Timestamp: 2025-08-19T20:07:15.621Z
Learning: In the Elasticsearch migration, the stringId property was removed from ElasticCase and ElasticTask, with the id property now containing the MongoDB case/task object ID directly. This consolidation eliminates redundancy and simplifies the data model.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticTask.javaapplication-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
🧬 Code graph analysis (14)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorFieldValue.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/GroupFieldValue.java (1)
Getter(8-45)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/UserFieldValue.java (1)
Getter(10-54)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/StringCollectionField.java (5)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(11-34)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TextField.java (1)
Data(9-24)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java (1)
Data(14-40)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/BooleanField.java (1)
NoArgsConstructor(11-32)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/NumberField.java (1)
NoArgsConstructor(11-29)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/GroupFieldValue.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorFieldValue.java (1)
Getter(10-26)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/UserFieldValue.java (1)
Getter(10-54)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ActorField.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ActorListField.java (1)
Data(9-17)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(11-34)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TextField.java (3)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(11-34)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java (1)
Data(10-31)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java (1)
Data(10-46)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (4)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ActorField.java (1)
Data(15-77)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java (1)
Data(13-70)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/StringCollectionField.java (1)
Data(10-21)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TextField.java (1)
Data(9-24)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorListField.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorFieldValue.java (1)
Getter(10-26)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorField.java (1)
Setter(10-41)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ActorListField.java (4)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ActorField.java (1)
Data(15-77)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(11-34)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/localised/LocalisedUserField.java (1)
Data(10-20)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/localised/LocalisedUserListField.java (1)
Data(10-20)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FilterField.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java (1)
Data(10-26)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java (1)
NoArgsConstructor(12-39)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(11-34)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TextField.java (1)
Data(9-24)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java (4)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java (1)
Data(11-27)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(11-34)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java (1)
Data(10-46)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TextField.java (1)
Data(9-24)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nStringUtils.java (1)
I18nStringUtils(8-18)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nStringUtils.java (1)
I18nStringUtils(8-18)
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskService.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/ActorTransformer.java (1)
ActorTransformer(10-131)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorListFieldValue.java (1)
ActorListFieldValue(9-39)
🪛 GitHub Actions: Pull Request
application-engine/src/main/java/com/netgrif/application/engine/importer/service/Importer.java
[error] 254-254: cannot find symbol: class CaseActorRef
[error] 747-747: cannot find symbol: class ActorRef
🪛 LanguageTool
docs/roles/permissions.md
[style] ~371-~371: Consider a more concise word here.
Context: ...d a permission from at least one source in order to be allowed to perform an operation. A g...
(IN_ORDER_TO_PREMIUM)
docs/roles/actorlist.md
[style] ~88-~88: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ission only for a set of actors, and we want to change the content of this set runtime,...
(REP_WANT_TO_VB)
🪛 markdownlint-cli2 (0.18.1)
docs/roles/permissions.md
298-298: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
300-300: Hard tabs
Column: 1
(MD010, no-hard-tabs)
301-301: Hard tabs
Column: 1
(MD010, no-hard-tabs)
302-302: Hard tabs
Column: 1
(MD010, no-hard-tabs)
303-303: Hard tabs
Column: 1
(MD010, no-hard-tabs)
304-304: Hard tabs
Column: 1
(MD010, no-hard-tabs)
305-305: Hard tabs
Column: 1
(MD010, no-hard-tabs)
314-314: Link fragments should be valid
Expected: #permissions; Actual: #Permissions
(MD051, link-fragments)
314-314: Link text should be descriptive
(MD059, descriptive-link-text)
318-318: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
320-320: Hard tabs
Column: 1
(MD010, no-hard-tabs)
321-321: Hard tabs
Column: 1
(MD010, no-hard-tabs)
322-322: Hard tabs
Column: 1
(MD010, no-hard-tabs)
323-323: Hard tabs
Column: 1
(MD010, no-hard-tabs)
324-324: Hard tabs
Column: 1
(MD010, no-hard-tabs)
325-325: Hard tabs
Column: 1
(MD010, no-hard-tabs)
326-326: Hard tabs
Column: 1
(MD010, no-hard-tabs)
327-327: Hard tabs
Column: 1
(MD010, no-hard-tabs)
328-328: Hard tabs
Column: 1
(MD010, no-hard-tabs)
334-334: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
336-336: Hard tabs
Column: 1
(MD010, no-hard-tabs)
337-337: Hard tabs
Column: 1
(MD010, no-hard-tabs)
338-338: Hard tabs
Column: 1
(MD010, no-hard-tabs)
339-339: Hard tabs
Column: 1
(MD010, no-hard-tabs)
340-340: Hard tabs
Column: 1
(MD010, no-hard-tabs)
341-341: Hard tabs
Column: 1
(MD010, no-hard-tabs)
342-342: Hard tabs
Column: 1
(MD010, no-hard-tabs)
343-343: Hard tabs
Column: 1
(MD010, no-hard-tabs)
344-344: Hard tabs
Column: 1
(MD010, no-hard-tabs)
345-345: Hard tabs
Column: 1
(MD010, no-hard-tabs)
370-370: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
395-395: Hard tabs
Column: 1
(MD010, no-hard-tabs)
396-396: Hard tabs
Column: 1
(MD010, no-hard-tabs)
397-397: Hard tabs
Column: 1
(MD010, no-hard-tabs)
398-398: Hard tabs
Column: 1
(MD010, no-hard-tabs)
399-399: Hard tabs
Column: 1
(MD010, no-hard-tabs)
400-400: Hard tabs
Column: 1
(MD010, no-hard-tabs)
401-401: Hard tabs
Column: 1
(MD010, no-hard-tabs)
402-402: Hard tabs
Column: 1
(MD010, no-hard-tabs)
445-445: Hard tabs
Column: 1
(MD010, no-hard-tabs)
446-446: Hard tabs
Column: 1
(MD010, no-hard-tabs)
447-447: Hard tabs
Column: 1
(MD010, no-hard-tabs)
448-448: Hard tabs
Column: 1
(MD010, no-hard-tabs)
449-449: Hard tabs
Column: 1
(MD010, no-hard-tabs)
450-450: Hard tabs
Column: 1
(MD010, no-hard-tabs)
451-451: Hard tabs
Column: 1
(MD010, no-hard-tabs)
docs/_sidebar.md
13-13: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
docs/roles/actorlist.md
21-21: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/search/elastic_mapping.md
167-167: Files should end with a single newline character
(MD047, single-trailing-newline)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
...vy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
Show resolved
Hide resolved
application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java
Show resolved
Hide resolved
application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java
Show resolved
Hide resolved
...c/main/java/com/netgrif/application/engine/elastic/service/ElasticViewPermissionService.java
Show resolved
Hide resolved
...c/main/java/com/netgrif/application/engine/elastic/service/ElasticViewPermissionService.java
Show resolved
Hide resolved
...main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ActorListField.java
Show resolved
Hide resolved
...ain/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/GroupFieldValue.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java
Show resolved
Hide resolved
...library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/Transition.java
Show resolved
Hide resolved
- fix NPE in ButtonField and value duplicates
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java (1)
55-68: Trailing-dot filenames lose the dot on reconstruction (edge case).The combination of
extractFileExtensionFromNameandnameWithExtensionslightly alters filenames that end with a dot, e.g."report.":
extractFileExtensionFromName("report.")producesname = "report",extension = "".nameWithExtension("report", "")then returns"report"(dot dropped).If you want exact round-tripping even for this edge case, you can treat a trailing dot as “no extension”:
private FileNameAndExtension extractFileExtensionFromName(String filename) { int index = filename.lastIndexOf('.'); - if (index > 0) { + if (index > 0 && index < filename.length() - 1) { return new FileNameAndExtension(filename.substring(0, index), filename.substring(index + 1)); } return new FileNameAndExtension(filename, null); }
nameWithExtensioncan then stay as-is and will reconstruct the original filename correctly for both “no extension” and “trailing dot” cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java(11 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java(2 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Retoocs
Repo: netgrif/application-engine PR: 397
File: application-engine/src/main/resources/petriNets/engine-processes/org_group.xml:36-39
Timestamp: 2025-12-12T12:35:57.760Z
Learning: In the org_group.xml file (application-engine/src/main/resources/petriNets/engine-processes/org_group.xml), variable names in action blocks like userField and user_selection should be kept as-is even when the field type changes from "user" to "actor".
Learnt from: Retoocs
Repo: netgrif/application-engine PR: 397
File: application-engine/src/test/groovy/com/netgrif/application/engine/impersonation/ImpersonationServiceTest.groovy:18-20
Timestamp: 2025-12-12T12:40:25.863Z
Learning: In the actor refactoring, UserFieldValue is a subclass of ActorFieldValue, and ActorListFieldValue accepts ActorFieldValue instances. Therefore, passing UserFieldValue instances to ActorListFieldValue is valid through polymorphism.
📚 Learning: 2025-06-23T13:30:13.096Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 318
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemConstants.java:60-62
Timestamp: 2025-06-23T13:30:13.096Z
Learning: In MenuItemConstants enum in nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemConstants.java, the field `attributeId` will be renamed to `value` to make it more generic and appropriate for both dataset attribute identifiers and technical constants like PATH_SEPARATOR.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.java
📚 Learning: 2025-12-12T13:39:52.238Z
Learnt from: Retoocs
Repo: netgrif/application-engine PR: 397
File: application-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java:98-104
Timestamp: 2025-12-12T13:39:52.238Z
Learning: In application-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java, the permission constraint logic in buildQuery() intentionally applies negative actor constraints globally: ((roleConstraint AND NOT negRoleConstraint) OR actorConstraint) AND NOT negActorConstraint. This means users in negativeViewActors are excluded even if they have positive role permissions, which is the intended authorization behavior.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java
📚 Learning: 2025-08-19T19:38:19.471Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 338
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/FileFieldValue.java:40-51
Timestamp: 2025-08-19T19:38:19.471Z
Learning: In FileFieldValue.getPath() method in the NAE codebase, the deprecated method returns an empty string instead of throwing an exception because it's still used in tests across multiple projects. Tests will be refactored in later snapshots, making the gradual deprecation approach necessary for backward compatibility.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java
📚 Learning: 2025-08-19T20:07:43.748Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 339
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java:16-16
Timestamp: 2025-08-19T20:07:43.748Z
Learning: In CaseField.java, the separate caseValue field (List<String>) is intentionally maintained alongside fulltextValue for specific Elasticsearch query requirements, rather than being derived on-the-fly from fulltextValue.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.javaapplication-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
📚 Learning: 2025-12-12T12:35:57.760Z
Learnt from: Retoocs
Repo: netgrif/application-engine PR: 397
File: application-engine/src/main/resources/petriNets/engine-processes/org_group.xml:36-39
Timestamp: 2025-12-12T12:35:57.760Z
Learning: In the org_group.xml file (application-engine/src/main/resources/petriNets/engine-processes/org_group.xml), variable names in action blocks like userField and user_selection should be kept as-is even when the field type changes from "user" to "actor".
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
📚 Learning: 2025-08-19T20:07:15.621Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 339
File: application-engine/src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovy:341-341
Timestamp: 2025-08-19T20:07:15.621Z
Learning: In the Elasticsearch migration, the stringId property was removed from ElasticCase and ElasticTask, with the id property now containing the MongoDB case/task object ID directly. This consolidation eliminates redundancy and simplifies the data model.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
📚 Learning: 2025-08-19T20:13:40.087Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 339
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java:16-16
Timestamp: 2025-08-19T20:13:40.087Z
Learning: In CaseField.java, fulltextValue is mapped as a keyword field type in Elasticsearch (for exact matches, filtering, aggregations), while the separate caseValue field serves different Elasticsearch query requirements, allowing the system to support multiple query patterns on the same data through different field mappings.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
📚 Learning: 2025-10-20T11:44:44.907Z
Learnt from: machacjozef
Repo: netgrif/application-engine PR: 367
File: application-engine/src/main/resources/application.yaml:24-24
Timestamp: 2025-10-20T11:44:44.907Z
Learning: In the netgrif/application-engine project, the correction of the Elasticsearch task index name from "_taks" to "_task" in application.yaml was approved by maintainer machacjozef, indicating that any data migration concerns for this typo fix are handled separately or not applicable to their deployment scenario.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
📚 Learning: 2025-12-12T12:40:25.863Z
Learnt from: Retoocs
Repo: netgrif/application-engine PR: 397
File: application-engine/src/test/groovy/com/netgrif/application/engine/impersonation/ImpersonationServiceTest.groovy:18-20
Timestamp: 2025-12-12T12:40:25.863Z
Learning: In the actor refactoring, UserFieldValue is a subclass of ActorFieldValue, and ActorListFieldValue accepts ActorFieldValue instances. Therefore, passing UserFieldValue instances to ActorListFieldValue is valid through polymorphism.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
🧬 Code graph analysis (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(11-34)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java (1)
Data(10-46)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nStringUtils.java (1)
I18nStringUtils(8-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (13)
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java (2)
47-75: LGTM! Well-structured permission resolution logic.The implementation correctly addresses all previous review concerns:
- Defensive copy prevents mutation of
docPermissionsbacking maps- Early returns for null checks and empty intersections improve readability
- Iterating over intersection keys directly is more efficient than scanning the entire map
77-84: LGTM!The null guard at lines 78-80 properly handles the case where
docPermissionsmight contain null permission maps.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.java (2)
13-13: LGTM! Encapsulation improved.Changing
buttonValuefrom public to protected properly encapsulates the field and forces callers to use the accessor methods.
20-24: LGTM! Synchronization and null safety properly implemented.The setter correctly clears
fulltextValuebefore updating (preventing duplicate accumulation) and handles null values gracefully. This addresses the critical issues raised in previous reviews.application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java (8)
37-37: LGTM! Generic type improves flexibility.Using
Field<?>enables polymorphic handling of various field types through instanceof checks, which aligns well with the actor-centric refactoring.
52-53: LGTM! Actor-centric routing aligns with refactoring objectives.The routing of
ActorFieldandActorListFieldto their respective transform methods is correct and consistent with the broader user→actor refactoring described in the PR objectives.Also applies to: 68-69
89-89: LGTM! More idiomatic Optional handling.Using
isEmpty()instead of!isPresent()improves readability and follows modern Java best practices.Also applies to: 152-152
111-121: LGTM! Simplified field transformations.The
transformCaseFieldFieldandtransformFilterFieldFieldmethods are correctly updated to construct their elastic domain objects with appropriate parameters.
123-129: LGTM! Unsafe cast issue resolved with stream conversion.The pattern matching and stream-based conversion properly handles any
Collection<?>type and converts it toList<String>safely. This addresses the unsafe cast concern raised in previous reviews.
214-229: Actor transform methods kept as-is per maintainer decision.The unsafe casts in
transformActorField(Line 216) andtransformActorListField(Line 224) were previously flagged. Based on learnings, the polymorphism betweenUserFieldValueandActorFieldValuesupports these casts, and the maintainer explicitly chose to keep the current implementation.
234-234: LGTM! Modern pattern matching improves readability.The use of pattern matching (
instanceof LocalDate date) is a clean, modern approach that eliminates the need for explicit casting.
292-295: LGTM! FileListField transformation updated correctly.The method properly extracts file name-path data using
getNamesPaths()and constructs the elasticFileFieldappropriately.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java (1)
22-24: Null-safe single-value constructor semantics look good, but confirm expected behaviour.
FileField(FileFieldValue)now treats anullvalue as “no files” by delegating tothis(null)instead of throwing aNullPointerException. This is a reasonable default and removes the earlier NPE risk fromList.of(fileFieldValue), but it subtly changes the contract.Please confirm that all call sites expect
new FileField(null)to result in an empty field rather than a hard failure; if some callers rely on a strict non-null contract, it might be safer to enforce it with a clear exception instead.
.../main/java/com/netgrif/application/engine/workflow/service/AbstractAuthorizationService.java
Show resolved
Hide resolved
...library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java
Outdated
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java
Outdated
Show resolved
Hide resolved
- add nullcheck in AbstractAuthorizationService
- fix NPE in ButtonField
- fix NPE in FileField
# Conflicts: # application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FilterField.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/StringCollectionField.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserListField.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java # nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java # nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java # nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java # nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/UserField.java # nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/UserListField.java
- post merge fixes
- post merge fixes
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java (1)
7-7: 🧹 Nitpick | 🔵 TrivialUnused import detected.
The
UserFieldValueimport on line 7 appears to be unused after the method was renamed tomakeActorFieldValuereturningActorFieldValue. Consider removing it.♻️ Proposed fix
-import com.netgrif.application.engine.objects.petrinet.domain.dataset.UserFieldValue;
🤖 Fix all issues with AI agents
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java`:
- Around line 254-273: Extract the duplicate logic in
WorkflowService.getExistingActors (also present in
TaskService.getExistingActors) into a shared utility method (e.g.,
ActorUtils.getExistingActors) that accepts an ActorListFieldValue and the
required services; in that utility, replace the exception-based group lookup by
adding and using a new GroupService.existsById(String id) method and keep the
user lookup via userService.findById as-is, then update both
WorkflowService.getExistingActors and TaskService.getExistingActors to delegate
to the new utility to avoid duplication and remove the try/catch around
groupService.findById.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ActorListField.java`:
- Around line 15-21: The ActorListField copy constructor redundantly re-copies
lists already deep-copied by the parent ActorField copy constructor: remove the
post-super assignments in ActorListField(ActorListField field) that reassign
usernameValue, fullNameValue, actorIdValue, and actorRealmIdValue so the
constructor simply calls super(field) and relies on ActorField's deep-copy
logic; keep the constructor signature and any other initialization only if it
adds unique behavior beyond the parent copy.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java`:
- Around line 25-27: The one-arg constructor DataField(String fulltextValue)
currently calls List.of(fulltextValue) which throws NullPointerException for
null; change it to call the list-backed constructor DataField(List<String>) with
null when fulltextValue is null and with a singleton list otherwise (i.e., if
fulltextValue == null pass null, else pass a List containing fulltextValue) so
existing callers that pass null keep working.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java`:
- Around line 24-28: The I18nField constructor that takes (Set<String> keys,
Set<String> values, Map<String, String> translations) can throw a
NullPointerException when values is null; update this constructor to defensively
handle a null values parameter (like the copy constructor does) by creating the
ArrayList from an empty collection when values == null, assign keyValue and
translations as before, and ensure the behavior matches the existing null-safe
copy constructor.
- Line 8: Replace the wildcard import in I18nField (currently "import
java.util.*;") with explicit imports for only the types used (e.g.,
java.util.Set, java.util.Map, java.util.ArrayList, java.util.HashSet,
java.util.HashMap) so dependencies are clear and unused types are not pulled in;
update the import statement at the top of the I18nField class accordingly.
- Around line 15-16: I18nField made a breaking change by reducing visibility of
keyValue and translations and by changing keyValue's type; restore backward
compatibility by making both fields public again in class I18nField (keyValue
and translations) or, if you must use Set internally, reintroduce a public
String[] keyValue (deprecated) that maps to the new Set<String> (with conversion
in getters/setters) while keeping translations public; ensure public getters
remain consistent (getKeyValue/getTranslations) and mark any reintroduced array
field as `@Deprecated` to signal migration.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TaskField.java`:
- Around line 18-21: The constructor TaskField(List<String> values) stores the
caller-owned List directly which allows external mutation; fix by creating a
defensive copy (e.g., new ArrayList<>(values) or
Collections.unmodifiableList(new ArrayList<>(values))) and use that copy both
when calling super(...) and when assigning to this.taskRefValue so the internal
state is not shared with the caller.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Task.java`:
- Around line 327-334: The method compareExistingActorPermissions risks a
NullPointerException when unboxing actors.get(actorId).get(permType); to fix it,
guard the boolean check by using a null-safe comparison (e.g., replace direct
unboxing with Boolean.TRUE.equals(...) or check for null before evaluating) and
simplify retrieval by caching actors.get(actorId) into a local Map variable;
update the conditional in compareExistingActorPermissions to use that null-safe
check when deciding to put(permType, permValue).
In
`@nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java`:
- Around line 14-17: The deprecated CaseField(String[] values, String[]
allowedNets) constructor should guard against null arrays to preserve the parent
class's null-list contract: check each array for null and only call
Arrays.asList(...) when non-null, otherwise pass null to the delegated
constructor (the CaseField(List<String> values, List<String> allowedNets)
overload); update the constructor in class CaseField to perform these null
checks so legacy callers passing null arrays do not hit a NullPointerException.
In
`@nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FileField.java`:
- Around line 28-31: The FileField(FileFieldValue[] values) constructor should
defensively handle null and make a mutable copy of the incoming array instead of
delegating to Arrays.asList(...) which throws on null and returns a fixed-size
list; modify the constructor (FileField(FileFieldValue[])) to check for null and
pass an empty list when null, otherwise wrap Arrays.asList(values) in a new
ArrayList<>(...) (matching the defensive-copy style used elsewhere, e.g. the
HashSet conversion and the superclass copy constructor) before delegating to the
other constructor.
- Around line 37-39: The public constructor FileField(HashSet<FileFieldValue>
values) should accept the interface type Set instead of the concrete HashSet;
change the parameter to Set<FileFieldValue> and keep the body delegating to the
List constructor via new ArrayList<>(values) so callers can pass any Set
implementation and behavior remains identical.
In
`@nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java`:
- Around line 15-18: The deprecated constructor FilterField(String value,
String[] allowedNets, Map<String, Object> filterMetadata) is not null-safe and
creates a fixed-size list via Arrays.asList; change it to convert null
allowedNets into an empty list and to pass a mutable defensive copy to the other
constructor (e.g., if allowedNets is null use Collections.emptyList(), otherwise
wrap Arrays.asList(allowedNets) with a new ArrayList<>(...) ) so no NPE is
thrown and callers cannot mutate the internal list through the fixed-size view.
In
`@nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java`:
- Around line 19-22: The deprecated constructor StringCollectionField(String[]
values) can throw NPE because Arrays.asList(values) fails when values is null;
modify the constructor in class StringCollectionField to guard against null
(e.g., if values == null use Collections.emptyList() or Arrays.asList(new
String[0])) before calling super, so pass a non-null List to the superclass
(refer to the StringCollectionField(String[] values) constructor and
Arrays.asList(values) call).
In
`@nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/TextField.java`:
- Around line 26-33: The deprecated TextField(String[] values) constructor
should guard against null and avoid passing the fixed-size list from
Arrays.asList directly to the parent; update the TextField constructor to check
if values is null and convert to an empty mutable list, and otherwise wrap
Arrays.asList(values) with a mutable copy (e.g., new
ArrayList<>(Arrays.asList(values))) before calling the List-based constructor so
the parent’s textValue is not a fixed-size or null reference.
...on-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java
Show resolved
Hide resolved
...rary/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ActorListField.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java
Show resolved
Hide resolved
...er/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FileField.java
Show resolved
Hide resolved
...er/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FileField.java
Show resolved
Hide resolved
.../src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java
Show resolved
Hide resolved
...java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java
Show resolved
Hide resolved
...er/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/TextField.java
Show resolved
Hide resolved
- fix issues in PR
# Conflicts: # application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java # application-engine/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java # application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskAuthorizationService.java # application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskService.java # application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowAuthorizationService.java # application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java # application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IWorkflowService.java # application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/elastic/DataSearchRequestTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/importer/ActorListTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/permissions/ElasticSearchViewPermissionTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/permissions/QueryDSLViewPermissionTest.groovy # nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Task.java
- post merge fixes
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java (1)
7-9: 🧹 Nitpick | 🔵 TrivialUnused import:
UserFieldValueon line 7.The explicit import
UserFieldValueon line 7 appears to be unused after the method signature change tomakeActorFieldValue. The wildcard import on line 9 covers all dataset classes includingActorFieldValue.🧹 Proposed fix to remove unused import
import com.netgrif.application.engine.objects.petrinet.domain.dataset.Field; import com.netgrif.application.engine.objects.petrinet.domain.dataset.FileField; import com.netgrif.application.engine.objects.petrinet.domain.dataset.FileListField; -import com.netgrif.application.engine.objects.petrinet.domain.dataset.UserFieldValue; import com.netgrif.application.engine.files.throwable.StorageException; import com.netgrif.application.engine.objects.petrinet.domain.dataset.*;nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Task.java (1)
318-323:⚠️ Potential issue | 🟡 MinorOutdated Javadoc: references
viewUsersinstead ofviewActors.The Javadoc comment still references
viewUsersandusers, but these have been renamed toviewActorsandactors.📝 Suggested Javadoc update
/** - * Initializes {`@link` `#viewUsers`} collection. Any user defined in {`@link` `#users`} with permission {`@link` RolePermission#VIEW} - * of true value is added to the {`@link` `#viewUsers`} collection. + * Initializes {`@link` `#viewActors`} collection. Any actor defined in {`@link` `#actors`} with permission {`@link` RolePermission#VIEW} + * of true value is added to the {`@link` `#viewActors`} collection. * - * `@return` true if the {`@link` `#viewUsers`} was modified, false otherwise + * `@return` true if the {`@link` `#viewActors`} was modified, false otherwise */
🤖 Fix all issues with AI agents
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java`:
- Around line 229-233: In the ACTOR branch of CaseSearchService where
ActorFieldValue is queried, replace the numeric coercion that builds a Long
constant (the variable named constant) with a string constant so the idPath
(Expressions.stringPath on ActorFieldValue.value.id) is compared to a String;
locate the block using ActorFieldValue, QCase.case$.dataSet, idPath and change
the Expressions.constant creation to use the String form of fieldValue (e.g.,
String.valueOf(fieldValue)) so non‑numeric actor IDs won’t trigger
NumberFormatException and the EQ predicate compares strings.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java`:
- Around line 333-340: In compareExistingActorPermissions, avoid auto-unboxing
NPEs by caching actors.get(actorId) into a local Map (e.g., actorPerms), read
the current permission into a local Boolean (e.g., currentPerm) and use
Boolean.TRUE.equals(currentPerm) (or currentPerm != null && currentPerm) for the
null-safe check instead of actors.get(actorId).get(permType); then use
actorPerms.put(permType, permValue) to update — this removes repeated map
lookups and prevents NullPointerException when stored Boolean values are null.
- Line 160: The constructor currently assigns actorRefs directly from
petriNet.getActorRefs(), creating a shared mutable reference; change this to a
defensive copy so actorRefs is a new HashMap initialized from
petriNet.getActorRefs() to match how tags and permissions are handled (see tags
and permissions initialization in the same constructor). Ensure you update the
actorRefs assignment in the Case constructor (reference: actorRefs and
petriNet.getActorRefs()) to create a new HashMap populated with the values from
petriNet.getActorRefs().
...-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java
Show resolved
Hide resolved
...bject-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java
Show resolved
Hide resolved
...bject-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java
Show resolved
Hide resolved
- fix schema link
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@nae-object-library/src/main/resources/petriflow_schema.xsd`:
- Line 6: The schema's version attribute (version="1.1.0") conflicts with the
xs:include pointing to v1.0.7; either confirm this is intentional or document
why the older include is used. Edit the petriflow_schema.xsd near the
version="1.1.0" declaration and the xs:include element to add an inline XML
comment explaining the version offset (e.g., "intentionally extends Petriflow
v1.0.7 to add X changes"), or if it was accidental, update the xs:include to the
matching v1.1.0 schema URL; reference the version attribute and the xs:include
element when making the change.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Task.java (1)
318-323:⚠️ Potential issue | 🟡 MinorStale Javadoc:
@linkreferences point to non-existent fields.The Javadoc still references
{@link#viewUsers}and{@link#users}, which were renamed toviewActorsandactors. This will produce broken@linkwarnings in generated Javadoc and mislead readers.📝 Proposed fix
/** - * Initializes {`@link` `#viewUsers`} collection. Any user defined in {`@link` `#users`} with permission {`@link` RolePermission#VIEW} - * of true value is added to the {`@link` `#viewUsers`} collection. + * Initializes {`@link` `#viewActors`} collection. Any actor defined in {`@link` `#actors`} with permission {`@link` RolePermission#VIEW} + * of true value is added to the {`@link` `#viewActors`} collection. * - * `@return` true if the {`@link` `#viewUsers`} was modified, false otherwise + * `@return` true if the {`@link` `#viewActors`} was modified, false otherwise */
🤖 Fix all issues with AI agents
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java`:
- Around line 264-274: The null-check in resolveActorRefPermissions is unsafe
because !permission.get(ProcessRolePermission.VIEW.getValue()) can NPE when the
map value is null; change the condition to use
Boolean.FALSE.equals(permission.get(ProcessRolePermission.VIEW.getValue())) so
it only treats an explicit false as denied before calling
useCase.getNegativeViewActors().addAll(actorIds); keep the existing actorIds
check and the call to useCase.addActors(new HashSet<>(actorIds), permission)
unchanged.
- Around line 276-295: The getExistingActors(ActorListFieldValue) method
currently returns null for a null input and swallows IllegalArgumentException
from groupService.findById; change it to return Collections.emptyList() instead
of null, and when filtering actor IDs keep the same logic but replace the silent
catch with a debug-level log (e.g., log.debug) that records the actorId and
exception before treating it as non-existent; update callers can then assume a
non-null list. Ensure references: getExistingActors, userService.findById,
groupService.findById, and use Collections.emptyList() and debug logging in the
catch block.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java`:
- Line 203: The current copy assigns this.actorRefs = petriNet.getActorRefs(),
creating a shared reference so later mutations via addActorPermission will
affect the original; change the copy to a defensive deep copy: allocate a new
Map for this.actorRefs and for each entry from petriNet.getActorRefs() insert
the key and a new Map copy of the inner map (e.g., new HashMap<>(inner)),
preserving keys and values; apply the same deep-copy approach used for
permissions if needed and ensure you update the copy constructor/clone code that
uses getActorRefs() so addActorPermission mutates only the new instance.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java`:
- Around line 138-144: The collection types for view-related fields are
inconsistent: negativeViewActors is an ArrayList while viewRoles, viewActorRefs,
viewActors and negativeViewRoles are LinkedLists; standardize these for
consistency and performance by switching viewRoles, viewActorRefs, viewActors,
and negativeViewRoles to ArrayList (or alternatively make negativeViewActors a
LinkedList) — update the initializations for actorRefs, actors, viewRoles,
viewActorRefs, viewActors, negativeViewRoles, and negativeViewActors in the Case
class so all view/negative-view collections use the same concrete List
implementation.
- Around line 209-214: Consolidate duplicate role-mapping logic by introducing a
shared interface (e.g., HasRoles with method getRoles()) implemented by
ActorField and ActorListField (in the dataset package), then in Case.java
replace the two identical branches with a single check: if (field instanceof
HasRoles) { dataField.setChoices(((HasRoles)
field).getRoles().stream().map(I18nString::new).collect(Collectors.toSet())); }
— reference ActorField, ActorListField, HasRoles, dataField.setChoices, and
I18nString to locate and update the code.
...on-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java
Show resolved
Hide resolved
...on-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java
Show resolved
Hide resolved
...bject-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java
Show resolved
Hide resolved
...bject-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java
Show resolved
Hide resolved
# Conflicts: # application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskService.java # nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java # nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/workflow/domain/Task.java
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Task.java (1)
317-322:⚠️ Potential issue | 🟡 MinorStale Javadoc: references non-existent
#viewUsersand#users.The Javadoc still references
{@link#viewUsers}and{@link#users}, but these fields have been renamed toviewActorsandactorsrespectively. This will produce broken@linkreferences.📝 Proposed fix
/** - * Initializes {`@link` `#viewUsers`} collection. Any user defined in {`@link` `#users`} with permission {`@link` RolePermission#VIEW} - * of true value is added to the {`@link` `#viewUsers`} collection. + * Initializes {`@link` `#viewActors`} collection. Any actor defined in {`@link` `#actors`} with permission {`@link` RolePermission#VIEW} + * of true value is added to the {`@link` `#viewActors`} collection. * - * `@return` true if the {`@link` `#viewUsers`} was modified, false otherwise + * `@return` true if the {`@link` `#viewActors`} was modified, false otherwise */
🤖 Fix all issues with AI agents
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskSearchService.java`:
- Around line 157-159: Rename the misleading method actorRefQuery to actorQuery
and update its usages (notably in buildActorRefQueryConstraint) so the method
name matches the field queried (QTask.task.actors.containsKey(actorId)); ensure
the method signature remains the same (returning Predicate) and replace all
references to actorRefQuery with actorQuery.
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskService.java`:
- Around line 857-860: The getExistingActors(ActorListFieldValue
actorListFieldValue) method currently returns null when actorListFieldValue is
null; change it to return Collections.emptyList() instead so callers of
getExistingActors no longer need a null check. Update the method body in
TaskService to return Collections.emptyList() for a null parameter, ensure
java.util.Collections is imported, and leave the return type as List<String> so
callers (which expect a list) can simply check emptiness.
- Around line 838-847: The loop over task.getActorRefs() can NPE when
useCase.getDataSet().get(actorFieldId) is null; before calling getValue() cast
to ActorListFieldValue in the lambda, retrieve the dataset entry into a local
variable (e.g., field = useCase.getDataSet().get(actorFieldId)), check field !=
null and field.getValue() != null, and only then call
getExistingActors((ActorListFieldValue) field.getValue()) and proceed with
task.addActors(...), isTaskModified.set(true), and negative view handling; if
the field is missing, skip the actor addition (optionally log a warning) to
avoid the NullPointerException.
- Around line 857-876: Replace the exception-based group existence check in
getExistingActors(ActorListFieldValue) by adding and using a non-throwing
existsById(String) on GroupService: add boolean existsById(String id) to the
GroupService interface and implement it in the concrete GroupService class to
delegate to GroupRepository.existsById(id); then change the filter in
TaskService.getExistingActors to call groupService.existsById(actorId) instead
of calling groupService.findById(actorId) inside a try/catch. Ensure
getExistingActors still uses userService.findById(actorId, null) for user checks
and returns the same collected List<String>.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticTask.java`:
- Around line 130-133: In update(), avoid assigning shared mutable Set instances
directly: replace direct assignments to viewActorRefs, viewActors,
negativeViewActors (and also ensure viewRoles and negativeViewRoles follow the
same pattern) with defensive copies using new HashSet<>(...) so ElasticTask does
not share the source task's Set instances; update the update() method to mirror
the constructor's behavior of creating new HashSet copies for these fields.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java`:
- Around line 60-66: In getValue(), fileExtensionValue is accessed without
defensive checks which can cause NPE/IndexOutOfBounds; update the lambda in the
multi-file branch of getValue() (the IntStream over fileNameValue) to mirror the
filePath guard: verify fileExtensionValue != null and fileExtensionValue.size()
>= i+1 and fileExtensionValue.get(i) != null, and use a safe fallback (e.g.,
empty string) when missing before calling nameWithExtension; alternatively
(preferred) ensure the three lists (fileNameValue, fileExtensionValue, filePath)
are maintained in lockstep on setters/deserialization so callers of
nameWithExtension can assume aligned lists.
- Around line 44-50: The three parallel lists fileNameValue, fileExtensionValue
and filePath are getting out of sync because fileNameValue and
fileExtensionValue are always appended but filePath is only appended when
fileFieldValue.getPath() != null; update the code in FileField (where
extractFileExtensionFromName and fileFieldValue are handled) to always append to
filePath — append the actual path when non-null and append a placeholder (null
or empty string, consistent with how missing paths are represented elsewhere)
when getPath() is null so the indexes in filePath align with
fileNameValue/fileExtensionValue for use in getValue().
Description
change myActorField value { loggedUser().stringId }Implements NAE-2285
Dependencies
No new dependencies were introduced
Third party dependencies
No new dependencies were introduced
Blocking Pull requests
The build of this changes depends on PR: petriflow/petriflow#27.
How Has Been This Tested?
Manually and by existing tests
Test Configuration
Checklist:
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests