Skip to content

Relations: Fix descendants query to exclude parent item#21162

Merged
AndyButland merged 2 commits intomainfrom
fix/descendants-in-references-exclude-self
Dec 16, 2025
Merged

Relations: Fix descendants query to exclude parent item#21162
AndyButland merged 2 commits intomainfrom
fix/descendants-in-references-exclude-self

Conversation

@iOvergaard
Copy link
Contributor

@iOvergaard iOvergaard commented Dec 16, 2025

Summary

  • Fixed the GetPagedDescendantsInReferences SQL query to only return actual descendants, not the parent item itself
  • The trash confirmation dialog was incorrectly showing the item being deleted in the "descending items with dependencies" list

Fixes AD#61182

Root Cause

The WhereLike query was using just % as suffix, which matched both the parent path and descendants:

  • WHERE path LIKE '-1,1000%' matches -1,1000 (parent) AND -1,1000,1001 (child)

Fix

Added , before the % wildcard to only match items whose path continues after the parent:

  • WHERE path LIKE '-1,1000,%' only matches -1,1000,1001 (child), NOT -1,1000 (parent)

Screenshot before (notice the Home node)

image

Screenshot after

image

API output before

{
    "total": 3,
    "items": [
        {
            "id": "3af0f367-d7ca-4438-a38a-17ef4699c254"
        },
        {
            "id": "607c17c1-d6cf-4739-b330-452436e25064"
        },
        {
            "id": "4db95763-5f0b-4065-9c03-58b0702e4483"
        }
    ]
}

API output after

{
    "total": 2,
    "items": [
        {
            "id": "607c17c1-d6cf-4739-b330-452436e25064"
        },
        {
            "id": "4db95763-5f0b-4065-9c03-58b0702e4483"
        }
    ]
}

Test plan

  • Create content with child items that have references (e.g., linked by other content)
  • Try to trash the parent item
  • Verify the "descending items with dependencies" section only shows children, not the parent itself

Related

🤖 Generated with Claude Code

The GetPagedDescendantsInReferences query was using a path LIKE without
the comma delimiter, causing it to match both the parent item and its
descendants. This resulted in the trash confirmation dialog showing
the item being deleted in the "descending items with dependencies"
list.

Changed the WhereLike call to use ",%" suffix instead of just "%",
so the query only matches actual descendants (items whose path
starts with the parent's path followed by a comma).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a SQL query bug in the GetPagedDescendantsInReferences method that was incorrectly including the parent item in its own descendants list. The issue occurred when showing the trash confirmation dialog, where the item being deleted would appear in the "descending items with dependencies" section. The fix adds a comma before the wildcard in the LIKE pattern (,% instead of just %) to ensure only actual descendants are matched, not the parent path itself.

Key changes:

  • Modified the WhereLike query to use ",%" suffix instead of default "%" to properly exclude the parent item
  • Updated the inline comment to document this exclusion behavior

Copy link
Contributor

@AndyButland AndyButland left a comment

Choose a reason for hiding this comment

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

Works as expected, thanks @iOvergaard. I'll merge this and mark it as fixed for 17.1.

@AndyButland AndyButland merged commit b207fa3 into main Dec 16, 2025
26 checks passed
@AndyButland AndyButland deleted the fix/descendants-in-references-exclude-self branch December 16, 2025 12:44
iOvergaard added a commit that referenced this pull request Jan 13, 2026
…21162)

This fixes the same path comparison bug we fixed in PR #21162 but in C# string
comparisons instead of SQL queries.

## Root Cause
Path comparisons without trailing commas caused false matches:
- Path "-1,1001" incorrectly matched prefix "-1,100"
- This marked nodes as ancestors/descendants when they weren't related

## Examples of False Matches
- child.Path = "-1,1001", startNodePath = "-1,100"
  - OLD: "-1,1001".StartsWith("-1,100") = TRUE (bug!)
  - NEW: "-1,1001,".StartsWith("-1,100,") = FALSE (correct!)

- child.Path = "-1,100", startNodePath = "-1,1001"
  - OLD: "-1,1001".StartsWith("-1,100") = TRUE (bug!)
  - NEW: "-1,1001,".StartsWith("-1,100,") = FALSE (correct!)

## Fix Applied (Two Locations)
1. Line 146 (ancestor check): Added comma suffix to child.Path
2. Line 226 (IsDescendantOrSelf): Added comma suffix to both paths

This matches the pattern already used correctly in lines 92 and 191 of the
same file, and mirrors the SQL fix from PR #21162.

## Test Impact
This should fix the failing E2E test where child media folders were incorrectly
marked as noAccess when they were actually the user's start node.

Related: #21162

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
nielslyngsoe pushed a commit that referenced this pull request Jan 15, 2026
…ss (#21365)

* Tree pickers: Implement noAccess property UI handling for user start nodes

- Add noAccess observable to document and media tree item contexts
- Add visual styling (grayed out, italic) for noAccess items in tree views
- Update document and media picker input contexts to prevent selection of noAccess items
- Items with noAccess are shown for navigation but cannot be selected in pickers

This implements the UI handling for Feature 63060 "Handle Start Nodes"

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix noAccess implementation and add E2E tests

This commit combines all improvements made to the noAccess property feature:

1. Refactored to use Lit lifecycle methods (updated()) instead of property watchers
2. Added click and keyboard event handlers to prevent navigation
3. Removed disabled attribute that was blocking tree expansion
4. Added comprehensive E2E tests for document and media trees

Critical bug fix: Removed disabled attribute that prevented expansion
- The disabled attribute was blocking ALL interactions including expanding
  tree items to show accessible children underneath noAccess ancestors
- Now only sets aria-disabled="true" for screen readers and removes href
- Click and keyboard event handlers still prevent navigation as intended
- Users can now properly navigate through noAccess ancestors to reach
  their accessible child nodes

E2E test coverage:
- Display noAccess styling (opacity, italic)
- Prevent navigation when clicking noAccess nodes
- Allow expansion of noAccess nodes to show children
- Picker tests skipped pending infrastructure improvements

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* Remove aria-disabled manipulation that interferes with tree expansion

The previous implementation set aria-disabled="true" and removed href
from the menu-item in #updateMenuItemAccessibility(). This approach
caused issues with tree expansion functionality.

Removed:
- #updateMenuItemAccessibility() method
- updated() lifecycle hook that called it
- UUIMenuItemElement import (no longer needed)

The click and keyboard event handlers already prevent navigation to
noAccess nodes, so additional DOM manipulation is not necessary.

Test results:
✅ 4 passing: Display styling and prevent navigation work correctly
❌ 2 failing: These appear to be backend issues:
   1. Document expansion: Caret button disabled (backend marking noAccess
      items as not selectable, which disables entire menu-item)
   2. Media expansion: Child media folder incorrectly has noAccess attribute
      (backend data issue - child should be accessible as it's the start node)

The UI implementation is sound. The remaining test failures indicate
backend API issues that need investigation.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* Fix path comparison bug in UserStartNodeEntitiesService (similar to #21162)

This fixes the same path comparison bug we fixed in PR #21162 but in C# string
comparisons instead of SQL queries.

## Root Cause
Path comparisons without trailing commas caused false matches:
- Path "-1,1001" incorrectly matched prefix "-1,100"
- This marked nodes as ancestors/descendants when they weren't related

## Examples of False Matches
- child.Path = "-1,1001", startNodePath = "-1,100"
  - OLD: "-1,1001".StartsWith("-1,100") = TRUE (bug!)
  - NEW: "-1,1001,".StartsWith("-1,100,") = FALSE (correct!)

- child.Path = "-1,100", startNodePath = "-1,1001"
  - OLD: "-1,1001".StartsWith("-1,100") = TRUE (bug!)
  - NEW: "-1,1001,".StartsWith("-1,100,") = FALSE (correct!)

## Fix Applied (Two Locations)
1. Line 146 (ancestor check): Added comma suffix to child.Path
2. Line 226 (IsDescendantOrSelf): Added comma suffix to both paths

This matches the pattern already used correctly in lines 92 and 191 of the
same file, and mirrors the SQL fix from PR #21162.

## Test Impact
This should fix the failing E2E test where child media folders were incorrectly
marked as noAccess when they were actually the user's start node.

Related: #21162

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* Fix remaining merge conflict markers in media-tree-item.element.ts

* Remove E2E agent markdown file (moved to personal space)

* test: adds mock data for noAccess

* feat: moves noAccess subscriber to base class

* test: adds mock data for media

* feat: moves no-access styling to the base class

* fix: media tree items should inherit styling from the base class

* feat: observes noAccess from children and reports back to the base class

* test: spec file should use undefined instead of null

* docs: add comprehensive comments explaining noAccess opt-in pattern

- Document why noAccess is not in base interface (breaking change)
- Explain opt-in pattern with code examples
- Add JSDoc comments to property, event handlers, and CSS
- Reference accessibility considerations (keyboard users)
- Link child class implementations to base class documentation

* test: adds timeout for URL to settle

* fix: allow clicks on accessible children of noAccess tree items

When a tree item has noAccess, child tree items are rendered in its slot.
Previously, the parent's click handler blocked ALL clicks due to event bubbling,
preventing users from navigating to accessible descendants.

Now checks if click originated from a child tree item element using closest().
If it's a child, allow the click. Only block clicks on the noAccess item itself.

Applied to both mouse clicks and keyboard navigation (Enter/Space).

This enables users to navigate through noAccess ancestors to reach their
accessible start nodes (e.g., Root[noAccess] → Child[noAccess] → Grandchild[accessible]).

Fixes tests:
- should allow expansion of noAccess ancestor node to show children (documents)
- should allow expansion of noAccess ancestor media node to show children (media)

* compare with the closest element to see if we are clicking on the element that is blocked or a sub-element that is not

* fix: adds forbidden route in case of no variants

* test: corrects label locator

* test: adds test to check if you can click or deeplink to restricted media

* test: removes .only

* test: removes duplicated tests

* test: adds test for document no-access

* test: add unit tests for user start node path comparison logic

Adds comprehensive unit tests documenting the path comparison fix that prevents
false matches when node IDs are numeric prefixes of other IDs (e.g., 100 vs 1001).

The fix uses trailing commas on both paths to ensure accurate comparison:
- Without fix: "-1,100".StartsWith("-1,10") = true ❌ (incorrect)
- With fix: "-1,100,".StartsWith("-1,10,") = false ✅ (correct)

Tests cover:
- Numeric prefix edge cases (1 vs 10, 10 vs 100, 100 vs 1001)
- Self comparison (start node itself)
- Descendant relationships
- Deep path hierarchies
- Demonstrates the bug without the fix for documentation

19 test cases total, all passing.

* test: removes .only

* fix: do not overwrite forbidden route

* docs: fixes line number in comment

* test: fixes comment

* feat: uses isSelectableContext to disable and scrub 'href' from base element

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants