Fix duplicated search criteria in Knowledge Base tree view (#24090)

[aymericcucherousset]

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

• It fixes Duplicate search entries in Knowledge Base tree view keep increasing #24090

In the Knowledge Base Browse (tree) view, the search form displayed two criteria on first load instead of one, and each search added another one. The List view was not affected.

Cause

ajax/treebrowse.php and TreeBrowse::showBrowseView() add a hidden category criterion flagged as virtual. This criterion was also passed to QueryBuilder::showGenericSearch(), which rendered it as a regular search field.

Since the virtual criterion was not stored in the session, the form displayed an additional default criterion. Submitting the search saved it as a regular criterion, causing the duplication to increase with each search.

Fix

QueryBuilder::showGenericSearch() now filters out virtual criteria before rendering the search form.

• Original criterion keys are preserved to maintain alignment with session criteria.
• The category filter remains applied server-side.
• The change does not affect other search forms that do not use virtual criteria.

Tests

• Functional (QueryBuilderTest): verifies that regular criteria are rendered correctly, virtual criteria are excluded, and the "add rule" counter reflects only visible criteria.
• E2E (Knowbase/tree_browse.spec.ts): verifies that the Browse view displays only one criterion, that submitting a search does not duplicate it or submit virtual criteria, and that category filtering continues to work.

Screenshots

[cconard96]

Please follow the pull request template and be sure to link any relevant issues using closing keywords. I think there is at least one issue currently open about this.

[qodo-free-for-open-source-projects]

Code Review by Qodo

View medium (3)

🟠 **Medium**

1. Tree browse test bypasses page objects ✓ Resolved

Description

The new tree browse test declares criteria_fields and items_list locators in the spec and
performs navigation and filter interactions directly on page instead of through a page class.
SearchEnginePage already exposes the search filters panel and button, so the new test duplicates
those selectors and the filter-opening interaction outside that abstraction.

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 tree browse test keeps selectors and reusable search interactions in the spec rather than a page object.
## Fix Focus Areas
- tests/e2e/specs/Knowbase/tree_browse.spec.ts[100-124]
- tests/e2e/pages/SearchEnginePage.ts[35-69]
## Recommended Fix
Use or extend a page object for the tree browse and search interactions. Put selectors and reusable actions there, then call its methods from the spec.

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

2. Tree browse checks use raw selectors ✓ Resolved

Description

The new test selects the category with page.locator('#tree_category .fancytree-title') and scopes
result checks with page.locator('#items_list'), with only lint-disable comments rather than an
explanation for avoiding semantic locators. The category interaction can target its visible text,
while the article checks already target links by role and name.

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 tree browse interactions use raw CSS selectors without explaining why semantic locators cannot be used.
## Fix Focus Areas
- tests/e2e/specs/Knowbase/tree_browse.spec.ts[118-123]
## Recommended Fix
Select the category by its visible text and locate article links by role and name, using the existing tree-browse test ID for scoping if needed. If a raw selector remains unavoidable, add an immediately preceding explanation.

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

3. Adding a rule can duplicate an existing rule ✓ Resolved

Description

showGenericSearch() removes virtual criteria without changing the remaining keys, while the form
uses the number of remaining criteria as the next rule index. If a virtual criterion precedes a
regular one, clicking Add rule reuses the regular criterion’s index, producing two rows with the
same submitted field names instead of a new rule.

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Filtering virtual criteria preserves the keys of visible rows, but the Add rule counter uses their count. A virtual criterion before a regular criterion therefore makes the next rule reuse an existing index.
## Fix Focus Areas
- src/Glpi/Search/Input/QueryBuilder.php[110-113]
- templates/components/search/query_builder/main.html.twig[68-72]
## Recommended Fix
Keep the existing criterion keys so rows still match their session entries, but initialize the form’s next-rule counter above the highest remaining key rather than from the filtered array length. Add a test with a virtual criterion before a regular criterion.

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