Skip to content

Fix/unused workflow controller return types#419

Merged
machacjozef merged 2 commits intorelease/7.0.0-rev10from
fix/unused_workflow_controller_return_types
Mar 3, 2026
Merged

Fix/unused workflow controller return types#419
machacjozef merged 2 commits intorelease/7.0.0-rev10from
fix/unused_workflow_controller_return_types

Conversation

@renczesstefan
Copy link
Member

@renczesstefan renczesstefan commented Mar 3, 2026

Description

Fixes unused workflowcontroller response, and adds additional logging to search endpoint.

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.

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 @machacjozef
  • 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

Summary by CodeRabbit

  • Refactor

    • Simplified and standardized workflow endpoint response construction for more consistent behavior.
  • Chores

    • Added trace-level logging and resource relation metadata to improve observability and API resource descriptions.

Replaced manual PagedModel creation with assembler-generated resources for consistency. Added detailed debug logging to CaseResource creation and search methods, providing more insights into operations. Introduced @slf4j and @relation annotations for better resource structure and traceability.
@renczesstefan renczesstefan self-assigned this Mar 3, 2026
@renczesstefan renczesstefan changed the base branch from master to release/7.0.0-rev10 March 3, 2026 13:45
@netgrif netgrif deleted a comment from coderabbitai bot Mar 3, 2026
@netgrif netgrif deleted a comment from coderabbitai bot Mar 3, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

@machacjozef: I'll scan and review the changes in this PR.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai bot added improvement A change that improves on an existing feature Extra Small labels Mar 3, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

Walkthrough

Refactors WorkflowController endpoints to return pre-built HATEOAS resources instead of constructing new PagedModel instances and adds logging and HATEOAS relation metadata to CaseResource.

Changes

Cohort / File(s) Summary
WorkflowController Endpoint Refactoring
application-engine/src/main/java/com/netgrif/application/engine/workflow/web/WorkflowController.java
Replaced construction of PagedModel.of(...) with returning the already assembled resources in endpoints (getAll, search2, searchMongo, findAllByAuthor). Added trace logging in the Elasticsearch search path to log found elements and final resources.
CaseResource Logging & HATEOAS Metadata
application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/CaseResource.java
Added @Slf4j and @Relation(collectionRelation = "cases", itemRelation = "cases") annotations, imported corresponding classes, and added a constructor log.trace call for id, locale, and class name; minor link-building formatting changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

Medium

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 'Fix/unused workflow controller return types' clearly summarizes the main change: fixing unused return type constructions in the WorkflowController by switching from PagedModel.of() to returning pre-assembled resources.

✏️ 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.

❤️ Share

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

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: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/CaseResource.java`:
- Line 20: The constructor debug log in CaseResource currently exposes
content.get_id() and class name; remove the identifier from the debug-level
message to avoid leaking IDs and reduce noise, and if you still need the ID for
troubleshooting move that detailed log to trace-level (e.g., use
log.debug("Creating CaseResource, locale={}", LocaleContextHolder.getLocale())
and log.trace(...) including content.get_id() and content.getClass().getName());
update the logging call in the CaseResource constructor accordingly so debug is
minimal and detailed identifiers are logged only at trace.

In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/web/WorkflowController.java`:
- Around line 132-142: The current debug logs in WorkflowController expose
identifiers and full PagedModel content; change them to only log non-sensitive
summaries: replace the first debug (which uses cases.map(Case::getStringId))
with a message that logs just counts like cases.getNumberOfElements() and
cases.getTotalElements(), and replace the later debug that logs the entire
PagedModel resources with a concise summary of paging info (pageable/page
number, size, total pages) and the assembler/selfLink relation; update the debug
calls around the variables cases and resources accordingly to avoid logging
identifiers or full objects.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a9eb74 and 7f1c9b2.

📒 Files selected for processing (2)
  • application-engine/src/main/java/com/netgrif/application/engine/workflow/web/WorkflowController.java
  • application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/CaseResource.java

@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

Oops, something went wrong! Please try again later. 🐰 💔

@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

Caution

Review failed

The head commit changed during the review from 7f1c9b2 to 7fca3b0.


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.

Updated logging level in `CaseResource` and `WorkflowController` to TRACE to capture granular debugging details without cluttering DEBUG-level logs. This change ensures better log granularity and improved log management.
@coderabbitai coderabbitai bot added the Medium label Mar 3, 2026
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.

♻️ Duplicate comments (1)
application-engine/src/main/java/com/netgrif/application/engine/workflow/web/WorkflowController.java (1)

132-148: ⚠️ Potential issue | 🟠 Major

Avoid logging case IDs and full paged payloads in TRACE logs.

Line 133 logs identifiers, and Line 141/Line 147 log the full PagedModel. This can leak internal data and create oversized logs under trace-enabled troubleshooting.

Proposed fix
-        if (log.isTraceEnabled()) {
-            log.trace("Found {} cases with id {}.", cases.getNumberOfElements(), cases.map(Case::getStringId));
-        }
+        if (log.isTraceEnabled()) {
+            log.trace("Search returned page: elementsOnPage={}, totalElements={}, page={}, size={}",
+                    cases.getNumberOfElements(), cases.getTotalElements(), cases.getNumber(), cases.getSize());
+        }
@@
-        if (log.isTraceEnabled()) {
-            log.trace("Paged model of {}", resources);
-        }
+        if (log.isTraceEnabled()) {
+            log.trace("Paged model built: elements={}, links={}",
+                    resources.getContent().size(), resources.getLinks().size());
+        }
@@
-        if (log.isTraceEnabled()) {
-            log.trace("Returning paged model with link: {}", resources);
-        }
+        if (log.isTraceEnabled()) {
+            log.trace("Returning search response for relation={}", selfLink.getRel());
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/web/WorkflowController.java`
around lines 132 - 148, Replace the TRACE logs that output case identifiers and
the full PagedModel in WorkflowController: remove or change the calls that log
cases.map(Case::getStringId) and the full resources object (the log.trace lines
referencing cases and resources) and instead log only non-sensitive metadata
such as cases.getNumberOfElements(), pageable info, or the selfLink relation.
Update the trace statements around the assembler.toModel(...) and
ResourceLinkAssembler.addLinks(...) sections to avoid printing Case::getStringId
or resources; keep minimal contextual messages (e.g., "Returning paged model
with N elements and rel X") referencing cases.getNumberOfElements() and
selfLink.getRel() to locate the code to modify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/workflow/web/WorkflowController.java`:
- Around line 132-148: Replace the TRACE logs that output case identifiers and
the full PagedModel in WorkflowController: remove or change the calls that log
cases.map(Case::getStringId) and the full resources object (the log.trace lines
referencing cases and resources) and instead log only non-sensitive metadata
such as cases.getNumberOfElements(), pageable info, or the selfLink relation.
Update the trace statements around the assembler.toModel(...) and
ResourceLinkAssembler.addLinks(...) sections to avoid printing Case::getStringId
or resources; keep minimal contextual messages (e.g., "Returning paged model
with N elements and rel X") referencing cases.getNumberOfElements() and
selfLink.getRel() to locate the code to modify.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f1c9b2 and 7fca3b0.

📒 Files selected for processing (2)
  • application-engine/src/main/java/com/netgrif/application/engine/workflow/web/WorkflowController.java
  • application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/CaseResource.java

@machacjozef machacjozef merged commit 7457097 into release/7.0.0-rev10 Mar 3, 2026
6 checks passed
@machacjozef machacjozef deleted the fix/unused_workflow_controller_return_types branch March 3, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Extra Small 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