Skip to content

Bugfix: Validation consumption #225

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
giovanny07 wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Bugfix: Validation consumption #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4513f81
fix credit consumption validation
giovanny07 Apr 24, 2026
a240bfc
update changelog for credit validation fix
giovanny07 Apr 24, 2026
9f28a7f
address credit validation review
giovanny07 May 4, 2026
9661e81
fix rector ticket class reference
giovanny07 Jun 22, 2026
976bf03
add credit validation tests
giovanny07 Jun 23, 2026
862b9db
fix ticket right class reference
giovanny07 Jun 23, 2026
ddbf3fd
fix phpunit bootstrap initialization
giovanny07 Jun 23, 2026
9970336
fix credit date test fixtures
giovanny07 Jun 23, 2026
bec00c8
skip normalized voucher dates in tests
giovanny07 Jun 23, 2026
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Improve consumed credits modal readability and open related tickets in a new tab
- Show credit vouchers list in entity tab for read-only users while keeping add/config forms restricted to editable contexts.
- Improve credit validation and voucher handling.

## [1.15.2] - 2025-12-22

Expand Down
45 changes: 17 additions & 28 deletions front/ticket.form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,36 +31,25 @@

use Glpi\Exception\Http\BadRequestHttpException;

Session::haveRight("ticket", UPDATE);

@Rom1-B Rom1-B Apr 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
Session::haveRight("ticket", UPDATE);
Session::checkRight(\Ticket::class, UPDATE);

Session::checkRight(\Ticket::class, UPDATE);

$PluginCreditTicket = new PluginCreditTicket();
if ($_REQUEST['plugin_credit_entities_id'] == 0) {
Session::addMessageAfterRedirect(
__s('Credit voucher entity must be selected.', 'credit'),
true,
ERROR,
);
Html::back();
} elseif ($_REQUEST['plugin_credit_quantity'] == 0) {
Session::addMessageAfterRedirect(
__s('Credit voucher quantity must be greater than 0.', 'credit'),
true,
ERROR,
);
Html::back();
Comment on lines -37 to -50

@Rom1-B Rom1-B Apr 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why remove these checks?

}
$input = [
'tickets_id' => $_REQUEST['tickets_id'],
'plugin_credit_entities_id' => $_REQUEST['plugin_credit_entities_id'],
'consumed' => $_REQUEST['plugin_credit_quantity'],
'users_id' => Session::getLoginUserID(),
];
if ($PluginCreditTicket->add($input)) {
Session::addMessageAfterRedirect(
__s('Credit voucher successfully added.', 'credit'),
true,
INFO,
);
if (isset($_POST["add"])) {
$input = [
'tickets_id' => $_POST['tickets_id'] ?? null,
'plugin_credit_entities_id' => $_POST['plugin_credit_entities_id'] ?? null,
'consumed' => $_POST['plugin_credit_quantity'] ?? null,
'users_id' => Session::getLoginUserID(),
];

if ($PluginCreditTicket->add($input)) {
Session::addMessageAfterRedirect(
__s('Credit voucher successfully added.', 'credit'),
true,
INFO,
);
}

Html::back();
}

Expand Down
37 changes: 28 additions & 9 deletions inc/entity.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,14 +569,29 @@ public static function getActiveFilter()
global $DB;
return [
'glpi_plugin_credit_entities.is_active' => 1,
'OR' => [
'glpi_plugin_credit_entities.end_date' => null,
new QueryExpression(
sprintf(
'NOW() < %s',
$DB->quoteName('glpi_plugin_credit_entities.end_date'),
),
),
'AND' => [
[
'OR' => [
'glpi_plugin_credit_entities.begin_date' => null,
new QueryExpression(
sprintf(
'%s <= NOW()',
$DB->quoteName('glpi_plugin_credit_entities.begin_date'),
),
),
],
],
[
'OR' => [
'glpi_plugin_credit_entities.end_date' => null,
new QueryExpression(
sprintf(
'NOW() <= %s',
$DB->quoteName('glpi_plugin_credit_entities.end_date'),
),
),
],
],
],
];
}
Expand All @@ -591,9 +606,13 @@ public static function getMaximumConsumptionForCredit(int $credit_id)
'FROM' => 'glpi_plugin_credit_entities',
'WHERE' => [
'id' => $credit_id,
],
] + self::getActiveFilter(),
];
$entity_result = $DB->request($entity_query)->current();
if ($entity_result === false) {
return 0;
}

$overconsumption_allowed = $entity_result['overconsumption_allowed'];
$quantity_sold = (int) $entity_result['quantity'];

Expand Down
211 changes: 168 additions & 43 deletions inc/ticket.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,169 @@ public static function getConsumedForCreditEntity($ID)
return $tot;
}

private static function canUpdateCreditsForTicket(Ticket $ticket): bool
{
if (Session::haveRight(Entity::$rightname, UPDATE)) {
return true;
}

return $ticket->canEdit($ticket->getID())
&& !in_array($ticket->fields['status'], array_merge(Ticket::getSolvedStatusArray(), Ticket::getClosedStatusArray()));
}

private static function checkInput(array $input, ?self $current_credit = null, bool $skip_status_check = false): array|false
{
$ticket_id = filter_var(
$input['tickets_id'] ?? null,
FILTER_VALIDATE_INT,
['options' => ['min_range' => 1]],
);
if ($ticket_id === false) {
Session::addMessageAfterRedirect(
__s('Ticket is mandatory.', 'credit'),
true,
ERROR,
);
return false;
}

$credit_id = filter_var(
$input['plugin_credit_entities_id'] ?? null,
FILTER_VALIDATE_INT,
['options' => ['min_range' => 1]],
);
if ($credit_id === false) {
Session::addMessageAfterRedirect(
__s('Credit voucher entity must be selected.', 'credit'),
true,
ERROR,
);
return false;
}

$consumed = filter_var(
$input['consumed'] ?? null,
FILTER_VALIDATE_INT,
['options' => ['min_range' => 1]],
);
if ($consumed === false) {
Session::addMessageAfterRedirect(
__s('Credit voucher quantity must be greater than 0.', 'credit'),
true,
ERROR,
);
return false;
}

$ticket = new Ticket();
if (
!$ticket->getFromDB($ticket_id)
|| !$ticket->can($ticket_id, READ)
|| (!$skip_status_check && !self::canUpdateCreditsForTicket($ticket))
) {
Comment on lines +247 to +252

@stonebuzz stonebuzz Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
$ticket = new Ticket();
if (
!$ticket->getFromDB($ticket_id)
|| !$ticket->can($ticket_id, READ)
|| !self::canUpdateCreditsForTicket($ticket)
) {
$ticket = new Ticket();
if (
!$ticket->getFromDB($ticket_id)
|| !$ticket->can($ticket_id, READ)
|| (!$skip_status_check && !self::canUpdateCreditsForTicket($ticket))
) {

Session::addMessageAfterRedirect(
__s('You do not have rights to update credit vouchers for this ticket.', 'credit'),
true,
ERROR,
);
return false;
}

$credit_entity = new PluginCreditEntity();
$credit_criteria = getEntitiesRestrictCriteria('', '', $ticket->getEntityID(), true);
$credit_criteria['id'] = $credit_id;
$credit_criteria += PluginCreditEntity::getActiveFilter();

if (!$credit_entity->getFromDBByCrit($credit_criteria)) {
Session::addMessageAfterRedirect(
__s('Selected credit voucher is not available for this ticket.', 'credit'),
true,
ERROR,
);
return false;
}

$quantity_sold = (int) $credit_entity->fields['quantity'];
$quantity_consumed = self::getConsumedForCreditEntity($credit_entity->getID());

if (
$current_credit instanceof self
&& (int) $current_credit->fields['plugin_credit_entities_id'] === $credit_entity->getID()
) {
$quantity_consumed = max(0, $quantity_consumed - (int) $current_credit->fields['consumed']);
}

$quantity_remaining = max(0, $quantity_sold - $quantity_consumed);

if (0 !== $quantity_sold && $quantity_remaining < $consumed) {
$message = sprintf(
__s('Quantity consumed exceeds remaining credits: %d', 'credit'),
$quantity_remaining,
);

if ($credit_entity->getField('overconsumption_allowed')) {
Session::addMessageAfterRedirect($message, true, WARNING);
} else {
Session::addMessageAfterRedirect($message, true, ERROR);
return false;
}
}

return [
'tickets_id' => $ticket->getID(),
'plugin_credit_entities_id' => $credit_entity->getID(),
'consumed' => $consumed,
];
}

public function prepareInputForAdd($input)
{
$skip_status_check = (bool) ($input['_skip_status_check'] ?? false);
unset($input['_skip_status_check']);

$validated_input = self::checkInput($input, null, $skip_status_check);
if ($validated_input === false) {
return false;
}

$input = $validated_input + $input;

return $input;
}

public function prepareInputForUpdate($input)
{
$credit = new self();
if (
!isset($input['id'])
|| filter_var($input['id'], FILTER_VALIDATE_INT, ['options' => ['min_range' => 1]]) === false
|| !$credit->getFromDB((int) $input['id'])
) {
Session::addMessageAfterRedirect(
__s('Unable to find the requested credit consumption.', 'credit'),
true,
ERROR,
);
return false;
}

$input += [
'tickets_id' => $credit->fields['tickets_id'],
'plugin_credit_entities_id' => $credit->fields['plugin_credit_entities_id'],
'consumed' => $credit->fields['consumed'],
];

$validated_input = self::checkInput($input, $credit);
if ($validated_input === false) {
return false;
}

$input = $validated_input + $input;
$input['users_id'] = $credit->fields['users_id'];

return $input;
}

/**
* Show credit vouchers consumed for a ticket
*
Expand All @@ -205,15 +368,7 @@ public static function showForTicket(Ticket $ticket)
return false;
}

$canedit = false;
if (Session::haveRight(Entity::$rightname, UPDATE)) {
$canedit = true; // Entity admin has always right to update credits
} elseif (
$ticket->canEdit($ID)
&& !in_array($ticket->fields['status'], array_merge(Ticket::getSolvedStatusArray(), Ticket::getClosedStatusArray()))
) {
$canedit = true;
}
$canedit = self::canUpdateCreditsForTicket($ticket);

$number = self::countForItem($ticket);
$rand = mt_rand();
Expand Down Expand Up @@ -480,44 +635,14 @@ public static function consumeVoucher(CommonDBTM $item)
return;
}

$credit_ticket = new self();

$credit_entity = new PluginCreditEntity();
$credit_entity->getFromDB($item->input['plugin_credit_entities_id']);

$quantity_sold = (int) $credit_entity->fields['quantity'];
$quantity_consumed = $credit_ticket->getConsumedForCreditEntity($item->input['plugin_credit_entities_id']);
$quantity_remaining = max(0, $quantity_sold - $quantity_consumed);

if (0 !== $quantity_sold && $quantity_remaining < $item->input['plugin_credit_quantity']) {
if ($credit_entity->getField('overconsumption_allowed')) {
Session::addMessageAfterRedirect(
sprintf(
__s('Quantity consumed exceeds remaining credits: %d', 'credit'),
$quantity_remaining,
),
true,
WARNING,
);
} else {
Session::addMessageAfterRedirect(
sprintf(
__s('Quantity consumed exceeds remaining credits: %d', 'credit'),
$quantity_remaining,
),
true,
ERROR,
);
return;
}
}

$input = [
'tickets_id' => $ticket->getID(),
'plugin_credit_entities_id' => $item->input['plugin_credit_entities_id'],
'consumed' => $item->input['plugin_credit_quantity'],
'plugin_credit_entities_id' => $item->input['plugin_credit_entities_id'] ?? null,
'consumed' => $item->input['plugin_credit_quantity'] ?? null,
'users_id' => Session::getLoginUserID(),
'_skip_status_check' => true,
];
$credit_ticket = new self();
if ($credit_ticket->add($input)) {
Session::addMessageAfterRedirect(
__s('Credit voucher successfully added.', 'credit'),
Expand Down
8 changes: 8 additions & 0 deletions phpunit.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit bootstrap="tests/bootstrap.php" colors="true">
<testsuites>
<testsuite name="Credit plugin tests">
<directory>tests</directory>
</testsuite>
</testsuites>
</phpunit>
4 changes: 4 additions & 0 deletions templates/tickets/form.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,15 @@
}

if (val == 0) {
quantity.closest('.form-field').classList.add('d-none');
quantity.disabled = true;
quantity.max = 0;
quantity.value = 0;
form_help.setAttribute('data-bs-title', __('Maximum consumable quantity', 'credit') + ' : 0 ');
form_help.setAttribute('data-bs-content', __('Maximum consumable quantity', 'credit') + ' : 0 ');
} else {
quantity.closest('.form-field').classList.remove('d-none');
quantity.disabled = false;
$.ajax({
type: 'POST',
url: '{{ path('plugins/credit/ajax/dropdownQuantity.php') }}',
Expand Down
Loading