Skip to content

Issue 36459 add has children field to folder search api to support accurate UI tree rendering - #36497

Merged
freddyDOTCMS merged 11 commits into
mainfrom
issue_36459_Add_hasChildren_field_to_folder_search_API_to_support_accurate_UI_tree_rendering
Jul 9, 2026
Merged

Issue 36459 add has children field to folder search api to support accurate UI tree rendering#36497
freddyDOTCMS merged 11 commits into
mainfrom
issue_36459_Add_hasChildren_field_to_folder_search_API_to_support_accurate_UI_tree_rendering

Conversation

@freddyDOTCMS

@freddyDOTCMS freddyDOTCMS commented Jul 9, 2026

Copy link
Copy Markdown
Member

Fixes: #36459

Summary

Adds a hasChildren boolean field to every folder entry returned by GET /api/v1/folder/search, so UI tree components can accurately decide whether to render an expand arrow — without issuing a second request per folder.

Problem: The endpoint returned folder data but gave no signal about sub-folders. Tree UIs were forced to choose between an N+1 query per folder or rendering expand arrows on all folders (including leaves). Clicking a leaf's arrow loaded nothing, breaking the UX.

Solution: hasChildren is computed in batch per page — two SQL round-trips total: one to fetch all direct children of the page's folders in a single IN (...) query, one batch permission check via permissionAPI.filterCollection. No per-folder queries. hasChildren: true only when at least one child is visible to the requesting user.

What changed

  • FolderSearchView — added hasChildren boolean field to the response record.
  • FolderAPIImpl#searchFolders — calls new findParentPathsWithVisibleChildren after the existing page query; uses batch permissionAPI.filterCollection (not per-item doesUserHavePermission).
  • FolderFactory / FolderFactoryImpl — new findDirectChildFolders(hostInode, parentPaths) fetches all direct children for the page in one SQL query.
  • Integration tests — new cases in FolderResourceSearchTest and FolderAPIImplFilterTest / FolderFactoryImplTest covering: child exists → true, leaf folder → false, permission-hidden child → false, user with read permission → visible child counted, empty page guard.
  • Postman — updated FolderResource collection with hasChildren assertions.

API contract

The change is fully backwards-compatible. No existing fields are removed or modified.

{
  "id": "...",
  "inode": "...",
  "name": "blog",
  "path": "/content/blog/",
  "addChildrenAllowed": true,
  "hasChildren": true
}

Checklist

  • Tests
  • Translations
  • Security Implications Contemplated (batch permission check uses the existing permissionAPI.filterCollection path — no new permission surface)

Closes #36459

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @freddyDOTCMS's task in 7m 44s —— View job


Review: PR #36497hasChildren field for folder search

  • Read the diff
  • Review backend logic (FolderAPIImpl, FolderFactoryImpl, FolderSearchView)
  • Check SQL / permission / batch correctness
  • Review tests + Postman
  • Post review

I traced the full path: FolderResource.searchFoldersFolderAPIImpl.searchFoldersfindParentPathsWithVisibleChildrenFolderFactoryImpl.findDirectChildFolders, plus the transformer, Identifier.getPath(), and the existing search SQL. The implementation is correct and well-structured.

Verified correct

  • SQL is injection-safefindDirectChildFolders uses ? placeholders + addParam() for both host_inode and every path in the IN (...) list. No string concatenation of user input. (FolderFactoryImpl.java:1230-1238)
  • Batch permission model — one bulk child fetch + one permissionAPI.filterCollection(READ) per page, exactly as the PR claims and consistent with the batch-over-scalar convention in CLAUDE.md. No N+1. (FolderAPIImpl.java:860-879)
  • Path reconstruction is safechildPath.substring(0, len - name.length() - 1) only depends on the length of the name, and getPath() is served from the cached path field the transformer already populated (FolderTransformer.java:60), so there's no hidden per-child identifier lookup. The /parent/ value it produces matches the page folder's getPath(). (FolderAPIImpl.java:874-877)
  • Case consistencyparent_path is stored non-lowercased (the existing non-recursive search matches parent_path = ? un-lowercased at line 1211), and folder.getPath() is also original-case, so the IN match is consistent.
  • Empty-page guard — double guarded: findDirectChildFolders short-circuits on empty/null parentPaths, and findParentPathsWithVisibleChildren returns Set.of() on no children.
  • OpenAPI — no regeneration needed; the endpoint's @Schema is the generic ResponseEntityPaginatedDataView, so FolderSearchView is not materialized in openapi.yaml. Confirmed FolderSearchView/hasChildren do not appear there.
  • Backwards compatible — additive record field only; no existing field changed.
  • Test coverage is solid — factory level (direct children, empty paths, grandchild exclusion), API level (true/leaf/permission-hidden), resource level (same three), and Postman assertions.

New Issues

  • 🟡 Medium: FolderFactoryImpl.java:1231findDirectChildFolders does SELECT folder.* and materializes every direct child (full folder rows + a transformer that resolves each Identifier) for all folders on the page, only to reduce it to a boolean. For a page of folders that each have many sub-folders this loads a large result set. It's bounded by page size × fan-out and permission filtering does require candidate objects, so this is acceptable, but a hasChildren signal is inherently existence-only. Assumption: typical folders have modest child counts. What to verify: behavior for a folder with thousands of direct children — if that's realistic, consider a permission-aware existence/count strategy rather than loading full rows.

Minor (non-blocking)

  • FolderSearchView.java:18 — file has no trailing newline (pre-existing, but the diff touches the last line).

Nice work — the batch approach and the test matrix are exactly right. Only the potential row-materialization cost is worth a second thought before merge; it's not a blocker.

…search_API_to_support_accurate_UI_tree_rendering
@freddyDOTCMS
freddyDOTCMS added this pull request to the merge queue Jul 9, 2026
@mergify

mergify Bot commented Jul 9, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Merged via the queue into main with commit 27191bc Jul 9, 2026
62 checks passed
@freddyDOTCMS
freddyDOTCMS deleted the issue_36459_Add_hasChildren_field_to_folder_search_API_to_support_accurate_UI_tree_rendering branch July 9, 2026 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Add hasChildren field to folder search API to support accurate UI tree rendering

2 participants