Skip to content

Fix knowledge base aside renaming all nested articles - #25486

Closed
marifex wants to merge 1 commit into
glpi-project:12.0/bugfixesfrom
marifex:fix/kb-aside-title-descendants
Closed

Fix knowledge base aside renaming all nested articles#25486
marifex wants to merge 1 commit into
glpi-project:12.0/bugfixesfrom
marifex:fix/kb-aside-title-descendants

Conversation

@marifex

@marifex marifex commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Checklist before requesting a review

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.

Description

The knowledge base aside renders the article tree by nesting each child article inside its parent element. When an article was renamed (or its illustration changed), the title and illustration were updated with a descendant selector [data-glpi-kb-article-id="<id>"] [data-glpi-kb-article-title], which also matched every nested child. Renaming a parent (in particular the root, which contains everything) therefore visually renamed all its descendants until the page was refreshed. The database was never affected.

Both #updateAsideTitle() and #updateAsideIllustration() now update only the entry that belongs to the current article, using the same closest(...) === article guard already used by AsideController::findEmptyMenus() for the exact same nesting reason.

Note on tests

The update methods are private and rely on the full controller and DOM, so a unit test would need significant setup. I verified the fix against the real aside DOM structure: the descendant selector matched the parent and all nested children, while the corrected logic matches only the current article. Happy to add a test where you prefer.

The aside tree nests child articles inside their parent element, so the descendant selectors used when updating an article title and illustration also matched every child. Renaming or changing the illustration of a parent (in particular the root) visually changed all its descendants until the page was refreshed. Update only the entry that belongs to the current article, as already done in AsideController::findEmptyMenus().

Fixes glpi-project#25394
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Powered by Qodo

@cconard96

Copy link
Copy Markdown
Member

#25396 is open to resolve this issue as well. I missed linking it to the original issue.

@marifex

marifex commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, I missed #25396. Yours is more complete since it also covers the search results template, so let's go with that one. Closing this in favor of #25396.

@marifex marifex closed this Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants