Skip to content

Fix(Perf): remove per-row SQL queries from voucher views and the init activation check #280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jdurand-teclib wants to merge 1 commit into
base: 11.0/bugfixes
Choose a base branch
from
Open

Fix(Perf): remove per-row SQL queries from voucher views and the init activation check #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Fixed

- Improve performance of the consumed details, the ticket Credit tab and the entity Credit tab, which no longer run SQL queries for each displayed row
- Remove 2 SQL queries from every GLPI page by dropping the plugin's own activation check
- Delete the default voucher options of a ticket when it is purged

## [1.15.8] - 2026-10-27

### Fixed
Expand Down
61 changes: 53 additions & 8 deletions inc/entity.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,21 +208,30 @@ public static function showForItemtype(Entity $entity, $itemtype = 'Entity')
];
}

$vouchers = self::getAllForEntity($ID, $sqlfilter);
$consumed = self::getConsumedForCredits(array_keys($vouchers));
$type_links = [];

$entries = [];
foreach (self::getAllForEntity($ID, $sqlfilter) as $data) {
foreach ($vouchers as $data) {
$quantity_sold = (int) $data['quantity'];
if (0 === $quantity_sold) {
$quantity_sold = __('Unlimited');
}

// The row already holds every field getById() would load.
$item = new self();
$item = $item->getById($data['id']);
$item->fields = $data;
$consumed_for_credit = $consumed[$data['id']] ?? 0;

if (!empty($data['plugin_credit_types_id'])) {
$type = new PluginCreditType();
$type = $type->getById($data['plugin_credit_types_id']);
if ($type) {
$data['plugin_credit_types_id'] = $type->getLink();
$types_id = $data['plugin_credit_types_id'];
if (!array_key_exists($types_id, $type_links)) {
$type = PluginCreditType::getById((int) $types_id);
$type_links[$types_id] = $type ? $type->getLink() : null;
}
if ($type_links[$types_id] !== null) {
$data['plugin_credit_types_id'] = $type_links[$types_id];
}
} else {
$data['plugin_credit_types_id'] = '';
Expand All @@ -238,15 +247,15 @@ public static function showForItemtype(Entity $entity, $itemtype = 'Entity')
],
);

$link = "<a href='#' data-bs-toggle='modal' data-bs-target='#displaycreditconsumed_{$data["id"]}' title='" . __('Consumed details', 'credit') . "' alt='" . __('Consumed details', 'credit') . "'>" . PluginCreditEntity::getConsumedForCredit($data['id']) . "</a>";
$link = "<a href='#' data-bs-toggle='modal' data-bs-target='#displaycreditconsumed_{$data["id"]}' title='" . __('Consumed details', 'credit') . "' alt='" . __('Consumed details', 'credit') . "'>" . $consumed_for_credit . "</a>";

$entries[] = array_merge($data, [
'name' => $item->getLink(),
'quantity' => $quantity_sold,
'itemtype' => PluginCreditEntity::class,
'low_credit_alert' => $data['low_credit_alert'] == -1 ? __('Disabled') : $data['low_credit_alert'] . '%',
'quantity_consumed' => $modal . $link,
'quantity_remaining' => $data['quantity'] > 0 ? $data['quantity'] - PluginCreditEntity::getConsumedForCredit($data['id']) : 'Unlimited',
'quantity_remaining' => $data['quantity'] > 0 ? $data['quantity'] - $consumed_for_credit : 'Unlimited',
'entities_id' => Entity::badgeCompletenameLink($entity),
]);
}
Expand Down Expand Up @@ -674,4 +683,40 @@ public static function getConsumedForCredit(int $credit_id)

return (int) $ticket_result['consumed_total'];
}

/**
* Get the total consumption of several credit vouchers in one query.
*
* @param int[] $credit_ids IDs of the credit vouchers
*
* @return array<int, int> Total consumption by voucher ID, vouchers without consumption are absent
*/
public static function getConsumedForCredits(array $credit_ids): array
{
/** @var DBmysql $DB */
global $DB;

if ($credit_ids === []) {
return [];
}

$iterator = $DB->request([
'SELECT' => [
'plugin_credit_entities_id',
'SUM' => 'consumed AS consumed_total',
],
'FROM' => 'glpi_plugin_credit_tickets',
'WHERE' => [
'plugin_credit_entities_id' => $credit_ids,
],
'GROUPBY' => 'plugin_credit_entities_id',
]);

$consumed = [];
foreach ($iterator as $row) {
$consumed[$row['plugin_credit_entities_id']] = (int) $row['consumed_total'];
}

return $consumed;
}
}
135 changes: 97 additions & 38 deletions inc/ticket.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,11 @@ public static function getIcon()

