-
Notifications
You must be signed in to change notification settings - Fork 6
[NAE-2228] Invalid arguments for collectRolesForPreferenceItem #368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Replaced multiple calls with a single paginated query to improve performance and reduce unnecessary iterations. This simplifies the logic and ensures the first global role is efficiently retrieved.
WalkthroughReplace GLOBAL role lookup in ActionDelegate.collectRolesForPreferenceItem to use a paginated call to ProcessRoleService.findAllByImportId(..., Pageable.ofSize(1)); proceed only if the returned page has at least one entry and use its first element. Several module parent POM versions updated from 7.0.0-RC8 to 7.0.0-RC8.1. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant ActionDelegate
participant ProcessRoleService
Caller->>ActionDelegate: collectRolesForPreferenceItem(roles)
alt GLOBAL role key present
ActionDelegate->>ProcessRoleService: findAllByImportId(importId, Pageable.ofSize(1))
ProcessRoleService-->>ActionDelegate: Page<ProcessRole>
alt page not empty
ActionDelegate->>ActionDelegate: select first ProcessRole (getFirst)
ActionDelegate-->>Caller: continue with selected global role
else page empty
ActionDelegate-->>Caller: skip GLOBAL role handling
end
else non-global role
ActionDelegate->>ActionDelegate: existing non-global role handling (unchanged)
ActionDelegate-->>Caller: continue
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (6)
⏰ 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). (1)
🔇 Additional comments (6)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(1 hunks)
⏰ 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). (5)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
...vy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
Outdated
Show resolved
Hide resolved
Updated logic to correctly handle cases where no global roles are found by checking for an empty list instead of null. This ensures robustness and prevents potential runtime errors.
The base branch was changed.
Description
Corrected wrong function call and simplified resolution of global role in collectRolesForPreferenceItem, as importId for global roles always will be unique.
Fixes NAE-2228
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
Bug Fixes
Performance
Chores