[NAE-2197] Elastic Mapping Fixes #352
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add `StringCollectionField` and integrate it into ElasticCase mapping logic - Annotate additional fields in `ElasticCase` with proper Elasticsearch mappings - Fix initialization of `booleanValue` in `BooleanField` constructor
WalkthroughAdds Elastic support for string collection fields, including a new domain and adapter class, updates case mappings (title, ids, authorName), fixes BooleanField constructor assignment, and revises ElasticCaseService search to build explicit sorts with unmapped type and missing handling while bypassing previous unmapped-sort resolver. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant ElasticCaseService
participant QueryBuilder as NativeQueryBuilder
participant ES as Elasticsearch
Client->>ElasticCaseService: search(query, pageable)
activate ElasticCaseService
ElasticCaseService->>ElasticCaseService: buildQuery(query, pageable)
activate QueryBuilder
Note over ElasticCaseService,QueryBuilder: Build NativeQuery with query + PageRequest
ElasticCaseService->>QueryBuilder: For each sort order<br/>- field, asc/desc<br/>- unmappedType=Keyword<br/>- missing="_last"
deactivate QueryBuilder
ElasticCaseService->>ES: execute(query)
ES-->>ElasticCaseService: results
ElasticCaseService-->>Client: Page<ElasticCase>
deactivate ElasticCaseService
sequenceDiagram
autonumber
participant Mapper as ElasticCaseMappingService
participant DF as DataField
participant NetField as Net StringCollectionField
participant EDoc as Elastic Document
DF->>Mapper: transformDataField(DF, NetField)
alt NetField is StringCollectionField
Mapper->>Mapper: transformStringCollectionField(DF, NetField)
alt collection null/empty
Mapper-->>DF: Optional.empty()
else collection has values
Mapper-->>DF: Optional.of(Elastic StringCollectionField with String[])
Mapper->>EDoc: include field
end
else other field types
Mapper->>Mapper: existing transform paths
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/StringCollectionField.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/BooleanField.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java:16-16
Timestamp: 2025-08-19T20:13:40.087Z
Learning: In CaseField.java, fulltextValue is mapped as a keyword field type in Elasticsearch (for exact matches, filtering, aggregations), while the separate caseValue field serves different Elasticsearch query requirements, allowing the system to support multiple query patterns on the same data through different field mappings.
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: application-engine/src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovy:341-341
Timestamp: 2025-08-19T20:07:15.621Z
Learning: In the Elasticsearch migration, the stringId property was removed from ElasticCase and ElasticTask, with the id property now containing the MongoDB case/task object ID directly. This consolidation eliminates redundancy and simplifies the data model.
📚 Learning: 2025-08-19T20:13:40.087Z
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java:16-16
Timestamp: 2025-08-19T20:13:40.087Z
Learning: In CaseField.java, fulltextValue is mapped as a keyword field type in Elasticsearch (for exact matches, filtering, aggregations), while the separate caseValue field serves different Elasticsearch query requirements, allowing the system to support multiple query patterns on the same data through different field mappings.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.javanae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
📚 Learning: 2025-08-19T20:07:15.621Z
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: application-engine/src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovy:341-341
Timestamp: 2025-08-19T20:07:15.621Z
Learning: In the Elasticsearch migration, the stringId property was removed from ElasticCase and ElasticTask, with the id property now containing the MongoDB case/task object ID directly. This consolidation eliminates redundancy and simplifies the data model.
Applied to files:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
📚 Learning: 2025-08-19T20:07:43.748Z
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java:16-16
Timestamp: 2025-08-19T20:07:43.748Z
Learning: In CaseField.java, the separate caseValue field (List<String>) is intentionally maintained alongside fulltextValue for specific Elasticsearch query requirements, rather than being derived on-the-fly from fulltextValue.
Applied to files:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
🧬 Code graph analysis (2)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/StringCollectionField.java (1)
Data(8-19)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/BooleanField.java (1)
NoArgsConstructor(9-30)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/StringCollectionField.java (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java (1)
Data(11-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (3)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/BooleanField.java (1)
15-18: Constructor fix correctly initializes booleanValueAssigning the field after super(value) prevents null mapping/serialization issues. LGTM.
Please run an index round-trip test to confirm boolean fields now persist and read back correctly.
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java (1)
31-35: Add @field(type = Keyword) to id — OKKeyword mapping for id aligns with exact-match querying. LGTM.
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java (1)
86-88: Type added: StringCollectionField path recognized — OKBranching into a dedicated transformer keeps the mapper cohesive.
Comment on lines
+132
to
+138
| protected Optional<DataField> transformStringCollectionField(com.netgrif.application.engine.objects.workflow.domain.DataField dataField, com.netgrif.application.engine.objects.petrinet.domain.dataset.StringCollectionField netField) { | ||
| if (dataField.getValue() != null && dataField.getValue() instanceof Collection && !((Collection<?>) dataField.getValue()).isEmpty()) { | ||
| String[] values = ((Collection<?>) dataField.getValue()).toArray(new String[0]); | ||
| return Optional.of(new com.netgrif.application.engine.adapter.spring.elastic.domain.StringCollectionField(values)); | ||
| } | ||
| return Optional.empty(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Unsafe cast and null-element handling in transformStringCollectionField
Direct toArray(new String[0]) will throw if the collection contains non-String elements; nulls are not filtered. Make it type-safe and robust.
Apply this diff:
- if (dataField.getValue() != null && dataField.getValue() instanceof Collection && !((Collection<?>) dataField.getValue()).isEmpty()) {
- String[] values = ((Collection<?>) dataField.getValue()).toArray(new String[0]);
- return Optional.of(new com.netgrif.application.engine.adapter.spring.elastic.domain.StringCollectionField(values));
- }
- return Optional.empty();
+ if (!(dataField.getValue() instanceof Collection<?> c) || c.isEmpty()) {
+ return Optional.empty();
+ }
+ List<String> values = c.stream()
+ .filter(Objects::nonNull)
+ .filter(String.class::isInstance)
+ .map(String.class::cast)
+ .toList();
+ if (values.isEmpty()) {
+ return Optional.empty();
+ }
+ return Optional.of(new com.netgrif.application.engine.adapter.spring.elastic.domain.StringCollectionField(values.toArray(new String[0])));I can add unit tests covering: null/empty collections, mixed-type collections (ensure filtering), and successful mapping.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| protected Optional<DataField> transformStringCollectionField(com.netgrif.application.engine.objects.workflow.domain.DataField dataField, com.netgrif.application.engine.objects.petrinet.domain.dataset.StringCollectionField netField) { | |
| if (dataField.getValue() != null && dataField.getValue() instanceof Collection && !((Collection<?>) dataField.getValue()).isEmpty()) { | |
| String[] values = ((Collection<?>) dataField.getValue()).toArray(new String[0]); | |
| return Optional.of(new com.netgrif.application.engine.adapter.spring.elastic.domain.StringCollectionField(values)); | |
| } | |
| return Optional.empty(); | |
| } | |
| protected Optional<DataField> transformStringCollectionField( | |
| com.netgrif.application.engine.objects.workflow.domain.DataField dataField, | |
| com.netgrif.application.engine.objects.petrinet.domain.dataset.StringCollectionField netField) { | |
| // If value is not a non-empty Collection, bail out | |
| if (!(dataField.getValue() instanceof Collection<?> c) || c.isEmpty()) { | |
| return Optional.empty(); | |
| } | |
| // Filter out nulls and non-String instances | |
| List<String> values = c.stream() | |
| .filter(Objects::nonNull) | |
| .filter(String.class::isInstance) | |
| .map(String.class::cast) | |
| .toList(); | |
| // If no valid String elements remain, bail out | |
| if (values.isEmpty()) { | |
| return Optional.empty(); | |
| } | |
| // Build the elastic StringCollectionField from the filtered array | |
| return Optional.of( | |
| new com.netgrif.application.engine.adapter.spring.elastic.domain.StringCollectionField( | |
| values.toArray(new String[0]) | |
| ) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
around lines 132 to 138, the current transformStringCollectionField does an
unsafe cast using toArray(new String[0]) and doesn't handle nulls or non-String
elements; change it to iterate the collection safely, filter out nulls and
non-String elements, convert remaining elements to String (e.g., via
String.valueOf or cast after instanceof), collect into a String[] and only
return Optional.of(...) when the resulting array is non-empty; if the input is
null/empty or filtering yields no elements, return Optional.empty().
Comment on lines
+13
to
+18
| public String[] collectionValue; | ||
|
|
||
| public StringCollectionField(String[] values) { | ||
| super(values); | ||
| this.collectionValue = values; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Encapsulation of collectionValue
Public mutable array invites accidental external mutation. Prefer at least protected to limit exposure (the adapter uses an accessor anyway).
Apply this minimal change:
- public String[] collectionValue;
+ protected String[] collectionValue;If feasible, consider returning an unmodifiable copy in the getter to prevent aliasing.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public String[] collectionValue; | |
| public StringCollectionField(String[] values) { | |
| super(values); | |
| this.collectionValue = values; | |
| } | |
| protected String[] collectionValue; | |
| public StringCollectionField(String[] values) { | |
| super(values); | |
| this.collectionValue = values; | |
| } |
🤖 Prompt for AI Agents
In
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/StringCollectionField.java
around lines 13–18, the public mutable array collectionValue exposes internal
state; change its visibility to protected (or private) and update the
constructor to defensively copy the incoming String[] (e.g., Arrays.copyOf) to
avoid aliasing. Also adjust or add the getter to return a defensive copy or an
unmodifiable list/array to prevent external mutation, and update any callers
(the adapter) to use the accessor rather than direct field access.
.../src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
Outdated
Show resolved
Hide resolved
.../src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
Outdated
Show resolved
Hide resolved
...java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java
Outdated
Show resolved
Hide resolved
...java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java (1)
351-357: Javadoc now misleading; update to reflect default-realm behaviorDocs still say “from users in specified realms” but the method no longer accepts realm IDs and, per impl, operates on the default realm. Also, prefer parameter name petriNet for consistency.
Apply this diff to align the contract:
- /** - * Removes roles associated with a deleted Petri net from users in specified realms. - * - * @param process the deleted Petri net - */ - void removeRoleOfDeletedPetriNet(PetriNet process); + /** + * Removes all roles of the deleted Petri net from users in the default realm. + * + * @param petriNet the deleted Petri net + */ + void removeRoleOfDeletedPetriNet(PetriNet petriNet);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java(1 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java(2 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: application-engine/src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovy:341-341
Timestamp: 2025-08-19T20:07:15.621Z
Learning: In the Elasticsearch migration, the stringId property was removed from ElasticCase and ElasticTask, with the id property now containing the MongoDB case/task object ID directly. This consolidation eliminates redundancy and simplifies the data model.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: task-list-completed
🔇 Additional comments (1)
nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java (1)
19-19: LGTM: Set importImport addition is correct for the new overload.
| log.info("[" + processId + "]: Initiating deletion of Petri net " + petriNet.getIdentifier() + " version " + petriNet.getVersion().toString()); | ||
|
|
||
| userService.removeRoleOfDeletedPetriNet(petriNet, null); | ||
| userService.removeRoleOfDeletedPetriNet(petriNet); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Realm scope change: confirm intended behavior and tenant impact
Switching to userService.removeRoleOfDeletedPetriNet(petriNet) implies cleanup only in the default realm per the new impl. If multi-realm deployments are supported, this narrows scope versus the previous multi-realm variant and may leave orphaned roles in non-default realms.
If multi-realm cleanup is required, consider a service variant that iterates all realms or accepts an explicit realm set and use that here.
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java
around line 535, the call was changed to
userService.removeRoleOfDeletedPetriNet(petriNet) which only cleans up roles in
the default realm; verify whether multi-realm deployments must also have roles
removed. If multi-realm cleanup is required, replace this call with a
userService method that iterates all realms or accepts an explicit realm set
(e.g., removeRoleOfDeletedPetriNetAcrossRealms(petriNet, realms) or loop through
realm list and call existing removal per realm); if only default-realm cleanup
is intended, add an explicit comment and/or assert to document that behavior so
it’s clear non-default realms are intentionally unchanged.
Comment on lines
525
to
536
| public void removeRoleOfDeletedPetriNet(PetriNet petriNet) { | ||
| String defaultRealm = collectionNameProvider.getDefaultRealmCollection(); | ||
| Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize()); | ||
| Page<AbstractUser> users; | ||
| do { | ||
| users = findAllByProcessRoles(petriNet.getRoles().values().stream().map(ProcessRole::get_id).collect(Collectors.toSet()), defaultRealm, pageable); | ||
| users.forEach(u -> { | ||
| petriNet.getRoles().forEach((k, role) -> removeRole(u, role.get_id())); | ||
| }); | ||
| pageable = pageable.next(); | ||
| } while (users.hasNext()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Critical: passing collection name where realmId is expected + pagination skip bug
- defaultRealm is a collection name; findAllByProcessRoles expects a realmId and internally calls getCollectionNameForRealm(realmId). Passing a collection name yields a wrong collection lookup.
- Incrementing pageable while removing matches can skip users due to page reordering.
Fix both issues and batch-remove roles per user:
- public void removeRoleOfDeletedPetriNet(PetriNet petriNet) {
- String defaultRealm = collectionNameProvider.getDefaultRealmCollection();
- Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize());
- Page<AbstractUser> users;
- do {
- users = findAllByProcessRoles(petriNet.getRoles().values().stream().map(ProcessRole::get_id).collect(Collectors.toSet()), defaultRealm, pageable);
- users.forEach(u -> {
- petriNet.getRoles().forEach((k, role) -> removeRole(u, role.get_id()));
- });
- pageable = pageable.next();
- } while (users.hasNext());
- }
+ public void removeRoleOfDeletedPetriNet(PetriNet petriNet) {
+ String defaultCollection = collectionNameProvider.getDefaultRealmCollection();
+ String realmId = collectionNameProvider.getRealmIdFromCollectionName(defaultCollection);
+ Set<ProcessResourceId> roleIds = petriNet.getRoles().values()
+ .stream()
+ .map(ProcessRole::get_id)
+ .collect(Collectors.toSet());
+ Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize());
+ Page<AbstractUser> users;
+ do {
+ users = findAllByProcessRoles(roleIds, realmId, pageable);
+ users.forEach(u -> removeRolesById(u, roleIds));
+ // Reset to page 0 to avoid skipping due to deletions changing page boundaries
+ pageable = PageRequest.of(0, paginationProperties.getBackendPageSize());
+ } while (users.hasContent());
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public void removeRoleOfDeletedPetriNet(PetriNet petriNet) { | |
| String defaultRealm = collectionNameProvider.getDefaultRealmCollection(); | |
| Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize()); | |
| Page<AbstractUser> users; | |
| do { | |
| users = findAllByProcessRoles(petriNet.getRoles().values().stream().map(ProcessRole::get_id).collect(Collectors.toSet()), defaultRealm, pageable); | |
| users.forEach(u -> { | |
| petriNet.getRoles().forEach((k, role) -> removeRole(u, role.get_id())); | |
| }); | |
| pageable = pageable.next(); | |
| } while (users.hasNext()); | |
| } | |
| public void removeRoleOfDeletedPetriNet(PetriNet petriNet) { | |
| String defaultCollection = collectionNameProvider.getDefaultRealmCollection(); | |
| String realmId = collectionNameProvider.getRealmIdFromCollectionName(defaultCollection); | |
| Set<ProcessResourceId> roleIds = petriNet.getRoles().values() | |
| .stream() | |
| .map(ProcessRole::get_id) | |
| .collect(Collectors.toSet()); | |
| Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize()); | |
| Page<AbstractUser> users; | |
| do { | |
| users = findAllByProcessRoles(roleIds, realmId, pageable); | |
| users.forEach(u -> removeRolesById(u, roleIds)); | |
| // Reset to page 0 to avoid skipping due to deletions changing page boundaries | |
| pageable = PageRequest.of(0, paginationProperties.getBackendPageSize()); | |
| } while (users.hasContent()); | |
| } |
🤖 Prompt for AI Agents
In
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java
around lines 525-536, the code currently passes a collection name into
findAllByProcessRoles and advances pages which can skip results; fix by using
the real realmId instead of collectionNameProvider.getDefaultRealmCollection()
(use whatever method returns the default realm id, e.g.
collectionNameProvider.getDefaultRealmId() or resolve the realm from the
collection via collectionNameProvider.getRealmForCollection(...)) so
findAllByProcessRoles receives a realmId, and avoid pageable.next() reordering
issues by not advancing pages while matching/removing roles — either collect all
matching user ids up front in safe-sized batches or repeatedly fetch page 0
until no more matches; for each user, compute the set of role ids to remove
(intersection of petriNet.getRoles() ids and user roles) and call the batch
role-removal API (e.g. removeRoles(user, roleIds)) rather than removing roles
one-by-one.
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java
Show resolved
Hide resolved
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java
Show resolved
Hide resolved
nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java
Show resolved
Hide resolved
machacjozef
changed the base branch from
release/7.0.0-rev7
to
release/7.0.0-rev6.1
machacjozef
changed the base branch from
release/7.0.0-rev6.1
to
release/7.0.0-rev7
- Updated `@Field` annotations in `StringCollectionField` and `ElasticCase` to improve clarity and consistency. - Removed redundant imports and fixed indentation for better code readability. - Ensured alignment with updated Elastic `FieldType` usage.
tuplle
previously approved these changes
Sep 5, 2025
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java (1)
26-30: Correct: collectionValue as Keyword (not Nested).
This resolves the earlier nested/field-shadowing issue.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java (1)
35-38: Consider multi-fields for title/visualId if full-text search is needed.
If only exact filters/aggregations are used, keep as Keyword. Otherwise switch to text+keyword subfield.Apply if needed:
- @Field(type = Keyword) - public String getTitle() { + @MultiField(mainField = @Field(type = Text), + otherFields = @InnerField(suffix = "keyword", type = Keyword)) + public String getTitle() { return super.getTitle(); } - @Field(type = Keyword) - public String getVisualId() { + @MultiField(mainField = @Field(type = Text), + otherFields = @InnerField(suffix = "keyword", type = Keyword)) + public String getVisualId() { return super.getVisualId(); }Also applies to: 40-44
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java(3 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-19T20:13:40.087Z
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java:16-16
Timestamp: 2025-08-19T20:13:40.087Z
Learning: In CaseField.java, fulltextValue is mapped as a keyword field type in Elasticsearch (for exact matches, filtering, aggregations), while the separate caseValue field serves different Elasticsearch query requirements, allowing the system to support multiple query patterns on the same data through different field mappings.
Applied to files:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.javanae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
📚 Learning: 2025-08-19T20:07:15.621Z
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: application-engine/src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovy:341-341
Timestamp: 2025-08-19T20:07:15.621Z
Learning: In the Elasticsearch migration, the stringId property was removed from ElasticCase and ElasticTask, with the id property now containing the MongoDB case/task object ID directly. This consolidation eliminates redundancy and simplifies the data model.
Applied to files:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
🧬 Code graph analysis (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java (3)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/StringCollectionField.java (1)
Data(8-19)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java (1)
NoArgsConstructor(17-139)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/BooleanField.java (1)
NoArgsConstructor(9-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (7)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/StringCollectionField.java (3)
11-14: Good: adapter maps via accessors, no field shadowing.
Class setup with Lombok + callSuper is appropriate.
16-18: Constructor correctly delegates without duplicating state.
Avoids reassigningcollectionValuelocally. LGTM.
20-24: fulltextValue → Text mapping looks consistent with other fields.
Keeps analyzed search behavior. LGTM.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java (4)
15-15: Static wildcard import is fine here.
Consistent with other mappings in this class.
30-33: Id as Keyword is appropriate.
No concerns.
45-49: Duplicate of id as caseId — verify necessity.
Confirm consumers require both; dual fields increase storage and mapping surface.
65-66: Date mapping with millis format looks correct.
Matches LocalDateTime usage.
Comment on lines
+26
to
+30
| @Override | ||
| @Field(type = Keyword) | ||
| public String[] getCollectionValue() { | ||
| return super.getCollectionValue(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Plan reindex if changing existing mappings.
If any existing index used nested for this field, switch via alias → build new index → reindex → alias swap.
# Conflicts: # application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java
- Modified the `findFilter` method to use `title` instead of `title.keyword` in the filter query string.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
...vy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
Outdated
Show resolved
Hide resolved
- Updated `ElasticCase` class to enhance field mapping using `@MultiField` for `title` and `authorName`. - Enabled support for both `Text` and `Keyword` types by defining main and inner fields.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java (4)
13-13: Static FieldType import consistency — goodUsing
import static ...FieldType.*;keeps the annotations concise and consistent with prior style feedback.
33-40: Title multi-field mapping — LGTMText +
keywordsubfield addresses full-text search and exact filters/sorts.
42-45: VisualId: confirm exact-match-only; consider multi-field if search/sort needsIf UI/queries ever require full-text search or case-insensitive sorting on
visualId, mirror thetitlemapping. Otherwise keep asKeyword.Optional diff:
- @Field(type = Keyword) - public String getVisualId() { + @MultiField( + mainField = @Field(type = Text), + otherFields = { @InnerField(suffix = "keyword", type = Keyword) } + ) + public String getVisualId() {
47-50: caseId duplicates id; plan compatibility and eventual cleanupPer prior learnings,
stringIdwas removed andidholds the Mongo ObjectId. Re-introducingcaseId(mirror ofid) is likely for compatibility. Make the migration plan explicit to avoid long-term duplication.I used retrieved learnings about id/stringId consolidation when assessing this.
Optional diffs:
- Auto-populate
caseIdfromidon index (keeps writers simple):- @Field(type = Keyword) + @Field(type = Keyword, copyTo = {"caseId"}) public String getId() { return super.getId(); }
- Mark
caseIdas transitional:- @Field(type = Keyword) + @Deprecated(since = "7.0.0-rev7", forRemoval = true) + @Field(type = Keyword) public String getCaseId() { return super.getId(); }Please confirm whether
caseIdis needed beyond migration; if not, add a deprecation note and removal ticket.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-19T20:13:40.087Z
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java:16-16
Timestamp: 2025-08-19T20:13:40.087Z
Learning: In CaseField.java, fulltextValue is mapped as a keyword field type in Elasticsearch (for exact matches, filtering, aggregations), while the separate caseValue field serves different Elasticsearch query requirements, allowing the system to support multiple query patterns on the same data through different field mappings.
Applied to files:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
📚 Learning: 2025-08-19T20:07:15.621Z
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: application-engine/src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovy:341-341
Timestamp: 2025-08-19T20:07:15.621Z
Learning: In the Elasticsearch migration, the stringId property was removed from ElasticCase and ElasticTask, with the id property now containing the MongoDB case/task object ID directly. This consolidation eliminates redundancy and simplifies the data model.
Applied to files:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (2)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java (2)
28-31: Be explicit about dual-mapping id (_id + source field).Annotating
getId()with@Field(type = Keyword)stores the id both as Elasticsearch_idand as akeywordfieldid. This is fine if you need aggregations/filters onid, but it increases index size and can confuse consumers.Please confirm:
- Consumers expect both
_idandidfields.- Index templates and dashboards use
idfor terms/sorts; otherwise, consider dropping the@Fieldto save storage.
1-146: Manual verification required for Elasticsearch field usage
- The initial grep for
caseId,id, andvisualIdreturned excessive noise due to generic matches; please refine your search (e.g., target query/DSL builder code, Spring Data repositories, or Elasticsearch templates) or manually inspect relevant repository and query methods to confirm that the new mappings (KeywordvsTextand.keywordsub-fields) align with existing usage.- Ensure no consumers rely on full-text search on
visualIdor expect inverted-index behavior forcaseId/id, and verify that any.keywordreferences (e.g., in sorting or aggregations) correspond to the newly defined sub-fields.
Comment on lines
+28
to
31
| @Field(type = Keyword) | ||
| public String getId() { | ||
| return super.getId(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mapping changes: plan safe rollout (templates, reindex, aliases)
- Update index templates to reflect new/changed mappings.
- Create a new index (vN+1),
_reindexfrom old, then atomically switch the write/read alias. - Backfill and verify dashboards/queries for
idvscaseId, andtitle/authorName.keywordusage.
Also applies to: 33-40, 42-45, 47-50, 67-70, 82-89
🤖 Prompt for AI Agents
In
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
around lines 28-31 (and similarly for ranges 33-40, 42-45, 47-50, 67-70, 82-89),
the mapping change (e.g., annotating getters with @Field(type = Keyword))
requires a safe rollout: update your Elasticsearch index templates to include
the new mappings, create a new index version (vN+1) with the updated template,
reindex data from the old index into the new one, atomically switch the
application read/write alias to the new index, and then verify/backfill
dashboards and queries to use the correct field names (`id` vs `caseId`,
`title`/`authorName` and their `.keyword` variants) so consumers are not broken
by the mapping change.
Comment on lines
+67
to
70
| @Field(type = Date, format = DateFormat.date_hour_minute_second_millis) | ||
| public LocalDateTime getCreationDate() { | ||
| return super.getCreationDate(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Date format change: ensure backward compatibility
Switching to DateFormat.date_hour_minute_second_millis may break reads/writes if existing indices stored creationDate with date_optional_time. Support both formats if mixed data exists or clients send varying precisions.
Suggested change:
- @Field(type = Date, format = DateFormat.date_hour_minute_second_millis)
+ @Field(type = Date, format = {
+ DateFormat.date_hour_minute_second_millis,
+ DateFormat.date_optional_time
+ })
public LocalDateTime getCreationDate() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Field(type = Date, format = DateFormat.date_hour_minute_second_millis) | |
| public LocalDateTime getCreationDate() { | |
| return super.getCreationDate(); | |
| } | |
| @Field(type = Date, format = { | |
| DateFormat.date_hour_minute_second_millis, | |
| DateFormat.date_optional_time | |
| }) | |
| public LocalDateTime getCreationDate() { | |
| return super.getCreationDate(); | |
| } |
🤖 Prompt for AI Agents
In
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
around lines 67-70, the @Field annotation currently forces
DateFormat.date_hour_minute_second_millis which can break compatibility with
existing indices using date_optional_time; change the mapping to accept both
formats by switching to DateFormat.custom and providing a pattern that includes
both formats separated by "||" (i.e., include the millisecond ISO pattern and
the strict_date_optional_time/date_optional_time variants) so reads/writes
tolerate mixed precisions and legacy values.
Comment on lines
+82
to
89
| @MultiField( | ||
| mainField = @Field(type = Text), | ||
| otherFields = { | ||
| @InnerField(suffix = "keyword", type = Keyword) | ||
| }) | ||
| public String getAuthorName() { | ||
| return super.getAuthorName(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
AuthorName multi-field mapping — LGTM
Covers full-text search and exact filters/sorts. If you need case-insensitive sorting, consider a normalizer-backed keyword subfield in the index settings.
🤖 Prompt for AI Agents
In
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
around lines 82 to 89, the current MultiField mapping on getAuthorName()
provides a Text main field and a keyword subfield but does not support
case-insensitive sorting; if you need case-insensitive sorts, add an additional
InnerField (e.g., suffix "keyword_normalized") of type Keyword that uses a
normalizer, and update the Elasticsearch index settings to define that
normalizer (lowercase filter) so the new keyword subfield can be used for
case-insensitive exact matching and sorting.
- Updated the `findFilter` method to use `title.keyword` instead of `title` in the filter query string to align with changes in ElasticSearch mapping for enhanced precision.
- Updated the `buildQuery` method to enhance query creation: - Added explicit handling for unmapped fields by setting `FieldType.Keyword`. - Included fallback handling for missing values using `_last`.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: application-engine/src/test/groovy/com/netgrif/application/engine/filters/FilterImportExportTest.groovy:341-341
Timestamp: 2025-08-19T20:07:15.621Z
Learning: In the Elasticsearch migration, the stringId property was removed from ElasticCase and ElasticTask, with the id property now containing the MongoDB case/task object ID directly. This consolidation eliminates redundancy and simplifies the data model.
📚 Learning: 2025-08-19T20:13:40.087Z
Learnt from: renczesstefan
PR: netgrif/application-engine#339
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java:16-16
Timestamp: 2025-08-19T20:13:40.087Z
Learning: In CaseField.java, fulltextValue is mapped as a keyword field type in Elasticsearch (for exact matches, filtering, aggregations), while the separate caseValue field serves different Elasticsearch query requirements, allowing the system to support multiple query patterns on the same data through different field mappings.
Applied to files:
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (3)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java (3)
4-4: Import for FieldType is correct.Needed for
.unmappedType(...)on sort; no issues.
194-197: Pageable sort intentionally dropped; confirm callers don’t rely on Page sort metadata.You build ES sorts explicitly and pass an unsorted
PageRequestto the query. Results are fine, but returnedPagemetadata (fromSearchHitSupport.searchPageFor(hits, query.getPageable())) won’t reflect sort. Verify UI/clients don’t read sort fromPageable.
392-395: Drop.keywordsuffix forfulltextValuequeries
Term queries must targetdataSet.<key>.fulltextValue, asfulltextValueis already mapped as a keyword field.- dataQuery.must(termQuery("dataSet." + field.getKey() + ".fulltextValue.keyword", field.getValue())._toQuery()); + dataQuery.must(termQuery("dataSet." + field.getKey() + ".fulltextValue", field.getValue())._toQuery());
...-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
Show resolved
Hide resolved
Comment on lines
+198
to
+207
| for (org.springframework.data.domain.Sort.Order o : pageable.getSort()) { | ||
| builder.withSort(s -> s.field(f -> f | ||
| .field(o.getProperty()) | ||
| .order(o.isAscending() | ||
| ? co.elastic.clients.elasticsearch._types.SortOrder.Asc | ||
| : co.elastic.clients.elasticsearch._types.SortOrder.Desc) | ||
| .unmappedType(FieldType.Keyword) | ||
| .missing("_last") | ||
| )); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coding unmappedType=Keyword can break date/numeric sorts (e.g., creationDate).
Elastic expects unmapped_type to match the field’s actual mapping. Using Keyword for dates/numbers can yield incorrect ordering or errors. Also handle _score via score sort, not field sort.
Apply this diff to select unmappedType per field and support _score:
- for (org.springframework.data.domain.Sort.Order o : pageable.getSort()) {
- builder.withSort(s -> s.field(f -> f
- .field(o.getProperty())
- .order(o.isAscending()
- ? co.elastic.clients.elasticsearch._types.SortOrder.Asc
- : co.elastic.clients.elasticsearch._types.SortOrder.Desc)
- .unmappedType(FieldType.Keyword)
- .missing("_last")
- ));
- }
+ for (org.springframework.data.domain.Sort.Order o : pageable.getSort()) {
+ String prop = o.getProperty();
+ if ("_score".equals(prop)) {
+ builder.withSort(s -> s.score(sc -> sc.order(o.isAscending()
+ ? co.elastic.clients.elasticsearch._types.SortOrder.Asc
+ : co.elastic.clients.elasticsearch._types.SortOrder.Desc)));
+ continue;
+ }
+ builder.withSort(s -> s.field(f -> f
+ .field(prop)
+ .order(o.isAscending()
+ ? co.elastic.clients.elasticsearch._types.SortOrder.Asc
+ : co.elastic.clients.elasticsearch._types.SortOrder.Desc)
+ .unmappedType(resolveSortUnmappedType(prop))
+ .missing("_last")));
+ }Then add this helper in the class:
// Heuristic mapping; extend as needed to cover your mapped fields
private FieldType resolveSortUnmappedType(String property) {
if (property.endsWith(".keyword")) return FieldType.Keyword;
if ("creationDate".equals(property) || property.endsWith("Date")) return FieldType.Date;
if ("priority".equals(property) || property.endsWith(".priority")) return FieldType.Long;
if (property.equals("_id") || property.endsWith("Id") || property.endsWith("Name")) return FieldType.Keyword;
return FieldType.Keyword; // safe default for string-ish fields
}🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
around lines 198 to 207, the code hard-codes unmappedType=Keyword for all sorts
and doesn't handle score sorts; change the sort builder to pick an unmappedType
based on the field property (use a helper that returns FieldType.Date for
date-like fields, FieldType.Long for numeric/priority, FieldType.Keyword for
ids/names/strings, etc.) and special-case "_score" to emit a score sort rather
than a field sort; also add the suggested resolveSortUnmappedType(String) helper
method in the class and call it when building the sort to supply the correct
unmappedType per field.
tuplle
approved these changes
Sep 6, 2025
machacjozef
approved these changes
Sep 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
StringCollectionFieldand integrate it into ElasticCase mapping logicElasticCasewith proper Elasticsearch mappingsbooleanValueinBooleanFieldconstructorFixes [NAE-2197]
Dependencies
None
Third party dependencies
None
Blocking Pull requests
Merge after #351
How Has Been This Tested?
User hands on test.
Test Configuration
Checklist:
Summary by CodeRabbit
New Features
Refactor
Bug Fixes