public function getTabNameForItem(CommonGLPI $item, $withtemplate = 0)
{
$nb = $item instanceof CommonDBTM ? self::countForItem($item) : 0;
if ($item instanceof Ticket) {
if ($_SESSION['glpishow_count_on_tabs']) {
return self::createTabEntry(self::getTypeName($nb), $nb);
} else {
return self::getTypeName($nb);
}
} else {
return self::getTypeName($nb);
if ($item instanceof Ticket && $_SESSION['glpishow_count_on_tabs']) {
$nb = self::countForItem($item);
return self::createTabEntry(self::getTypeName($nb), $nb);
}
return self::getTypeName(Session::getPluralNumber());
}

public static function displayTabContentForItem(CommonGLPI $item, $tabnum = 1, $withtemplate = 0)
Expand Down Expand Up @@ -232,9 +227,27 @@ public static function showForTicket(Ticket $ticket)
],
];
}
foreach (self::getAllForTicket($ID) as $data) {
$credit_entity = new PluginCreditEntity();
$credit_entity->getFromDB($data['plugin_credit_entities_id']);
$consumptions = self::getAllForTicket($ID);

// Voucher names and user links are loaded once, not once per consumption.
$voucher_names = [];
$voucher_ids = array_unique(array_column($consumptions, 'plugin_credit_entities_id'));
if ($voucher_ids !== []) {
$vouchers = $DB->request([
'SELECT' => ['id', 'name'],
'FROM' => PluginCreditEntity::getTable(),
'WHERE' => ['id' => $voucher_ids],
]);
foreach ($vouchers as $voucher) {
$voucher_names[$voucher['id']] = $voucher['name'];
}
}
$showuserlink = Session::haveRight('user', READ);
$usernames = [];

foreach ($consumptions as $data) {
$users_id = $data['users_id'];
$usernames[$users_id] ??= $showuserlink ? getUserLink($users_id) : getUserName($users_id);

if (!empty($data['plugin_credit_types_id'])) {
$type = new PluginCreditType();
Expand All @@ -248,10 +261,12 @@ public static function showForTicket(Ticket $ticket)

$entries[] = array_merge($data, [
'id' => $data['id'],
'name' => $credit_entity->getName(),
'name' => (string) ($voucher_names[$data['plugin_credit_entities_id']] ?? '') !== ''
? $voucher_names[$data['plugin_credit_entities_id']]
: NOT_AVAILABLE,
'plugin_credit_types_id' => $data['plugin_credit_types_id'],
'date_creation' => $data['date_creation'],
'users_id' => Session::haveRight('user', READ) == true ? getUserLink($data["users_id"]) : getUserName($data["users_id"]),
'users_id' => $usernames[$users_id],
'consumed' => $data['consumed'],
'itemtype' => PluginCreditTicket::class,
]);
Expand All @@ -271,10 +286,6 @@ public static function showForTicket(Ticket $ticket)
'entries' => $entries,
'massiveactionparams' => $massiveactionparams ?? [],
]);

$Entity = new Entity();
$Entity->getFromDB($ticket->fields['entities_id']);
PluginCreditEntity::showForItemtype($Entity, 'Ticket');
}

