[NAE-2433] Implement Migration Helper Integration for Engine 6.x#445
Open
renczesstefan wants to merge 18 commits into
Open
[NAE-2433] Implement Migration Helper Integration for Engine 6.x#445renczesstefan wants to merge 18 commits into
renczesstefan wants to merge 18 commits into
Conversation
Added `MigrationTest` class to validate case migration processes and introduced the `MigrationHelper` utility to streamline case and task migrations. These changes improve testing coverage and simplify migration workflows through reusable and efficient methods.
Introduced abstract and task-specific migration helpers to enhance reusability and efficiency of MongoDB migration logic. Improved test coverage with `MigrationBenchmarkTest`, consolidated redundant methods, and standardized operations for cases, tasks, and PetriNet migrations.
Streamlined the MigrationHelper class by delegating migration logic to specialized helper classes (CaseMigrationHelper, TaskMigrationHelper, and PetriNetMigrationHelper). This improves code modularity, maintainability, and reduces redundancy by centralizing shared operations.
Refactored `CaseMigrationHelper`, `TaskMigrationHelper`, and `PetriNetMigrationHelper` to include static helper methods for flexible reuse. Added comprehensive Javadoc comments to improve maintainability and make these methods self-explanatory. Simplified `MigrationHelper` to leverage updated helpers.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduces a complete framework for bulk migrating workflow Cases, Tasks, and Petri Nets in MongoDB with configurable error handling, pagination, Elasticsearch indexing, role management, and data field transformations. Includes error models, configuration, abstract migration base, three concrete helpers, a top-level facade, documentation, and integration tests. ChangesMigration Framework with Error Handling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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: 23
🤖 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
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/AbstractMigrationHelper.groovy`:
- Line 123: The bulkOps call in AbstractMigrationHelper uses a hardcoded
Case.class which targets the wrong collection for non-case migrations; change
the mongoTemplate.bulkOps(...) invocation(s) (the one calling
BulkOperations.BulkMode.UNORDERED and any other similar call) to use a generic
document type (e.g. org.bson.Document) instead of Case.class so the operation
runs against the intended collection regardless of domain type; update both
occurrences (the one at the shown line and the other similar call) to use
Document as the entity parameter.
- Around line 114-118: The iterate method uses pageSize in pagination math
without validating it; add a guard at the start of iterate (method name: iterate
in class AbstractMigrationHelper) that ensures pageSize is a positive integer
(e.g., if pageSize <= 0 then set pageSize = getPageSize() or a sensible minimum
like 1), then compute numOfPages using a safe integer-safe pagination formula
(e.g., ceil-like calculation) so division by zero is impossible and batching
behaves correctly; update any downstream logic that relies on pageSize to use
the validated value.
- Around line 80-89: The handleBulkOps method currently catches
BulkWriteException and only logs errors, which swallows failures; modify
handleBulkOps(BulkOperations bulkOps) so that after logging the
BulkWriteException (including per-write errors) it rethrows the exception (or
throws a new RuntimeException/checked exception) to propagate failure to
callers; keep the existing logging of BulkWriteResult on success and ensure you
reference BulkOperations, BulkWriteResult, and BulkWriteException when making
the change so callers of handleBulkOps can detect and handle migration failures.
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy`:
- Around line 207-218: In changeDataFieldsValueFromTextToNumber, the catch block
sets useCase.dataSet[dataFieldID].value to null before logging, so the log
prints null; capture the original value into a local variable (e.g.,
originalValue = useCase.dataSet[dataFieldID].value) before assigning null, then
use that originalValue in the processLogger/log.error call (reference:
changeDataFieldsValueFromTextToNumber, variable dataFieldID and
useCase.dataSet[dataFieldID].value) so the logged message shows the
pre-conversion value and include the exception.
- Around line 292-296: The method changeFileFieldToFileList creates a
FileListFieldValue and unconditionally adds useCase.dataSet[fieldId].value cast
to FileFieldValue into fileListFieldValue.namesPaths, which can add a null entry
if the original value is null; update changeFileFieldToFileList to first check
whether useCase.dataSet[fieldId].value is non-null and an instance of
FileFieldValue before adding it to FileListFieldValue.namesPaths, and only add
when valid (otherwise leave namesPaths empty), then set
useCase.dataSet[fieldId].value to the new FileListFieldValue.
- Around line 275-285: In removeChoices, removing List<String> entries from sets
of I18nString fails due to type/equality mismatch; update remove logic in
removeChoices so that for both useCase.dataSet[dataFieldID].value (the Set) and
useCase.dataSet[dataFieldID].choices you map/filter the String ids in
choicesToRemove to the actual I18nString objects (e.g., by matching
I18nString.getDefault() or its string representation) and then call removeAll
with that Set, or use removeIf with a predicate comparing each I18nString to the
Strings in choicesToRemove; ensure changes are applied inside the removeChoices
method and reference useCase.dataSet[dataFieldID].value and
useCase.dataSet[dataFieldID].choices accordingly.
- Around line 322-342: The method updateCasePermissionsFromNet is static but
references the instance field taskMigrationHelper (used when calling
taskMigrationHelper.updateTaskPermissions), causing a compilation error; fix by
either removing the static modifier from updateCasePermissionsFromNet so it
becomes an instance method that can access the taskMigrationHelper field, or
change the method signature to accept a TaskMigrationHelper (or appropriate
type) parameter and replace uses of the field with that parameter when calling
updateTaskPermissions; ensure callers are updated accordingly (pass the existing
taskMigrationHelper instance when invoking the modified method).
- Around line 141-154: Both updateCasesCursor and updateAllCasesCursor declare
pageSize as double but immediately cast it to int when calling iterate; change
the pageSize parameter type to int (and its default value from 100.0 to 100) in
both method signatures (updateCasesCursor(Closure update, String
processIdentifier, int pageSize = 100) and updateAllCasesCursor(Closure update,
int pageSize = 100)), and remove the unnecessary "as int" casts when calling
iterate so iterate(update, DEFAULT_PROCESS_OPERATIONS, query, 0, pageSize) and
iterate(update, DEFAULT_PROCESS_OPERATIONS, new Query(), 0, pageSize) are used.
- Around line 193-200: The condition in changeDataFieldsValueFromNumberToText
incorrectly uses OR and a redundant truthy check; update the guard for
useCase.dataSet[dataFieldID].value to explicitly ensure it is neither null nor
empty by using && (e.g. if (useCase.dataSet[dataFieldID].value != null &&
useCase.dataSet[dataFieldID].value != "") ) or simply remove the initial truthy
check and use that && check; then safely cast the value to double and assign
back as String as currently done.
- Around line 161-175: In elasticIndex(Case useCase) replace the runtime-unsafe
"assert useCase.petriNet" with an explicit null-check after calling
PetriNetMigrationHelper.setPetriNet(useCase,
petriNetService.getNewestVersionByIdentifier(useCase.processIdentifier)); if the
petriNet is still null log a warn/info and skip indexing; when setting
useCase.lastModified and retrying
elasticCaseService.indexNow(elasticCaseMappingService.transform(useCase)) wrap
that retry in its own try-catch so any second failure is logged (use log.error
with the exception) instead of propagating, and change the initial corrective
log line to warn or info rather than error.
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovy`:
- Around line 239-248: The Collator is hardcoded to Slovak via new
Locale("sk","SK") in resolveDataOrder; change this to use a configurable or
neutral locale instead of a hardcoded one: either accept a Locale parameter on
resolveDataOrder or obtain the locale from a configuration/service (e.g.,
application settings or Locale.getDefault()/Locale.ROOT) and replace the
skCollator creation with Collator.getInstance(configuredLocale); ensure
petriNet.dataSet sorting logic (fields stream, skCollator usage) remains
identical but uses the injected/configured collator.
- Around line 116-122: The log messages in
PetriNetMigrationHelper.prepareOperations incorrectly say "case" instead of
"petri net"; update the two logging statements inside prepareOperations (the
calls referencing document.stringId and document.toString()) to use "petri net"
(e.g., "Updating petri net with ID ..." and "Updating petri net ...") so log
context correctly reflects the PetriNet document before calling update(document)
and bulkOperations.replaceOne(...).
- Around line 139-146: The call to importPetriNet().get() in
updateNetIgnoreRoles(String identifier, String fileName, ...) is unsafe and can
throw NoSuchElementException; update the code to handle the Optional properly by
replacing .get() with a safe alternative (e.g., use orElseThrow with a
descriptive IllegalStateException including identifier and fileName, or check
isPresent() and log/throw a clear error) so that failures to import the petri
net from getImporter().importPetriNet(...) produce a meaningful error message or
are handled gracefully before calling updateNetIgnoreRoles(currentNet,
reimported, ...).
- Around line 417-428: In updateRoleEvents(PetriNet existing, PetriNet
reimported) ensure you null-check the result of oldRoles.find for each newRole
before dereferencing: if role is null, skip updating/saving (or optionally log a
warning) instead of setting role.events and calling processRoleRepository.save;
only assign role.events and save when role != null to avoid the NPE.
- Around line 257-272: The two overloads of updateTransitionRoles need null
checks after lookups to avoid NPEs: after finding Transition trans via
net.transitions.values().find { it.importId == transitionId } in
updateTransitionRoles(PetriNet, String, ProcessRole, Map) ensure trans is not
null before accessing trans.roles and handle the missing-transition case (log
via a static logger or throw a clear exception); similarly, in
updateTransitionRoles(PetriNet, String, String, Map) check the ProcessRole found
by net.roles.values().find { it.importId == roleImportId } for null and handle
it before calling the other overload. Use a static logger reference if you want
to log from these static methods.
- Around line 292-306: The updateDataSet method currently constructs a File with
a hardcoded path ("src/main/resources/petriNets/" + fileName) which fails in
packaged JARs; change it to load the resource from the classpath (similar to
updateNetIgnoreRoles) using ClassPathResource or getResourceAsStream, pass the
resource stream or URL into getImporter().importPetriNet(...) instead of new
File(...), and ensure the stream is closed; update references inside
updateDataSet (and keep replaceUserFieldRoleReferences, getImporter(),
petriNetService.save(existing) logic unchanged) so the importer reads from the
classpath resource rather than the filesystem.
- Around line 129-132: PetriNetMigrationHelper.updateNetIgnoreRoles calls
userService.system.transformToLoggedUser() but userService is not declared; add
a UserService injection to PetriNetMigrationHelper (e.g., a private UserService
userService field injected via `@Autowired/`@Inject or constructor) or obtain the
logged user via an existing helper method in AbstractMigrationHelper if
preferred; ensure the new field name matches the call (userService) so
updateNetIgnoreRoles compiles and uses the injected UserService instance.
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/TaskMigrationHelper.groovy`:
- Around line 119-125: The log messages in prepareOperations (class
TaskMigrationHelper) incorrectly say "case" instead of "task"; update the calls
to log.debug and log.trace to reference "task" and use the Task identifiers
(e.g., log.debug("Updating task with ID ${document.stringId}") and
log.trace("Updating task ${document.toString()}")) so the logs correctly reflect
the entity being processed while leaving the rest of the method
(update(document) and
bulkOperations.replaceOne(Query.query(Criteria.where("_id").is(document.getObjectId())),
document)) unchanged.
- Around line 237-250: Remove the redundant roles clearing in
updateTaskPermissions: in the method updateTaskPermissions remove the call to
oldTask.getRoles().clear() before assigning new roles; simply fetch the Task
(oldTask) and Transition (newTransition) as done, then call
oldTask.setRoles(newTransition.roles) and continue with setNegativeViewRoles,
resolveViewRoles and taskService.save(oldTask) so the behavior is unchanged but
the unnecessary getRoles().clear() call is eliminated.
In
`@src/main/groovy/com/netgrif/application/engine/migration/MigrationHelper.groovy`:
- Around line 250-252: The createGlobalRole method in MigrationHelper is
forwarding the wrong variable name (uses undefined or incorrect 'event') to
petriNetMigrationHelper.createGlobalRole; update the call inside
createGlobalRole(String id, String title, Map<EventType, Event> events = [:]) to
pass the method parameter 'events' to
petriNetMigrationHelper.createGlobalRole(id, title, events) so the provided
events map is correctly forwarded.
In
`@src/test/groovy/com/netgrif/application/engine/migration/MigrationBenchmarkTest.groovy`:
- Around line 56-60: The test currently creates the migration report in
src/main/resources (variable report in MigrationBenchmarkTest), which mutates
source files; change it to write into a test output directory (e.g.,
"test-output" or build/test-results) instead, ensure the parent directory is
created if missing, and construct the File and FileWriter against that path
(replace creation of new File("src/main/resources/migration_report.txt") and
writer = new FileWriter(report) with a File pointing into the test-output folder
and creating dirs via report.getParentFile().mkdirs() before creating the
FileWriter).
- Around line 67-72: The test opens FileInputStreams for
petriNetService.importPetriNet (creating ImportPetriNetEventOutcome netV1Outcome
and netV2Outcome) but never closes them, which can leak file descriptors during
repeated runs; fix by wrapping each FileInputStream in a try-with-resources (or
Groovy withStream/withCloseable) so the stream passed into
petriNetService.importPetriNet is automatically closed after use (or assign to a
variable and explicitly close in finally), ensuring both the v1 and v2 streams
are closed.
- Around line 87-117: The benchmark labels are mismatched with the helpers
invoked: in migrateCasesWitLegacyCursor() (labelled "LEGACY MIGRATION HELPER")
you're calling migrationHelper.updateCasesCursor, and in
migrateCasesWithCursor() (labelled "NEW MIGRATION HELPER") you're calling
caseMigrationHelper.updateCasesCursor; swap the helper calls so the
legacy-labelled method uses caseMigrationHelper.updateCasesCursor(...) and the
new-labelled method uses migrationHelper.updateCasesCursor(...), keeping the
same updateCasePermissionsFromNet lambda and writer output.
🪄 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: 5f20c328-fecd-4258-ba93-e7e7e00c2736
📒 Files selected for processing (11)
src/main/groovy/com/netgrif/application/engine/migration/MigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/config/properties/MigrationConfigurationProperties.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/AbstractMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/TaskMigrationHelper.groovysrc/main/java/com/netgrif/application/engine/utils/MongodbUtils.javasrc/main/java/com/netgrif/application/engine/workflow/domain/DataField.javasrc/test/groovy/com/netgrif/application/engine/migration/MigrationBenchmarkTest.groovysrc/test/resources/nae_2432_v1.xmlsrc/test/resources/nae_2432_v2.xml
Optimized test setup to reduce case count from 10,000 to 100 for efficiency. Removed deprecated legacy migration code and file-writing logic. Enhanced assertions to validate updated data fields and task permissions post-migration.
Retoocs
requested changes
May 28, 2026
machacjozef
reviewed
May 28, 2026
| /** | ||
| * Service for managing PetriNet operations. | ||
| */ | ||
| private IPetriNetService petriNetService |
Member
There was a problem hiding this comment.
Maybe I would change this from private to protected in case something needs it later.
- removed unused data annotation from MigrationConfigurationProperties - Add Elasticsearch indexing helpers for cases and tasks with safer error handling - Use configured PetriNet references when indexing cases - Support task permission updates when migrating case permissions - Harden case data field migration utilities against null values and conversion failures - Add helper methods for task reloads, role assignment, and permission synchronization - Extend PetriNet migration utilities with custom update hooks and role event updates - Make migration helper dependencies immutable - Replace Lombok-based migration configuration properties with explicit accessors - Update migration tests for the enhanced helper behavior
- changed properties to protected
- Improve bulk write error logging in AbstractMigrationHelper
- Remove redundant trace log from `CaseMigrationHelper`
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/groovy/com/netgrif/application/engine/migration/config/properties/MigrationConfigurationProperties.groovy (1)
17-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake nested
pageSizebindable for Spring@ConfigurationProperties
CaseMigrationProperties,TaskMigrationProperties, andPetriNetMigrationPropertieseach defineprivate int pageSize = 100but only exposegetPageSize()(nosetPageSize(...)) at lines 19/28/37. With JavaBean-style@ConfigurationPropertiesbinding, Spring can’t write a nestedpageSizeproperty without a writable accessor, so externalnetgrif.migration.*.page-sizeoverrides may never apply and the default100will remain.💡 Proposed fix
static class CaseMigrationProperties { private int pageSize = 100 int getPageSize() { return pageSize } + + void setPageSize(int pageSize) { + this.pageSize = pageSize + } } static class TaskMigrationProperties { private int pageSize = 100 int getPageSize() { return pageSize } + + void setPageSize(int pageSize) { + this.pageSize = pageSize + } } static class PetriNetMigrationProperties { private int pageSize = 100 int getPageSize() { return pageSize } + + void setPageSize(int pageSize) { + this.pageSize = pageSize + } }🤖 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 `@src/main/groovy/com/netgrif/application/engine/migration/config/properties/MigrationConfigurationProperties.groovy` around lines 17 - 42, The nested migration property classes (CaseMigrationProperties, TaskMigrationProperties, PetriNetMigrationProperties) only expose getPageSize() so Spring `@ConfigurationProperties` cannot bind external netgrif.migration.*.page-size values; add a public setPageSize(int pageSize) setter to each class (alongside the existing getPageSize()) so the pageSize field becomes writable by the binder and external overrides are applied.src/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovy (1)
136-152:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClose the resource streams after import.
Lines 137, 148, and 310 open resource streams without closing them. Re-running these helpers leaks JAR/file handles and makes failures harder to diagnose.
♻️ Use `withCloseable` around the imports
void updateNetIgnoreRoles(String identifier, Resource resource, List<Closure<PetriNet>> customUpdates = null) { - PetriNet reimported = petriNetService.importPetriNet(resource.inputStream, VersionType.MAJOR, userService.getSystem().transformToLoggedUser()).getNet() + PetriNet reimported = resource.inputStream.withCloseable { inputStream -> + petriNetService.importPetriNet(inputStream, VersionType.MAJOR, userService.getSystem().transformToLoggedUser()).getNet() + } updateNetIgnoreRoles(petriNetService.getNewestVersionByIdentifier(identifier), reimported, customUpdates) } void updateNetIgnoreRoles(String identifier, String fileName, List<Closure<PetriNet>> customUpdates = null) { PetriNet currentNet = petriNetService.getNewestVersionByIdentifier(identifier) - InputStream inputStream = new ClassPathResource("petriNets/$fileName" as String).inputStream - ByteArrayOutputStream outputStream = new ByteArrayOutputStream() - IOUtils.copy(inputStream, outputStream) - PetriNet reimported = getImporter().importPetriNet(new ByteArrayInputStream(outputStream.toByteArray())) - .orElseThrow { new IllegalStateException("Failed to import Petri Net from file: $fileName") } + PetriNet reimported = new ClassPathResource("petriNets/$fileName" as String).inputStream.withCloseable { inputStream -> + ByteArrayOutputStream outputStream = new ByteArrayOutputStream() + IOUtils.copy(inputStream, outputStream) + getImporter().importPetriNet(new ByteArrayInputStream(outputStream.toByteArray())) + .orElseThrow { new IllegalStateException("Failed to import Petri Net from file: $fileName") } + } updateNetIgnoreRoles(currentNet, reimported, customUpdates) } void updateDataSet(String identifier, String fileName, Closure<PetriNet> customUpdate = null) { PetriNet existing = petriNetService.getNewestVersionByIdentifier(identifier) - InputStream inputStream = new ClassPathResource("petriNets/$fileName" as String).inputStream - PetriNet reimported = getImporter().importPetriNet(inputStream) - .orElseThrow { new IllegalStateException("Failed to import Petri Net from file: $fileName") } + PetriNet reimported = new ClassPathResource("petriNets/$fileName" as String).inputStream.withCloseable { inputStream -> + getImporter().importPetriNet(inputStream) + .orElseThrow { new IllegalStateException("Failed to import Petri Net from file: $fileName") } + }Also applies to: 308-312
🤖 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 `@src/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovy` around lines 136 - 152, The helper methods updateNetIgnoreRoles (both overloads) open InputStream and ByteArrayOutputStream resources (via resource.inputStream, new ClassPathResource(...).inputStream and new ByteArrayInputStream/ByteArrayOutputStream) without closing them; wrap these streams in withCloseable (or try/finally/try-with-resources) so the InputStream and ByteArrayOutputStream used to read/import the PetriNet are closed after import, and likewise ensure any stream created by getImporter().importPetriNet(...) is closed or consumed within a withCloseable block to prevent JAR/file handle leaks.src/main/groovy/com/netgrif/application/engine/migration/helpers/TaskMigrationHelper.groovy (1)
152-178:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
int pageSizefor task cursor helpers (remove silentdouble→inttruncation).
AbstractMigrationHelper.iterate(...)already takesint pageSizeand validates it, butTaskMigrationHelpercursor helpers acceptdoubleand then dopageSize as int, silently truncating values.- Align the task cursor API with
CaseMigrationHelperby changing bothTaskMigrationHelper.groovyand the forwarding methods inMigrationHelper.groovytoint pageSize(and update Javadoc/defaults), removing theas intcasts.Proposed change (TaskMigrationHelper cursor helpers)
-void updateTasksCursor(Closure update, String processIdentifier, double pageSize = 100.0) { +void updateTasksCursor(Closure update, String processIdentifier, int pageSize = 100) { String processId = petriNetService.getNewestVersionByIdentifier(processIdentifier).stringId Query query = new Query(Criteria.where("processId").is(processId)) - iterate(update, DEFAULT_PROCESS_OPERATIONS, query, 0, pageSize as int) + iterate(update, DEFAULT_PROCESS_OPERATIONS, query, 0, pageSize) } -void updateSpecificTasksCursor(Closure update, String processIdentifier, List<String> transitionIds, double pageSize = 100.0) { +void updateSpecificTasksCursor(Closure update, String processIdentifier, List<String> transitionIds, int pageSize = 100) { String processId = petriNetService.getNewestVersionByIdentifier(processIdentifier).stringId Query query = new Query(Criteria.where("processId").is(processId)) query.addCriteria(Criteria.where("transitionId").in(transitionIds)) - iterate(update, DEFAULT_PROCESS_OPERATIONS, query, 0, pageSize as int) + iterate(update, DEFAULT_PROCESS_OPERATIONS, query, 0, pageSize) } -void updateAllTasksCursor(Closure update, double pageSize = 100.0) { - iterate(update, DEFAULT_PROCESS_OPERATIONS, new Query(), 0, pageSize as int) +void updateAllTasksCursor(Closure update, int pageSize = 100) { + iterate(update, DEFAULT_PROCESS_OPERATIONS, new Query(), 0, pageSize) }🤖 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 `@src/main/groovy/com/netgrif/application/engine/migration/helpers/TaskMigrationHelper.groovy` around lines 152 - 178, Change the pageSize parameter types from double to int in TaskMigrationHelper methods updateTasksCursor, updateSpecificTasksCursor, and updateAllTasksCursor, remove the "pageSize as int" casts and update Javadocs/defaults from 100.0 to 100; also update the corresponding forwarding method signatures in MigrationHelper (those that call iterate/these task helpers) to accept int pageSize so the API matches AbstractMigrationHelper.iterate(int pageSize) and no silent double→int truncation occurs.
🤖 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
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy`:
- Around line 216-225: The helper currently assumes useCase.dataSet[dataFieldID]
exists (e.g., in changeDataFieldsValueFromTextToNumber) and will NPE on
mixed-version data; modify each transform routine to first retrieve DataField df
= useCase.dataSet[dataFieldID], check if df is null and if so skip that field
(optionally log a debug/warn referencing useCase.stringId and dataFieldID),
otherwise operate on df.value as before; apply this null-guard pattern to all
similar blocks that access useCase.dataSet[...] (the blocks around lines
referenced in the review).
- Line 16: Remove the unused JUnit4 import and harden CaseMigrationHelper by
deleting the import org.junit.Assert and adding null checks around DataField
lookups: in methods changeDataFieldsValueFromTextToNumber,
changeDataFieldsValueFromEnumerationToMultichoice, addChoices, removeChoices,
and changeFileFieldToFileList, guard the DataField retrieval (DataField
dataField = useCase.dataSet[dataFieldID]) with an existence check and skip or
log a warning when dataField is null before accessing dataField.value or
dataField.choices; this removes the test-only dependency and prevents NPEs
during migrations in the CaseMigrationHelper class.
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovy`:
- Around line 247-254: resolveDataOrder currently uses
Collator.getInstance(Locale.getDefault()), which makes sorting depend on host
JVM locale; change it to a stable locale (e.g.,
Collator.getInstance(Locale.ROOT) or Locale.ENGLISH) so petriNet.getDataSet
ordering is deterministic across environments, keep the existing comparator
logic in resolveDataOrder and ensure the collator instance (variable collator)
is created with the chosen fixed locale.
In
`@src/test/groovy/com/netgrif/application/engine/migration/MigrationTest.groovy`:
- Line 95: The test in MigrationTest.groovy asserts task order with it.tasks[0]
and it.tasks[1], which is brittle; change the assertion to verify the tasks
collection contains the expected transitions regardless of order by mapping
transitions and asserting size == 2 and that the set/list of transitions
contains both "person_info" and "recreate_person" (e.g., assert it.tasks.size()
== 2 && it.tasks*.transition.containsAll(["person_info","recreate_person"])).
- Around line 66-68: The test setup in MigrationTest uses
(1..10).stream().parallel().forEach which induces nondeterminism; change the
setup to create cases sequentially by removing .parallel() and iterating in
order (e.g., use a simple for loop or stream().forEach) when calling
workflowService.createCase(netV1.stringId, "Net V1 " + it, null,
superCreator.loggedSuper, Locale.default) inside the `@BeforeEach` so case
creation is deterministic and avoids exposing thread-safety issues.
- Around line 54-61: The test currently opens XMLs with filesystem paths (new
FileInputStream("src/test/resources/nae_2432_v1.xml") and v2) which is fragile;
change both imports to load resources from the classpath (e.g., use
this.getClass().getResourceAsStream("/nae_2432_v1.xml") and "/nae_2432_v2.xml")
and pass those InputStreams into petriNetService.importPetriNet so netV1Outcome
and netV2Outcome are obtained from classpath streams and ensure the streams are
closed (withCloseable or try-with-resources equivalent).
---
Outside diff comments:
In
`@src/main/groovy/com/netgrif/application/engine/migration/config/properties/MigrationConfigurationProperties.groovy`:
- Around line 17-42: The nested migration property classes
(CaseMigrationProperties, TaskMigrationProperties, PetriNetMigrationProperties)
only expose getPageSize() so Spring `@ConfigurationProperties` cannot bind
external netgrif.migration.*.page-size values; add a public setPageSize(int
pageSize) setter to each class (alongside the existing getPageSize()) so the
pageSize field becomes writable by the binder and external overrides are
applied.
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovy`:
- Around line 136-152: The helper methods updateNetIgnoreRoles (both overloads)
open InputStream and ByteArrayOutputStream resources (via resource.inputStream,
new ClassPathResource(...).inputStream and new
ByteArrayInputStream/ByteArrayOutputStream) without closing them; wrap these
streams in withCloseable (or try/finally/try-with-resources) so the InputStream
and ByteArrayOutputStream used to read/import the PetriNet are closed after
import, and likewise ensure any stream created by
getImporter().importPetriNet(...) is closed or consumed within a withCloseable
block to prevent JAR/file handle leaks.
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/TaskMigrationHelper.groovy`:
- Around line 152-178: Change the pageSize parameter types from double to int in
TaskMigrationHelper methods updateTasksCursor, updateSpecificTasksCursor, and
updateAllTasksCursor, remove the "pageSize as int" casts and update
Javadocs/defaults from 100.0 to 100; also update the corresponding forwarding
method signatures in MigrationHelper (those that call iterate/these task
helpers) to accept int pageSize so the API matches
AbstractMigrationHelper.iterate(int pageSize) and no silent double→int
truncation occurs.
🪄 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: 4acb3ad7-3915-4022-8e06-bd7ecc8a7944
📒 Files selected for processing (7)
src/main/groovy/com/netgrif/application/engine/migration/MigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/config/properties/MigrationConfigurationProperties.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/AbstractMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/TaskMigrationHelper.groovysrc/test/groovy/com/netgrif/application/engine/migration/MigrationTest.groovy
| import com.netgrif.application.engine.workflow.domain.DataField | ||
| import com.querydsl.core.types.Predicate | ||
| import groovy.util.logging.Slf4j | ||
| import org.junit.Assert |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== JUnit imports under main sources ==="
rg -n "import org\.junit|org\.junit\.Assert|Assert\." src/main/groovy src/main/java || true
echo
echo "=== Dependency declarations mentioning JUnit ==="
fd -HI '^pom\.xml$' . -x sh -c '
echo "--- $1 ---"
rg -n -C2 "junit|org\.junit" "$1" || true
' sh {}Repository: netgrif/application-engine
Length of output: 3335
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy"
echo "=== CaseMigrationHelper.groovy (imports + Assert usage) ==="
nl -ba "$FILE" | sed -n '1,120p'
echo
echo "--- occurrences of Assert in this file ---"
rg -n "\bAssert\b" "$FILE" || true
echo
echo "=== pom.xml: any JUnit deps not scoped to test ==="
rg -n "junit|org\.junit" pom.xml || true
echo
echo "=== pom.xml: exact junit dependency blocks with scope ==="
python3 - <<'PY'
import re, itertools
p=open("pom.xml","r",encoding="utf-8").read().splitlines()
for i,line in enumerate(p,1):
if re.search(r'org\.junit', line) or re.search(r'junit-jupiter', line):
start=max(1,i-6); end=min(len(p), i+10)
print(f"\n--- context around line {i} ---")
for j in range(start,end+1):
if j==i: pass
print(f"{j:4d}:{p[j-1]}")
PYRepository: netgrif/application-engine
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy"
echo "=== CaseMigrationHelper.groovy: top imports + Assert usage ==="
cat -n "$FILE" | sed -n '1,120p'
echo
echo "--- occurrences of 'org.junit.Assert' or 'Assert.' in this file ---"
rg -n "org\.junit\.Assert|Assert\." "$FILE" || true
echo
echo "--- occurrences of word boundary 'Assert' in this file ---"
rg -n "\bAssert\b" "$FILE" || true
echo
echo "=== pom.xml: exact junit dependency blocks with scope ==="
python3 - <<'PY'
import re
p=open("pom.xml","r",encoding="utf-8").read().splitlines()
# collect indices where junit artifacts appear
idx=[]
for i,line in enumerate(p):
if "org.junit" in line or "junit-jupiter" in line:
idx.append(i)
for i in idx:
start=max(0,i-6); end=min(len(p)-1,i+14)
print(f"\n--- context around pom.xml line {i+1} ---")
for j in range(start,end+1):
print(f"{j+1:4d}:{p[j]}")
PY
echo
echo "=== pom.xml: detect any org.junit/junit-jupiter deps not scoped to test ==="
python3 - <<'PY'
import re
from xml.etree import ElementTree as ET
# best-effort parse with namespace-stripping
text=open("pom.xml","r",encoding="utf-8").read()
text=re.sub(r'xmlns="[^"]+"', '', text, count=1) # crude remove default namespace
text=re.sub(r'xmlns:[^=]+="[^"]+"', '', text) # crude remove prefixed namespaces
root=ET.fromstring(text)
deps=[]
for dep in root.findall(".//dependency"):
gid=dep.findtext("groupId","").strip()
aid=dep.findtext("artifactId","").strip()
scope=dep.findtext("scope","").strip()
if "org.junit" in gid or aid.startswith("junit-") or aid.startswith("junit"):
deps.append((gid,aid,scope))
print("JUnit-related deps found:", len(deps))
for gid,aid,scope in deps:
print(f"- {gid}:{aid} scope={scope or '(none)'}")
non_test=[d for d in deps if d[2] not in ("test","")]
# Actually "none" means no scope => compile/runtime; treat as non-test
non_test=[d for d in deps if d[2] not in ("test")]
print("\nNon-test scoped (scope != test):", len(non_test))
for gid,aid,scope in non_test:
print(f"- {gid}:{aid} scope={scope or '(none)'}")
PYRepository: netgrif/application-engine
Length of output: 16704
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) pom.xml: find junit deps and whether scope is test (block-based text extraction)
echo "=== pom.xml: JUnit dependency blocks and their scope ==="
python3 - <<'PY'
import re
path="pom.xml"
lines=open(path,"r",encoding="utf-8").read().splitlines()
blocks=[]
start=None
for i,l in enumerate(lines):
if "<dependency>" in l:
start=i
if start is not None and "</dependency>" in l:
end=i
block="\n".join(lines[start:end+1])
if re.search(r"<groupId>\s*org\.junit\b", block) or re.search(r"<artifactId>\s*junit-", block) or re.search(r"org\.junit", block):
blocks.append(block)
start=None
print(f"Found {len(blocks)} dependency blocks containing org.junit/junit- artifacts.")
for idx,b in enumerate(blocks,1):
# extract groupId/artifactId/scope
def get(tag):
m=re.search(rf"<{tag}>\s*([^<]+)\s*</{tag}>", b)
return m.group(1).strip() if m else None
gid=get("groupId"); aid=get("artifactId"); scope=get("scope")
scope_disp=scope if scope is not None else "(none)"
print(f"\n--- block {idx} ---")
print(f"groupId={gid} artifactId={aid} scope={scope_disp}")
PY
echo
# 2) CaseMigrationHelper.groovy: find dataSet dereferences and DataField guards
FILE="src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy"
echo "=== CaseMigrationHelper.groovy: occurrences of useCase.dataSet / dataSet[...] ==="
rg -n "useCase\.dataSet|\bdataSet\[" "$FILE" || true
echo
echo "=== CaseMigrationHelper.groovy: occurrences of DataField lookups (dataSet + DataField) ==="
rg -n "DataField|dataSet\[" "$FILE" -C2 || true
echo
echo "=== CaseMigrationHelper.groovy: sample around each dataSet occurrence ==="
rg -n "useCase\.dataSet|\bdataSet\[" "$FILE" | while IFS=: read -r ln rest; do
echo "--- around line $ln ---"
sed -n "$((ln-10)),$((ln+10))p" "$FILE" | nl -ba -w2 -s": " | head -n 60
doneRepository: netgrif/application-engine
Length of output: 5112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== pom.xml: look for JUnit 4 / vintage / junit artifact that contains org.junit.Assert ==="
rg -n "junit4|vintage|artifactId>\s*junit\s*<|groupId>\s*org\.junit\s*<|org\.junit:" pom.xml || true
echo
echo "=== pom.xml: list all occurrences of org.junit/junit- with a bit of context ==="
rg -n "org\.junit|junit-jupiter|junit-platform-surefire-provider|junit-vintage" -C2 pom.xml || true
echo
echo "=== CaseMigrationHelper.groovy: inspect around dataSet dereference lines ==="
python3 - <<'PY'
import itertools
path="src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy"
lines=open(path,'r',encoding='utf-8').read().splitlines()
def show(start,end):
for i in range(start-1,end):
if 1<=i+1<=len(lines):
print(f"{i+1:4d}:{lines[i]}")
# ranges covering the dereference sites shown earlier
for s,e in [(175,260),(260,340),(340,360)]:
print(f"\n--- {s}-{e} ---")
show(s,e)
PYRepository: netgrif/application-engine
Length of output: 10654
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== pom.xml: any JUnit 4 artifact (org.junit:junit) or org.junit group beyond junit-jupiter ==="
rg -n "groupId>\s*org\.junit\b|artifactId>\s*junit\b|junit:junit" pom.xml || true
echo
echo "=== pom.xml: Groovy compilation plugin configuration ==="
rg -n "gmavenplus|groovy-maven-plugin|maven-groovy-plugin|kotlin-maven-plugin" pom.xml || true
echo
echo "=== pom.xml: any mention of test classpath / compile classpath mixing ==="
rg -n "testClasspath|compileClasspath|classpathScope|scope>test" pom.xml || trueRepository: netgrif/application-engine
Length of output: 1267
Remove org.junit.Assert from src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy
import org.junit.Assert(line 16) is unused and is JUnit4;pom.xmlonly declaresjunit-jupiter*underscope=test, so this production compile should not depend on test-only (and JUnit4) artifacts.- Migration helpers dereference
DataField dataField = useCase.dataSet[dataFieldID]and then usedataField.value/dataField.choiceswithout null guards (e.g.,changeDataFieldsValueFromTextToNumber,changeDataFieldsValueFromEnumerationToMultichoice,addChoices,removeChoices,changeFileFieldToFileList), which can abort migrations with missing field ids.
🤖 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
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy`
at line 16, Remove the unused JUnit4 import and harden CaseMigrationHelper by
deleting the import org.junit.Assert and adding null checks around DataField
lookups: in methods changeDataFieldsValueFromTextToNumber,
changeDataFieldsValueFromEnumerationToMultichoice, addChoices, removeChoices,
and changeFileFieldToFileList, guard the DataField retrieval (DataField
dataField = useCase.dataSet[dataFieldID]) with an existence check and skip or
log a warning when dataField is null before accessing dataField.value or
dataField.choices; this removes the test-only dependency and prevents NPEs
during migrations in the CaseMigrationHelper class.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy"
echo "=== JUnit import check ==="
rg -n "import org\.junit" "$FILE" || echo "(no org.junit imports found)"
echo
echo "=== Null guard check in relevant methods ==="
python3 - <<'PY'
path = "src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy"
lines = open(path, 'r', encoding='utf-8').read().splitlines()
methods = [
"changeDataFieldsValueFromTextToNumber",
"changeDataFieldsValueFromEnumerationToMultichoice",
"addChoices",
"removeChoices",
"changeFileFieldToFileList",
]
for method in methods:
for i, line in enumerate(lines):
if method in line and ("static" in line or "def " in line or "void " in line):
start = i
# Find method end (next static/def/void method or EOF)
end = len(lines)
for j in range(i+1, len(lines)):
if j < len(lines) and ("static " in lines[j] or ("def " in lines[j] and "static" not in lines[j-5:j])):
if ("void " in lines[j] or "def " in lines[j]) and not lines[j].strip().startswith("//"):
end = j
break
block = "\n".join(f"{start+k+1}: {lines[start+k]}" for k in range(min(end - start, 30)))
print(f"\n--- method: {method} (starting line {i+1}) ---")
print(block)
break
PYLength of output: 1559
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Fetch file content from the PR branch via GitHub API
gh api repos/netgrif/application-engine/contents/src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy \
--jq '.content' | base64 -d > /tmp/CaseMigrationHelper.groovy
echo "=== JUnit import check ==="
rg -n "import org\.junit" /tmp/CaseMigrationHelper.groovy || echo "(no org.junit imports found)"
echo
echo "=== Null guard check in relevant methods ==="
python3 - <<'PY'
path = "/tmp/CaseMigrationHelper.groovy"
lines = open(path, 'r', encoding='utf-8').read().splitlines()
methods_to_find = [
"changeDataFieldsValueFromTextToNumber",
"changeDataFieldsValueFromEnumerationToMultichoice",
"addChoices",
"removeChoices",
"changeFileFieldToFileList",
]
for method in methods_to_find:
for i, line in enumerate(lines):
if method in line and "static" in line:
# Print 25 lines starting from method definition
block = "\n".join(f"{i+k+1}: {lines[i+k]}" for k in range(min(25, len(lines)-i)))
print(f"\n--- method: {method} (line {i+1}) ---")
print(block)
break
PYLength of output: 1469
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the PR branch name
gh pr view 445 --repo netgrif/application-engine --json headRefName,headRepository
echo "---"
# Fetch from the PR branch
gh api "repos/netgrif/application-engine/contents/src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy?ref=NAE-2433" \
--jq '.content' | base64 -d > /tmp/CaseMigrationHelper.groovy 2>&1 && echo "File fetched OK" || echo "Fetch failed"
echo "=== JUnit import check ==="
rg -n "import org\.junit" /tmp/CaseMigrationHelper.groovy || echo "(no org.junit imports found)"
echo
echo "=== Null guard check in relevant methods ==="
python3 - <<'PY'
path = "/tmp/CaseMigrationHelper.groovy"
try:
lines = open(path, 'r', encoding='utf-8').read().splitlines()
except Exception as e:
print(f"Could not open file: {e}")
exit(1)
print(f"Total lines: {len(lines)}")
methods_to_find = [
"changeDataFieldsValueFromTextToNumber",
"changeDataFieldsValueFromEnumerationToMultichoice",
"addChoices",
"removeChoices",
"changeFileFieldToFileList",
]
for method in methods_to_find:
found = False
for i, line in enumerate(lines):
if method in line and "static" in line:
block = "\n".join(f"{i+k+1}: {lines[i+k]}" for k in range(min(25, len(lines)-i)))
print(f"\n--- method: {method} (line {i+1}) ---")
print(block)
found = True
break
if not found:
print(f"\n--- method: {method} NOT FOUND ---")
PYLength of output: 8781
@Retoocs, mostly yes — here's the breakdown:
| Issue | Status |
|---|---|
import org.junit.Assert removed |
✅ Fixed |
changeDataFieldsValueFromTextToNumber — safe nav dataField?.value |
✅ Fixed |
addChoices — if (!dataField) { return } guard added |
✅ Fixed |
removeChoices — if (!dataField) { return } guard added |
✅ Fixed |
changeFileFieldToFileList — if (!dataField) { return } guard added |
✅ Fixed |
changeDataFieldsValueFromEnumerationToMultichoice |
In changeDataFieldsValueFromEnumerationToMultichoice (line 261-262), dataField itself is not checked before accessing .value:
DataField dataField = useCase.dataSet[dataFieldID]
if (dataField.value && dataField.value != null) { // NPE if dataField is nullThe fix is straightforward — add the same existence check used in the other methods:
DataField dataField = useCase.dataSet[dataFieldID]
if (!dataField) {
return
}
if (dataField.value && dataField.value != null) {🧠 Learnings used
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 Groovy/Java, if a parameter or collection expects ActorFieldValue, you can pass UserFieldValue (a subclass) due to polymorphism. In tests, verify that ActorListFieldValue accepts ActorFieldValue instances and that passing UserFieldValue instances is valid without casts. This pattern applies to similar Groovy files across the repository.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
- Introduced debug and trace logs across `MigrationHelper`, `CaseMigrationHelper`, `TaskMigrationHelper`, and `PetriNetMigrationHelper`. - Logs include method parameters, processing state, and context to enhance debugging and analysis during migrations.
… imports and assertions
…ingMode and MigrationErrorException - Add shared migration error cache with get, pop, clear and has-error helpers - Support configurable error policies for continuing, failing immediately, failing after a limit, or failing after processing - Wire migration error policy handling into case, task and Petri net migration helpers - Add scoped MigrationHelper.withErrorPolicy support for migration scripts - Move migration page size and error policy settings into MigrationProperties - Remove obsolete MigrationConfigurationProperties - Improve helper constructors and page-size resolution to use unified migration properties - Add migration error handling quick usage documentation - Add migration test Petri nets and tests for case migration, Petri net update, and error handling policiesRefactor and enhance migration error handling: - Replaced `MigrationConfigurationProperties` with granular error-handling mechanisms using `MigrationErrorPolicy` and `MigrationErrorHandlingMode
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovy (1)
205-207:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle missing reimported role before dereferencing
importId.Both remapping blocks dereference
newRole.importIdwithout guardingnewRole. If a role ID exists in permissions/transition map but is absent inreimported.roles, this throws and aborts migration.Suggested patch
} else { + if (!newRole) { + log.warn("Role mapping missing for role id $id in reimported net ${reimported.identifier}") + return + } def oldRole = oldProcessRoles.values().find { it.importId == newRole.importId }} else { + if (!newRole) { + log.warn("Role mapping missing for transition role id $roleMongoId in reimported net ${reimported.identifier}") + return + } def oldRole = oldProcessRoles.values().find { it.importId == newRole.importId }Also applies to: 231-233
🤖 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 `@src/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovy` around lines 205 - 207, The code dereferences newRole.importId without checking newRole exists, which can throw if a role ID is present in permissions/transition maps but missing from reimported.roles; update both remapping blocks that use oldProcessRoles.values().find { it.importId == newRole.importId } (also the similar block near the other occurrence) to first null-check newRole (e.g., skip or continue when newRole is null) before accessing importId, or guard the find with safe navigation so the find call only runs when newRole != null and handle the missing-role case gracefully (e.g., log and skip remapping).
♻️ Duplicate comments (2)
src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy (2)
277-279:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard missing DataField before dereferencing value.
dataFieldcan be null for mixed-version cases; Line 278 dereferences it directly and can abort the batch with NPE.Suggested patch
toChange.each { dataFieldID -> DataField dataField = useCase.dataSet[dataFieldID] - if (dataField.value && dataField.value != null) { + if (dataField?.value != null) { def value if (dataField.value instanceof I18nString) {🤖 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 `@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy` around lines 277 - 279, The code dereferences dataField without guarding for null (DataField dataField = useCase.dataSet[dataFieldID]), which can throw an NPE for mixed-version cases; update the logic in CaseMigrationHelper so you first check whether dataField is non-null (e.g., if (dataField == null) continue or skip processing for that dataFieldID) before accessing dataField.value and before declaring/using value, ensuring the rest of the block only runs when dataField is present.
353-355:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate type before casting to
FileFieldValue.Line 354 casts any non-null value to
FileFieldValue. If the field is already migrated (or malformed), this throwsClassCastExceptionand interrupts migration.Suggested patch
def existingValue = dataField.value - if (existingValue != null) { + if (existingValue instanceof FileFieldValue) { fileListFieldValue.namesPaths.add(existingValue as FileFieldValue) }🤖 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 `@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy` around lines 353 - 355, The code casts existingValue to FileFieldValue without checking its type, risking ClassCastException during migrations; update the logic in CaseMigrationHelper to first check the runtime type (e.g., if (existingValue instanceof FileFieldValue)) before adding to fileListFieldValue.namesPaths, and for non-null but unexpected types handle gracefully (skip, convert if possible, or log a warning/error with identifying context) so the migration continues safely.
🤖 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 `@docs/migration/migration_error_handling_quick_usage.md`:
- Around line 1-92: Change the first heading to a top-level heading (use "#
Migration Error Handling — Quick Usage") to satisfy MD041, ensure there is a
single blank line before and after each heading and code fence to fix
MD022/MD031 (adjust spacing around all "###", "####" headings and all
triple-backtick blocks), and remove any extra trailing blank lines so the file
ends with exactly one newline to address MD047; check examples and helper lists
referencing MigrationErrorPolicy, migrationHelper, MigrationErrorException, and
methods like clearErrors(), withErrorPolicy(), popErrors(), getErrors(),
hasErrors() remain unchanged aside from spacing.
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/AbstractMigrationHelper.groovy`:
- Line 65: MIGRATION_ERRORS is a static shared CopyOnWriteArrayList on
AbstractMigrationHelper which creates global mutable state and can cause
cross-contamination between concurrent or sequential migrations; change the
design to use an instance-level (non-static) List<MigrationError> field on
AbstractMigrationHelper (or a ThreadLocal/List passed into the migration
methods) instead of the static MIGRATION_ERRORS, update all references
(including methods that add errors and clearErrors()) to use the instance field,
and remove or deprecate the static constant; if you cannot change the API, at
minimum update documentation on AbstractMigrationHelper.clearErrors() to require
callers to invoke it before each run and add synchronization or ThreadLocal
scoping to prevent cross-run leakage.
- Around line 148-152: popErrors() currently does a non-atomic copy-then-clear
on MIGRATION_ERRORS which can lose items if another thread adds between those
calls; change popErrors() to perform the copy-and-clear atomically (e.g.,
synchronize on the MIGRATION_ERRORS object or a dedicated lock) so that creating
the new ArrayList and clearing MIGRATION_ERRORS occur inside the same
synchronized block; update other methods that access MIGRATION_ERRORS
(reads/writes) to use the same lock for consistency.
- Around line 239-247: The catch in AbstractMigrationHelper around
processOperations currently calls cacheError(...) directly, bypassing
handleMigrationError(...) so bulk operation failures ignore the configured error
policy; change the catch to call handleMigrationError(this.class.simpleName,
"iterate", type, null, message, e) (or otherwise route through the existing
handleMigrationError method) so THROW_IMMEDIATELY / THROW_AFTER_LIMIT behaviors
are respected for processOperations failures.
- Around line 259-266: The outer catch in the try/catch/finally around the
cursor iteration can have its rethrown exception masked if
finishMigrationErrorPolicy(...) in the finally throws; modify the block so
finishMigrationErrorPolicy does not overwrite the original error: either wrap
the call to finishMigrationErrorPolicy(errorPolicy) in its own try/catch and
log/suppress any exception, or track whether the try block completed normally
(e.g., a boolean like iterationThrew) and only invoke finishMigrationErrorPolicy
when no exception was thrown; apply this change around the code that calls
finishMigrationErrorPolicy, and ensure handleMigrationError(...) and the
original exception thrown from the catch remain the primary propagated error.
- Line 19: Remove the unused import AtomicInteger from
AbstractMigrationHelper.groovy: delete the line importing
java.util.concurrent.atomic.AtomicInteger so the file no longer contains an
unused import and imports only classes actually referenced in the
AbstractMigrationHelper class.
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy`:
- Around line 189-191: In CaseMigrationHelper (around the block that logs
"Failed to index $useCase.stringId after setting lastModified"), change the call
to handleMigrationError to pass the retry exception (retryEx) as the cause
instead of the original exception variable (ex); locate the lines where
log.error(message, retryEx) and handleMigrationError(errorPolicy,
"elasticIndex", type, useCase.stringId, message, ex) are used and replace the
last argument with retryEx so the actual retry failure is propagated to
handleMigrationError/throwOriginal.
- Around line 174-178: Replace the direct cache-and-return when a PetriNet is
missing with the standard migration error handling: instead of calling
cacheError(...) and returning in CaseMigrationHelper (where you check if
(!useCase.petriNet)), call handleMigrationError(...) with the same message and
context (class simple name, "elasticIndex", type, useCase.stringId and the
constructed message) so the MigrationErrorPolicy is respected; ensure the method
signature/overload you call matches other usages of handleMigrationError in this
class so immediate-throw and other policies behave consistently.
In
`@src/main/groovy/com/netgrif/application/engine/migration/MigrationHelper.groovy`:
- Around line 570-574: collectErrors currently calls clearErrors() then
migrationCode.call(), but if migrationCode throws the error cache isn't
popped/cleared; update collectErrors to call clearErrors() first, then execute
migrationCode.call() inside a try/finally so that popErrors() is always invoked
in the finally block and its result returned; reference collectErrors,
clearErrors, migrationCode.call and popErrors when making the change.
In
`@src/main/groovy/com/netgrif/application/engine/migration/model/MigrationErrorPolicy.groovy`:
- Around line 91-95: The factory method MigrationErrorPolicy.throwAfterLimit
currently accepts non-positive maxErrors which makes THROW_AFTER_LIMIT semantics
ambiguous; validate that maxErrors > 0 and throw an IllegalArgumentException (or
similar) if not. Update MigrationErrorPolicy.throwAfterLimit (and the other
factory variant that sets mode to MigrationErrorHandlingMode.THROW_AFTER_LIMIT)
to check the maxErrors parameter before constructing the object, referencing the
maxErrors field and MigrationErrorHandlingMode.THROW_AFTER_LIMIT to locate the
change. Ensure the exception message clearly states that maxErrors must be a
positive integer.
- Around line 55-61: In MigrationErrorPolicy.defaultErrorPolicy, don't call
MigrationErrorHandlingMode.valueOf(props.mode) directly; instead normalize
props.mode (trim and toUpperCase) and validate it against
MigrationErrorHandlingMode.values() before using valueOf, and if invalid throw a
clear IllegalArgumentException that includes the bad value and the list of
allowed enum names; update the defaultErrorPolicy method to perform this
normalization/validation (referencing MigrationErrorHandlingMode and
MigrationProperties.ErrorPolicy) so misconfigured
nae.migration.error-policy.mode yields an actionable error message.
In
`@src/main/groovy/com/netgrif/application/engine/migration/throwable/MigrationErrorException.groovy`:
- Around line 26-29: The constructor MigrationErrorException currently assigns
this.errors = Collections.unmodifiableList(errors ?: []), which wraps the
caller's list and allows external mutations to affect the stored list; make a
defensive copy before wrapping to preserve immutability by creating a new
ArrayList from the incoming errors (or an empty list when null) and then
assigning Collections.unmodifiableList(copy) to this.errors so the exception
retains an immutable snapshot.
---
Outside diff comments:
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovy`:
- Around line 205-207: The code dereferences newRole.importId without checking
newRole exists, which can throw if a role ID is present in
permissions/transition maps but missing from reimported.roles; update both
remapping blocks that use oldProcessRoles.values().find { it.importId ==
newRole.importId } (also the similar block near the other occurrence) to first
null-check newRole (e.g., skip or continue when newRole is null) before
accessing importId, or guard the find with safe navigation so the find call only
runs when newRole != null and handle the missing-role case gracefully (e.g., log
and skip remapping).
---
Duplicate comments:
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy`:
- Around line 277-279: The code dereferences dataField without guarding for null
(DataField dataField = useCase.dataSet[dataFieldID]), which can throw an NPE for
mixed-version cases; update the logic in CaseMigrationHelper so you first check
whether dataField is non-null (e.g., if (dataField == null) continue or skip
processing for that dataFieldID) before accessing dataField.value and before
declaring/using value, ensuring the rest of the block only runs when dataField
is present.
- Around line 353-355: The code casts existingValue to FileFieldValue without
checking its type, risking ClassCastException during migrations; update the
logic in CaseMigrationHelper to first check the runtime type (e.g., if
(existingValue instanceof FileFieldValue)) before adding to
fileListFieldValue.namesPaths, and for non-null but unexpected types handle
gracefully (skip, convert if possible, or log a warning/error with identifying
context) so the migration continues safely.
🪄 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: 71c9710d-ab7a-4f37-9cad-85cca6ae0f77
📒 Files selected for processing (14)
docs/migration/migration_error_handling_quick_usage.mdsrc/main/groovy/com/netgrif/application/engine/migration/MigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/AbstractMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/TaskMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/model/MigrationError.groovysrc/main/groovy/com/netgrif/application/engine/migration/model/MigrationErrorHandlingMode.groovysrc/main/groovy/com/netgrif/application/engine/migration/model/MigrationErrorPolicy.groovysrc/main/groovy/com/netgrif/application/engine/migration/throwable/MigrationErrorException.groovysrc/main/java/com/netgrif/application/engine/configuration/properties/MigrationProperties.javasrc/test/groovy/com/netgrif/application/engine/migration/MigrationTest.groovysrc/test/resources/petriNets/nae_2432_v1.xmlsrc/test/resources/petriNets/nae_2432_v2.xml
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 (2)
src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy (2)
236-254:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winStatic method cannot call instance methods
defaultErrorPolicy()andhandleMigrationError().
changeDataFieldsValueFromTextToNumberis declaredstaticbut usesdefaultErrorPolicy()as a default parameter value (line 236) and callshandleMigrationError(...)(line 250). Both are instance methods inherited fromAbstractMigrationHelper. This will cause a compilation or runtime error.Either remove the
staticmodifier or remove the error policy integration from this method.🐛 Option 1: Remove static modifier
- static void changeDataFieldsValueFromTextToNumber(Case useCase, Set<String> toChange, MigrationErrorPolicy errorPolicy = defaultErrorPolicy()) { + void changeDataFieldsValueFromTextToNumber(Case useCase, Set<String> toChange, MigrationErrorPolicy errorPolicy = defaultErrorPolicy()) {🐛 Option 2: Remove error policy (keep static, just log)
- static void changeDataFieldsValueFromTextToNumber(Case useCase, Set<String> toChange, MigrationErrorPolicy errorPolicy = defaultErrorPolicy()) { + static void changeDataFieldsValueFromTextToNumber(Case useCase, Set<String> toChange) { log.debug("Starting changeDataFieldsValueFromTextToNumber for case: ${useCase.stringId}, fields to change: ${toChange}") toChange.each { dataFieldID -> DataField dataField = useCase.dataSet[dataFieldID] if (dataField?.value != null && dataField.value != "") { try { def originalValue = dataField.value dataField.value = dataField.value as double log.trace("Converted field ${dataFieldID} from text ${originalValue} to number in case: ${useCase.stringId}") } catch (Exception e) { def originalValue = dataField.value dataField.value = null String message = "[${useCase.stringId}] could not convert value ${originalValue} in field ${dataFieldID}" log.error(message, e) - handleMigrationError(errorPolicy, "changeDataFieldsValueFromTextToNumber", type, useCase.stringId, message, e) + // Error is logged above; caller can check case state if needed } } } }🤖 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 `@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy` around lines 236 - 254, The static method changeDataFieldsValueFromTextToNumber illegally references instance methods defaultErrorPolicy() and handleMigrationError(); either make the method non-static or remove/replace the instance-dependent error handling: to fix, change the method signature to non-static (remove static) so it can call defaultErrorPolicy() and handleMigrationError() as inherited from AbstractMigrationHelper, or if you must keep it static, remove the defaultErrorPolicy() parameter and replace the handleMigrationError(...) call with only logging (e.g., log.error with context) and avoid invoking instance methods; update references to the method accordingly.
274-292:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing null guard for
dataFieldbefore accessing.value.In
changeDataFieldsValueFromEnumerationToMultichoice, line 278 accessesdataField.valuewithout first checking ifdataFieldis null. IfuseCase.dataSet[dataFieldID]returns null (missing field), this will throw an NPE.A past review comment flagged this same issue, and it was reported as still outstanding.
🛡️ Proposed fix
static void changeDataFieldsValueFromEnumerationToMultichoice(Case useCase, Set<String> toChange) { log.debug("Starting changeDataFieldsValueFromEnumerationToMultichoice for case: ${useCase.stringId}, fields to change: ${toChange}") toChange.each { dataFieldID -> DataField dataField = useCase.dataSet[dataFieldID] + if (!dataField) { + return + } if (dataField.value && dataField.value != 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 `@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy` around lines 274 - 292, In changeDataFieldsValueFromEnumerationToMultichoice, guard against useCase.dataSet[dataFieldID] returning null by checking the retrieved DataField (variable dataField) before accessing dataField.value; if dataField is null, log a debug/warn message referencing dataFieldID and useCase.stringId and continue to the next ID, otherwise proceed with the existing conversion logic that builds the I18nString and HashSet and assigns it back to dataField.value.
🤖 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
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy`:
- Around line 236-254: The static method changeDataFieldsValueFromTextToNumber
illegally references instance methods defaultErrorPolicy() and
handleMigrationError(); either make the method non-static or remove/replace the
instance-dependent error handling: to fix, change the method signature to
non-static (remove static) so it can call defaultErrorPolicy() and
handleMigrationError() as inherited from AbstractMigrationHelper, or if you must
keep it static, remove the defaultErrorPolicy() parameter and replace the
handleMigrationError(...) call with only logging (e.g., log.error with context)
and avoid invoking instance methods; update references to the method
accordingly.
- Around line 274-292: In changeDataFieldsValueFromEnumerationToMultichoice,
guard against useCase.dataSet[dataFieldID] returning null by checking the
retrieved DataField (variable dataField) before accessing dataField.value; if
dataField is null, log a debug/warn message referencing dataFieldID and
useCase.stringId and continue to the next ID, otherwise proceed with the
existing conversion logic that builds the I18nString and HashSet and assigns it
back to dataField.value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 857dc56f-a693-4e34-ae57-9607e89e46fc
📒 Files selected for processing (7)
docs/migration/migration_error_handling_quick_usage.mdsrc/main/groovy/com/netgrif/application/engine/migration/MigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/AbstractMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/TaskMigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/model/MigrationErrorPolicy.groovysrc/main/groovy/com/netgrif/application/engine/migration/throwable/MigrationErrorException.groovy
…ll DataField values in `CaseMigrationHelper`.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy (1)
151-162: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRemove redundant
as intcasts.Since
pageSizeis already declared asintin both method signatures (lines 147 and 160), theas intcasts on lines 151 and 162 are unnecessary.🧹 Suggested cleanup
void updateCasesCursor(Closure update, String processIdentifier, int pageSize = 100, MigrationErrorPolicy errorPolicy = defaultErrorPolicy()) { log.debug("Starting updateCasesCursor for processIdentifier: ${processIdentifier}, pageSize: ${pageSize}") Query query = new Query(Criteria.where("processIdentifier").is(processIdentifier)) - iterate(update, null, query, 0, pageSize as int, errorPolicy) + iterate(update, null, query, 0, pageSize, errorPolicy) } ... void updateAllCasesCursor(Closure update, int pageSize = 100, MigrationErrorPolicy errorPolicy = defaultErrorPolicy()) { log.debug("Starting updateAllCasesCursor with pageSize: ${pageSize}") - iterate(update, null, new Query(), 0, pageSize as int, errorPolicy) + iterate(update, null, new Query(), 0, pageSize, errorPolicy) }🤖 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 `@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy` around lines 151 - 162, The calls to iterate(...) in updateAllCases and updateAllCasesCursor include unnecessary "as int" casts for the pageSize argument; remove the "as int" casts so the calls pass pageSize directly (e.g., change iterate(update, null, query, 0, pageSize as int, errorPolicy) to iterate(update, null, query, 0, pageSize, errorPolicy)), touching the methods updateAllCases and updateAllCasesCursor and the iterate invocation.
🤖 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
`@src/main/groovy/com/netgrif/application/engine/migration/MigrationHelper.groovy`:
- Around line 407-409: The method changeDataFieldsValueFromTextToNumber
currently delegates to caseMigrationHelper.changeDataFieldsValueFromTextToNumber
without forwarding the configured error policy, so calls via
withErrorPolicy(...) are ignored; update this method to pass
getCurrentErrorPolicy() to the delegate (i.e., call
caseMigrationHelper.changeDataFieldsValueFromTextToNumber(useCase, toChange,
getCurrentErrorPolicy())) consistent with other methods like
updateCases/elasticIndex/updateCasePermissionsFromNet.
---
Outside diff comments:
In
`@src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy`:
- Around line 151-162: The calls to iterate(...) in updateAllCases and
updateAllCasesCursor include unnecessary "as int" casts for the pageSize
argument; remove the "as int" casts so the calls pass pageSize directly (e.g.,
change iterate(update, null, query, 0, pageSize as int, errorPolicy) to
iterate(update, null, query, 0, pageSize, errorPolicy)), touching the methods
updateAllCases and updateAllCasesCursor and the iterate invocation.
🪄 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: ac237aa8-aa0e-4350-8906-202a209bc36f
📒 Files selected for processing (2)
src/main/groovy/com/netgrif/application/engine/migration/MigrationHelper.groovysrc/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovy
…er` and simplify `pageSize` casting in `CaseMigrationHelper`.
Member
Author
|
@coderabbitai is there anything I can do? Or could it be approved? |
|
Your plan includes 1 PR review per hour. Reviews are available now. |
Retoocs
approved these changes
May 29, 2026
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-2433
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 automatically and with unit tests.
Test Configuration
Checklist:
Summary by CodeRabbit