[NAE-2423] Extend Action API with wrapper support#440
Merged
Conversation
Introduced methods to handle additional parameters across case and task search and count operations. Created a constants class for default process parameter names and added overloads with parameter maps to improve flexibility.
Replaced the `AuthPrincipalDto` class with a Java record for improved immutability and concise representation. Adjusted related code to align with the record's accessors and updated the system username constant in `UserConstants` for consistency.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR converts ChangesAPI parameter support and data model migration
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
coderabbitai
Bot
added
improvement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java`:
- Around line 125-128: The new params-aware overloads (e.g.,
ActionApiImpl.searchCases(String processIdentifier, Predicate predicate,
Pageable pageable, Map<String,String> params)) currently ignore the params map;
update each of these methods to incorporate or forward params into the
search/count operation instead of discarding them. Concretely: in searchCases
and the analogous countCases wrappers, either merge params into the existing
Predicate (e.g., build or augment predicate from params) or call the
workflowService overload that accepts params (e.g., workflowService.search(...)
/ workflowService.count(...) with params) so the incoming Map<String,String> is
actually used; apply the same change to every method signature that accepts
Map<String,String> params in this class. Ensure you reference and update the
methods named searchCases and the corresponding countCases wrappers so params
are read and propagated to workflowService.
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java`:
- Around line 11-16: AuthPrincipalDto is a Java record whose compiler-generated
equals, hashCode and toString include sessionId despite Lombok annotations; to
fix this either convert the record to a class-based DTO or add an explicit
record body that overrides equals(Object), hashCode(), and toString() to exclude
the sessionId component (keep the existing components username and realmId
semantics and retain Serializable), ensuring all loggable toString() calls and
equality/hash checks do not expose sessionId.
In
`@nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java`:
- Around line 216-223: The Javadoc for the countTasks method incorrectly states
it returns "a page of tasks"; update the `@return` description for the countTasks
method to reflect that it returns a Long representing the count of tasks (e.g.,
"the number of tasks matching the criteria") so the documentation matches the
method's return type Long in ActionApi.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c8e2e965-a2aa-4753-92f2-6f91200ee187
📒 Files selected for processing (6)
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/constants/UserConstants.javanae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.javanae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.javanae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApiDefaultParamNames.javanae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java
Modified the countTasks method to return the number of tasks instead of a page of tasks. Also updated the corresponding JavaDoc to reflect the change accurately. Removed an unused import to improve code cleanliness.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java`:
- Line 108: The six new abstract overloads (searchCases(String, Predicate,
Pageable, Map<String,String>), searchCases(List<String>, AuthPrincipalDto,
Pageable, Boolean, Map<String,String>), countCases(String, Predicate,
Map<String,String>), searchTasks(String, Predicate, Pageable,
Map<String,String>), searchTasks(List<String>, AuthPrincipalDto, Pageable,
Boolean, Map<String,String>), countTasks(String, Predicate, Map<String,String>))
must be converted to default methods on the ActionApi interface that delegate to
the existing 3- or 4-parameter variants to preserve binary compatibility;
implement each default to simply call the older overload (e.g.,
searchCases(processIdentifier, predicate, pageable) or searchTasks(...,
pageable)) ignoring the extra params Map or passing a default, so external
implementers are not forced to recompile.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b7ea28a6-c6f8-4740-a2ef-be4b961aa530
📒 Files selected for processing (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
Contributor
|
tests are failing |
Updated test cases to consistently use `username` instead of `email` for identifying users. This ensures better alignment with domain constants and improved code maintainability. Changes include replacing hardcoded email references with `UserConstants.SYSTEM_USER_USERNAME` or user `username` fields where applicable.
There was a problem hiding this comment.
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/test/groovy/com/netgrif/application/engine/event/GroovyShellFactoryTest.groovy (1)
75-86:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winInconsistent system user lookup causes test failure.
Line 75 correctly looks up the user by
getUsername(), but line 84 still usesgetEmail(). If the system user's username differs from its email, line 84 will fail to find the updated user or retrieve a different user entirely, breaking the assertion on line 85.This inconsistency is likely the root cause of the test failures mentioned in the PR comments.
🐛 Proposed fix to use consistent username lookup
- user = userService.findUserByUsername(userService.getSystem().getEmail(), null) + user = userService.findUserByUsername(userService.getSystem().getUsername(), null)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application-engine/src/test/groovy/com/netgrif/application/engine/event/GroovyShellFactoryTest.groovy` around lines 75 - 86, The test uses inconsistent system user lookups causing failures: in GroovyShellFactoryTest replace the second lookup that calls userService.findUserByUsername(userService.getSystem().getEmail(), ...) with the same username-based lookup used earlier (userService.getSystem().getUsername()) so both searches use userService.findUserByUsername and the assertion on processRoles (after roleService.assignRolesToUser) checks the updated system user reliably.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@application-engine/src/test/groovy/com/netgrif/application/engine/event/GroovyShellFactoryTest.groovy`:
- Around line 75-86: The test uses inconsistent system user lookups causing
failures: in GroovyShellFactoryTest replace the second lookup that calls
userService.findUserByUsername(userService.getSystem().getEmail(), ...) with the
same username-based lookup used earlier (userService.getSystem().getUsername())
so both searches use userService.findUserByUsername and the assertion on
processRoles (after roleService.assignRolesToUser) checks the updated system
user reliably.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 11e1c546-2a64-416f-94cf-5ded1884da23
📒 Files selected for processing (3)
application-engine/src/test/groovy/com/netgrif/application/engine/elastic/DataSearchRequestTest.groovyapplication-engine/src/test/groovy/com/netgrif/application/engine/event/GroovyShellFactoryTest.groovyapplication-engine/src/test/groovy/com/netgrif/application/engine/orgstructure/groups/GroupServiceTest.groovy
…hCode Enhanced the AuthPrincipalDto class by overriding toString, equals, and hashCode methods for better readability and consistency in object comparisons and hashing. This change improves debugging and ensures hash-based collections work as expected. The sessionId remains excluded from these methods to preserve confidentiality.
Retoocs
requested changes
May 18, 2026
Contributor
Retoocs
left a comment
Retoocs
left a comment
There was a problem hiding this comment.
thx for the fix but the tests are still failing
Updated the test to retrieve the system user by username instead of email. This aligns with the correct method usage and improves test accuracy.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java`:
- Around line 26-32: The equals method in AuthPrincipalDto currently calls
username.equals(...) and realmId.equals(...) which can throw NPE if either
component is null; change the implementation of AuthPrincipalDto.equals(Object
o) to use Objects.equals(this.username, that.username) and
Objects.equals(this.realmId, that.realmId) for null-safe comparisons (consistent
with the existing Objects.hash usage in hashCode).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8e516e08-294e-40cc-afd1-41c9eddecc6b
📒 Files selected for processing (2)
application-engine/src/test/groovy/com/netgrif/application/engine/event/GroovyShellFactoryTest.groovynae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java
Added validation to prevent null pointer exceptions by checking if `username` or `realmId` is null before proceeding with equality checks. This change improves robustness and avoids potential runtime errors.
Retoocs
previously approved these changes
May 19, 2026
machacjozef
dismissed stale reviews from Retoocs and coderabbitai[bot]
via
d2c59a6
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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-2423
Dependencies
No new dependencies were introduced
Third party dependencies
No new dependencies were introduced
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
This was tested manually and with unit tests.
Test Configuration
Checklist:
Summary by CodeRabbit
New Features
Refactor