Skip to content

KB : Add home - #25576

Open
f2cmb wants to merge 15 commits into
glpi-project:12.0/bugfixesfrom
f2cmb:kb/faq/addHome
Open

f2cmb wants to merge 15 commits into
glpi-project:12.0/bugfixesfrom
f2cmb:kb/faq/addHome

Conversation

@f2cmb

@f2cmb f2cmb commented Sep 18, 2026

Copy link
Copy Markdown
Contributor
  • 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 helpdesk FAQ opens on the root Home article, as the central knowledge base does.

It also fixes two defects found on the way :

  • A user with the knowledge base right saw only the FAQ in the helpdesk interface.
  • An article disappeared from the aside tree when its parent was hidden.

Bonus : enriched the KB items creation command tools (needed content to fit to my case

Screenshot :

Capture d’écran 2026-09-18 à 16 27 51

@f2cmb f2cmb added this to the 12.0.0 milestone Sep 18, 2026
@f2cmb f2cmb linked an issue Sep 21, 2026 that may be closed by this pull request
@f2cmb
f2cmb marked this pull request as ready for review September 22, 2026 12:00
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo


🔴 High

1. Unauthorized readers can list Home 🐞 Bug
Description
getListRequest() always wraps the forced FAQ filter in withRootArm(), which adds the configured
root ID without testing canReadRoot(). When public FAQ access is disabled, or a logged-in user has
no knowledge-base right, list and aside queries can return Home even though can($rootId, READ)
denies access.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getListRequest()` admits the configured root through `withRootArm()` even when the current session cannot read that root. This makes browse/search consumers of the list criteria return Home for anonymous users while the public FAQ is disabled and for authenticated users without FAQ or knowledge-base read rights, contradicting the item-level authorization check.

## Fix Focus Areas
- src/KnowbaseItem.php[2158-2164]
- src/KnowbaseItem.php[295-315]

## Recommended Fix
Only add the root-ID arm to the FAQ filter when `canReadRoot()` is true. Keep the normal FAQ predicate unchanged for sessions that cannot read the root, so root visibility in query results remains consistent with `can($rootId, READ)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (3)
🟠 **Medium**
2. FAQ navigation skips action naming 📘 Rule violation
Description
gotoFaqHome() is a public user-facing navigation action whose name does not start with the
required do prefix. Callers of this new page-object method cannot identify it consistently
alongside the class's existing doXxx actions.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new public FAQ navigation action does not use the required `doXxx` naming convention for end-to-end page-object user actions.

## Fix Focus Areas
- tests/e2e/pages/KnowbaseItemPage.ts[157-163]
- tests/e2e/specs/Knowbase/faq-aside.spec.ts[163-164]

## Recommended Fix
Rename `gotoFaqHome()` to an async `doGotoFaqHome()` method and update every caller to use the renamed action.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. FAQ tests cannot choose an initial tab 📘 Rule violation
Description
gotoFaqHome() accepts no optional tab argument and always passes the fixed
/front/helpdesk.faq.php URL to Playwright. A test that must initially open a specific FAQ tab
cannot map that request to forcetab through the page object and must bypass the helper or click
the tab afterward.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new FAQ page-object navigation method cannot select an initial tab through the required `forcetab` URL parameter.

## Fix Focus Areas
- tests/e2e/pages/KnowbaseItemPage.ts[157-162]

## Recommended Fix
Add an optional `tab` parameter and append an encoded `forcetab` query parameter only when it is supplied; otherwise retain the current URL.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. Large knowledge bases load slowly 🐞 Bug
Description
getChildArticlesInfo() materializes every readable knowledge-base article through an unrestricted
getListRequest() before examining the current article's descendants. Because showFull() invokes
this method for every article, even rendering a leaf now scales with the entire knowledge base,
while each returned child additionally incurs can() and translated-name lookups.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getChildArticlesInfo()` loads all readable knowledge-base IDs for every article render, even when the current article has no descendants, and then performs additional per-child database lookups.

## Fix Focus Areas
- src/KnowbaseItem.php[1642-1690]
- src/KnowbaseItem.php[1700-1725]

## Recommended Fix
Traverse direct-child frontiers first and restrict each visibility query to the IDs in that frontier. Load the fields needed for authorization and output in the same query where possible, while preserving publication-window checks, nearest-readable-descendant behavior, DAG deduplication, and cycle protection.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


@f2cmb
f2cmb removed the request for review from AdrienClairembault September 22, 2026 12:04

This branch has not been deployed

No deployments
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.

Add Home article to Helpdesk

1 participant