Skip to content

Conversation

@renczesstefan
Copy link
Member

@renczesstefan renczesstefan commented Dec 9, 2025

Description

Fixes NAE-2282

Dependencies

No new dependencies were introduced

Third party dependencies

No new dependencies were introduced

Blocking Pull requests

There are no dependencies on other PR

How Has Been This Tested?

This was tested manually and with unit tests.

Test Configuration

Name Tested on
OS macOS Tahoe 26.0.1
Runtime Java 21
Dependency Manager Maven 3.9.9n
Framework version Spring Boot 3.4.4
Run parameters
Other configuration

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes have been checked, personally or remotely, with @Retoocs
  • I have commented my code, particularly in hard-to-understand areas
  • I have resolved all conflicts with the target branch of the PR
  • I have updated and synced my code with the target branch
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes:
    • Lint test
    • Unit tests
    • Integration tests
  • I have checked my contribution with code analysis tools:
  • I have made corresponding changes to the documentation:
    • Developer documentation
    • User Guides
    • Migration Guides

Introduced the TaskField class to handle task references in elastic mappings. Updated ElasticCaseMappingService to transform TaskField objects and integrated this functionality into the application engine. This enhancement ensures proper indexing and retrieval of task-related data in Elasticsearch.
@coderabbitai
Copy link

coderabbitai bot commented Dec 9, 2025

Warning

Rate limit exceeded

@renczesstefan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 29 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between c54a2fa and 2f8a320.

📒 Files selected for processing (1)
  • nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TaskField.java (1 hunks)

Walkthrough

Adds TaskField support for Elasticsearch: new abstract TaskField domain class, a Spring adapter with ES annotations, and ElasticCaseMappingService logic to transform and index TaskField instances containing task ID references. (≈34 words)

Changes

Cohort / File(s) Summary
TaskField domain class
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TaskField.java
New abstract class extending DataField with a String[] taskRefValue, constructor and overridden getValue(); Lombok generates accessors and equals/hashCode.
TaskField Spring adapter
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/TaskField.java
New concrete adapter class extending the domain TaskField; overrides getters with Elasticsearch @Field mappings (Text for fulltext, Keyword for task refs) and delegates to superclass.
Elastic mapping service
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
transformDataField() extended to detect TaskField instances and delegate to new transformTaskField() which converts task ID references into an elastic TaskField representation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect ElasticCaseMappingService.transformTaskField for correct extraction/formatting of task IDs and null-safety.
  • Verify TaskField inheritance and visibility across modules.
  • Confirm ES annotations and field types in the Spring adapter match indexing/query requirements.

Possibly related PRs

  • [NAE-2197] Elastic Mapping Fixes #352 — Similar change pattern adding transform handling and a corresponding domain/spring adapter (StringCollectionField); likely related implementation approach.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly references the issue (NAE-2282) and accurately describes the main change: adding support for parsing large taskRef values from Elasticsearch through new TaskField handling across multiple modules.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot added bugfix A change that fixes a bug improvement A change that improves on an existing feature Medium labels Dec 9, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 46e49ec and cd27b36.

📒 Files selected for processing (3)
  • application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java (3 hunks)
  • nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TaskField.java (1 hunks)
  • nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/TaskField.java (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: machacjozef
Repo: netgrif/application-engine PR: 367
File: application-engine/src/main/resources/application.yaml:24-24
Timestamp: 2025-10-20T11:44:44.907Z
Learning: In the netgrif/application-engine project, the correction of the Elasticsearch task index name from "_taks" to "_task" in application.yaml was approved by maintainer machacjozef, indicating that any data migration concerns for this typo fix are handled separately or not applicable to their deployment scenario.
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 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.
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 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.
📚 Learning: 2025-10-20T11:44:44.907Z
Learnt from: machacjozef
Repo: netgrif/application-engine PR: 367
File: application-engine/src/main/resources/application.yaml:24-24
Timestamp: 2025-10-20T11:44:44.907Z
Learning: In the netgrif/application-engine project, the correction of the Elasticsearch task index name from "_taks" to "_task" in application.yaml was approved by maintainer machacjozef, indicating that any data migration concerns for this typo fix are handled separately or not applicable to their deployment scenario.

Applied to files:

  • nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/TaskField.java
  • application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
📚 Learning: 2025-08-19T20:13:40.087Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 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/TaskField.java
  • application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
📚 Learning: 2025-08-19T20:07:15.621Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 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:

  • application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
📚 Learning: 2025-08-19T20:07:43.748Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 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:

  • application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
🧬 Code graph analysis (2)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/TaskField.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TaskField.java (1)
  • Data (10-26)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TaskField.java (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/TaskField.java (1)
  • Data (11-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
  • GitHub Check: Build
  • GitHub Check: task-list-completed
🔇 Additional comments (2)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java (1)

81-82: TaskField mapping wiring is consistent with existing CaseField/StringCollection mappings

The new instanceof TaskField branch and transformTaskField implementation follow the same pattern as transformCaseFieldField/transformStringCollectionField (casting DataField.value to a List<String> and converting to String[] for the elastic domain). Given caseField.getValue() is already null‑checked, this looks correct for indexing task references.

Also applies to: 232-235

nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/TaskField.java (1)

11-30: Confirm Elasticsearch field types for TaskField mappings

You’ve mapped:

  • getFulltextValue() as @Field(type = Text)
  • getTaskRefValue() as @Field(type = Keyword)

For CaseField, prior design maps fulltextValue as a Keyword field for exact matches/filtering/aggregations (per existing learnings). Please double‑check that using Text for TaskField’s fulltextValue (vs Keyword) is intentional and aligns with how you plan to query task refs, especially for aggregations and filters.

Replaced `List.of` with `Arrays.asList` to allow null checks and ensure compatibility. Added a null check for `taskRefValue` to prevent potential `NullPointerException` and return an empty list when it is null. This improves robustness and aligns with safe coding practices.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd27b36 and c54a2fa.

📒 Files selected for processing (1)
  • nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TaskField.java (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: machacjozef
Repo: netgrif/application-engine PR: 367
File: application-engine/src/main/resources/application.yaml:24-24
Timestamp: 2025-10-20T11:44:44.907Z
Learning: In the netgrif/application-engine project, the correction of the Elasticsearch task index name from "_taks" to "_task" in application.yaml was approved by maintainer machacjozef, indicating that any data migration concerns for this typo fix are handled separately or not applicable to their deployment scenario.
Learnt from: Retoocs
Repo: netgrif/application-engine PR: 383
File: application-engine/src/main/java/com/netgrif/application/engine/startup/ApplicationRunnerOrderResolver.java:43-43
Timestamp: 2025-11-14T10:22:01.634Z
Learning: For the netgrif/application-engine repository, avoid flagging trivial or nitpick-level issues such as redundant null checks, minor code style improvements, or obvious simplifications that don't affect functionality or introduce bugs. Focus review comments on substantive issues like logic errors, security concerns, performance problems, or breaking changes.
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 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.
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 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.
📚 Learning: 2025-08-20T07:27:02.660Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 339
File: application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticTaskQueueManager.java:38-45
Timestamp: 2025-08-20T07:27:02.660Z
Learning: When reviewing ElasticTaskQueueManager changes, task.getTask().getId() returns the document identifier while task.getTaskId() returns the business task identifier. The queue operations should use consistent identifiers throughout the lifecycle (scheduling, processing, cleanup).

Applied to files:

  • nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TaskField.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). (6)
  • 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
  • GitHub Check: Build
🔇 Additional comments (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/TaskField.java (1)

22-28: getValue() implementation correctly fixed.

The method now properly returns a List<String> of task IDs instead of a List<String[]>. The null check and defensive copy via new ArrayList<>(Arrays.asList(taskRefValue)) are both appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix A change that fixes a bug improvement A change that improves on an existing feature Medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants