[NAE-2439] Implement new filter data types#446
Merged
Conversation
- introduce new filter fields - remove old filter field
- remove transactional in Importer - handle deprecated filter data type when importing process - remove use of deleted filter attributes
- implement FilterTest
- force component use for dedicated filter fields - fix cloning filter fields
WalkthroughThis PR decomposes a generic ChangesFilter Field Architecture Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
new feature
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/netgrif/application/engine/menu/domain/FilterBody.java (1)
18-23:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRemoving these fields also removes accessors that current callers still use.
This change drops the Lombok-generated
setAllowedNets(...),setIcon(...), andsetMetadata(...)methods, butActionDelegate.groovystill calls them in the provided context (for example around Lines 1619-1622, 2036-2039, and 2466-2469). Those filter/menu creation paths will now fail at runtime in Groovy with a missing-method error unless the callers are migrated in the same PR or compatibility setters are kept temporarily.🤖 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/java/com/netgrif/application/engine/menu/domain/FilterBody.java` around lines 18 - 23, The removal of fields from FilterBody removed Lombok-generated setters (setAllowedNets, setIcon, setMetadata) that ActionDelegate.groovy still invokes; restore binary compatibility by reintroducing those setters on FilterBody (either add back the removed fields with appropriate types or add explicit public setter methods named setAllowedNets, setIcon, setMetadata that accept the same parameter types as before) so ActionDelegate.groovy calls (around the spots that used those setters) continue to work without runtime missing-method errors.
🤖 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/java/com/netgrif/application/engine/importer/service/FieldFactory.java`:
- Around line 251-254: The code in FieldFactory currently maps the deprecated
FILTER token directly to buildCaseFilterField, which will misclassify legacy
filters; update the FILTER branch in the switch inside FieldFactory to inspect
the incoming legacy definition (e.g., a "filterType" discriminator or other
distinguishing attributes on the data object) and route to the correct builder
(e.g., buildCaseFilterField or the task/process-specific builder), and if the
discriminator is missing or ambiguous, fail fast by throwing a descriptive
exception rather than silently coercing; reference the FILTER and CASE_FILTER
enum values and the buildCaseFilterField method when implementing the
discriminator check and error path.
In `@src/main/java/com/netgrif/application/engine/importer/service/Importer.java`:
- Line 243: The method signature change to protected void
resolveUserRef(CaseActorRef userRef) and related uses reference types
(CaseActorRef, ActorRef) that don't exist in
com.netgrif.application.engine.importer.model in this branch; revert the
signature and any other changed method/type usages to the existing importer
model types (restore the original parameter/type declarations used before the
PR) or update imports to the regenerated importer model only if that regenerated
model is landed in this stack; specifically update resolveUserRef and any
methods referencing CaseActorRef/ActorRef to use the existing model classes
present in the branch (or postpone these signature changes until the new
importer model is merged).
In
`@src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovy`:
- Line 40: The test class FilterImportExportTest has been globally disabled with
`@Disabled` because it references removed DataField properties filterMetadata and
allowedNets; add a tracking reference so we don’t lose the TODO — insert a
concise TODO/JIRA comment next to the `@Disabled` annotation or at the top of the
class mentioning the ticket (e.g. TODO: RE-ENABLE after migrating to new filter
field architecture — JIRA-XXXX) and note which symbols need updating
(filterMetadata, allowedNets on DataField) so the future change and re-enabling
steps are clear.
In `@src/test/groovy/com/netgrif/application/engine/filters/FilterTest.groovy`:
- Line 61: The FileInputStream created for PROCESS_PATH when calling
petriNetService.importPetriNet(...) is never closed and may leak; wrap the
stream creation and the import call in a try-with-resources (or use Groovy's
withCloseable) so the FileInputStream is automatically closed after use.
Specifically, ensure the FileInputStream used to set PetriNet testProcess via
petriNetService.importPetriNet(new FileInputStream(PROCESS_PATH),
VersionType.MAJOR, superCreator.loggedSuper).getNet() is created inside a
try(resource) block (or passed through withCloseable) so the stream is closed
even on exceptions.
---
Outside diff comments:
In `@src/main/java/com/netgrif/application/engine/menu/domain/FilterBody.java`:
- Around line 18-23: The removal of fields from FilterBody removed
Lombok-generated setters (setAllowedNets, setIcon, setMetadata) that
ActionDelegate.groovy still invokes; restore binary compatibility by
reintroducing those setters on FilterBody (either add back the removed fields
with appropriate types or add explicit public setter methods named
setAllowedNets, setIcon, setMetadata that accept the same parameter types as
before) so ActionDelegate.groovy calls (around the spots that used those
setters) continue to work without runtime missing-method errors.
🪄 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: 9b66e756-f883-4739-bd3b-ff0706ec6d0e
📒 Files selected for processing (24)
src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/CaseFilterField.groovysrc/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FieldType.groovysrc/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FilterField.groovysrc/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/ProcessFilterField.groovysrc/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/TaskFilterField.groovysrc/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovysrc/main/java/com/netgrif/application/engine/importer/service/FieldFactory.javasrc/main/java/com/netgrif/application/engine/importer/service/Importer.javasrc/main/java/com/netgrif/application/engine/menu/domain/FilterBody.javasrc/main/java/com/netgrif/application/engine/menu/services/MenuItemService.javasrc/main/java/com/netgrif/application/engine/workflow/domain/Case.javasrc/main/java/com/netgrif/application/engine/workflow/domain/DataField.javasrc/main/java/com/netgrif/application/engine/workflow/service/DataService.javasrc/main/java/com/netgrif/application/engine/workflow/service/FilterImportExportService.javasrc/main/java/com/netgrif/application/engine/workflow/service/interfaces/IFilterImportExportService.javasrc/main/java/com/netgrif/application/engine/workflow/web/responsebodies/LocalisedCaseFilterField.javasrc/main/java/com/netgrif/application/engine/workflow/web/responsebodies/LocalisedFieldFactory.javasrc/main/java/com/netgrif/application/engine/workflow/web/responsebodies/LocalisedFilterField.javasrc/main/java/com/netgrif/application/engine/workflow/web/responsebodies/LocalisedProcessFilterField.javasrc/main/java/com/netgrif/application/engine/workflow/web/responsebodies/LocalisedTaskFilterField.javasrc/main/resources/petriNets/engine-processes/import_filters.xmlsrc/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovysrc/test/groovy/com/netgrif/application/engine/filters/FilterTest.groovysrc/test/resources/petriNets/filter_test.xml
💤 Files with no reviewable changes (8)
- src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FilterField.groovy
- src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IFilterImportExportService.java
- src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/LocalisedFilterField.java
- src/main/resources/petriNets/engine-processes/import_filters.xml
- src/main/java/com/netgrif/application/engine/menu/services/MenuItemService.java
- src/main/java/com/netgrif/application/engine/workflow/service/FilterImportExportService.java
- src/main/java/com/netgrif/application/engine/workflow/domain/DataField.java
- src/main/java/com/netgrif/application/engine/workflow/domain/Case.java
- close input stream - disable MenuItemApiTest.groovy
coderabbitai
Bot
added
the
improvement
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/test/groovy/com/netgrif/application/engine/filters/FilterTest.groovy (1)
61-63:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse exception-safe resource cleanup.
The explicit
close()call on line 63 won't execute if line 62 throws an exception, leaving the FileInputStream open. Wrap the resource in Groovy'swithCloseableto guarantee cleanup.🔒 Proposed fix using withCloseable
- InputStream inputStream = new FileInputStream(PROCESS_PATH) - PetriNet testProcess = petriNetService.importPetriNet(inputStream, VersionType.MAJOR, superCreator.loggedSuper).getNet() - inputStream.close() + PetriNet testProcess + new FileInputStream(PROCESS_PATH).withCloseable { inputStream -> + testProcess = petriNetService.importPetriNet(inputStream, VersionType.MAJOR, superCreator.loggedSuper).getNet() + }🤖 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/test/groovy/com/netgrif/application/engine/filters/FilterTest.groovy` around lines 61 - 63, The FileInputStream assigned to InputStream inputStream (opened from PROCESS_PATH) is closed explicitly but will leak if petriNetService.importPetriNet(...) throws; replace the manual open/close with Groovy's resource-safe pattern: create the FileInputStream and call withCloseable (or use try-with-resources) so the stream is passed into petriNetService.importPetriNet(INPUT_STREAM, VersionType.MAJOR, superCreator.loggedSuper).getNet() inside the withCloseable block, ensuring the InputStream is always closed even on exception.
🤖 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/test/groovy/com/netgrif/application/engine/action/MenuItemApiTest.groovy`:
- Line 38: Add a descriptive reason to the `@Disabled` annotation on the
MenuItemApiTest class explaining why it is disabled and whether that state is
temporary or permanent (e.g., "`@Disabled`(\"temporarily disabled: failing due to
new filter field types - needs migration\")"); locate the class MenuItemApiTest
and update the annotation accordingly, and if the intent is to re-enable later,
add a TODO comment referencing the required work (migrate tests to new filter
field types or remove) so maintainers know the next action.
---
Duplicate comments:
In `@src/test/groovy/com/netgrif/application/engine/filters/FilterTest.groovy`:
- Around line 61-63: The FileInputStream assigned to InputStream inputStream
(opened from PROCESS_PATH) is closed explicitly but will leak if
petriNetService.importPetriNet(...) throws; replace the manual open/close with
Groovy's resource-safe pattern: create the FileInputStream and call
withCloseable (or use try-with-resources) so the stream is passed into
petriNetService.importPetriNet(INPUT_STREAM, VersionType.MAJOR,
superCreator.loggedSuper).getNet() inside the withCloseable block, ensuring the
InputStream is always closed even on exception.
🪄 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: c7867d2e-ac6b-429c-b4c1-e7c34d157d46
📒 Files selected for processing (2)
src/test/groovy/com/netgrif/application/engine/action/MenuItemApiTest.groovysrc/test/groovy/com/netgrif/application/engine/filters/FilterTest.groovy
18 tasks
mazarijuraj
approved these changes
Jun 3, 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
2 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
Introduced new filter fields. Removed old
FilterField. New filter fields are more simpleImplements NAE-2439
Dependencies
No new dependencies were introduced
Third party dependencies
No new dependencies were introduced
Blocking Pull requests
Depends on petriflow/petriflow#30
How Has Been This Tested?
Manually and by unit test
FilterTest.groovy
Test Configuration
Checklist:
Summary by CodeRabbit
New Features
Improvements
Tests