[NAE-2221] NOL: DTO refactor #382
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- refactor of controller request and response objects to use dtos instead of domain classes - moved commonly used request and response objects to nae-object-library - work in progress
- added missing dtos - CaseResourceAssembler changes
SamuelPalaj
requested review from
Kovy95,
Retoocs,
camperko,
machacjozef,
renczesstefan and
tuplle
SamuelPalaj
added
improvement
|
Caution Review failedFailed to post review comments WalkthroughThis pull request performs a comprehensive refactoring to replace domain entities with Data Transfer Objects (DTOs) across the codebase. Changes introduce new DTO classes for users, petri nets, groups, tasks, cases, preferences, and authorities while removing legacy response body classes. Public APIs are updated to return and accept DTOs instead of domain objects. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant Service
participant Factory
participant DTO
participant Response
Controller->>Service: request with domain object
Service->>Factory: create DTO from domain
Factory->>DTO: factory method (e.g., fromPetriNet)
DTO-->>Factory: new DTO instance
Factory-->>Service: DTO
Service-->>Controller: DTO wrapped in Page/List
Controller->>Response: serialize to HTTP response
Response-->>Client: JSON with DTO fields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Pre-merge checks✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java (1)
294-301: Fix JavaDoc to match the updated method signature.The JavaDoc references
@param negateRoleIds excluded role identifiersat line 298, but this parameter was removed from the method signature. Update the documentation to reflect the current signature.Apply this diff to fix the JavaDoc:
/** * Advanced search for co-members with role filtering. * * @param query the search query * @param roleIds required role identifiers - * @param negateRoleIds excluded role identifiers * @param loggedUser the logged user * @param pageable pagination information * @return page of matching co-members */nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java (2)
410-412: Remove redundant null check.The null/empty check for
roleIdson line 410 is redundant because lines 402-403 already return early whenroleIdsis null or empty. This code block will never execute with null or emptyroleIds.Apply this diff to simplify:
- if (roleIds != null && !roleIds.isEmpty()) { - predicate = predicate.and(QUser.user.processRoles.any()._id.in(roleIds)); - } + predicate = predicate.and(QUser.user.processRoles.any()._id.in(roleIds));
401-417: Breaking change properly handled; remove redundant null check.All call sites throughout the codebase have been successfully updated to match the new 4-parameter signature (removing
negateRoleIds). The interface definition in UserService.java is consistent with the implementation, and no orphaned calls remain.One minor optimization: the null check at line 410 (
if (roleIds != null && !roleIds.isEmpty())) is redundant since the guard at lines 402-404 ensuresroleIdsis non-null and non-empty when execution reaches that point. Simplify to:predicate = predicate.and(QUser.user.processRoles.any()._id.in(roleIds));application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetReferenceResourceAssembler.java (1)
8-24: Consider removing or implementing the commented HATEOAS links.The
buildLinksmethod contains commented-out code for building HATEOAS links to related resources (transitions, data, search). If these links are no longer needed, remove the commented code; if they are needed, implement them.application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.java (1)
99-108: Update the Javadoc to reflect the new return type.The Javadoc comment on line 107 still references the old return type
Page<PetriNetReference>, but the method now returnsPage<PetriNetReferenceDto>.Apply this diff to update the Javadoc:
/** * Method for search of PetriNets in Elastic * * @param requests - search body, for now only title working * @param user - logged user * @param pageable - pageable for paging * @param locale - internacionalization * @param isIntersection - property for merging filter, not implemented now, use false - * @return Page<PetriNetReference> - page of PetriNetReferences + * @return Page<PetriNetReferenceDto> - page of PetriNetReferenceDtos */application-engine/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java (1)
73-81: Preserve assembler-generated hypermediaBy replacing the assembler result with
PagedModel.of(...)we lose every link thatTaskResourceAssembler,PagedResourcesAssembler, andResourceLinkAssembleradd (per-task self/actions, next/prev page, etc.). Clients depending on those HATEOAS affordances will suddenly receive bare items with no navigation or action links. Please keep the assembler output (and updateTaskResourceAssemblerto emitLocalisedTaskResourcebuilt fromTaskDto) instead of bypassing it. The same fix is needed for the other methods that now stream-map tasks (see Lines 90, 160, 169, 178, 187, 196).- return PagedModel.of(page.stream() - .map(t -> new LocalisedTaskResource(TaskDto.fromTask(t, locale))).toList(), - new PagedModel.PageMetadata(pageable.getPageSize(), pageable.getPageNumber(), page.getTotalElements())); + return resources;application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java (2)
65-88: Restore bean validation on createUser inputsWith the switch to
UserCreateRequestDto, any Jakarta Bean Validation annotations on the record will be ignored unless the controller parameter is annotated with@Valid. That opens a path for malformed payloads (e.g., blank usernames, invalid emails) to reachuserService.createUser, which can trigger lower-level exceptions or create inconsistent users. Please reinstate validation on this endpoint.- public ResponseEntity<UserDto> createUser(@PathVariable String realmId, @RequestBody UserCreateRequestDto request, Locale locale) { + public ResponseEntity<UserDto> createUser(@PathVariable String realmId, @Valid @RequestBody UserCreateRequestDto request, Locale locale) {(Don't forget to import
jakarta.validation.Valid.)
277-285: Fix success/error payloads in assignAuthorityToUserThe responses are inverted: on failure we send a “success” message with a 400 status, and on success we send an “error” message with 200 OK. Clients relying on the message text (as they historically have in this API) will mis-handle the outcome. Please swap the factories so that the payload matches the HTTP status.
- return ResponseEntity.badRequest().body(ResponseMessage.createSuccessMessage("Assigning authority to user " + userId + " has failed!")); + return ResponseEntity.badRequest().body(ResponseMessage.createErrorMessage("Assigning authority to user " + userId + " has failed!")); ... - return ResponseEntity.ok(ResponseMessage.createErrorMessage("Authority was assigned to user successfully")); + return ResponseEntity.ok(ResponseMessage.createSuccessMessage("Authority was assigned to user successfully"));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (107)
application-engine/src/main/java/com/netgrif/application/engine/auth/service/UserResourceHelperService.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/service/interfaces/IUserResourceHelperService.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/PublicUserController.java(7 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java(9 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/ProcessRoleFactoryImpl.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/UserResource.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/configuration/ElasticsearchConfiguration.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.java(4 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticTaskService.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/interfaces/IElasticPetriNetService.java(2 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/orgstructure/web/GroupController.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/orgstructure/web/responsebodies/Group.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/orgstructure/web/responsebodies/GroupsResource.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java(8 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IPetriNetService.java(4 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.java(5 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PublicPetriNetController.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/ArcImportReference.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/DataFieldReference.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/DataFieldReferencesResource.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetImportReference.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetReference.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetReferenceResource.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetReferenceResourceAssembler.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetReferenceResources.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetReferenceWithMessage.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PlaceImportReference.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/ProcessRole.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/ProcessRolesAndPermissions.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/ProcessRolesResource.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/Reference.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/Transaction.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/TransactionResource.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/TransitionImportReference.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/TransitionReference.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/TransitionReferencesResource.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/CaseRepositoryImpl.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java(4 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/ConfigurableMenuService.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskSearchService.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java(10 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/WorkflowController.java(11 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/CaseResource.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/CaseResourceAssembler.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/DataFieldReference.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/ElasticTaskResourceAssembler.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/LocalisedTaskResource.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/ResourceLinkAssembler.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/Task.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/TaskResourceAssembler.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/eventoutcomes/base/LocalisedCaseEventOutcome.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/eventoutcomes/base/LocalisedPetriNetEventOutcome.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/eventoutcomes/base/LocalisedTaskEventOutcome.java(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/PetriNetTest.groovy(3 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthoritySearchDto.java(0 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/GroupSearchDto.java(0 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/PreferencesDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/authority/AuthoritySearchRequestDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/group/CreateGroupRequestDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/group/GroupSearchRequestDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/impersonation/ImpersonationSearchRequestDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/realm/RealmSearchDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/user/PasswordResetRequestDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/user/UserCreateRequestDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/user/UserSearchRequestBody.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/authority/AuthorityDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/group/GroupDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/impersonation/ImpersonationNotAvailableResponseDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/ArcImportReferenceDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/DataFieldReferenceDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/PetriNetImportReferenceDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/PetriNetReferenceDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/PlaceImportReferenceDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/PositionDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/ProcessRoleDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/TransactionDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/TransitionImportReferenceDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/TransitionReferenceDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/realm/RealmDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/task/TaskDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/user/UserDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/workflow/CaseDto.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/impersonation/web/requestbodies/SearchRequest.java(0 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/impersonation/web/responsebodies/ImpersonationNotAvailableResponse.java(0 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/petrinet/web/responsebodies/ProcessRole.java(0 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/preferences/Preferences.java(1 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/AuthorityServiceImpl.java(2 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/GroupServiceImpl.java(2 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/RealmServiceImpl.java(2 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserFactoryImpl.java(2 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java(1 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/realm/RealmDto.java(0 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/realm/request/RealmSearch.java(0 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/realm/response/RealmSearchResponse.java(0 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/RealmRepository.java(2 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/AuthorityService.java(2 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/GroupService.java(2 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/ProcessRoleFactory.java(1 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/RealmService.java(1 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserFactory.java(2 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java(1 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/requestbodies/PreferencesRequest.java(0 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/requestbodies/UserCreateRequest.java(0 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/requestbodies/UserSearchRequestBody.java(0 hunks)
⛔ Files not processed due to max files limit (2)
- nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/PreferencesResource.java
- nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/User.java
💤 Files with no reviewable changes (26)
- nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/requestbodies/PreferencesRequest.java
- nae-user-common/src/main/java/com/netgrif/application/engine/auth/realm/RealmDto.java
- nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/requestbodies/UserSearchRequestBody.java
- nae-user-common/src/main/java/com/netgrif/application/engine/auth/realm/request/RealmSearch.java
- application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/Task.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/TransitionImportReference.java
- application-engine/src/main/java/com/netgrif/application/engine/orgstructure/web/responsebodies/Group.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PlaceImportReference.java
- application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/ElasticTaskResourceAssembler.java
- nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/requestbodies/UserCreateRequest.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetReference.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/ProcessRole.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/GroupSearchDto.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/Reference.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/petrinet/web/responsebodies/ProcessRole.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/impersonation/web/requestbodies/SearchRequest.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/TransitionReference.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetImportReference.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/DataFieldReference.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthoritySearchDto.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/Transaction.java
- nae-user-common/src/main/java/com/netgrif/application/engine/auth/realm/response/RealmSearchResponse.java
- application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/DataFieldReference.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/impersonation/web/responsebodies/ImpersonationNotAvailableResponse.java
- application-engine/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/ArcImportReference.java
🧰 Additional context used
🧠 Learnings (10)
📚 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/ConfigurableMenuService.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/workflow/service/ConfigurableMenuService.javaapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.javaapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PublicPetriNetController.javaapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IPetriNetService.javaapplication-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/PetriNetTest.groovyapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java
📚 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/workflow/service/ConfigurableMenuService.javaapplication-engine/src/main/java/com/netgrif/application/engine/auth/service/interfaces/IUserResourceHelperService.javanae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserFactory.javaapplication-engine/src/main/java/com/netgrif/application/engine/auth/web/PublicUserController.javaapplication-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.javaapplication-engine/src/main/java/com/netgrif/application/engine/auth/service/UserResourceHelperService.java
📚 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/workflow/service/ConfigurableMenuService.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/PetriNetReferenceDto.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/CaseRepositoryImpl.javaapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetReferenceWithMessage.javaapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PublicPetriNetController.javaapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IPetriNetService.javaapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.javaapplication-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/ProcessRoleFactoryImpl.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java
📚 Learning: 2025-07-29T17:19:18.300Z
Learnt from: tuplle
Repo: netgrif/application-engine PR: 331
File: application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.java:45-46
Timestamp: 2025-07-29T17:19:18.300Z
Learning: In ElasticPetriNetService class, petriNetService is properly initialized using Lazy setter injection rather than constructor injection. This pattern with Lazy Autowired setter methods is commonly used in Spring to resolve circular dependencies and is a valid alternative to constructor injection.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/ConfigurableMenuService.javaapplication-engine/src/main/java/com/netgrif/application/engine/elastic/service/interfaces/IElasticPetriNetService.javaapplication-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.javaapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java
📚 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:
nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/RealmRepository.javanae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/RealmServiceImpl.javanae-user-common/src/main/java/com/netgrif/application/engine/auth/service/RealmService.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/realm/RealmSearchDto.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/realm/RealmDto.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/ElasticCaseService.javaapplication-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticTaskService.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/ElasticTaskService.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskSearchService.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.javaapplication-engine/src/main/java/com/netgrif/application/engine/configuration/ElasticsearchConfiguration.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/elastic/service/ElasticTaskService.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java
📚 Learning: 2025-11-04T11:47:03.897Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 378
File: nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java:16-16
Timestamp: 2025-11-04T11:47:03.897Z
Learning: In the netgrif/application-engine project, ElasticsearchRunner is responsible for creating Elasticsearch indexes automatically based on configuration. Spring Data's automatic index creation via the Document annotation's createIndex attribute should be disabled (set to false) to prevent conflicts with this centralized index management approach.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/configuration/ElasticsearchConfiguration.java
🧬 Code graph analysis (8)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/ArcImportReferenceDto.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/arcs/InhibitorArc.java (1)
InhibitorArc(11-58)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/task/TaskDto.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/localised/LocalisedFieldFactory.java (1)
LocalisedFieldFactory(7-112)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.java (1)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/PetriNetReferenceResource.java (1)
PetriNetReferenceResource(8-36)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PublicPetriNetController.java (1)
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/dto/response/workflow/CaseDto.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/localised/LocalisedFieldFactory.java (1)
LocalisedFieldFactory(7-112)
application-engine/src/main/java/com/netgrif/application/engine/workflow/web/WorkflowController.java (1)
application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/ResourceLinkAssembler.java (1)
ResourceLinkAssembler(12-61)
application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/common/web/responsebodies/ResponseMessage.java (1)
ResponseMessage(4-60)
application-engine/src/main/java/com/netgrif/application/engine/orgstructure/web/GroupController.java (2)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java (1)
ProcessRoleService(46-528)application-engine/src/main/java/com/netgrif/application/engine/orgstructure/web/responsebodies/GroupsResource.java (1)
GroupsResource(15-61)
⏰ 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). (6)
- 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: Build
Comment on lines
+314
to
323
| public ResponseEntity<ResponseMessage> savePreferences(@RequestBody PreferencesDto preferences, Authentication auth) { | ||
| try { | ||
| String userId = ((LoggedUser) auth.getPrincipal()).getStringId(); | ||
| preferences.setUserId(userId); | ||
| preferencesService.save(preferences.toPreferences()); | ||
| Preferences domainPreferences = com.netgrif.application.engine.adapter.spring.preferences.Preferences.fromDto(preferences, userId); | ||
| preferencesService.save(domainPreferences); | ||
| return ResponseEntity.ok(ResponseMessage.createSuccessMessage("User preferences saved")); | ||
| } catch (Exception e) { | ||
| log.error("Saving user preferences failed", e); | ||
| return ResponseEntity.badRequest().body(ResponseMessage.createErrorMessage("Saving user preferences failed")); | ||
| } |
There was a problem hiding this comment.
Validate PreferencesDto before persisting
PreferencesDto now reaches the service layer directly, but without @Valid on the controller parameter any field-level constraints on the DTO (e.g., @NotBlank, size limits) are skipped. That allows malformed preference data to be saved or to blow up deeper in Preferences.fromDto. Add @Valid to keep the bean validation contract intact.
- public ResponseEntity<ResponseMessage> savePreferences(@RequestBody PreferencesDto preferences, Authentication auth) {
+ public ResponseEntity<ResponseMessage> savePreferences(@Valid @RequestBody PreferencesDto preferences, Authentication auth) {(Requires jakarta.validation.Valid import if not already added.)
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java
around lines 314 to 323, the controller method parameter for PreferencesDto is
not annotated with @Valid so bean validation constraints on the DTO are skipped;
update the method signature to annotate the preferences parameter with
jakarta.validation.Valid and add the required import (jakarta.validation.Valid)
so Spring triggers validation before calling Preferences.fromDto, optionally
handle MethodArgumentNotValidException globally or add a BindingResult parameter
if you want to return field-level errors.
Comment on lines
+266
to
+273
| public ResponseEntity<Page<GroupDto>> search(@RequestBody GroupSearchRequestDto query, Pageable pageable, Locale locale) { | ||
| List<Group> groups = service.search(query, pageable).getContent(); | ||
| return ResponseEntity.ok(transformPageContent(groups, pageable, locale)); | ||
| } | ||
|
|
||
| private Page<GroupDto> transformPageContent(List<Group> groups, Pageable pageable, Locale locale) { | ||
| return new PageImpl<>(groups.stream().map(group -> GroupDto.fromGroup(group, locale)).toList(), pageable, groups.size()); | ||
| } |
There was a problem hiding this comment.
Preserve pagination metadata when mapping search results.
search() discards the Page<Group> returned by the service and rebuilds a page with totalElements = groups.size(). Any result set spanning multiple pages now reports the wrong total, breaking pagination. Please keep the original Page metadata.
- List<Group> groups = service.search(query, pageable).getContent();
- return ResponseEntity.ok(transformPageContent(groups, pageable, locale));
+ Page<Group> groups = service.search(query, pageable);
+ return ResponseEntity.ok(transformPageContent(groups, locale));
...
- private Page<GroupDto> transformPageContent(List<Group> groups, Pageable pageable, Locale locale) {
- return new PageImpl<>(groups.stream().map(group -> GroupDto.fromGroup(group, locale)).toList(), pageable, groups.size());
+ private Page<GroupDto> transformPageContent(Page<Group> groups, Locale locale) {
+ List<GroupDto> content = groups.getContent().stream().map(group -> GroupDto.fromGroup(group, locale)).toList();
+ return new PageImpl<>(content, groups.getPageable(), groups.getTotalElements());🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/orgstructure/web/GroupController.java
around lines 266-273, the controller discards the original Page<Group> metadata
and rebuilds a Page with totalElements = groups.size(), breaking pagination; fix
by keeping the Page returned by service and mapping its content to DTOs while
preserving metadata (e.g., obtain Page<Group> page = service.search(...); then
return page.map(g -> GroupDto.fromGroup(g, locale)) or construct a new PageImpl
using page.getTotalElements() and page.getPageable()).
Comment on lines
+135
to
137
| PetriNetReferenceDto reference = service.getReference(identifier, converter.convert(version), user, locale); | ||
| return ResponseEntity.ok(new PageImpl<>(Collections.singletonList(reference), pageable, reference == null ? 0 : 1)); | ||
| } else { |
There was a problem hiding this comment.
Return an empty page when the reference is missing.
If service.getReference(...) returns null, we currently emit a page whose content is [null] yet totalElements is 0. Clients now receive a null entry instead of an empty list. Please build the content list conditionally.
- PetriNetReferenceDto reference = service.getReference(identifier, converter.convert(version), user, locale);
- return ResponseEntity.ok(new PageImpl<>(Collections.singletonList(reference), pageable, reference == null ? 0 : 1));
+ PetriNetReferenceDto reference = service.getReference(identifier, converter.convert(version), user, locale);
+ List<PetriNetReferenceDto> content = reference == null ? List.of() : List.of(reference);
+ return ResponseEntity.ok(new PageImpl<>(content, pageable, content.size()));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| PetriNetReferenceDto reference = service.getReference(identifier, converter.convert(version), user, locale); | |
| return ResponseEntity.ok(new PageImpl<>(Collections.singletonList(reference), pageable, reference == null ? 0 : 1)); | |
| } else { | |
| PetriNetReferenceDto reference = service.getReference(identifier, converter.convert(version), user, locale); | |
| List<PetriNetReferenceDto> content = reference == null ? List.of() : List.of(reference); | |
| return ResponseEntity.ok(new PageImpl<>(content, pageable, content.size())); | |
| } else { |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.java
around lines 135 to 137, the code currently wraps a possibly null reference in
Collections.singletonList(reference) producing a page with a null element;
change this to conditionally create the content list (use
Collections.emptyList() when reference is null, otherwise
Collections.singletonList(reference)) and pass the correct total (0 when null, 1
otherwise) so the ResponseEntity returns an empty page instead of a list
containing null.
Comment on lines
+5
to
+6
| public record CreateGroupRequestDto(String displayName, String realmId, String identifier, String ownerId) implements Serializable { | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding validation constraints.
This DTO lacks validation annotations. Depending on where validation is performed in your architecture, consider adding constraints such as @NotNull or @NotBlank for required fields like displayName or identifier to provide early validation feedback and clearer API contracts.
🤖 Prompt for AI Agents
In
nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/group/CreateGroupRequestDto.java
around lines 5-6, the record fields have no validation annotations; add
appropriate bean validation constraints to the record components (e.g., annotate
displayName and identifier with @NotBlank and realmId/ownerId with @NotNull or
@NotBlank as required), import the javax.validation or jakarta.validation
constraint annotations, and ensure consuming controllers/services validate
incoming DTOs (use @Valid on controller method params) so the annotations are
enforced.
| /** | ||
| * DTO for {@link Group} search request | ||
| */ | ||
| public record GroupSearchRequestDto(String fullText, String realmId) implements Serializable { } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding validation annotations for clarity.
While search fields are often optional, adding validation annotations (e.g., @Size(max = ...) for fullText, or @Pattern for realmId) would clarify constraints and prevent potential issues with excessively large inputs or malformed realm identifiers.
🤖 Prompt for AI Agents
In
nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/request/group/GroupSearchRequestDto.java
around line 10, the record parameters lack validation annotations; add Jakarta
Bean Validation annotations on the record components (e.g., @Size(max=255) or an
appropriate max for fullText, and @Pattern(...) or @Size(...) for realmId with a
suitable regex/length) and import the corresponding
jakarta.validation.constraints classes so the DTO enforces input constraints at
binding time.
Comment on lines
+10
to
+13
| public static TransitionImportReferenceDto fromTransition(Transition transition) { | ||
| return new TransitionImportReferenceDto(PositionDto.fromPosition(transition.getPosition()), transition.getTitle(), | ||
| transition.getObjectId().toString(), transition.getImportId()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify null safety in factory method.
The fromTransition factory method doesn't perform null checks before accessing transition.getPosition(), transition.getTitle(), etc. If any of these getters can return null, consider adding defensive null checks or document the preconditions.
Run the following script to check if the Transition domain class has nullable fields:
🏁 Script executed:
#!/bin/bash
# Description: Check Transition class for nullable annotations or documentation
# Search for Transition class definition and methods
ast-grep --pattern $'class Transition {
$$$
}'
# Look for nullable annotations on getters
rg -nP --type=java -C3 '@Nullable|@CheckForNull' -g '**/Transition.java'Length of output: 0
I need to search more broadly for the Transition class. Let me locate it first and then inspect its structure.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find Transition.java file
fd -type f -name "Transition.java" --exec echo {} \;
# Also search for Transition class using broader rg pattern
rg -n "class Transition\b" --type=java -A 30 | head -100
Length of output: 7205
I found the Transition class. Now I need to inspect the Node parent class to check the getter implementations.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find Node.java file
fd -type f -name "Node.java"
# Search for Node class and its getters
rg -n "class Node\b" -A 50 | head -150
Length of output: 5708
I need to see the full Node class to find the getter methods, and also check the PetriNetObject parent class.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full Node.java file
cat nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/Node.java
# Also search for PetriNetObject class
rg -n "class PetriNetObject" -A 50 | head -120
Length of output: 4784
I need to check the Imported parent class to complete the analysis.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find Imported class and check getImportId()
rg -n "class Imported" -A 30
Length of output: 901
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full Imported.java file
cat nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/Imported.java
# Also search for getImportId method
rg -n "getImportId" -B 2 -A 2
Length of output: 279437
Add null checks or document preconditions for potentially null values.
The review concern is partially valid. Analysis shows:
getPosition()is safe (initialized in Node constructor)getObjectId()is safe (initialized in Node constructor)getTitle()is unsafe – the title field is not initialized in the Node constructor and can be nullgetImportId()is unsafe – the importId String field has no initialization guarantee
Recommend adding null checks before accessing transition.getTitle() and transition.getImportId(), or document the precondition that the Transition instance must have these values set.
🤖 Prompt for AI Agents
In
nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/TransitionImportReferenceDto.java
around lines 10 to 13, the call sites transition.getTitle() and
transition.getImportId() can be null; add null checks for these two values
before constructing the DTO and fail-fast with a clear exception if either is
missing (e.g., throw IllegalArgumentException with a message identifying which
field and which transition), or alternatively document in the method Javadoc
that the Transition must have non-null title and importId and validate at the
start to convert that contract into runtime checks.
Comment on lines
+11
to
+16
| public TransitionReferenceDto(String stringId, String title, String petriNetId, List<DataFieldReferenceDto> immediateData) { | ||
| this.stringId = stringId; | ||
| this.title = title; | ||
| this.petriNetId = petriNetId; | ||
| this.immediateData = immediateData; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove redundant canonical constructor.
Records automatically generate a canonical constructor that performs the same assignments. This explicit constructor is redundant and should be removed to leverage the implicit record behavior.
Apply this diff to remove the redundant constructor:
public record TransitionReferenceDto(String stringId,
String title,
String petriNetId,
List<DataFieldReferenceDto> immediateData) implements Serializable {
-
- public TransitionReferenceDto(String stringId, String title, String petriNetId, List<DataFieldReferenceDto> immediateData) {
- this.stringId = stringId;
- this.title = title;
- this.petriNetId = petriNetId;
- this.immediateData = immediateData;
- }
}🤖 Prompt for AI Agents
In
nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/petrinet/TransitionReferenceDto.java
around lines 11 to 16, remove the explicit canonical constructor (the public
TransitionReferenceDto(String stringId, String title, String petriNetId,
List<DataFieldReferenceDto> immediateData) { ... }) since Java records already
generate the same canonical constructor; delete those lines so the record uses
its implicit constructor and keep the rest of the record unchanged.
Comment on lines
+53
to
+55
| task.getDataFocusPolicy().toString(), | ||
| task.getFinishPolicy().toString(), | ||
| task.getTranslatedEventTitle(EventType.FINISH, locale), |
There was a problem hiding this comment.
Guard dataFocusPolicy against null.
task.getDataFocusPolicy() is optional (field marked @Nullable), so calling .toString() unconditionally will blow up for tasks without that policy. Please propagate null safely.
- task.getAssignPolicy().toString(),
- task.getDataFocusPolicy().toString(),
+ task.getAssignPolicy().toString(),
+ task.getDataFocusPolicy() != null ? task.getDataFocusPolicy().toString() : null,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
nae-object-library/src/main/java/com/netgrif/application/engine/objects/dto/response/task/TaskDto.java
around lines 53 to 55, task.getDataFocusPolicy() is @Nullable so calling
.toString() unguarded can NPE; change the mapping to guard for null and
propagate null (e.g., use a null check or Optional/ternary to set the DTO value
to task.getDataFocusPolicy() == null ? null :
task.getDataFocusPolicy().toString()), ensuring the DTO field remains nullable
and consistent with other policies.
Comment on lines
+19
to
+22
| preferences.setDrawerWidth(preferencesDto.drawerWidth() <= 0 ? preferencesDto.drawerWidth() : 200); | ||
| preferences.setTaskFilters(preferencesDto.taskFilters() != null ? preferencesDto.taskFilters() : new HashMap<>()); | ||
| preferences.setCaseFilters(preferencesDto.caseFilters() != null ? preferencesDto.caseFilters() : new HashMap<>()); | ||
| preferences.setHeaders(preferencesDto.headers() != null ? preferencesDto.headers() : new HashMap<>()); |
There was a problem hiding this comment.
Fix inverted drawer width fallback
When the DTO supplies a positive drawer width, the current ternary overwrites it with the fallback 200, and when the DTO provides a non-positive value, the invalid value is retained. This breaks persisted drawer sizing for every user. Please invert the condition (and guard against null) so that only missing or non-positive values fall back to 200.
- preferences.setDrawerWidth(preferencesDto.drawerWidth() <= 0 ? preferencesDto.drawerWidth() : 200);
+ Integer drawerWidth = preferencesDto.drawerWidth();
+ preferences.setDrawerWidth(drawerWidth != null && drawerWidth > 0 ? drawerWidth : 200);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| preferences.setDrawerWidth(preferencesDto.drawerWidth() <= 0 ? preferencesDto.drawerWidth() : 200); | |
| preferences.setTaskFilters(preferencesDto.taskFilters() != null ? preferencesDto.taskFilters() : new HashMap<>()); | |
| preferences.setCaseFilters(preferencesDto.caseFilters() != null ? preferencesDto.caseFilters() : new HashMap<>()); | |
| preferences.setHeaders(preferencesDto.headers() != null ? preferencesDto.headers() : new HashMap<>()); | |
| Integer drawerWidth = preferencesDto.drawerWidth(); | |
| preferences.setDrawerWidth(drawerWidth != null && drawerWidth > 0 ? drawerWidth : 200); | |
| preferences.setTaskFilters(preferencesDto.taskFilters() != null ? preferencesDto.taskFilters() : new HashMap<>()); | |
| preferences.setCaseFilters(preferencesDto.caseFilters() != null ? preferencesDto.caseFilters() : new HashMap<>()); | |
| preferences.setHeaders(preferencesDto.headers() != null ? preferencesDto.headers() : new HashMap<>()); |
🤖 Prompt for AI Agents
In
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/preferences/Preferences.java
around lines 19 to 22, the ternary for drawerWidth is inverted so positive DTO
values get replaced by the fallback; change the logic to check for null or
non-positive and only then use 200, otherwise use the DTO value (e.g., if
preferencesDto.drawerWidth() == null || preferencesDto.drawerWidth() <= 0 then
200 else preferencesDto.drawerWidth()); keep the other null checks for maps
as-is.
| Optional<Realm> findAdminRealm(); | ||
|
|
||
| default Page<Realm> searchRealms(RealmSearch realmSerach, Pageable pageable, MongoTemplate mongoTemplate) { | ||
| default Page<Realm> searchRealms(RealmSearchDto realmSerach, Pageable pageable, MongoTemplate mongoTemplate) { |
There was a problem hiding this comment.
Fix the parameter name typo.
The parameter name realmSerach contains a typo and should be realmSearch. This typo was likely carried over during the DTO refactor.
Apply this diff to fix the typo:
- default Page<Realm> searchRealms(RealmSearchDto realmSerach, Pageable pageable, MongoTemplate mongoTemplate) {
+ default Page<Realm> searchRealms(RealmSearchDto realmSearch, Pageable pageable, MongoTemplate mongoTemplate) {
Criteria searchCriteria = new Criteria();
List<Criteria> criteriaList = new ArrayList<>();
- if (realmSerach != null) {
- if (realmSerach.id() != null && !realmSerach.id().isBlank()) {
- criteriaList.add(Criteria.where("id").regex(realmSerach.id(), "i"));
+ if (realmSearch != null) {
+ if (realmSearch.id() != null && !realmSearch.id().isBlank()) {
+ criteriaList.add(Criteria.where("id").regex(realmSearch.id(), "i"));
}
- if (realmSerach.name() != null && !realmSerach.name().isBlank()) {
- criteriaList.add(Criteria.where("name").regex(realmSerach.name(), "i"));
+ if (realmSearch.name() != null && !realmSearch.name().isBlank()) {
+ criteriaList.add(Criteria.where("name").regex(realmSearch.name(), "i"));
}
- if (realmSerach.description() != null && !realmSerach.description().isBlank()) {
- criteriaList.add(Criteria.where("description").regex(realmSerach.description(), "i"));
+ if (realmSearch.description() != null && !realmSearch.description().isBlank()) {
+ criteriaList.add(Criteria.where("description").regex(realmSearch.description(), "i"));
}
- if (Boolean.TRUE.equals(realmSerach.adminRealm())) {
+ if (Boolean.TRUE.equals(realmSearch.adminRealm())) {
criteriaList.add(Criteria.where("adminRealm").is(true));
}
- if (Boolean.TRUE.equals(realmSerach.defaultRealm())) {
+ if (Boolean.TRUE.equals(realmSearch.defaultRealm())) {
criteriaList.add(Criteria.where("defaultRealm").is(true));
}
- if (Boolean.TRUE.equals(realmSerach.enableBlocking())) {
+ if (Boolean.TRUE.equals(realmSearch.enableBlocking())) {
criteriaList.add(Criteria.where("enableBlocking").is(true));
}
}🤖 Prompt for AI Agents
In
nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/RealmRepository.java
around line 35, the method parameter name realmSerach is misspelled; rename it
to realmSearch in the method signature and update any references within the
method body to use the corrected name so the DTO parameter matches its intended
name.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Implements NAE-2221
Dependencies
none
Third party dependencies
none
How Has Been This Tested?
Test Configuration
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Refactor