/**
Expand Down Expand Up @@ -381,38 +392,86 @@ public static function displayVoucherInTicketProcessingForm($params)
*/
public static function displayConsumed($ID)
{
/** @var DBmysql $DB */
global $DB;

$consumed_credits = self::getConsumedForCreditEntity($ID);
$tickets_data = [];

if ($consumed_credits > 0) {
foreach (self::getAllForCreditEntity($ID) as $data) {
// Tickets and categories come with the consumptions, in a single query.
$iterator = $DB->request([
'SELECT' => [
'glpi_tickets.*',
'glpi_itilcategories.name AS credit_category_name',
'glpi_itilcategories.completename AS credit_category_completename',
self::getTable() . '.date_creation AS credit_date_creation',
self::getTable() . '.users_id AS credit_users_id',
self::getTable() . '.consumed AS credit_consumed',
],
'FROM' => self::getTable(),
'LEFT JOIN' => [
'glpi_tickets' => [
'ON' => [
'glpi_tickets' => 'id',
self::getTable() => 'tickets_id',
],
],
'glpi_itilcategories' => [
'ON' => [
'glpi_itilcategories' => 'id',
'glpi_tickets' => 'itilcategories_id',
],
],
],
'WHERE' => [
self::getTable() . '.plugin_credit_entities_id' => $ID,
],
'ORDER' => [self::getTable() . '.id DESC'],
]);

$showuserlink = Session::haveRight('user', READ);
$usernames = [];

foreach ($iterator as $data) {
$Ticket = new Ticket();
$Ticket->getFromDB($data['tickets_id']);
$ticket_link = '';
$status = '';
$type = '';
// The ticket may have been purged since the consumption.
if ($data['id'] !== null) {
$Ticket->fields = $data;
$ticket_link = sprintf(
'<a href="%s" target="_blank" rel="noopener noreferrer">%s</a>',
htmlescape($Ticket->getLinkURL()),
htmlescape($Ticket->getNameID()),
);
$status = Ticket::getStatus($data['status']);
$type = Ticket::getTicketTypeName($data['type']);
}

$itilcat = new ITILCategory();
$category = __('None');
if ($itilcat->getFromDB($Ticket->fields['itilcategories_id'])) {
$category = $itilcat->getName(['comments' => true]);
if ($data['credit_category_name'] !== null) {
$itilcat = new ITILCategory();
$itilcat->fields = [
'id' => $data['itilcategories_id'],
'name' => $data['credit_category_name'],
'completename' => $data['credit_category_completename'],
];
$category = $itilcat->getName();
}

$showuserlink = Session::haveRight('user', READ) ? 1 : 0;

$ticket_url = $Ticket->getLinkURL();
$ticket_name = $Ticket->getNameID();
$username = $showuserlink !== 0 ? getUserLink($data['users_id']) : getUserName($data['users_id']);
$users_id = $data['credit_users_id'];
$usernames[$users_id] ??= $showuserlink ? getUserLink($users_id) : getUserName($users_id);

$tickets_data[] = [
'ticket_link' => sprintf(
'<a href="%s" target="_blank" rel="noopener noreferrer">%s</a>',
htmlescape($ticket_url),
htmlescape($ticket_name),
),
'status' => Ticket::getStatus($Ticket->fields['status']),
'type' => Ticket::getTicketTypeName($Ticket->fields['type']),
'ticket_link' => $ticket_link,
'status' => $status,
'type' => $type,
'category' => $category,
'date_creation' => $data["date_creation"],
'username' => $username,
'consumed' => $data['consumed'],
'date_creation' => $data['credit_date_creation'],
'username' => $usernames[$users_id],
'consumed' => $data['credit_consumed'],
];
}
}
Expand Down
9 changes: 9 additions & 0 deletions inc/ticketconfig.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,15 @@ public static function updateConfig(Ticket $ticket)
}
}

/**
* Remove the default voucher options of a purged ticket.
* Consumptions are kept: they still count against their voucher.
*/
public static function cleanForTicket(Ticket $ticket): void
{
(new self())->deleteByCriteria(['tickets_id' => $ticket->getID()]);
}

public static function install(Migration $migration)
{
/** @var DBmysql $DB */
Expand Down
9 changes: 6 additions & 3 deletions setup.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,14 @@ function plugin_init_credit()
/** @var array $PLUGIN_HOOKS */
global $PLUGIN_HOOKS;

$plugin = new Plugin();

$PLUGIN_HOOKS['csrf_compliant']['credit'] = true;

if (Session::getLoginUserID() && $plugin->isActivated('credit')) {
// Outside the session check: purges also run from cron and CLI.
$PLUGIN_HOOKS['item_purge']['credit'] = [
'Ticket' => ['PluginCreditTicketConfig', 'cleanForTicket'],
];

if (Session::getLoginUserID()) {
Plugin::registerClass(
'PluginCreditEntity',
[
Expand Down
Loading