diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ba2cce..b525a72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Added + +- Credit vouchers can be restricted to incidents and/or requests (both enabled by default) + ### Fixed - Fix access denied error when consuming a credit voucher from the ticket Credit tab diff --git a/front/ticket.form.php b/front/ticket.form.php index c07fe19..753dc96 100644 --- a/front/ticket.form.php +++ b/front/ticket.form.php @@ -63,6 +63,15 @@ throw new AccessDeniedHttpException(); } +if (!PluginCreditEntity::canBeUsedForTicket((int) $_REQUEST['plugin_credit_entities_id'], $ticket)) { + Session::addMessageAfterRedirect( + __s('This credit voucher cannot be used for this type of ticket', 'credit'), + true, + ERROR, + ); + Html::back(); +} + $input = [ 'tickets_id' => $_REQUEST['tickets_id'], diff --git a/inc/entity.class.php b/inc/entity.class.php index 3cd4367..87c03c0 100644 --- a/inc/entity.class.php +++ b/inc/entity.class.php @@ -197,6 +197,8 @@ public static function showForItemtype(Entity $entity, $itemtype = 'Entity') 'quantity_remaining' => __('Quantity remaining', 'credit'), 'overconsumption_allowed' => __('Allow overconsumption', 'credit'), 'low_credit_alert' => __('Low credits alert', 'credit'), + 'is_for_incidents' => __('Usable for incidents', 'credit'), + 'is_for_requests' => __('Usable for requests', 'credit'), 'entities_id' => __('Entity'), 'is_recursive' => __('Child entities'), ]; @@ -342,6 +344,22 @@ public function rawSearchOptions() 'unit' => '%', ]; + $tab[] = [ + 'id' => 998, + 'table' => self::getTable(), + 'field' => 'is_for_incidents', + 'name' => __('Usable for incidents', 'credit'), + 'datatype' => 'bool', + ]; + + $tab[] = [ + 'id' => 999, + 'table' => self::getTable(), + 'field' => 'is_for_requests', + 'name' => __('Usable for requests', 'credit'), + 'datatype' => 'bool', + ]; + $tab[] = [ 'id' => 80, 'table' => 'glpi_entities', @@ -544,6 +562,8 @@ public static function install(Migration $migration) `quantity` int NOT NULL DEFAULT '0', `overconsumption_allowed` tinyint NOT NULL DEFAULT '0', `low_credit_alert` int DEFAULT NULL, + `is_for_incidents` tinyint NOT NULL DEFAULT '1', + `is_for_requests` tinyint NOT NULL DEFAULT '1', PRIMARY KEY (`id`), KEY `name` (`name`), KEY `entities_id` (`entities_id`), @@ -568,6 +588,10 @@ public static function install(Migration $migration) // 1.13.2 $migration->addField($table, 'low_credit_alert', 'int', ['update' => "NULL"]); + + // Restrict vouchers to incidents and/or requests + $migration->addField($table, 'is_for_incidents', 'bool', ['value' => '1', 'update' => '1']); + $migration->addField($table, 'is_for_requests', 'bool', ['value' => '1', 'update' => '1']); } return true; @@ -604,6 +628,99 @@ public static function getActiveFilter() ]; } + /** + * Get criteria restricting credit vouchers to those usable for the given ticket type. + * + * @param int $ticket_type Ticket type (Ticket::INCIDENT_TYPE or Ticket::DEMAND_TYPE) + * + * @return array + */ + public static function getTicketTypeFilter(int $ticket_type): array + { + return match ($ticket_type) { + Ticket::INCIDENT_TYPE => ['glpi_plugin_credit_entities.is_for_incidents' => 1], + Ticket::DEMAND_TYPE => ['glpi_plugin_credit_entities.is_for_requests' => 1], + default => [], + }; + } + + /** + * Get criteria restricting credit vouchers to active ones usable for the given ticket. + * + * @param Ticket $ticket + * + * @return array + */ + public static function getActiveFilterForTicket(Ticket $ticket): array + { + return array_merge( + self::getActiveFilter(), + self::getTicketTypeFilter((int) ($ticket->fields['type'] ?? 0)), + ); + } + + /** + * Check if at least one active credit voucher can be consumed on the given ticket. + * + * @param Ticket $ticket + * + * @return bool + */ + public static function hasUsableVoucherForTicket(Ticket $ticket): bool + { + return countElementsInTable(self::getTable(), [ + 'WHERE' => [ + getEntitiesRestrictCriteria(self::getTable(), '', $ticket->getEntityID(), true), + self::getActiveFilterForTicket($ticket), + ], + ]) > 0; + } + + /** + * Check if the credit voucher can be used for the given ticket type. + * + * @param int $ticket_type Ticket type (Ticket::INCIDENT_TYPE or Ticket::DEMAND_TYPE) + * + * @return bool + */ + public function isUsableForTicketType(int $ticket_type): bool + { + return match ($ticket_type) { + Ticket::INCIDENT_TYPE => (bool) $this->fields['is_for_incidents'], + Ticket::DEMAND_TYPE => (bool) $this->fields['is_for_requests'], + default => true, + }; + } + + /** + * Check if the credit voucher can be used for the given ticket, based on the ticket type. + * + * @param int $credit_id ID of the credit voucher + * @param Ticket $ticket + * + * @return bool + */ + public static function canBeUsedForTicket(int $credit_id, Ticket $ticket): bool + { + return self::canBeUsedForTicketType($credit_id, (int) ($ticket->fields['type'] ?? 0)); + } + + /** + * Check if the credit voucher can be used for the given ticket type. + * + * @param int $credit_id ID of the credit voucher + * @param int $ticket_type Ticket type (Ticket::INCIDENT_TYPE or Ticket::DEMAND_TYPE) + * + * @return bool + */ + public static function canBeUsedForTicketType(int $credit_id, int $ticket_type): bool + { + $credit_entity = new self(); + + return $credit_entity->getFromDB($credit_id) + && $credit_entity->isUsableForTicketType($ticket_type); + } + /** * Check if current user can consume a credit voucher, based on the voucher entity. * diff --git a/inc/ticket.class.php b/inc/ticket.class.php index 16aaea0..e543af9 100644 --- a/inc/ticket.class.php +++ b/inc/ticket.class.php @@ -265,7 +265,7 @@ public static function showForTicket(Ticket $ticket) 'type_name' => self::getTypeName(2), 'creditentityclass' => PluginCreditEntity::class, 'form_url' => self::getFormUrl(), - 'conditions' => PluginCreditEntity::getActiveFilter(), + 'conditions' => PluginCreditEntity::getActiveFilterForTicket($ticket), 'canedit' => $canedit, 'ID' => $ID, 'entries' => $entries, @@ -327,6 +327,11 @@ public static function displayVoucherInTicketProcessingForm($params) return; } + // No voucher can be consumed for this ticket type in this entity + if (!PluginCreditEntity::hasUsableVoucherForTicket($ticket)) { + return; + } + $canedit = $ticket->canEdit($ticket->getID()); if ( in_array($ticket->fields['status'], Ticket::getSolvedStatusArray()) @@ -351,10 +356,14 @@ public static function displayVoucherInTicketProcessingForm($params) if ($canedit) { //get default value for ticket $default_credit = PluginCreditTicketConfig::getDefaultForTicket($ticket->getID(), $item->getType()); - if ($default_credit == 0) { + if ($default_credit == 0 || !PluginCreditEntity::canBeUsedForTicket($default_credit, $ticket)) { //get default value for entity $default_credit = PluginCreditEntityConfig::getDefaultForEntityAndType($ticket->getEntityID(), $item->getType()); } + if ($default_credit != 0 && !PluginCreditEntity::canBeUsedForTicket($default_credit, $ticket)) { + // Default voucher cannot be used for this ticket type + $default_credit = 0; + } if ($default_credit != 0) { $max = PluginCreditEntity::getMaximumConsumptionForCredit($default_credit); @@ -368,7 +377,7 @@ public static function displayVoucherInTicketProcessingForm($params) 'default_credit_max' => $max ?? 0, 'entity_id' => $ticket->getEntityID(), 'type_name' => self::getTypeName(2), - 'condition' => PluginCreditEntity::getActiveFilter(), + 'condition' => PluginCreditEntity::getActiveFilterForTicket($ticket), 'creditentityclass' => PluginCreditEntity::class, 'plugin_credit_geturl' => plugin_credit_geturl(), ]); @@ -496,6 +505,15 @@ public static function consumeVoucher(CommonDBTM $item) return; } + if (!$credit_entity->isUsableForTicketType((int) $ticket->fields['type'])) { + Session::addMessageAfterRedirect( + __s('This credit voucher cannot be used for this type of ticket', 'credit'), + true, + ERROR, + ); + return; + } + $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); @@ -538,6 +556,57 @@ public static function consumeVoucher(CommonDBTM $item) } } + public function prepareInputForAdd($input) + { + if (!$this->checkVoucherTicketType($input)) { + return false; + } + + return parent::prepareInputForAdd($input); + } + + public function prepareInputForUpdate($input) + { + $relation_changed = false; + foreach (['tickets_id', 'plugin_credit_entities_id'] as $field) { + if (array_key_exists($field, $input) && $input[$field] != $this->fields[$field]) { + $relation_changed = true; + } + } + + if ($relation_changed && !$this->checkVoucherTicketType($input + $this->fields)) { + return false; + } + + return parent::prepareInputForUpdate($input); + } + + /** + * Check that the credit voucher can be consumed on the ticket type. + * + * @param array $input Input containing `tickets_id` and `plugin_credit_entities_id` keys + * + * @return bool + */ + private function checkVoucherTicketType(array $input): bool + { + $ticket = new Ticket(); + if (!$ticket->getFromDB((int) ($input['tickets_id'] ?? 0))) { + return true; + } + + if (PluginCreditEntity::canBeUsedForTicket((int) ($input['plugin_credit_entities_id'] ?? 0), $ticket)) { + return true; + } + + Session::addMessageAfterRedirect( + __s('This credit voucher cannot be used for this type of ticket', 'credit'), + true, + ERROR, + ); + return false; + } + public function rawSearchOptions() { $tab = parent::rawSearchOptions(); diff --git a/inc/ticketconfig.class.php b/inc/ticketconfig.class.php index 1ce4d52..b3e1caa 100644 --- a/inc/ticketconfig.class.php +++ b/inc/ticketconfig.class.php @@ -125,7 +125,7 @@ public static function showForTicket(Ticket $ticket, bool $embed_in_ticket_form 'uncollapsed' => $uncollapsed ?? false, 'type_name' => self::getTypeName(), 'creditentitytype' => PluginCreditEntity::class, - 'condition' => PluginCreditEntity::getActiveFilter(), + 'condition' => PluginCreditEntity::getActiveFilterForTicket($ticket), 'entity_id' => $ticket->getEntityID(), 'ticket' => $ticket, 'ticket_config' => $ticket_config, @@ -133,6 +133,59 @@ public static function showForTicket(Ticket $ticket, bool $embed_in_ticket_form ]); } + public function prepareInputForAdd($input) + { + return parent::prepareInputForAdd($this->removeUnusableDefaults($input)); + } + + public function prepareInputForUpdate($input) + { + return parent::prepareInputForUpdate($this->removeUnusableDefaults($input)); + } + + /** + * Reset default vouchers that cannot be used for the ticket type. + * + * @param array $input + * + * @return array + */ + private function removeUnusableDefaults(array $input): array + { + if (array_key_exists('_ticket_type', $input)) { + $ticket_type = (int) $input['_ticket_type']; + } else { + $ticket = new Ticket(); + $tickets_id = $input['tickets_id'] ?? $this->fields['tickets_id'] ?? 0; + if (!$ticket->getFromDB((int) $tickets_id)) { + return $input; + } + $ticket_type = (int) $ticket->fields['type']; + } + + $config_fields = [ + 'plugin_credit_entities_id_followups', + 'plugin_credit_entities_id_tasks', + 'plugin_credit_entities_id_solutions', + ]; + foreach ($config_fields as $field) { + if ( + array_key_exists($field, $input) + && (int) $input[$field] !== 0 + && !PluginCreditEntity::canBeUsedForTicketType((int) $input[$field], $ticket_type) + ) { + $input[$field] = 0; + Session::addMessageAfterRedirect( + __s('This credit voucher cannot be used for this type of ticket', 'credit'), + true, + WARNING, + ); + } + } + + return $input; + } + public static function updateConfig(Ticket $ticket) { if (!Session::haveRight(self::$rightname, self::TICKET_FORM)) { @@ -156,6 +209,8 @@ public static function updateConfig(Ticket $ticket) return; } $input['tickets_id'] = $ticket->getID(); + // Ticket type may be changed in the same form, and is not yet stored in DB on `pre_item_update` + $input['_ticket_type'] = $ticket->input['type'] ?? $ticket->fields['type'] ?? 0; $ticket_config = new self(); if ($ticket_config->getFromDBByCrit(['tickets_id' => $ticket->getID()])) { diff --git a/templates/creditentity.hmtl.twig b/templates/creditentity.hmtl.twig index 1e97baa..2b016cc 100644 --- a/templates/creditentity.hmtl.twig +++ b/templates/creditentity.hmtl.twig @@ -109,6 +109,18 @@ 'unit': '%' } ) }} + + {{ fields.dropdownYesNo( + 'is_for_incidents', + 1, + __('Usable for incidents', 'credit'), + ) }} + + {{ fields.dropdownYesNo( + 'is_for_requests', + 1, + __('Usable for requests', 'credit'), + ) }}
@@ -137,6 +149,8 @@ 'overconsumption_allowed': 'yesno', 'is_active': 'yesno', 'is_recursive': 'yesno', + 'is_for_incidents': 'yesno', + 'is_for_requests': 'yesno', 'entities_id': 'raw_html', 'quantity_consumed': 'raw_html', }, diff --git a/templates/creditentity_form.html.twig b/templates/creditentity_form.html.twig index 5207a92..931d224 100644 --- a/templates/creditentity_form.html.twig +++ b/templates/creditentity_form.html.twig @@ -69,6 +69,20 @@ field_options ) }} + {{ fields.dropdownYesNo( + 'is_for_incidents', + item.fields['is_for_incidents'], + __('Usable for incidents', 'credit'), + field_options + ) }} + + {{ fields.dropdownYesNo( + 'is_for_requests', + item.fields['is_for_requests'], + __('Usable for requests', 'credit'), + field_options + ) }} +
{{ __('Validity') }} diff --git a/tests/Units/TicketTypeVoucherTest.php b/tests/Units/TicketTypeVoucherTest.php new file mode 100644 index 0000000..eeac014 --- /dev/null +++ b/tests/Units/TicketTypeVoucherTest.php @@ -0,0 +1,243 @@ +. + * ------------------------------------------------------------------------- + * @author François Legastelois + * @copyright Copyright (C) 2017-2023 by Credit plugin team. + * @license GPLv3 https://www.gnu.org/licenses/gpl-3.0.html + * @link https://github.com/pluginsGLPI/credit + * ------------------------------------------------------------------------- + */ + +namespace GlpiPlugin\Credit\Tests\Units; + +use Entity; +use Glpi\Tests\DbTestCase; +use ITILFollowup; +use PluginCreditEntity; +use PluginCreditTicket; +use PluginCreditTicketConfig; +use Ticket; + +final class TicketTypeVoucherTest extends DbTestCase +{ + public function testVoucherIsUsableForBothTicketTypesByDefault(): void + { + $this->login(); + + $credit = $this->createItem(PluginCreditEntity::class, [ + 'name' => 'Default voucher', + 'entities_id' => 0, + 'is_active' => 1, + ]); + + $this->assertTrue($credit->isUsableForTicketType(Ticket::INCIDENT_TYPE)); + $this->assertTrue($credit->isUsableForTicketType(Ticket::DEMAND_TYPE)); + } + + public function testVoucherConsumptionIsRejectedForDisabledTicketType(): void + { + $this->login(); + + $credit_id = $this->createItem(PluginCreditEntity::class, [ + 'name' => 'Requests only voucher', + 'entities_id' => 0, + 'is_recursive' => 1, + 'is_active' => 1, + 'quantity' => 10, + 'is_for_incidents' => 0, + 'is_for_requests' => 1, + ])->getID(); + + $incident_id = $this->createItem(Ticket::class, [ + 'name' => 'Incident', + 'content' => 'Test', + 'entities_id' => 0, + 'type' => Ticket::INCIDENT_TYPE, + ])->getID(); + + $request_id = $this->createItem(Ticket::class, [ + 'name' => 'Request', + 'content' => 'Test', + 'entities_id' => 0, + 'type' => Ticket::DEMAND_TYPE, + ])->getID(); + + foreach ([$incident_id, $request_id] as $ticket_id) { + $followup = new ITILFollowup(); + $this->assertGreaterThan(0, $followup->add([ + 'itemtype' => Ticket::class, + 'items_id' => $ticket_id, + 'content' => 'Followup consuming a voucher', + ])); + + // `item_add` hooks are registered on plugin init, which occurs before login in tests, + // so the hook is called explicitly. + $followup->input = [ + 'plugin_credit_consumed_voucher' => 1, + 'plugin_credit_entities_id' => $credit_id, + 'plugin_credit_quantity' => 1, + ]; + PluginCreditTicket::consumeVoucher($followup); + } + + $credit_ticket = new PluginCreditTicket(); + $this->assertFalse( + $credit_ticket->getFromDBByCrit(['tickets_id' => $incident_id]), + 'Voucher consumption must not be recorded on an incident when the voucher is not usable for incidents', + ); + $this->assertTrue( + $credit_ticket->getFromDBByCrit(['tickets_id' => $request_id]), + 'Voucher consumption must be recorded on a request when the voucher is usable for requests', + ); + } + + public function testConsumptionCannotBeAddedOrMovedToVoucherNotUsableForTicketType(): void + { + $this->login(); + + $incidents_credit_id = $this->createItem(PluginCreditEntity::class, [ + 'name' => 'Incidents only voucher', + 'entities_id' => 0, + 'is_recursive' => 1, + 'is_active' => 1, + 'is_for_incidents' => 1, + 'is_for_requests' => 0, + ])->getID(); + + $requests_credit_id = $this->createItem(PluginCreditEntity::class, [ + 'name' => 'Requests only voucher', + 'entities_id' => 0, + 'is_recursive' => 1, + 'is_active' => 1, + 'is_for_incidents' => 0, + 'is_for_requests' => 1, + ])->getID(); + + $incident_id = $this->createItem(Ticket::class, [ + 'name' => 'Incident', + 'content' => 'Test', + 'entities_id' => 0, + 'type' => Ticket::INCIDENT_TYPE, + ])->getID(); + + $credit_ticket = new PluginCreditTicket(); + $this->assertFalse($credit_ticket->add([ + 'tickets_id' => $incident_id, + 'plugin_credit_entities_id' => $requests_credit_id, + 'consumed' => 1, + ])); + + $credit_ticket_id = $credit_ticket->add([ + 'tickets_id' => $incident_id, + 'plugin_credit_entities_id' => $incidents_credit_id, + 'consumed' => 1, + ]); + $this->assertGreaterThan(0, $credit_ticket_id); + + // Quantity can still be updated + $this->assertTrue($credit_ticket->update(['id' => $credit_ticket_id, 'consumed' => 2])); + + // Voucher cannot be changed to one not usable for incidents + $this->assertFalse($credit_ticket->update([ + 'id' => $credit_ticket_id, + 'plugin_credit_entities_id' => $requests_credit_id, + ])); + $this->assertTrue($credit_ticket->getFromDB($credit_ticket_id)); + $this->assertEquals($incidents_credit_id, $credit_ticket->fields['plugin_credit_entities_id']); + } + + public function testTicketDefaultVoucherNotUsableForTicketTypeIsReset(): void + { + $this->login(); + + $incidents_credit_id = $this->createItem(PluginCreditEntity::class, [ + 'name' => 'Incidents only voucher', + 'entities_id' => 0, + 'is_recursive' => 1, + 'is_active' => 1, + 'is_for_incidents' => 1, + 'is_for_requests' => 0, + ])->getID(); + + $request_id = $this->createItem(Ticket::class, [ + 'name' => 'Request', + 'content' => 'Test', + 'entities_id' => 0, + 'type' => Ticket::DEMAND_TYPE, + ])->getID(); + + $ticket_config = new PluginCreditTicketConfig(); + $ticket_config_id = $ticket_config->add([ + 'tickets_id' => $request_id, + 'plugin_credit_entities_id_followups' => $incidents_credit_id, + ]); + $this->assertGreaterThan(0, $ticket_config_id); + $this->assertTrue($ticket_config->getFromDB($ticket_config_id)); + $this->assertEquals(0, $ticket_config->fields['plugin_credit_entities_id_followups']); + + // Type given explicitly (ticket form, type changed in the same submission) + $this->assertTrue($ticket_config->update([ + 'id' => $ticket_config_id, + 'plugin_credit_entities_id_followups' => $incidents_credit_id, + '_ticket_type' => Ticket::INCIDENT_TYPE, + ])); + $this->assertTrue($ticket_config->getFromDB($ticket_config_id)); + $this->assertEquals($incidents_credit_id, $ticket_config->fields['plugin_credit_entities_id_followups']); + } + + public function testHasUsableVoucherForTicketDependsOnTicketType(): void + { + $this->login(); + + $entity_id = $this->createItem(Entity::class, [ + 'name' => 'Incidents only entity', + 'entities_id' => 0, + ])->getID(); + + $this->createItem(PluginCreditEntity::class, [ + 'name' => 'Incidents only voucher', + 'entities_id' => $entity_id, + 'is_active' => 1, + 'is_for_incidents' => 1, + 'is_for_requests' => 0, + ]); + + $incident = $this->createItem(Ticket::class, [ + 'name' => 'Incident', + 'content' => 'Test', + 'entities_id' => $entity_id, + 'type' => Ticket::INCIDENT_TYPE, + ]); + + $request = $this->createItem(Ticket::class, [ + 'name' => 'Request', + 'content' => 'Test', + 'entities_id' => $entity_id, + 'type' => Ticket::DEMAND_TYPE, + ]); + + $this->assertTrue(PluginCreditEntity::hasUsableVoucherForTicket($incident)); + $this->assertFalse(PluginCreditEntity::hasUsableVoucherForTicket($request)); + } +}