Skip to content

Feature: Used and managed items user count - #24668

Open
monta990 wants to merge 12 commits into
glpi-project:mainfrom
monta990:used-managed-items-user-count
Open

monta990 wants to merge 12 commits into
glpi-project:mainfrom
monta990:used-managed-items-user-count

Conversation

@monta990

@monta990 monta990 commented Jun 24, 2026 •

Copy link
Copy Markdown
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

  • 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.
  • This change requires a documentation update.

Description

Screenshots (if appropriate):

image

Comment thread src/User.php Resolved
Comment thread src/User.php Resolved
Comment thread src/User.php Outdated
$criteria = $user_criteria['criteria'];

$groups = [];
$groups = [];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This variable is build from a SQL query but is never used.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right. The $groups array is no longer used after the refactoring.

The issue was introduced when I extracted the common group criteria logic into getItemsForUserCriteria(). The original showItems() code populated $groups from the group query, but after the refactoring the display logic relies on $group_choices instead. This left the $groups array and its associated query unused.

I'll remove $groups and $groups_iterator from showItems(). The groups_ids query in getItemsForUserCriteria() will remain, since those IDs are still required to build the user/group criteria.

monta990 and others added 2 commits September 2, 2026 08:35
Co-authored-by: Adrien Clairembault <42734840+AdrienClairembault@users.noreply.github.com>
Co-authored-by: Adrien Clairembault <42734840+AdrienClairembault@users.noreply.github.com>
@monta990

monta990 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for pointing this out. I checked the current version of showItems() again: the result from $groups_iterator is consumed immediately by the foreach to populate $groups.

So, in the current commit, the SQL result is actually used. I believe this comment may refer to an earlier state of the diff before the subsequent commits updated this section.

@AdrienClairembault

Copy link
Copy Markdown
Member

Thanks for pointing this out. I checked the current version of showItems() again: the result from $groups_iterator is consumed immediately by the foreach to populate $groups.

So, in the current commit, the SQL result is actually used. I believe this comment may refer to an earlier state of the diff before the subsequent commits updated this section.

Can you please clarify how the content of $group is used after bien populated?

Removed unused groups fetching logic from user criteria.
@monta990

monta990 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

You're right. The $groups array is no longer used after the refactoring.

The issue was introduced when I extracted the common group criteria logic into getItemsForUserCriteria(). The original showItems() code populated $groups from the group query, but after the refactoring the display logic relies on $group_choices instead.

I'll remove $groups and $groups_iterator from showItems(). The groups_ids query in getItemsForUserCriteria() will remain, since those IDs are still required to build the user/group criteria.

Comment thread src/User.php Resolved
@cconard96 cconard96 changed the title Feature: Used and managed items user count glpi 12 Feature: Used and managed items user count Oct 2, 2026
Co-authored-by: Curtis Conard <cconard96@gmail.com>
@monta990
monta990 requested a review from cconard96 October 7, 2026 02:04

@cedric-anne cedric-anne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tests are failing.

Comment thread src/User.php

$query = [
'SELECT' => [
'COUNT DISTINCT' => "$table.id AS cpt",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fix for COUNT DISTINCT in select

@monta990

Copy link
Copy Markdown
Contributor Author

Tests are failing.

Thanks for pointing this out. I corrected the query by moving COUNT DISTINCT under the SELECT key, which resolved the SQL errors.

The latest CI run is now passing, including the PHP 8.3/MySQL and PHP 8.5/MariaDB test suites, lint checks, Playwright tests, and the security scan.

Thanks again for the feedback and for helping improve this PR.

Working in GLPI 12
image

@monta990
monta990 requested a review from cedric-anne October 21, 2026 03:51
@cedric-anne

Copy link
Copy Markdown
Member

IA;DR

@cedric-anne
cedric-anne removed their request for review October 21, 2026 07:02
@cedric-anne
cedric-anne dismissed their stale review October 21, 2026 07:02

Tests were fixed.

@trasher trasher added the external contribution Pull request from a contributor outside of the core team label Nov 4, 2026

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

Labels

external contribution Pull request from a contributor outside of the core team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants