From 61e6855a7f837072f9d60dbf71823d125093e082 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Mon, 20 Jul 2026 13:48:10 +0200 Subject: [PATCH 01/26] Add ReservationQuestionConfig --- .../ReservationQuestionConfig.php | 89 +++++++++++++++++++ .../ReservationQuestionConfigTest.php | 65 ++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 src/Model/QuestionType/ReservationQuestionConfig.php create mode 100644 tests/Model/QuestionType/ReservationQuestionConfigTest.php diff --git a/src/Model/QuestionType/ReservationQuestionConfig.php b/src/Model/QuestionType/ReservationQuestionConfig.php new file mode 100644 index 0000000..ec85d65 --- /dev/null +++ b/src/Model/QuestionType/ReservationQuestionConfig.php @@ -0,0 +1,89 @@ + + * } $data + */ + #[Override] + public static function jsonDeserialize(array $data): self + { + return new self( + allowed_itemtypes: $data[self::ALLOWED_ITEMTYPES] ?? [], + ); + } + + /** + * @return array{ + * allowed_itemtypes: array + * } $data + */ + #[Override] + public function jsonSerialize(): array + { + return [ + self::ALLOWED_ITEMTYPES => $this->allowed_itemtypes, + ]; + } + + public function getAllowedItemtypes(): array + { + return $this->allowed_itemtypes; + } + + public function getEffectiveAllowedItemtypes(): array + { + global $CFG_GLPI; + + if ($this->allowed_itemtypes !== []) { + return $this->allowed_itemtypes; + } + + return $CFG_GLPI['reservation_types'] ?? []; + } +} diff --git a/tests/Model/QuestionType/ReservationQuestionConfigTest.php b/tests/Model/QuestionType/ReservationQuestionConfigTest.php new file mode 100644 index 0000000..1097032 --- /dev/null +++ b/tests/Model/QuestionType/ReservationQuestionConfigTest.php @@ -0,0 +1,65 @@ +jsonSerialize(); + $this->assertSame(['allowed_itemtypes' => ['Computer', 'Monitor']], $serialized); + + $rebuilt = ReservationQuestionConfig::jsonDeserialize($serialized); + $this->assertSame(['Computer', 'Monitor'], $rebuilt->getAllowedItemtypes()); + } + + public function testEffectiveAllowedItemtypesFallsBackToConfiguredReservationTypes(): void + { + global $CFG_GLPI; + $CFG_GLPI['reservation_types'] = ['Computer', 'Monitor', 'Peripheral']; + + $config = new ReservationQuestionConfig([]); + $this->assertSame(['Computer', 'Monitor', 'Peripheral'], $config->getEffectiveAllowedItemtypes()); + } + + public function testEffectiveAllowedItemtypesUsesExplicitListWhenSet(): void + { + $config = new ReservationQuestionConfig(['Monitor']); + $this->assertSame(['Monitor'], $config->getEffectiveAllowedItemtypes()); + } +} From 9d622ca4134533ec864038c35f57f4fa659b30b9 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Mon, 20 Jul 2026 13:55:59 +0200 Subject: [PATCH 02/26] Add ReservationQuestionAnswer --- .../ReservationQuestionAnswer.php | 103 ++++++++++++++++++ .../ReservationQuestionAnswerTest.php | 47 ++++++++ 2 files changed, 150 insertions(+) create mode 100644 src/Model/QuestionType/ReservationQuestionAnswer.php create mode 100644 tests/Model/QuestionType/ReservationQuestionAnswerTest.php diff --git a/src/Model/QuestionType/ReservationQuestionAnswer.php b/src/Model/QuestionType/ReservationQuestionAnswer.php new file mode 100644 index 0000000..d4192f4 --- /dev/null +++ b/src/Model/QuestionType/ReservationQuestionAnswer.php @@ -0,0 +1,103 @@ + $this->reservationitems_id, + 'begin' => $this->begin, + 'end' => $this->end, + ]; + } + + public function getReservationItemsId(): int + { + return $this->reservationitems_id; + } + + public function getBegin(): string + { + return $this->begin; + } + + public function getEnd(): string + { + return $this->end; + } + + public function isValidRange(): bool + { + return strtotime($this->begin) < strtotime($this->end); + } +} diff --git a/tests/Model/QuestionType/ReservationQuestionAnswerTest.php b/tests/Model/QuestionType/ReservationQuestionAnswerTest.php new file mode 100644 index 0000000..ecfa345 --- /dev/null +++ b/tests/Model/QuestionType/ReservationQuestionAnswerTest.php @@ -0,0 +1,47 @@ + 123, + 'begin' => '2026-01-15 09:00:00', + 'end' => '2026-01-15 12:00:00', + ]); + + $this->assertSame(123, $answer->getReservationItemsId()); + $this->assertSame('2026-01-15 09:00:00', $answer->getBegin()); + $this->assertSame('2026-01-15 12:00:00', $answer->getEnd()); + $this->assertSame([ + 'reservationitems_id' => 123, + 'begin' => '2026-01-15 09:00:00', + 'end' => '2026-01-15 12:00:00', + ], $answer->toArray()); + } + + public function testFromArrayThrowsOnMissingKeys(): void + { + $this->expectException(InvalidArgumentException::class); + ReservationQuestionAnswer::fromArray(['reservationitems_id' => 123]); + } + + public function testIsValidRange(): void + { + $valid = ReservationQuestionAnswer::fromArray([ + 'reservationitems_id' => 1, 'begin' => '2026-01-15 09:00:00', 'end' => '2026-01-15 12:00:00', + ]); + $this->assertTrue($valid->isValidRange()); + + $invalid = ReservationQuestionAnswer::fromArray([ + 'reservationitems_id' => 1, 'begin' => '2026-01-15 12:00:00', 'end' => '2026-01-15 09:00:00', + ]); + $this->assertFalse($invalid->isValidRange()); + } +} From c283899ce5fe108cab550ce5b1914e6117bd9f30 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Mon, 20 Jul 2026 14:08:15 +0200 Subject: [PATCH 03/26] Add missing license header to ReservationQuestionAnswerTest --- .../ReservationQuestionAnswerTest.php | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/Model/QuestionType/ReservationQuestionAnswerTest.php b/tests/Model/QuestionType/ReservationQuestionAnswerTest.php index ecfa345..921ca82 100644 --- a/tests/Model/QuestionType/ReservationQuestionAnswerTest.php +++ b/tests/Model/QuestionType/ReservationQuestionAnswerTest.php @@ -1,5 +1,36 @@ Date: Mon, 20 Jul 2026 14:28:25 +0200 Subject: [PATCH 04/26] Add ReservationQuestion question type --- .../QuestionType/ReservationQuestion.php | 193 ++++++++++++++++++ src/Service/ConfigManager.php | 2 + .../reservation_config.html.twig | 53 +++++ .../QuestionType/ReservationQuestionTest.php | 115 +++++++++++ tests/Service/ConfigManagerTest.php | 10 + 5 files changed, 373 insertions(+) create mode 100644 src/Model/QuestionType/ReservationQuestion.php create mode 100644 templates/editor/question_types/reservation_config.html.twig create mode 100644 tests/Model/QuestionType/ReservationQuestionTest.php diff --git a/src/Model/QuestionType/ReservationQuestion.php b/src/Model/QuestionType/ReservationQuestion.php new file mode 100644 index 0000000..49f4068 --- /dev/null +++ b/src/Model/QuestionType/ReservationQuestion.php @@ -0,0 +1,193 @@ + $input */ + #[Override] + public function validateExtraDataInput(array $input): bool + { + // All fields of `ReservationQuestionConfig` are optional: an empty + // `allowed_itemtypes` list means "every reservable type is allowed". + return true; + } + + #[Override] + public function renderAdministrationTemplate(Question|null $question): string + { + // Read extra config specific to this question type + $decoded_extra_data = []; + if ($question instanceof Question && is_string($question->fields['extra_data'])) { + $decoded_extra_data = json_decode( + $question->fields['extra_data'], + associative: true, + ); + + // Fallback to safe value + if (!is_array($decoded_extra_data)) { + $decoded_extra_data = []; + } + } + + $config = $this->getExtraDataConfig($decoded_extra_data); + if (!$config instanceof JsonFieldInterface) { + $config = new ReservationQuestionConfig(); + } + + global $CFG_GLPI; + $reservation_types = []; + $configured_types = $CFG_GLPI['reservation_types'] ?? []; + if (is_array($configured_types)) { + foreach ($configured_types as $itemtype) { + if (is_string($itemtype) && class_exists($itemtype)) { + $reservation_types[$itemtype] = $itemtype::getTypeName(); + } + } + } + + $twig = TemplateRenderer::getInstance(); + return $twig->render( + '@advancedforms/editor/question_types/reservation_config.html.twig', + [ + 'question' => $question, + 'extra_data' => $config, + 'reservation_types' => $reservation_types, + 'ALLOWED_ITEMTYPES' => ReservationQuestionConfig::ALLOWED_ITEMTYPES, + ], + ); + } + + #[Override] + public function renderEndUserTemplate(Question $question): string + { + // Minimal placeholder: the full Select2/Flatpickr widget is wired in + // a later task. For now, expose the raw wire format as empty hidden + // inputs so the end user answer processing pipeline can be tested. + return TemplateRenderer::getInstance()->renderFromStringTemplate( + << + + + TWIG, + ['question' => $question], + ); + } + + #[Override] + public function prepareEndUserAnswer(Question $question, mixed $answer): mixed + { + if (!is_array($answer)) { + return null; + } + + return ReservationQuestionAnswer::fromArray($answer)->toArray(); + } + + #[Override] + public function formatRawAnswer(mixed $answer, Question $question): string + { + if (!is_array($answer)) { + return ''; + } + + $parsed = ReservationQuestionAnswer::fromArray($answer); + + return sprintf('%s → %s', $parsed->getBegin(), $parsed->getEnd()); + } + + #[Override] + public static function getConfigKey(): string + { + return "enable_question_type_reservation"; + } + + #[Override] + public function getConfigTitle(): string + { + return __("Material reservation question type", 'advancedforms'); + } + + #[Override] + public function getConfigDescription(): string + { + return __("Allow users to reserve equipment from a form.", 'advancedforms'); + } + + #[Override] + public function getConfigIcon(): string + { + return $this->getIcon(); + } +} diff --git a/src/Service/ConfigManager.php b/src/Service/ConfigManager.php index b83f04d..a488cc6 100644 --- a/src/Service/ConfigManager.php +++ b/src/Service/ConfigManager.php @@ -43,6 +43,7 @@ use GlpiPlugin\Advancedforms\Model\QuestionType\IpAddressQuestion; use GlpiPlugin\Advancedforms\Model\QuestionType\LdapQuestion; use GlpiPlugin\Advancedforms\Model\QuestionType\TableQuestion; +use GlpiPlugin\Advancedforms\Model\QuestionType\ReservationQuestion; use GlpiPlugin\Advancedforms\Model\QuestionType\TreeCascadeDropdownQuestion; final class ConfigManager @@ -68,6 +69,7 @@ public function getConfigurableQuestionTypes(): array new LdapQuestion(), new TreeCascadeDropdownQuestion(), new TableQuestion(), + new ReservationQuestion(), ]; } diff --git a/templates/editor/question_types/reservation_config.html.twig b/templates/editor/question_types/reservation_config.html.twig new file mode 100644 index 0000000..21413da --- /dev/null +++ b/templates/editor/question_types/reservation_config.html.twig @@ -0,0 +1,53 @@ +{# + # ------------------------------------------------------------------------- + # advancedforms plugin for GLPI + # ------------------------------------------------------------------------- + # + # MIT License + # + # Permission is hereby granted, free of charge, to any person obtaining a copy + # of this software and associated documentation files (the "Software"), to deal + # in the Software without restriction, including without limitation the rights + # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + # copies of the Software, and to permit persons to whom the Software is + # furnished to do so, subject to the following conditions: + # + # The above copyright notice and this permission notice shall be included in all + # copies or substantial portions of the Software. + # + # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + # SOFTWARE. + # ------------------------------------------------------------------------- + # @copyright Copyright (C) 2025 by the advancedforms plugin team. + # @license MIT https://opensource.org/licenses/mit-license.php + # @link https://github.com/pluginsGLPI/advancedforms + # ------------------------------------------------------------------------- + #} + +{% import 'components/form/fields_macros.html.twig' as fields %} + +
+ {{ fields.dropdownArrayField( + 'extra_data[' ~ ALLOWED_ITEMTYPES ~ ']', + '', + reservation_types, + __('Allowed equipment types', 'advancedforms'), + { + 'values' : extra_data.getAllowedItemtypes(), + 'multiple' : true, + 'display_emptychoice': true, + 'emptylabel' : __('All reservable types', 'advancedforms'), + 'full_width' : true, + 'is_horizontal' : false, + 'init' : question is not null ? true : false, + } + ) }} +
diff --git a/tests/Model/QuestionType/ReservationQuestionTest.php b/tests/Model/QuestionType/ReservationQuestionTest.php new file mode 100644 index 0000000..e00ac73 --- /dev/null +++ b/tests/Model/QuestionType/ReservationQuestionTest.php @@ -0,0 +1,115 @@ +assertGreaterThan( + 0, + $html->filter('[data-glpi-form-editor-question-extra-details]')->count(), + ); + } + + #[Override] + protected function validateHelpdeskRenderingWhenEnabled( + Crawler $html, + ): void { + $this->assertGreaterThan(0, $html->filter('input[name$="[reservationitems_id]"]')->count()); + $this->assertGreaterThan(0, $html->filter('input[name$="[begin]"]')->count()); + $this->assertGreaterThan(0, $html->filter('input[name$="[end]"]')->count()); + } + + #[Override] + protected function validateHelpdeskRenderingWhenDisabled( + Crawler $html, + ): void { + $this->assertCount(0, $html->filter('input[name$="[reservationitems_id]"]')); + } + + public function testFormatRawAnswerAndPrepareEndUserAnswerRoundTrip(): void + { + $type = new ReservationQuestion(); + $raw = [ + 'reservationitems_id' => 42, + 'begin' => '2026-01-15 09:00:00', + 'end' => '2026-01-15 12:00:00', + ]; + + // `Glpi\Form\Question` is final and cannot be mocked, and neither + // `prepareEndUserAnswer()` nor `formatRawAnswer()` actually read + // from the question here, so a real question is built instead. + // The question type must be enabled or `Section::getQuestions()` + // will silently skip it (treating it as an unknown/disabled type). + $this->enableConfigurableItem($type); + $builder = new FormBuilder("My form"); + $builder->addQuestion("My question", ReservationQuestion::class); + $form = $this->createForm($builder); + $questions_id = $this->getQuestionId($form, "My question"); + $question = new Question(); + $this->assertTrue($question->getFromDB($questions_id)); + + $prepared = $type->prepareEndUserAnswer($question, $raw); + $this->assertSame([ + 'reservationitems_id' => 42, + 'begin' => '2026-01-15 09:00:00', + 'end' => '2026-01-15 12:00:00', + ], $prepared); + + $formatted = $type->formatRawAnswer($raw, $question); + $this->assertStringContainsString('2026-01-15 09:00:00', $formatted); + $this->assertStringContainsString('2026-01-15 12:00:00', $formatted); + } +} diff --git a/tests/Service/ConfigManagerTest.php b/tests/Service/ConfigManagerTest.php index 7857742..7837b67 100644 --- a/tests/Service/ConfigManagerTest.php +++ b/tests/Service/ConfigManagerTest.php @@ -35,6 +35,7 @@ use DOMElement; use GlpiPlugin\Advancedforms\Model\Config\ConfigurableItemInterface; +use GlpiPlugin\Advancedforms\Model\QuestionType\ReservationQuestion; use GlpiPlugin\Advancedforms\Service\ConfigManager; use GlpiPlugin\Advancedforms\Tests\AdvancedFormsTestCase; use PHPUnit\Framework\Attributes\DataProvider; @@ -43,6 +44,15 @@ final class ConfigManagerTest extends AdvancedFormsTestCase { + public function testReservationQuestionIsConfigurable(): void + { + $types = array_map( + fn($t): string => $t::class, + ConfigManager::getInstance()->getConfigurableQuestionTypes(), + ); + $this->assertContains(ReservationQuestion::class, $types); + } + #[DataProvider('provideQuestionTypes')] public function testQuestionTypeConfigFormWhenEnabled( ConfigurableItemInterface $item, From ed58ce966a9e06539dc8ec1395e4d1196b1d37f5 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Mon, 20 Jul 2026 15:14:59 +0200 Subject: [PATCH 05/26] Add reservation widget AJAX controllers --- .../CheckAvailabilityController.php | 89 +++++++++++ src/Controller/GetReservationsController.php | 104 +++++++++++++ src/Controller/ReservableItemsController.php | 132 +++++++++++++++++ .../CheckAvailabilityControllerTest.php | 138 +++++++++++++++++ .../GetReservationsControllerTest.php | 117 +++++++++++++++ .../ReservableItemsControllerTest.php | 139 ++++++++++++++++++ 6 files changed, 719 insertions(+) create mode 100644 src/Controller/CheckAvailabilityController.php create mode 100644 src/Controller/GetReservationsController.php create mode 100644 src/Controller/ReservableItemsController.php create mode 100644 tests/Controller/CheckAvailabilityControllerTest.php create mode 100644 tests/Controller/GetReservationsControllerTest.php create mode 100644 tests/Controller/ReservableItemsControllerTest.php diff --git a/src/Controller/CheckAvailabilityController.php b/src/Controller/CheckAvailabilityController.php new file mode 100644 index 0000000..3c508f6 --- /dev/null +++ b/src/Controller/CheckAvailabilityController.php @@ -0,0 +1,89 @@ +request->getInt('reservationitems_id'); + $begin = $request->request->getString('begin'); + $end = $request->request->getString('end'); + + if ($reservationitems_id <= 0 || $begin === '' || $end === '') { + throw new BadRequestHttpException(); + } + + global $DB; + $conflict = $DB->request([ + 'COUNT' => 'cpt', + 'FROM' => Reservation::getTable(), + 'WHERE' => [ + 'reservationitems_id' => $reservationitems_id, + 'end' => ['>', $begin], + 'begin' => ['<', $end], + ], + ])->current(); + + $count = 0; + if (is_array($conflict) && isset($conflict['cpt']) && is_numeric($conflict['cpt'])) { + $count = (int) $conflict['cpt']; + } + + return new JsonResponse(['available' => $count === 0]); + } +} diff --git a/src/Controller/GetReservationsController.php b/src/Controller/GetReservationsController.php new file mode 100644 index 0000000..6f0a243 --- /dev/null +++ b/src/Controller/GetReservationsController.php @@ -0,0 +1,104 @@ +request->getInt('reservationitems_id'); + if ($reservationitems_id <= 0) { + throw new BadRequestHttpException(); + } + + $begin = $request->request->getString('begin', ''); + $end = $request->request->getString('end', ''); + + $where = ['reservationitems_id' => $reservationitems_id]; + if ($begin !== '') { + $where['end'] = ['>', $begin]; + } + if ($end !== '') { + $where['begin'] = ['<', $end]; + } + + global $DB; + $rows = $DB->request([ + 'FROM' => Reservation::getTable(), + 'WHERE' => $where, + ]); + + $results = []; + foreach ($rows as $row) { + if (!is_array($row)) { + continue; + } + + $begin_value = $row['begin'] ?? null; + $end_value = $row['end'] ?? null; + $users_id_value = $row['users_id'] ?? null; + + $results[] = [ + 'begin' => is_scalar($begin_value) ? (string) $begin_value : '', + 'end' => is_scalar($end_value) ? (string) $end_value : '', + 'users_id' => is_numeric($users_id_value) ? (int) $users_id_value : 0, + ]; + } + + return new JsonResponse($results); + } +} diff --git a/src/Controller/ReservableItemsController.php b/src/Controller/ReservableItemsController.php new file mode 100644 index 0000000..617bfea --- /dev/null +++ b/src/Controller/ReservableItemsController.php @@ -0,0 +1,132 @@ + $allowed_itemtypes */ + $allowed_itemtypes = $request->request->all('allowed_itemtypes'); + $search = $request->request->getString('search', ''); + + global $CFG_GLPI; + $reservation_types = $CFG_GLPI['reservation_types'] ?? []; + $itemtypes = $allowed_itemtypes !== [] + ? $allowed_itemtypes + : (is_array($reservation_types) ? $reservation_types : []); + + $results = []; + foreach ($itemtypes as $itemtype) { + if (!is_string($itemtype) || !is_a($itemtype, CommonDBTM::class, true)) { + continue; + } + + $results = [...$results, ...$this->getReservableItemsForType($itemtype, $search)]; + } + + return new JsonResponse($results); + } + + /** + * @param class-string $itemtype + * @return array + */ + private function getReservableItemsForType(string $itemtype, string $search): array + { + global $DB; + + $item = getItemForItemtype($itemtype); + $where = [ + 'itemtype' => $itemtype, + 'is_active' => 1, + ]; + + if ($search !== '') { + $where['items_id'] = new QuerySubQuery([ + 'SELECT' => 'id', + 'FROM' => $itemtype::getTable(), + 'WHERE' => ['name' => ['LIKE', "%$search%"]], + ]); + } + + $rows = $DB->request([ + 'FROM' => ReservationItem::getTable(), + 'WHERE' => $where, + ]); + + $results = []; + foreach ($rows as $row) { + if (!is_array($row) || !isset($row['items_id']) || !is_numeric($row['items_id'])) { + continue; + } + + if (!$item->getFromDB((int) $row['items_id'])) { + continue; + } + + $id = $row['id'] ?? null; + $results[] = [ + 'id' => is_numeric($id) ? (int) $id : 0, + 'text' => sprintf('%s (%s)', $item->getName(), $itemtype::getTypeName(1)), + 'itemtype' => $itemtype, + ]; + } + + return $results; + } +} diff --git a/tests/Controller/CheckAvailabilityControllerTest.php b/tests/Controller/CheckAvailabilityControllerTest.php new file mode 100644 index 0000000..0390626 --- /dev/null +++ b/tests/Controller/CheckAvailabilityControllerTest.php @@ -0,0 +1,138 @@ +login(); + $_SESSION['glpiactiveprofile']['reservation'] = 0; + + $controller = new CheckAvailabilityController(); + $this->expectException(AccessDeniedHttpException::class); + $controller(Request::create('/', 'POST', [])); + } + + public function testMissingParamsThrowsBadRequest(): void + { + $this->login(); + $controller = new CheckAvailabilityController(); + $this->expectException(BadRequestHttpException::class); + $controller(Request::create('/', 'POST', [])); + } + + public function testAvailableSlotReturnsTrue(): void + { + $this->login(); + $item = $this->getReservableItem(); + + $controller = new CheckAvailabilityController(); + $response = $controller(Request::create('/', 'POST', [ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-02-01 09:00:00', + 'end' => '2026-02-01 10:00:00', + ])); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(['available' => true], json_decode((string) $response->getContent(), true)); + } + + public function testConflictingSlotReturnsFalse(): void + { + $this->login(); + $item = $this->getReservableItem(); + + $reservation = new Reservation(); + $this->assertGreaterThan(0, $reservation->add([ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-02-01 09:00:00', + 'end' => '2026-02-01 11:00:00', + 'users_id' => Session::getLoginUserID(), + 'comment' => '', + ])); + + $controller = new CheckAvailabilityController(); + $response = $controller(Request::create('/', 'POST', [ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-02-01 10:00:00', // overlaps + 'end' => '2026-02-01 12:00:00', + ])); + + $this->assertSame(['available' => false], json_decode((string) $response->getContent(), true)); + } + + public function testBackToBackSlotIsAvailable(): void + { + $this->login(); + $item = $this->getReservableItem(); + + $reservation = new Reservation(); + $this->assertGreaterThan(0, $reservation->add([ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-02-01 09:00:00', + 'end' => '2026-02-01 11:00:00', + 'users_id' => Session::getLoginUserID(), + 'comment' => '', + ])); + + $controller = new CheckAvailabilityController(); + $response = $controller(Request::create('/', 'POST', [ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-02-01 11:00:00', // starts exactly when the other ends: no overlap + 'end' => '2026-02-01 12:00:00', + ])); + + $this->assertSame(['available' => true], json_decode((string) $response->getContent(), true)); + } + + private function getReservableItem(): ReservationItem + { + $computer = $this->createItem('Computer', ['name' => 'test-computer', 'entities_id' => 0]); + return $this->createItem('ReservationItem', [ + 'itemtype' => 'Computer', + 'items_id' => $computer->getID(), + 'is_active' => 1, + ]); + } +} diff --git a/tests/Controller/GetReservationsControllerTest.php b/tests/Controller/GetReservationsControllerTest.php new file mode 100644 index 0000000..f42795f --- /dev/null +++ b/tests/Controller/GetReservationsControllerTest.php @@ -0,0 +1,117 @@ +login(); + $_SESSION['glpiactiveprofile']['reservation'] = 0; + + $controller = new GetReservationsController(); + $this->expectException(AccessDeniedHttpException::class); + $controller(Request::create('/', 'POST', [])); + } + + public function testMissingParamsThrowsBadRequest(): void + { + $this->login(); + $controller = new GetReservationsController(); + $this->expectException(BadRequestHttpException::class); + $controller(Request::create('/', 'POST', [])); + } + + public function testReturnsReservationsForItem(): void + { + $this->login(); + $item = $this->getReservableItem(); + + $reservation = new Reservation(); + $this->assertGreaterThan(0, $reservation->add([ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-02-01 09:00:00', + 'end' => '2026-02-01 11:00:00', + 'users_id' => Session::getLoginUserID(), + 'comment' => 'this should not leak', + ])); + + $controller = new GetReservationsController(); + $response = $controller(Request::create('/', 'POST', [ + 'reservationitems_id' => $item->getID(), + ])); + + $this->assertSame(200, $response->getStatusCode()); + $data = json_decode((string) $response->getContent(), true); + $this->assertIsArray($data); + $this->assertCount(1, $data); + $this->assertSame('2026-02-01 09:00:00', $data[0]['begin']); + $this->assertSame('2026-02-01 11:00:00', $data[0]['end']); + $this->assertSame(Session::getLoginUserID(), $data[0]['users_id']); + $this->assertArrayNotHasKey('comment', $data[0]); + } + + public function testReturnsEmptyArrayWhenNoReservations(): void + { + $this->login(); + $item = $this->getReservableItem(); + + $controller = new GetReservationsController(); + $response = $controller(Request::create('/', 'POST', [ + 'reservationitems_id' => $item->getID(), + ])); + + $data = json_decode((string) $response->getContent(), true); + $this->assertSame([], $data); + } + + private function getReservableItem(): ReservationItem + { + $computer = $this->createItem('Computer', ['name' => 'test-computer-resa', 'entities_id' => 0]); + return $this->createItem('ReservationItem', [ + 'itemtype' => 'Computer', + 'items_id' => $computer->getID(), + 'is_active' => 1, + ]); + } +} diff --git a/tests/Controller/ReservableItemsControllerTest.php b/tests/Controller/ReservableItemsControllerTest.php new file mode 100644 index 0000000..04e9c82 --- /dev/null +++ b/tests/Controller/ReservableItemsControllerTest.php @@ -0,0 +1,139 @@ +login(); + $_SESSION['glpiactiveprofile']['reservation'] = 0; + + $controller = new ReservableItemsController(); + $this->expectException(AccessDeniedHttpException::class); + $controller(Request::create('/', 'POST', [])); + } + + public function testReturnsActiveReservableItemsFilteredByAllowedItemtypes(): void + { + $this->login(); + + $computer = $this->createItem('Computer', ['name' => 'test-reservable-computer', 'entities_id' => 0]); + $res_item = $this->createItem('ReservationItem', [ + 'itemtype' => 'Computer', + 'items_id' => $computer->getID(), + 'is_active' => 1, + ]); + + // Item of an itemtype not in the allowed list must not be returned + $monitor = $this->createItem('Monitor', ['name' => 'test-reservable-monitor', 'entities_id' => 0]); + $this->createItem('ReservationItem', [ + 'itemtype' => 'Monitor', + 'items_id' => $monitor->getID(), + 'is_active' => 1, + ]); + + $controller = new ReservableItemsController(); + $response = $controller(Request::create('/', 'POST', [ + 'allowed_itemtypes' => ['Computer'], + ])); + + $this->assertSame(200, $response->getStatusCode()); + $data = json_decode((string) $response->getContent(), true); + $this->assertIsArray($data); + + $ids = array_column($data, 'id'); + $this->assertContains($res_item->getID(), $ids); + foreach ($data as $row) { + $this->assertSame('Computer', $row['itemtype']); + $this->assertSame('test-reservable-computer (Computer)', $row['text']); + } + } + + public function testInactiveItemIsExcluded(): void + { + $this->login(); + + $computer = $this->createItem('Computer', ['name' => 'test-inactive-computer', 'entities_id' => 0]); + $this->createItem('ReservationItem', [ + 'itemtype' => 'Computer', + 'items_id' => $computer->getID(), + 'is_active' => 0, + ]); + + $controller = new ReservableItemsController(); + $response = $controller(Request::create('/', 'POST', [ + 'allowed_itemtypes' => ['Computer'], + ])); + + $data = json_decode((string) $response->getContent(), true); + $names = array_column($data, 'text'); + $this->assertNotContains('test-inactive-computer (Computer)', $names); + } + + public function testSearchFiltersResults(): void + { + $this->login(); + + $computer1 = $this->createItem('Computer', ['name' => 'alpha-computer', 'entities_id' => 0]); + $this->createItem('ReservationItem', [ + 'itemtype' => 'Computer', + 'items_id' => $computer1->getID(), + 'is_active' => 1, + ]); + + $computer2 = $this->createItem('Computer', ['name' => 'beta-computer', 'entities_id' => 0]); + $this->createItem('ReservationItem', [ + 'itemtype' => 'Computer', + 'items_id' => $computer2->getID(), + 'is_active' => 1, + ]); + + $controller = new ReservableItemsController(); + $response = $controller(Request::create('/', 'POST', [ + 'allowed_itemtypes' => ['Computer'], + 'search' => 'alpha', + ])); + + $data = json_decode((string) $response->getContent(), true); + $names = array_column($data, 'text'); + $this->assertContains('alpha-computer (Computer)', $names); + $this->assertNotContains('beta-computer (Computer)', $names); + } +} From 93c9f803e686d1258c618205a67af354752d2d05 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Mon, 20 Jul 2026 15:25:52 +0200 Subject: [PATCH 06/26] Add missing test coverage --- .../GetReservationsControllerTest.php | 36 +++++++++++++++++++ .../ReservableItemsControllerTest.php | 34 ++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/tests/Controller/GetReservationsControllerTest.php b/tests/Controller/GetReservationsControllerTest.php index f42795f..030a542 100644 --- a/tests/Controller/GetReservationsControllerTest.php +++ b/tests/Controller/GetReservationsControllerTest.php @@ -91,6 +91,42 @@ public function testReturnsReservationsForItem(): void $this->assertArrayNotHasKey('comment', $data[0]); } + public function testFiltersReservationsByDateRange(): void + { + $this->login(); + $item = $this->getReservableItem(); + + $reservation_in_range = new Reservation(); + $this->assertGreaterThan(0, $reservation_in_range->add([ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-02-01 09:00:00', + 'end' => '2026-02-01 11:00:00', + 'users_id' => Session::getLoginUserID(), + ])); + + $reservation_out_of_range = new Reservation(); + $this->assertGreaterThan(0, $reservation_out_of_range->add([ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-02-02 09:00:00', + 'end' => '2026-02-02 11:00:00', + 'users_id' => Session::getLoginUserID(), + ])); + + $controller = new GetReservationsController(); + $response = $controller(Request::create('/', 'POST', [ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-02-01 00:00:00', + 'end' => '2026-02-01 23:59:59', + ])); + + $this->assertSame(200, $response->getStatusCode()); + $data = json_decode((string) $response->getContent(), true); + $this->assertIsArray($data); + $this->assertCount(1, $data); + $this->assertSame('2026-02-01 09:00:00', $data[0]['begin']); + $this->assertSame('2026-02-01 11:00:00', $data[0]['end']); + } + public function testReturnsEmptyArrayWhenNoReservations(): void { $this->login(); diff --git a/tests/Controller/ReservableItemsControllerTest.php b/tests/Controller/ReservableItemsControllerTest.php index 04e9c82..8251f57 100644 --- a/tests/Controller/ReservableItemsControllerTest.php +++ b/tests/Controller/ReservableItemsControllerTest.php @@ -107,6 +107,40 @@ public function testInactiveItemIsExcluded(): void $this->assertNotContains('test-inactive-computer (Computer)', $names); } + public function testFallsBackToConfigReservationTypesWhenAllowedItemtypesOmitted(): void + { + $this->login(); + + global $CFG_GLPI; + $original_reservation_types = $CFG_GLPI['reservation_types'] ?? null; + $CFG_GLPI['reservation_types'] = ['Computer']; + + try { + $computer = $this->createItem('Computer', ['name' => 'test-fallback-computer', 'entities_id' => 0]); + $res_item = $this->createItem('ReservationItem', [ + 'itemtype' => 'Computer', + 'items_id' => $computer->getID(), + 'is_active' => 1, + ]); + + $controller = new ReservableItemsController(); + $response = $controller(Request::create('/', 'POST', [])); + + $this->assertSame(200, $response->getStatusCode()); + $data = json_decode((string) $response->getContent(), true); + $this->assertIsArray($data); + + $ids = array_column($data, 'id'); + $this->assertContains($res_item->getID(), $ids); + + foreach ($data as $row) { + $this->assertSame('Computer', $row['itemtype']); + } + } finally { + $CFG_GLPI['reservation_types'] = $original_reservation_types; + } + } + public function testSearchFiltersResults(): void { $this->login(); From e8769a17c842eb6c088566672fa87c5143e6adeb Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Mon, 20 Jul 2026 16:33:33 +0200 Subject: [PATCH 07/26] ReservationQuestion end-user widget (Select2 + Flatpickr) --- .../js/modules/ReservationQuestionWidget.js | 186 ++++++++++++++++++ .../QuestionType/ReservationQuestion.php | 49 +++-- templates/reservation_question.html.twig | 76 +++++++ .../QuestionType/ReservationQuestionTest.php | 74 +++++++ 4 files changed, 372 insertions(+), 13 deletions(-) create mode 100644 public/js/modules/ReservationQuestionWidget.js create mode 100644 templates/reservation_question.html.twig diff --git a/public/js/modules/ReservationQuestionWidget.js b/public/js/modules/ReservationQuestionWidget.js new file mode 100644 index 0000000..722b782 --- /dev/null +++ b/public/js/modules/ReservationQuestionWidget.js @@ -0,0 +1,186 @@ +/** + * ------------------------------------------------------------------------- + * advancedforms plugin for GLPI + * ------------------------------------------------------------------------- + * + * MIT License + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2025 by the advancedforms plugin team. + * @license MIT https://opensource.org/licenses/mit-license.php + * @link https://github.com/pluginsGLPI/advancedforms + * ------------------------------------------------------------------------- + */ + +export class ReservationQuestionWidget { + #root; + #endpoint_url = `${CFG_GLPI.root_doc}/plugins/advancedforms/ReservationWidget`; + + /** + * @param {HTMLElement} root - The root element of the widget, as rendered + * by `templates/reservation_question.html.twig`. + */ + constructor(root) { + if (!root) { + return; + } + + this.#root = root; + + this.#initItemSelect(); + this.#initDatePickers(); + } + + #initItemSelect() { + const $select = $(this.#root.querySelector('[data-reservation-question-item-select]')); + if ($select.length === 0) { + return; + } + + const allowed_itemtypes = JSON.parse(this.#root.dataset.allowedItemtypes || '[]'); + + $select.select2({ + width: '100%', + allowClear: true, + placeholder: __('Select an item to reserve'), + ajax: { + url: `${this.#endpoint_url}/ReservableItems`, + type: 'POST', + delay: 250, + data: (params) => ({ + allowed_itemtypes: allowed_itemtypes, + search: params.term || '', + }), + processResults: (data) => ({ + results: this.#groupResultsByItemtype(Array.isArray(data) ? data : []), + }), + }, + }); + + $select.on('select2:select', () => this.#onItemSelected()); + $select.on('select2:clear select2:unselecting', () => this.#onItemCleared()); + } + + /** + * Group flat `{id, text, itemtype}` results into Select2 optgroups so + * items are presented per reservable itemtype. + * + * @param {Array<{id: number, text: string, itemtype: string}>} data + * @return {Array<{text: string, children: Array<{id: number, text: string}>}>} + */ + #groupResultsByItemtype(data) { + const groups = new Map(); + + for (const item of data) { + const group_label = item.itemtype || ''; + if (!groups.has(group_label)) { + groups.set(group_label, []); + } + groups.get(group_label).push({ + id: item.id, + text: item.text, + }); + } + + return Array.from(groups, ([text, children]) => ({ text, children })); + } + + #onItemSelected() { + const $select = $(this.#root.querySelector('[data-reservation-question-item-select]')); + this.#setFieldValue('reservationitems_id', $select.val()); + $(this.#root.querySelector('[data-reservation-question-dates]')).removeClass('d-none'); + this.#checkAvailability(); + } + + #onItemCleared() { + this.#setFieldValue('reservationitems_id', ''); + this.#setFieldValue('begin', ''); + this.#setFieldValue('end', ''); + $(this.#root.querySelector('[data-reservation-question-dates]')).addClass('d-none'); + this.#showAvailability(null); + } + + #initDatePickers() { + const begin_input = this.#root.querySelector('[data-reservation-question-begin-picker]'); + const end_input = this.#root.querySelector('[data-reservation-question-end-picker]'); + if (!begin_input || !end_input) { + return; + } + + const options = { + enableTime: true, + enableSeconds: true, + time_24hr: true, + dateFormat: 'Y-m-d H:i:S', + onChange: (selected_dates, date_str, instance) => { + const field = instance.element === begin_input ? 'begin' : 'end'; + this.#setFieldValue(field, date_str); + this.#checkAvailability(); + }, + }; + + flatpickr(begin_input, options); + flatpickr(end_input, options); + } + + #setFieldValue(field, value) { + const input = this.#root.querySelector(`[data-reservation-question-field="${field}"]`); + if (input) { + input.value = value ?? ''; + } + } + + #getFieldValue(field) { + const input = this.#root.querySelector(`[data-reservation-question-field="${field}"]`); + return input ? input.value : ''; + } + + #checkAvailability() { + const reservationitems_id = this.#getFieldValue('reservationitems_id'); + const begin = this.#getFieldValue('begin'); + const end = this.#getFieldValue('end'); + + if (!reservationitems_id || !begin || !end) { + this.#showAvailability(null); + return; + } + + $.post(`${this.#endpoint_url}/CheckAvailability`, { reservationitems_id, begin, end }) + .done((data) => this.#showAvailability(data.available)) + .fail(() => this.#showAvailability(null)); + } + + #showAvailability(available) { + const $result = $(this.#root.querySelector('[data-reservation-question-availability]')); + if ($result.length === 0) { + return; + } + + if (available === null || available === undefined) { + $result.text('').removeClass('text-danger text-success'); + return; + } + + $result + .text(available ? __('This slot is available') : __('This slot is no longer available')) + .toggleClass('text-danger', !available) + .toggleClass('text-success', available); + } +} diff --git a/src/Model/QuestionType/ReservationQuestion.php b/src/Model/QuestionType/ReservationQuestion.php index 49f4068..4de109f 100644 --- a/src/Model/QuestionType/ReservationQuestion.php +++ b/src/Model/QuestionType/ReservationQuestion.php @@ -39,6 +39,7 @@ use Glpi\Form\QuestionType\AbstractQuestionType; use Glpi\Form\QuestionType\QuestionTypeCategoryInterface; use GlpiPlugin\Advancedforms\Model\Config\ConfigurableItemInterface; +use InvalidArgumentException; use Override; use function Safe\json_decode; @@ -132,17 +133,28 @@ public function renderAdministrationTemplate(Question|null $question): string #[Override] public function renderEndUserTemplate(Question $question): string { - // Minimal placeholder: the full Select2/Flatpickr widget is wired in - // a later task. For now, expose the raw wire format as empty hidden - // inputs so the end user answer processing pipeline can be tested. - return TemplateRenderer::getInstance()->renderFromStringTemplate( - << - - - TWIG, - ['question' => $question], - ); + $decoded_extra_data = []; + if (is_string($question->fields['extra_data'])) { + $decoded_extra_data = json_decode( + $question->fields['extra_data'], + associative: true, + ); + + // Fallback to safe value + if (!is_array($decoded_extra_data)) { + $decoded_extra_data = []; + } + } + + $config = $this->getExtraDataConfig($decoded_extra_data); + if (!$config instanceof ReservationQuestionConfig) { + $config = new ReservationQuestionConfig(); + } + + return TemplateRenderer::getInstance()->render('@advancedforms/reservation_question.html.twig', [ + 'input_name' => $question->getEndUserInputName(), + 'allowed_itemtypes' => $config->getAllowedItemtypes(), + ]); } #[Override] @@ -152,7 +164,14 @@ public function prepareEndUserAnswer(Question $question, mixed $answer): mixed return null; } - return ReservationQuestionAnswer::fromArray($answer)->toArray(); + try { + return ReservationQuestionAnswer::fromArray($answer)->toArray(); + } catch (InvalidArgumentException) { + // The question is optional (or was simply left untouched) and + // the widget's hidden inputs are empty: treat this the same as + // "not answered" instead of failing the whole form submission. + return null; + } } #[Override] @@ -162,7 +181,11 @@ public function formatRawAnswer(mixed $answer, Question $question): string return ''; } - $parsed = ReservationQuestionAnswer::fromArray($answer); + try { + $parsed = ReservationQuestionAnswer::fromArray($answer); + } catch (InvalidArgumentException) { + return ''; + } return sprintf('%s → %s', $parsed->getBegin(), $parsed->getEnd()); } diff --git a/templates/reservation_question.html.twig b/templates/reservation_question.html.twig new file mode 100644 index 0000000..ea892ab --- /dev/null +++ b/templates/reservation_question.html.twig @@ -0,0 +1,76 @@ +{# + # ------------------------------------------------------------------------- + # advancedforms plugin for GLPI + # ------------------------------------------------------------------------- + # + # MIT License + # + # Permission is hereby granted, free of charge, to any person obtaining a copy + # of this software and associated documentation files (the "Software"), to deal + # in the Software without restriction, including without limitation the rights + # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + # copies of the Software, and to permit persons to whom the Software is + # furnished to do so, subject to the following conditions: + # + # The above copyright notice and this permission notice shall be included in all + # copies or substantial portions of the Software. + # + # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + # SOFTWARE. + # ------------------------------------------------------------------------- + # @copyright Copyright (C) 2025 by the advancedforms plugin team. + # @license MIT https://opensource.org/licenses/mit-license.php + # @link https://github.com/pluginsGLPI/advancedforms + # ------------------------------------------------------------------------- + #} + +
+ + +
+
+
+ +
+
+ +
+
+
+
+ + + + +
+ diff --git a/tests/Model/QuestionType/ReservationQuestionTest.php b/tests/Model/QuestionType/ReservationQuestionTest.php index e00ac73..dda3dab 100644 --- a/tests/Model/QuestionType/ReservationQuestionTest.php +++ b/tests/Model/QuestionType/ReservationQuestionTest.php @@ -70,6 +70,16 @@ protected function validateHelpdeskRenderingWhenEnabled( $this->assertGreaterThan(0, $html->filter('input[name$="[reservationitems_id]"]')->count()); $this->assertGreaterThan(0, $html->filter('input[name$="[begin]"]')->count()); $this->assertGreaterThan(0, $html->filter('input[name$="[end]"]')->count()); + + // The rendered helpdesk page contains several ` {% endif %} diff --git a/tests/Service/TimelineManagerTest.php b/tests/Service/TimelineManagerTest.php index ccd0db5..bba07de 100644 --- a/tests/Service/TimelineManagerTest.php +++ b/tests/Service/TimelineManagerTest.php @@ -78,6 +78,32 @@ public function testAddTimelineItemsAddsWaitingEntryWithApproveRefuseButtons(): ); } + public function testAddTimelineItemsWiresApproveRefuseButtonsToController(): void + { + $this->login(); + $ticket = $this->createItem('Ticket', ['name' => 't', 'content' => 'c', 'entities_id' => $this->getTestRootEntity(true)]); + $item = $this->getReservableItem(); + + $request = $this->createItem(TicketReservationRequest::class, [ + 'tickets_id' => $ticket->getID(), + 'reservationitems_id' => $item->getID(), + 'users_id' => Session::getLoginUserID(), + 'begin' => '2026-05-01 09:30:00', + 'end' => '2026-05-01 10:30:00', + 'status' => TicketReservationRequest::STATUS_WAITING, + ]); + + $timeline = []; + TimelineManager::addTimelineItems(['item' => $ticket, 'timeline' => &$timeline]); + + $key = "TicketReservationRequest_{$request->getID()}"; + $this->assertArrayHasKey($key, $timeline); + $content = $timeline[$key]['item']['content']; + + $this->assertStringContainsString('ReservationRequestTimelineActions.js', $content); + $this->assertStringContainsString('data-reservation-request-action="approve"', $content); + } + public function testAddTimelineItemsHidesApproveRefuseButtonsWhenUserCannotAnswer(): void { // Ticket is created by the default logged-in user (requester). From 6798332918247e4714dae2f92a25cbb4fa20a200 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Wed, 22 Jul 2026 10:44:37 +0200 Subject: [PATCH 18/26] Add reservation workflow test --- tests/ReservationWorkflowTest.php | 388 ++++++++++++++++++++++++++++++ 1 file changed, 388 insertions(+) create mode 100644 tests/ReservationWorkflowTest.php diff --git a/tests/ReservationWorkflowTest.php b/tests/ReservationWorkflowTest.php new file mode 100644 index 0000000..39d5804 --- /dev/null +++ b/tests/ReservationWorkflowTest.php @@ -0,0 +1,388 @@ +login(); + $requester_id = Session::getLoginUserID(); + $this->giveUserADefaultEmail($requester_id); + $this->activateReservationRequestNotification('reservation_request_created'); + $this->activateReservationRequestNotification('reservation_request_approved'); + + $queue_before_submission = countElementsInTable('glpi_queuednotifications'); + + [$ticket, $item] = $this->submitReservationForm(require_approval: true); + $this->assertGreaterThan(0, $ticket->getID()); + + $request = new TicketReservationRequest(); + $rows = $request->find(['tickets_id' => $ticket->getID()]); + $this->assertCount(1, $rows); + $this->assertTrue($request->getFromDB((int) array_key_first($rows))); + $this->assertSame(TicketReservationRequest::STATUS_WAITING, (int) $request->fields['status']); + + // The requester must have been notified that their request needs + // an answer. + $queue_after_submission = countElementsInTable('glpi_queuednotifications'); + $this->assertGreaterThan($queue_before_submission, $queue_after_submission); + + // A different (technician) user approves the request through the + // real HTTP controller, not by calling the model directly, so that + // the full stack (rights-checking included) is exercised. + $this->login('glpi', 'glpi'); + $this->assertNotSame($requester_id, Session::getLoginUserID()); + + $controller = new ReservationRequestController(); + $response = $controller(Request::create('/', 'POST', [ + 'id' => $request->getID(), + 'action' => 'approve', + 'comment' => 'ok', + ])); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(['success' => true], json_decode((string) $response->getContent(), true)); + + $this->assertTrue($request->getFromDB($request->getID())); + $this->assertSame(TicketReservationRequest::STATUS_ACCEPTED, (int) $request->fields['status']); + + // The resulting Reservation must be attributed to the original + // requester, not to the technician who approved it. + $reservation = new Reservation(); + $found = $reservation->find(['reservationitems_id' => $item->getID()]); + $this->assertCount(1, $found); + $this->assertSame($requester_id, (int) reset($found)['users_id']); + + $queue_after_approval = countElementsInTable('glpi_queuednotifications'); + $this->assertGreaterThan($queue_after_submission, $queue_after_approval); + } + + public function testFullRefusalFlow(): void + { + $this->login(); + $requester_id = Session::getLoginUserID(); + $this->giveUserADefaultEmail($requester_id); + $this->activateReservationRequestNotification('reservation_request_created'); + $this->activateReservationRequestNotification('reservation_request_refused'); + + [$ticket, $item] = $this->submitReservationForm(require_approval: true); + + $request = new TicketReservationRequest(); + $rows = $request->find(['tickets_id' => $ticket->getID()]); + $this->assertCount(1, $rows); + $this->assertTrue($request->getFromDB((int) array_key_first($rows))); + + $queue_before_refusal = countElementsInTable('glpi_queuednotifications'); + + $this->login('glpi', 'glpi'); + $controller = new ReservationRequestController(); + $response = $controller(Request::create('/', 'POST', [ + 'id' => $request->getID(), + 'action' => 'refuse', + 'comment' => 'no', + ])); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(['success' => true], json_decode((string) $response->getContent(), true)); + + $this->assertTrue($request->getFromDB($request->getID())); + $this->assertSame(TicketReservationRequest::STATUS_REFUSED, (int) $request->fields['status']); + + $reservation = new Reservation(); + $this->assertCount(0, $reservation->find(['reservationitems_id' => $item->getID()])); + + $queue_after_refusal = countElementsInTable('glpi_queuednotifications'); + $this->assertGreaterThan($queue_before_refusal, $queue_after_refusal); + } + + public function testDirectModeSlotFreeAutoApproves(): void + { + $this->login(); + $requester_id = Session::getLoginUserID(); + + [$ticket, $item] = $this->submitReservationForm(require_approval: false); + + $request = new TicketReservationRequest(); + $rows = $request->find(['tickets_id' => $ticket->getID()]); + $this->assertCount(1, $rows); + $row = reset($rows); + $this->assertSame(TicketReservationRequest::STATUS_ACCEPTED, (int) $row['status']); + // No technician ever answered this request: it was auto-approved. + $this->assertSame(0, (int) $row['users_id_validate']); + + $reservation = new Reservation(); + $found = $reservation->find(['reservationitems_id' => $item->getID()]); + $this->assertCount(1, $found); + $this->assertSame($requester_id, (int) reset($found)['users_id']); + } + + public function testDirectModeSlotConflictingCancelsAndNotifiesRequester(): void + { + $this->login(); + $requester_id = Session::getLoginUserID(); + $this->giveUserADefaultEmail($requester_id); + $this->activateReservationRequestNotification('reservation_request_slot_unavailable'); + + $item = $this->getReservableItem(); + + // Seed a conflicting reservation for the same slot before submitting. + $conflict = new Reservation(); + $this->assertGreaterThan(0, $conflict->add([ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-07-01 09:00:00', + 'end' => '2026-07-01 11:00:00', + 'users_id' => $requester_id, + 'comment' => '', + ])); + + $queue_before_submission = countElementsInTable('glpi_queuednotifications'); + + [$ticket] = $this->submitReservationForm( + require_approval: false, + item: $item, + begin: '2026-07-01 10:00:00', + end: '2026-07-01 12:00:00', + ); + + $request = new TicketReservationRequest(); + $rows = $request->find(['tickets_id' => $ticket->getID()]); + $this->assertCount(1, $rows); + $this->assertSame(TicketReservationRequest::STATUS_CANCELED, (int) reset($rows)['status']); + + // Only the pre-existing conflicting reservation must exist: no new + // Reservation was created for this request. + $reservation_found = (new Reservation())->find(['reservationitems_id' => $item->getID()]); + $this->assertCount(1, $reservation_found); + $this->assertSame('2026-07-01 09:00:00', reset($reservation_found)['begin']); + + $queue_after_submission = countElementsInTable('glpi_queuednotifications'); + $this->assertGreaterThan($queue_before_submission, $queue_after_submission); + } + + public function testApproveDeniedWithoutTicketUpdateRightLeavesRequestUnchanged(): void + { + [$ticket] = $this->submitReservationForm(require_approval: true); + + $request = new TicketReservationRequest(); + $rows = $request->find(['tickets_id' => $ticket->getID()]); + $this->assertCount(1, $rows); + $this->assertTrue($request->getFromDB((int) array_key_first($rows))); + + // Switch to a "post-only" session: this user is not the ticket's + // requester and uses the helpdesk interface, so Ticket::canUpdateItem() + // is guaranteed to return false regardless of any right assignment. + $this->login('post-only', 'postonly'); + + $controller = new ReservationRequestController(); + $denied = false; + try { + $controller(Request::create('/', 'POST', [ + 'id' => $request->getID(), + 'action' => 'approve', + ])); + } catch (AccessDeniedHttpException) { + $denied = true; + } + $this->assertTrue($denied, 'Expected an AccessDeniedHttpException to be thrown.'); + + // The denial must be a real gate, not just an exception with a + // silent side effect: the request must be untouched. + $this->assertTrue($request->getFromDB($request->getID())); + $this->assertSame(TicketReservationRequest::STATUS_WAITING, (int) $request->fields['status']); + } + + /** + * Build a form with a single `ReservationQuestion`, configure its + * `Ticket` destination's `PreReservationField` with the given approval + * mode, submit it as the current test user and return the created + * `Ticket` alongside the `ReservationItem` used for the answer. + * + * @return array{0: Ticket, 1: ReservationItem} + */ + private function submitReservationForm( + bool $require_approval, + ?ReservationItem $item = null, + string $begin = '2026-07-02 09:00:00', + string $end = '2026-07-02 10:00:00', + ): array { + $this->login(); + $this->enableConfigurableItem(new ReservationQuestion()); + + $item ??= $this->getReservableItem(); + + $builder = new FormBuilder("Reservation workflow form"); + $builder->addQuestion("Reservation", ReservationQuestion::class); + $form = $this->createForm($builder); + + $question_id = $this->getQuestionId($form, "Reservation"); + + $config = new PreReservationFieldConfig( + strategy: PreReservationFieldStrategy::FROM_SPECIFIC_QUESTION, + question_id: $question_id, + require_approval: $require_approval, + ); + + $destinations = $form->getDestinations(); + $this->assertCount(1, $destinations); + $destination = current($destinations); + $this->updateItem( + $destination::getType(), + $destination->getId(), + ['config' => [PreReservationField::getKey() => $config->jsonSerialize()]], + ['config'], + ); + + $ticket = $this->sendFormAndGetCreatedTicket($form, [ + "Reservation" => [ + 'reservationitems_id' => $item->getID(), + 'begin' => $begin, + 'end' => $end, + ], + ]); + + return [$ticket, $item]; + } + + private function getReservableItem(): ReservationItem + { + $computer = $this->createItem('Computer', [ + 'name' => 'reservationworkflow-test-computer', + 'entities_id' => $this->getTestRootEntity(true), + ]); + + return $this->createItem(ReservationItem::class, [ + 'itemtype' => 'Computer', + 'items_id' => $computer->getID(), + 'is_active' => 1, + ]); + } + + /** + * Give the given user a default email address: `NotificationTarget`'s + * `addItemAuthor()`/`addToRecipientsList()` silently drops recipients + * that have no resolvable email address, so without this a queued + * notification would never be created regardless of everything else + * being correctly configured. + */ + private function giveUserADefaultEmail(int $users_id): void + { + $this->createItem(UserEmail::class, [ + 'users_id' => $users_id, + 'email' => 'reservationworkflow-test-' . $users_id . '@example.com', + 'is_default' => 1, + ]); + } + + /** + * Register a minimal, active `Notification` (+ template + "requester" + * target) for the given `TicketReservationRequest` event, so that + * `NotificationEvent::raiseEvent()` actually queues something: this + * plugin does not ship any notification configuration of its own (that + * is left to the GLPI administrator), so tests must create their own. + * + * Also enables notifications globally in `$CFG_GLPI`. + */ + private function activateReservationRequestNotification(string $event): void + { + global $CFG_GLPI; + + $CFG_GLPI['use_notifications'] = 1; + $CFG_GLPI['notifications_mailing'] = 1; + + $notification = $this->createItem(Notification::class, [ + 'name' => 'test-' . $event, + 'itemtype' => TicketReservationRequest::class, + 'event' => $event, + 'entities_id' => 0, + 'is_recursive' => 1, + 'is_active' => 1, + ]); + + $template = $this->createItem(NotificationTemplate::class, [ + 'name' => 'test-template-' . $event, + 'itemtype' => TicketReservationRequest::class, + ]); + + $this->createItem(NotificationTemplateTranslation::class, [ + 'notificationtemplates_id' => $template->getID(), + 'language' => '', + 'subject' => 'test subject', + 'content_text' => 'test content', + 'content_html' => '', + ]); + + $this->createItem(Notification_NotificationTemplate::class, [ + 'notifications_id' => $notification->getID(), + 'mode' => Notification_NotificationTemplate::MODE_MAIL, + 'notificationtemplates_id' => $template->getID(), + ]); + + $this->createItem(NotificationTarget::class, [ + 'notifications_id' => $notification->getID(), + 'type' => Notification::USER_TYPE, + 'items_id' => Notification::AUTHOR, + ]); + } +} From a0d1d1b1ec2799d28ff806f35bf4125e565bdb9b Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Wed, 22 Jul 2026 11:49:49 +0200 Subject: [PATCH 19/26] Fix details --- src/Model/Destination/PreReservationField.php | 26 +++++- src/Model/TicketReservationRequest.php | 16 +++- src/Service/InstallManager.php | 18 +++- tests/Model/TicketReservationRequestTest.php | 35 ++++++++ tests/ReservationWorkflowTest.php | 87 +++++++++++++++++++ tests/Service/InstallManagerTest.php | 73 ++++++++++++++-- 6 files changed, 243 insertions(+), 12 deletions(-) diff --git a/src/Model/Destination/PreReservationField.php b/src/Model/Destination/PreReservationField.php index d6bc1eb..0e1e1c7 100644 --- a/src/Model/Destination/PreReservationField.php +++ b/src/Model/Destination/PreReservationField.php @@ -181,8 +181,9 @@ public function applyConfiguratedValueAfterDestinationCreation( return; } - $request = new TicketReservationRequest(); - $request_id = $request->add([ + $require_approval = $config->isApprovalRequired(); + + $add_input = [ 'tickets_id' => $ticket->getID(), 'reservationitems_id' => $answer->getReservationItemsId(), // @phpstan-ignore cast.int (CommonDBTM::$fields is not generically typed) @@ -190,13 +191,30 @@ public function applyConfiguratedValueAfterDestinationCreation( 'begin' => $answer->getBegin(), 'end' => $answer->getEnd(), 'status' => TicketReservationRequest::STATUS_WAITING, - ]); + ]; + + if (!$require_approval) { + // In direct/no-approval mode, the WAITING status set above is + // only a transient step before immediately transitioning this + // request to its real final state below (ACCEPTED/CANCELED), + // which each raise their own distinct notification event. The + // request never actually waits for approval, so the "please wait + // for approval" notification must be suppressed for this insert. + // (The key must only be present when disabling: `isset()` is what + // `TicketReservationRequest::post_addItem()` checks, and it is + // `true` even for an explicit `false` value, matching the + // `_disablenotif` convention used throughout GLPI core.) + $add_input['_disablenotif'] = true; + } + + $request = new TicketReservationRequest(); + $request_id = $request->add($add_input); if (!$request_id) { return; } - if (!$config->isApprovalRequired()) { + if (!$require_approval) { if ($request->isSlotStillAvailable()) { $request->approve(0, ''); } else { diff --git a/src/Model/TicketReservationRequest.php b/src/Model/TicketReservationRequest.php index 72573ba..d42f235 100644 --- a/src/Model/TicketReservationRequest.php +++ b/src/Model/TicketReservationRequest.php @@ -88,7 +88,21 @@ public function post_addItem(): void // created already accepted (e.g. a future "direct reservation, no // approval needed" path) has nothing to approve/refuse, so it must // not raise this event. - if ((int) $this->fields['status'] === self::STATUS_WAITING) { + // + // A caller can also explicitly opt out of this notification (via the + // standard `_disablenotif` input key, see `CommonDBTM`/`Ticket`/etc.) + // even while inserting the row in the WAITING state: this is used by + // `PreReservationField` for its "no approval required" (direct + // reservation) path, where the row is inserted as WAITING only as a + // transient step before immediately transitioning it to its real + // final state (ACCEPTED/CANCELED) via `approve()`/`markUnavailable()`, + // which each raise their own distinct event. Without this, the + // "please wait for approval" notification would fire for something + // that never actually waits for approval. + if ( + (int) $this->fields['status'] === self::STATUS_WAITING + && !isset($this->input['_disablenotif']) + ) { NotificationEvent::raiseEvent('reservation_request_created', $this); } } diff --git a/src/Service/InstallManager.php b/src/Service/InstallManager.php index 739963b..121b7e5 100644 --- a/src/Service/InstallManager.php +++ b/src/Service/InstallManager.php @@ -65,7 +65,13 @@ public function install(): bool private function installTicketReservationRequestsTable(\DBmysql $DB): void { $table = TicketReservationRequest::getTable(); - if ($DB->tableExists($table)) { + // `tableExists()`'s cache only ever grows (a table once found never + // leaves it), it is never invalidated when a table is dropped. Since + // `uninstall()` can genuinely drop this table within the same PHP + // process (e.g. an uninstall immediately followed by a reinstall), + // relying on the cache here could make this guard wrongly believe + // the table still exists and skip recreating it. Bypass the cache. + if ($DB->tableExists($table, false)) { return; } @@ -100,8 +106,18 @@ private function installTicketReservationRequestsTable(\DBmysql $DB): void public function uninstall(): bool { + global $DB; + $config = new Config(); $config->deleteByCriteria(['context' => 'advancedforms']); + + $table = TicketReservationRequest::getTable(); + // Bypass the cache here too, for the same reason as in + // installTicketReservationRequestsTable(). + if ($DB->tableExists($table, false)) { + $DB->dropTable($table); + } + return true; } } diff --git a/tests/Model/TicketReservationRequestTest.php b/tests/Model/TicketReservationRequestTest.php index b622ea7..3814f4d 100644 --- a/tests/Model/TicketReservationRequestTest.php +++ b/tests/Model/TicketReservationRequestTest.php @@ -270,6 +270,41 @@ public function testCreatingWaitingRequestRaisesCreatedNotification(): void $this->assertGreaterThan($queue_before, $queue_after); } + public function testCreatingWaitingRequestWithDisablenotifFlagDoesNotRaiseCreatedNotification(): void + { + // A request created in the WAITING state can still opt out of the + // "new request needs an answer" notification via the standard + // `_disablenotif` input key (same convention used throughout GLPI + // core, e.g. `Ticket`/`CommonITILObject`/`Reservation`). This is what + // `PreReservationField` relies on for its "no approval required" + // (direct reservation) path: the row is inserted as WAITING only as + // a transient step before immediately being transitioned to its real + // final state, so the "please wait for approval" notification must + // not fire for that insert. + $this->login(); + $requester_id = Session::getLoginUserID(); + $this->giveUserADefaultEmail($requester_id); + $this->activateReservationRequestNotification('reservation_request_created'); + + $ticket = $this->createItem('Ticket', ['name' => 't', 'content' => 'c', 'entities_id' => $this->getTestRootEntity(true)]); + $item = $this->getReservableItem(); + + $queue_before = countElementsInTable('glpi_queuednotifications'); + + $this->createItem(TicketReservationRequest::class, [ + 'tickets_id' => $ticket->getID(), + 'reservationitems_id' => $item->getID(), + 'users_id' => $requester_id, + 'begin' => '2026-05-07 09:00:00', + 'end' => '2026-05-07 10:00:00', + 'status' => TicketReservationRequest::STATUS_WAITING, + '_disablenotif' => true, + ]); + + $queue_after = countElementsInTable('glpi_queuednotifications'); + $this->assertSame($queue_before, $queue_after); + } + public function testCreatingAlreadyAcceptedRequestDoesNotRaiseCreatedNotification(): void { // A request created directly in the ACCEPTED state (no approval diff --git a/tests/ReservationWorkflowTest.php b/tests/ReservationWorkflowTest.php index 39d5804..ec5ecff 100644 --- a/tests/ReservationWorkflowTest.php +++ b/tests/ReservationWorkflowTest.php @@ -221,6 +221,80 @@ public function testDirectModeSlotConflictingCancelsAndNotifiesRequester(): void $this->assertGreaterThan($queue_before_submission, $queue_after_submission); } + public function testDirectModeSlotFreeDoesNotRaiseCreatedNotification(): void + { + // Regression test: in "no approval required" (direct reservation) + // mode, the underlying `TicketReservationRequest` row is still + // inserted as WAITING before immediately being auto-approved by + // `PreReservationField`. Only the "created" ("please wait for + // approval") notification is activated here: if it fired for this + // transient WAITING insert, the requester would wrongly receive both + // a "please wait" and an "approved" notification for one + // auto-confirmed action, and technicians-in-charge would be pinged + // with an approval request for something that needs no approval. + // + // The count is scoped to `TicketReservationRequest` notifications + // (rather than the whole queue) because submitting the form also + // creates a `Ticket` and a `Reservation`, which raise their own, + // unrelated core notifications ("New ticket"/"New reservation") in + // this test environment. + $this->login(); + $requester_id = Session::getLoginUserID(); + $this->giveUserADefaultEmail($requester_id); + $this->activateReservationRequestNotification('reservation_request_created'); + + $queue_before_submission = $this->countReservationRequestQueuedNotifications(); + + [$ticket] = $this->submitReservationForm(require_approval: false); + $this->assertGreaterThan(0, $ticket->getID()); + + $request = new TicketReservationRequest(); + $rows = $request->find(['tickets_id' => $ticket->getID()]); + $this->assertCount(1, $rows); + $this->assertSame(TicketReservationRequest::STATUS_ACCEPTED, (int) reset($rows)['status']); + + $queue_after_submission = $this->countReservationRequestQueuedNotifications(); + $this->assertSame($queue_before_submission, $queue_after_submission); + } + + public function testDirectModeSlotConflictingDoesNotRaiseCreatedNotification(): void + { + // Same regression as above, but for the "slot no longer available" + // (markUnavailable) branch of direct mode. + $this->login(); + $requester_id = Session::getLoginUserID(); + $this->giveUserADefaultEmail($requester_id); + $this->activateReservationRequestNotification('reservation_request_created'); + + $item = $this->getReservableItem(); + + $conflict = new Reservation(); + $this->assertGreaterThan(0, $conflict->add([ + 'reservationitems_id' => $item->getID(), + 'begin' => '2026-07-03 09:00:00', + 'end' => '2026-07-03 11:00:00', + 'users_id' => $requester_id, + 'comment' => '', + ])); + + $queue_before_submission = $this->countReservationRequestQueuedNotifications(); + + [$ticket] = $this->submitReservationForm( + require_approval: false, + item: $item, + begin: '2026-07-03 10:00:00', + end: '2026-07-03 12:00:00', + ); + + $request = new TicketReservationRequest(); + $rows = $request->find(['tickets_id' => $ticket->getID()]); + $this->assertCount(1, $rows); + $this->assertSame(TicketReservationRequest::STATUS_CANCELED, (int) reset($rows)['status']); + + $queue_after_submission = $this->countReservationRequestQueuedNotifications(); + $this->assertSame($queue_before_submission, $queue_after_submission); + } + public function testApproveDeniedWithoutTicketUpdateRightLeavesRequestUnchanged(): void { [$ticket] = $this->submitReservationForm(require_approval: true); @@ -253,6 +327,19 @@ public function testApproveDeniedWithoutTicketUpdateRightLeavesRequestUnchanged( $this->assertSame(TicketReservationRequest::STATUS_WAITING, (int) $request->fields['status']); } + /** + * Count queued notifications for `TicketReservationRequest`, ignoring + * unrelated notifications also queued by submitting the form (e.g. the + * core "New ticket"/"New reservation" notifications triggered by the + * `Ticket`/`Reservation` created along the way). + */ + private function countReservationRequestQueuedNotifications(): int + { + return countElementsInTable('glpi_queuednotifications', [ + 'itemtype' => TicketReservationRequest::class, + ]); + } + /** * Build a form with a single `ReservationQuestion`, configure its * `Ticket` destination's `PreReservationField` with the given approval diff --git a/tests/Service/InstallManagerTest.php b/tests/Service/InstallManagerTest.php index 43cb9f5..6a06c06 100644 --- a/tests/Service/InstallManagerTest.php +++ b/tests/Service/InstallManagerTest.php @@ -42,6 +42,8 @@ final class InstallManagerTest extends AdvancedFormsTestCase { public function testUninstallRemoveConfig(): void { + global $DB; + // Arrange: set multiples config values $config_values = []; foreach (self::provideQuestionTypes() as $type) { @@ -50,14 +52,38 @@ public function testUninstallRemoveConfig(): void Config::setConfigurationValues('advancedforms', $config_values); - // Act: uninstall plugin $config_before = Config::getConfigurationValues('advancedforms'); - InstallManager::getInstance()->uninstall(); - $config_after = Config::getConfigurationValues('advancedforms'); - - // Assert: config should be empty after uninstallation $this->assertNotEmpty($config_before); - $this->assertEmpty($config_after); + + // `uninstall()` also drops the plugin's table (real `DROP TABLE`), + // which core's `DBmysql::checkForDDLInsideTransaction()` forbids + // while inside an active transaction (DDL causes an implicit commit + // that would otherwise silently break `DbTestCase`'s + // rollback-per-test isolation). `DbTestCase::setUp()` always opens + // one such transaction, so it must be committed first, and a fresh + // one reopened afterward so `DbTestCase::tearDown()`'s own rollback + // still finds a transaction to close. This means the DB changes made + // in this test are real and not rolled back by `tearDown()`; the + // plugin's full state (config + table) is explicitly restored below + // so any test running afterward in this same process/DB is + // unaffected. See testUninstallDropsTicketReservationRequestsTable() + // for the same pattern applied more narrowly to just the table. + $DB->commit(); + + try { + InstallManager::getInstance()->uninstall(); + $config_after = Config::getConfigurationValues('advancedforms'); + + // Assert: config should be empty after uninstallation + $this->assertEmpty($config_after); + + // Restore full plugin state (table) for any test that runs + // afterward in this same process/DB. + $this->assertTrue(InstallManager::getInstance()->install()); + $this->assertTrue($DB->tableExists(TicketReservationRequest::getTable(), false)); + } finally { + $DB->beginTransaction(); + } } public function testInstallCreatesTicketReservationRequestsTable(): void @@ -73,4 +99,39 @@ public function testInstallIsIdempotent(): void $this->assertTrue(InstallManager::getInstance()->install()); $this->assertTrue(InstallManager::getInstance()->install()); } + + public function testUninstallDropsTicketReservationRequestsTable(): void + { + global $DB; + + // See the detailed explanation in testUninstallRemoveConfig(): the + // real `DROP TABLE` performed by `uninstall()` cannot run inside the + // transaction `DbTestCase::setUp()` opens, so it is committed first + // and a fresh one is reopened afterward for `tearDown()`. + $DB->commit(); + + try { + // Arrange: make sure the table exists before uninstalling. + $this->assertTrue(InstallManager::getInstance()->install()); + $this->assertTrue($DB->tableExists(TicketReservationRequest::getTable(), false)); + + // Act + $this->assertTrue(InstallManager::getInstance()->uninstall()); + + // Assert: the table must be gone (no orphaned table left behind). + // `usecache: false` is required here: `tableExists()`'s cache + // only ever grows and is never invalidated by a drop, so the + // `true` cached just above by the "Arrange" step would otherwise + // make this assertion pass regardless of the real DB state. + $this->assertFalse($DB->tableExists(TicketReservationRequest::getTable(), false)); + + // Restore the table for any other test that runs afterward in + // this same process/DB: `install()` is idempotent and safe to + // call again here. + $this->assertTrue(InstallManager::getInstance()->install()); + $this->assertTrue($DB->tableExists(TicketReservationRequest::getTable(), false)); + } finally { + $DB->beginTransaction(); + } + } } From a5bb8bdd5075d949c2838f27e89f457e9b1a0f4b Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Wed, 22 Jul 2026 11:59:57 +0200 Subject: [PATCH 20/26] Rector --- src/Controller/GetReservationsController.php | 1 + src/Controller/ReservableItemsController.php | 2 +- src/Model/Destination/PreReservationField.php | 4 +++- src/Model/Destination/PreReservationFieldConfig.php | 2 ++ src/Model/QuestionType/ReservationQuestionAnswer.php | 8 ++++---- src/Model/TicketReservationRequest.php | 5 +++++ src/Service/InitManager.php | 2 +- src/Service/InstallManager.php | 5 +++-- src/Service/TimelineManager.php | 2 +- 9 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/Controller/GetReservationsController.php b/src/Controller/GetReservationsController.php index 6f0a243..9dc35d4 100644 --- a/src/Controller/GetReservationsController.php +++ b/src/Controller/GetReservationsController.php @@ -72,6 +72,7 @@ public function __invoke(Request $request): Response if ($begin !== '') { $where['end'] = ['>', $begin]; } + if ($end !== '') { $where['begin'] = ['<', $end]; } diff --git a/src/Controller/ReservableItemsController.php b/src/Controller/ReservableItemsController.php index 617bfea..b4ffad1 100644 --- a/src/Controller/ReservableItemsController.php +++ b/src/Controller/ReservableItemsController.php @@ -100,7 +100,7 @@ private function getReservableItemsForType(string $itemtype, string $search): ar $where['items_id'] = new QuerySubQuery([ 'SELECT' => 'id', 'FROM' => $itemtype::getTable(), - 'WHERE' => ['name' => ['LIKE', "%$search%"]], + 'WHERE' => ['name' => ['LIKE', sprintf('%%%s%%', $search)]], ]); } diff --git a/src/Model/Destination/PreReservationField.php b/src/Model/Destination/PreReservationField.php index 0e1e1c7..eb781d8 100644 --- a/src/Model/Destination/PreReservationField.php +++ b/src/Model/Destination/PreReservationField.php @@ -33,6 +33,7 @@ namespace GlpiPlugin\Advancedforms\Model\Destination; +use Glpi\Form\Answer; use Glpi\Application\View\TemplateRenderer; use Glpi\DBAL\JsonFieldInterface; use Glpi\Form\AnswersSet; @@ -92,6 +93,7 @@ public function getStrategiesForDropdown(): array foreach (PreReservationFieldStrategy::cases() as $strategy) { $values[$strategy->value] = $strategy->getLabel(); } + return $values; } @@ -163,7 +165,7 @@ public function applyConfiguratedValueAfterDestinationCreation( } $question_answer = $answers_set->getAnswerByQuestionId($question_id); - if ($question_answer === null) { + if (!$question_answer instanceof Answer) { // The question was left unanswered (or skipped): nothing to do. return; } diff --git a/src/Model/Destination/PreReservationFieldConfig.php b/src/Model/Destination/PreReservationFieldConfig.php index 6ef68b8..c5592f6 100644 --- a/src/Model/Destination/PreReservationFieldConfig.php +++ b/src/Model/Destination/PreReservationFieldConfig.php @@ -43,7 +43,9 @@ { // Unique reference to hardcoded names used for serialization and forms input names public const STRATEGY = 'strategy'; + public const QUESTION_ID = 'question_id'; + public const REQUIRE_APPROVAL = 'require_approval'; public function __construct( diff --git a/src/Model/QuestionType/ReservationQuestionAnswer.php b/src/Model/QuestionType/ReservationQuestionAnswer.php index d4192f4..11bf20d 100644 --- a/src/Model/QuestionType/ReservationQuestionAnswer.php +++ b/src/Model/QuestionType/ReservationQuestionAnswer.php @@ -54,14 +54,14 @@ public static function fromArray(array $data): self { foreach (['reservationitems_id', 'begin', 'end'] as $key) { if (!isset($data[$key]) || $data[$key] === '') { - throw new InvalidArgumentException("Missing or empty key: $key"); + throw new InvalidArgumentException('Missing or empty key: ' . $key); } } return new self( - reservationitems_id: (int) $data['reservationitems_id'], - begin: (string) $data['begin'], - end: (string) $data['end'], + reservationitems_id: $data['reservationitems_id'], + begin: $data['begin'], + end: $data['end'], ); } diff --git a/src/Model/TicketReservationRequest.php b/src/Model/TicketReservationRequest.php index d42f235..43606a8 100644 --- a/src/Model/TicketReservationRequest.php +++ b/src/Model/TicketReservationRequest.php @@ -48,12 +48,17 @@ final class TicketReservationRequest extends CommonDBChild { public static $itemtype = 'Ticket'; + public static $items_id = 'tickets_id'; + public static $rightname = 'ticket'; public const STATUS_WAITING = 1; + public const STATUS_ACCEPTED = 2; + public const STATUS_REFUSED = 3; + public const STATUS_CANCELED = 4; #[Override] diff --git a/src/Service/InitManager.php b/src/Service/InitManager.php index 14ce8db..7f257a9 100644 --- a/src/Service/InitManager.php +++ b/src/Service/InitManager.php @@ -132,6 +132,6 @@ private function registerTimelineHooks(): void global $PLUGIN_HOOKS; // @phpstan-ignore offsetAccess.nonOffsetAccessible (we don't have type hint for this array at this time) - $PLUGIN_HOOKS[Hooks::TIMELINE_ITEMS]['advancedforms'] = [TimelineManager::class, 'addTimelineItems']; + $PLUGIN_HOOKS[Hooks::TIMELINE_ITEMS]['advancedforms'] = TimelineManager::addTimelineItems(...); } } diff --git a/src/Service/InstallManager.php b/src/Service/InstallManager.php index 121b7e5..3370146 100644 --- a/src/Service/InstallManager.php +++ b/src/Service/InstallManager.php @@ -33,6 +33,7 @@ namespace GlpiPlugin\Advancedforms\Service; +use DBmysql; use Config; use DBConnection; use Glpi\Toolbox\SingletonTrait; @@ -62,7 +63,7 @@ public function install(): bool return true; } - private function installTicketReservationRequestsTable(\DBmysql $DB): void + private function installTicketReservationRequestsTable(DBmysql $DB): void { $table = TicketReservationRequest::getTable(); // `tableExists()`'s cache only ever grows (a table once found never @@ -79,7 +80,7 @@ private function installTicketReservationRequestsTable(\DBmysql $DB): void $default_collation = DBConnection::getDefaultCollation(); $default_key_sign = DBConnection::getDefaultPrimaryKeySignOption(); - $query = "CREATE TABLE `$table` ( + $query = "CREATE TABLE `{$table}` ( `id` int {$default_key_sign} NOT NULL AUTO_INCREMENT, `tickets_id` int {$default_key_sign} NOT NULL DEFAULT '0', `reservationitems_id` int {$default_key_sign} NOT NULL DEFAULT '0', diff --git a/src/Service/TimelineManager.php b/src/Service/TimelineManager.php index e816d38..f44478c 100644 --- a/src/Service/TimelineManager.php +++ b/src/Service/TimelineManager.php @@ -95,7 +95,7 @@ public static function addTimelineItems(array $params): void $users_id = $request->fields['users_id'] ?? 0; $users_id = is_numeric($users_id) ? (int) $users_id : 0; - $timeline["TicketReservationRequest_{$request->getID()}"] = [ + $timeline['TicketReservationRequest_' . $request->getID()] = [ 'type' => TicketReservationRequest::class, 'item' => [ 'id' => $request->getID(), From 299856b01d72ea7561955e646b895bcdf427b7be Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Wed, 22 Jul 2026 13:52:54 +0200 Subject: [PATCH 21/26] Fix phpstan --- src/Model/Destination/PreReservationField.php | 1 - .../ReservationQuestionAnswer.php | 22 ++++++---- .../ReservationQuestionConfig.php | 15 ++++++- src/Model/TicketReservationRequest.php | 40 ++++++++++++++----- 4 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/Model/Destination/PreReservationField.php b/src/Model/Destination/PreReservationField.php index eb781d8..09babf4 100644 --- a/src/Model/Destination/PreReservationField.php +++ b/src/Model/Destination/PreReservationField.php @@ -176,7 +176,6 @@ public function applyConfiguratedValueAfterDestinationCreation( } try { - // @phpstan-ignore argument.type (the exact shape is validated at runtime by fromArray() itself) $answer = ReservationQuestionAnswer::fromArray($raw_answer); } catch (InvalidArgumentException) { // Incomplete/invalid answer: do not block the ticket creation. diff --git a/src/Model/QuestionType/ReservationQuestionAnswer.php b/src/Model/QuestionType/ReservationQuestionAnswer.php index 11bf20d..678f911 100644 --- a/src/Model/QuestionType/ReservationQuestionAnswer.php +++ b/src/Model/QuestionType/ReservationQuestionAnswer.php @@ -35,6 +35,8 @@ use InvalidArgumentException; +use function Safe\strtotime; + final readonly class ReservationQuestionAnswer { public function __construct( @@ -44,11 +46,7 @@ public function __construct( ) {} /** - * @param array{ - * reservationitems_id: int, - * begin: string, - * end: string - * } $data + * @param array $data */ public static function fromArray(array $data): self { @@ -58,10 +56,18 @@ public static function fromArray(array $data): self } } + $reservationitems_id = $data['reservationitems_id']; + $begin = $data['begin']; + $end = $data['end']; + + if (!is_numeric($reservationitems_id) || !is_string($begin) || !is_string($end)) { + throw new InvalidArgumentException('Invalid type for reservation answer data'); + } + return new self( - reservationitems_id: $data['reservationitems_id'], - begin: $data['begin'], - end: $data['end'], + reservationitems_id: (int) $reservationitems_id, + begin: $begin, + end: $end, ); } diff --git a/src/Model/QuestionType/ReservationQuestionConfig.php b/src/Model/QuestionType/ReservationQuestionConfig.php index ec85d65..863e2a8 100644 --- a/src/Model/QuestionType/ReservationQuestionConfig.php +++ b/src/Model/QuestionType/ReservationQuestionConfig.php @@ -41,6 +41,9 @@ // Unique reference to hardcoded name used for serialization public const ALLOWED_ITEMTYPES = 'allowed_itemtypes'; + /** + * @param array $allowed_itemtypes + */ public function __construct( private array $allowed_itemtypes = [], ) {} @@ -71,11 +74,17 @@ public function jsonSerialize(): array ]; } + /** + * @return array + */ public function getAllowedItemtypes(): array { return $this->allowed_itemtypes; } + /** + * @return array + */ public function getEffectiveAllowedItemtypes(): array { global $CFG_GLPI; @@ -84,6 +93,10 @@ public function getEffectiveAllowedItemtypes(): array return $this->allowed_itemtypes; } - return $CFG_GLPI['reservation_types'] ?? []; + $reservation_types = $CFG_GLPI['reservation_types'] ?? []; + + return is_array($reservation_types) + ? array_values(array_filter($reservation_types, is_string(...))) + : []; } } diff --git a/src/Model/TicketReservationRequest.php b/src/Model/TicketReservationRequest.php index 43606a8..75d3f39 100644 --- a/src/Model/TicketReservationRequest.php +++ b/src/Model/TicketReservationRequest.php @@ -105,7 +105,7 @@ public function post_addItem(): void // "please wait for approval" notification would fire for something // that never actually waits for approval. if ( - (int) $this->fields['status'] === self::STATUS_WAITING + $this->getIntField('status') === self::STATUS_WAITING && !isset($this->input['_disablenotif']) ) { NotificationEvent::raiseEvent('reservation_request_created', $this); @@ -133,7 +133,11 @@ public function isSlotStillAvailable(): bool ], ])->current(); - return ((int) $conflicts['cpt']) === 0; + $count = is_array($conflicts) && is_numeric($conflicts['cpt'] ?? null) + ? (int) $conflicts['cpt'] + : 0; + + return $count === 0; } /** @@ -237,14 +241,14 @@ public function getTimelineInfo(): array { return [ 'id' => $this->getID(), - 'status' => (int) $this->fields['status'], - 'reservationitems_id' => (int) $this->fields['reservationitems_id'], - 'begin' => $this->fields['begin'], - 'end' => $this->fields['end'], - 'comment_validation' => $this->fields['comment_validation'] ?? '', + 'status' => $this->getIntField('status'), + 'reservationitems_id' => $this->getIntField('reservationitems_id'), + 'begin' => $this->getNullableStringField('begin'), + 'end' => $this->getNullableStringField('end'), + 'comment_validation' => $this->getNullableStringField('comment_validation') ?? '', 'can_answer' => $this->canAnswer(), - 'is_direct_reservation' => (int) $this->fields['status'] === self::STATUS_ACCEPTED - && (int) $this->fields['users_id_validate'] === 0, + 'is_direct_reservation' => $this->getIntField('status') === self::STATUS_ACCEPTED + && $this->getIntField('users_id_validate') === 0, ]; } @@ -255,15 +259,29 @@ public function getTimelineInfo(): array */ public function canAnswer(): bool { - if ((int) $this->fields['status'] !== self::STATUS_WAITING) { + if ($this->getIntField('status') !== self::STATUS_WAITING) { return false; } $ticket = new Ticket(); - if (!$ticket->getFromDB($this->fields['tickets_id'])) { + if (!$ticket->getFromDB($this->getIntField('tickets_id'))) { return false; } return $ticket->canUpdateItem(); } + + private function getIntField(string $field): int + { + $value = $this->fields[$field] ?? null; + + return is_numeric($value) ? (int) $value : 0; + } + + private function getNullableStringField(string $field): ?string + { + $value = $this->fields[$field] ?? null; + + return is_string($value) ? $value : null; + } } From ce5dddc62bbfa8c1a8949697f158ef0093f36de9 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Thu, 23 Jul 2026 12:11:08 +0200 Subject: [PATCH 22/26] Fix details --- .../js/modules/ReservationQuestionWidget.js | 71 +++++++--------- .../ReservationRequestTimelineActions.js | 5 +- .../ReservationRequestController.php | 2 - src/Model/Destination/PreReservationField.php | 26 +----- .../Destination/PreReservationFieldConfig.php | 5 +- ...ficationTargetTicketReservationRequest.php | 18 +---- .../QuestionType/ReservationQuestion.php | 8 -- .../ReservationQuestionAnswer.php | 12 +-- .../ReservationQuestionConfig.php | 24 ++---- src/Model/TicketReservationRequest.php | 69 ++-------------- src/Service/InitManager.php | 8 +- src/Service/InstallManager.php | 15 +--- src/Service/TimelineManager.php | 32 ++------ .../prereservation_config_field.html.twig | 31 ++++--- templates/reservation_question.html.twig | 50 ++++++------ .../timeline/reservation_request.html.twig | 29 ++++--- .../ReservationRequestControllerTest.php | 5 +- .../Destination/PreReservationFieldTest.php | 34 +------- .../QuestionType/ReservationQuestionTest.php | 15 ---- tests/Model/TicketReservationRequestTest.php | 42 +--------- tests/ReservationWorkflowTest.php | 80 +++---------------- tests/Service/InstallManagerTest.php | 67 ---------------- tests/Service/TimelineManagerTest.php | 4 +- 23 files changed, 128 insertions(+), 524 deletions(-) diff --git a/public/js/modules/ReservationQuestionWidget.js b/public/js/modules/ReservationQuestionWidget.js index 722b782..874a3b5 100644 --- a/public/js/modules/ReservationQuestionWidget.js +++ b/public/js/modules/ReservationQuestionWidget.js @@ -33,10 +33,7 @@ export class ReservationQuestionWidget { #root; #endpoint_url = `${CFG_GLPI.root_doc}/plugins/advancedforms/ReservationWidget`; - /** - * @param {HTMLElement} root - The root element of the widget, as rendered - * by `templates/reservation_question.html.twig`. - */ + /** @param {HTMLElement} root - root element rendered by templates/reservation_question.html.twig */ constructor(root) { if (!root) { return; @@ -45,7 +42,7 @@ export class ReservationQuestionWidget { this.#root = root; this.#initItemSelect(); - this.#initDatePickers(); + this.#initDateChangeListeners(); } #initItemSelect() { @@ -78,13 +75,7 @@ export class ReservationQuestionWidget { $select.on('select2:clear select2:unselecting', () => this.#onItemCleared()); } - /** - * Group flat `{id, text, itemtype}` results into Select2 optgroups so - * items are presented per reservable itemtype. - * - * @param {Array<{id: number, text: string, itemtype: string}>} data - * @return {Array<{text: string, children: Array<{id: number, text: string}>}>} - */ + /** Groups flat {id, text, itemtype} results into Select2 optgroups per itemtype. */ #groupResultsByItemtype(data) { const groups = new Map(); @@ -104,58 +95,54 @@ export class ReservationQuestionWidget { #onItemSelected() { const $select = $(this.#root.querySelector('[data-reservation-question-item-select]')); - this.#setFieldValue('reservationitems_id', $select.val()); + this.#setReservationItemsId($select.val()); $(this.#root.querySelector('[data-reservation-question-dates]')).removeClass('d-none'); this.#checkAvailability(); } #onItemCleared() { - this.#setFieldValue('reservationitems_id', ''); - this.#setFieldValue('begin', ''); - this.#setFieldValue('end', ''); + this.#setReservationItemsId(''); + if (this.#getBeginInput()) { + this.#getBeginInput().value = ''; + } + if (this.#getEndInput()) { + this.#getEndInput().value = ''; + } $(this.#root.querySelector('[data-reservation-question-dates]')).addClass('d-none'); this.#showAvailability(null); } - #initDatePickers() { - const begin_input = this.#root.querySelector('[data-reservation-question-begin-picker]'); - const end_input = this.#root.querySelector('[data-reservation-question-end-picker]'); + /** begin/end are rendered by the datetimeField macro (self-initializing Flatpickr); just react to changes. */ + #initDateChangeListeners() { + const begin_input = this.#getBeginInput(); + const end_input = this.#getEndInput(); if (!begin_input || !end_input) { return; } - const options = { - enableTime: true, - enableSeconds: true, - time_24hr: true, - dateFormat: 'Y-m-d H:i:S', - onChange: (selected_dates, date_str, instance) => { - const field = instance.element === begin_input ? 'begin' : 'end'; - this.#setFieldValue(field, date_str); - this.#checkAvailability(); - }, - }; + $(begin_input).on('change', () => this.#checkAvailability()); + $(end_input).on('change', () => this.#checkAvailability()); + } - flatpickr(begin_input, options); - flatpickr(end_input, options); + #getBeginInput() { + return this.#root.querySelector('[data-reservation-question-begin-picker]'); } - #setFieldValue(field, value) { - const input = this.#root.querySelector(`[data-reservation-question-field="${field}"]`); + #getEndInput() { + return this.#root.querySelector('[data-reservation-question-end-picker]'); + } + + #setReservationItemsId(value) { + const input = this.#root.querySelector('[data-reservation-question-field="reservationitems_id"]'); if (input) { input.value = value ?? ''; } } - #getFieldValue(field) { - const input = this.#root.querySelector(`[data-reservation-question-field="${field}"]`); - return input ? input.value : ''; - } - #checkAvailability() { - const reservationitems_id = this.#getFieldValue('reservationitems_id'); - const begin = this.#getFieldValue('begin'); - const end = this.#getFieldValue('end'); + const reservationitems_id = this.#root.querySelector('[data-reservation-question-field="reservationitems_id"]')?.value ?? ''; + const begin = this.#getBeginInput()?.value ?? ''; + const end = this.#getEndInput()?.value ?? ''; if (!reservationitems_id || !begin || !end) { this.#showAvailability(null); diff --git a/public/js/modules/ReservationRequestTimelineActions.js b/public/js/modules/ReservationRequestTimelineActions.js index 509eeb9..275ed91 100644 --- a/public/js/modules/ReservationRequestTimelineActions.js +++ b/public/js/modules/ReservationRequestTimelineActions.js @@ -33,10 +33,7 @@ export class ReservationRequestTimelineActions { #root; #endpoint_url = `${CFG_GLPI.root_doc}/plugins/advancedforms/ReservationRequest`; - /** - * @param {HTMLElement} root - The root element of a single timeline card, - * as rendered by `templates/timeline/reservation_request.html.twig`. - */ + /** @param {HTMLElement} root - one timeline card, rendered by templates/timeline/reservation_request.html.twig */ constructor(root) { if (!root) { return; diff --git a/src/Controller/ReservationRequestController.php b/src/Controller/ReservationRequestController.php index e6dd0df..71d11e5 100644 --- a/src/Controller/ReservationRequestController.php +++ b/src/Controller/ReservationRequestController.php @@ -72,8 +72,6 @@ public function __invoke(Request $request): Response throw new AccessDeniedHttpException(); } - // The controller requires an authenticated session (see the - // SecurityStrategy attribute above), so this is always a real user id. $users_id_validate = (int) Session::getLoginUserID(); if ($action === 'approve') { diff --git a/src/Model/Destination/PreReservationField.php b/src/Model/Destination/PreReservationField.php index 09babf4..48d026e 100644 --- a/src/Model/Destination/PreReservationField.php +++ b/src/Model/Destination/PreReservationField.php @@ -48,11 +48,7 @@ use Override; use Ticket; -/** - * Destination config field allowing a form to automatically create a - * `TicketReservationRequest` from the answer of a `ReservationQuestion` once - * the destination `Ticket` has been created. - */ +/** Turns a ReservationQuestion answer into a TicketReservationRequest once the destination Ticket is created. */ final class PreReservationField extends AbstractConfigField { #[Override] @@ -112,13 +108,10 @@ public function renderConfigForm( $twig = TemplateRenderer::getInstance(); return $twig->render('@advancedforms/destination/prereservation_config_field.html.twig', [ - // Possible configuration constant that will be used to hide/show additional fields 'CONFIG_FROM_SPECIFIC_QUESTION' => PreReservationFieldStrategy::FROM_SPECIFIC_QUESTION->value, - // General display options 'options' => $display_options, - // Additional config displayed only for the FROM_SPECIFIC_QUESTION strategy 'question_extra_field' => [ 'empty_label' => __("Select a question...", 'advancedforms'), 'value' => $config->getQuestionId(), @@ -143,10 +136,8 @@ public function applyConfiguratedValueAfterDestinationCreation( return; } - // Only one strategy is allowed $strategy = current($config->getStrategies()); if ($strategy !== PreReservationFieldStrategy::FROM_SPECIFIC_QUESTION) { - // Nothing to do: no pre-reservation was requested for this destination. return; } @@ -155,9 +146,6 @@ public function applyConfiguratedValueAfterDestinationCreation( return; } - // `$created_objects` maps each destination's id to the items it - // created (see `DestinationFieldInterface::applyConfiguratedValueAfterDestinationCreation()` - // and `AnswersHandler::createDestinations()`), it is not a flat list. $destination_items = $created_objects[$destination->getID()] ?? []; $ticket = $destination_items[0] ?? null; if (!$ticket instanceof Ticket) { @@ -166,7 +154,6 @@ public function applyConfiguratedValueAfterDestinationCreation( $question_answer = $answers_set->getAnswerByQuestionId($question_id); if (!$question_answer instanceof Answer) { - // The question was left unanswered (or skipped): nothing to do. return; } @@ -178,7 +165,6 @@ public function applyConfiguratedValueAfterDestinationCreation( try { $answer = ReservationQuestionAnswer::fromArray($raw_answer); } catch (InvalidArgumentException) { - // Incomplete/invalid answer: do not block the ticket creation. return; } @@ -195,16 +181,6 @@ public function applyConfiguratedValueAfterDestinationCreation( ]; if (!$require_approval) { - // In direct/no-approval mode, the WAITING status set above is - // only a transient step before immediately transitioning this - // request to its real final state below (ACCEPTED/CANCELED), - // which each raise their own distinct notification event. The - // request never actually waits for approval, so the "please wait - // for approval" notification must be suppressed for this insert. - // (The key must only be present when disabling: `isset()` is what - // `TicketReservationRequest::post_addItem()` checks, and it is - // `true` even for an explicit `false` value, matching the - // `_disablenotif` convention used throughout GLPI core.) $add_input['_disablenotif'] = true; } diff --git a/src/Model/Destination/PreReservationFieldConfig.php b/src/Model/Destination/PreReservationFieldConfig.php index c5592f6..e6c2b0f 100644 --- a/src/Model/Destination/PreReservationFieldConfig.php +++ b/src/Model/Destination/PreReservationFieldConfig.php @@ -41,7 +41,6 @@ #[HasFieldWithQuestionId(self::QUESTION_ID)] final readonly class PreReservationFieldConfig implements JsonFieldInterface, ConfigFieldWithStrategiesInterface { - // Unique reference to hardcoded names used for serialization and forms input names public const STRATEGY = 'strategy'; public const QUESTION_ID = 'question_id'; @@ -99,9 +98,7 @@ public static function getStrategiesInputName(): string return self::STRATEGY; } - /** - * @return array - */ + /** @return array */ #[Override] public function getStrategies(): array { diff --git a/src/Model/NotificationTargetTicketReservationRequest.php b/src/Model/NotificationTargetTicketReservationRequest.php index aa71f9d..7cdb714 100644 --- a/src/Model/NotificationTargetTicketReservationRequest.php +++ b/src/Model/NotificationTargetTicketReservationRequest.php @@ -39,14 +39,7 @@ use Override; use Ticket; -/** - * Notification target for `TicketReservationRequest`. Auto-discovered by - * core (see `NotificationTarget::getInstanceClass()`) because it lives in - * the same namespace as `TicketReservationRequest` and is named - * `NotificationTarget`. - * - * @extends NotificationTarget - */ +/** @extends NotificationTarget */ final class NotificationTargetTicketReservationRequest extends NotificationTarget { #[Override] @@ -63,12 +56,8 @@ public function getEvents(): array #[Override] public function addAdditionalTargets($event = ''): void { - // The original requester (the request's own `users_id`, not the - // validator) is always notified. $this->addTarget(Notification::AUTHOR, __('Requester')); - // Only notify the parent ticket's technician(s) when a new request - // is created: they are the ones expected to approve/refuse it. if ($event === 'reservation_request_created') { $this->addTarget(Notification::ITEM_TECH_IN_CHARGE, __('Technician in charge')); $this->addTarget(Notification::ITEM_TECH_GROUP_IN_CHARGE, __('Group in charge')); @@ -114,11 +103,6 @@ public function getTags(): void #[Override] public function getObjectItem($event = '') { - // Technician-in-charge related targets are resolved from - // `$this->target_object` (see `NotificationTarget::addUserByField()`), - // and technicians are attached to the parent `Ticket`, not to the - // `TicketReservationRequest` itself. Point `target_object` at the - // parent ticket so those targets resolve correctly. if ($this->obj instanceof TicketReservationRequest) { $tickets_id = $this->obj->fields['tickets_id']; $ticket = new Ticket(); diff --git a/src/Model/QuestionType/ReservationQuestion.php b/src/Model/QuestionType/ReservationQuestion.php index 4de109f..a1cee1a 100644 --- a/src/Model/QuestionType/ReservationQuestion.php +++ b/src/Model/QuestionType/ReservationQuestion.php @@ -80,15 +80,12 @@ public function getExtraDataConfigClass(): string #[Override] public function validateExtraDataInput(array $input): bool { - // All fields of `ReservationQuestionConfig` are optional: an empty - // `allowed_itemtypes` list means "every reservable type is allowed". return true; } #[Override] public function renderAdministrationTemplate(Question|null $question): string { - // Read extra config specific to this question type $decoded_extra_data = []; if ($question instanceof Question && is_string($question->fields['extra_data'])) { $decoded_extra_data = json_decode( @@ -96,7 +93,6 @@ public function renderAdministrationTemplate(Question|null $question): string associative: true, ); - // Fallback to safe value if (!is_array($decoded_extra_data)) { $decoded_extra_data = []; } @@ -140,7 +136,6 @@ public function renderEndUserTemplate(Question $question): string associative: true, ); - // Fallback to safe value if (!is_array($decoded_extra_data)) { $decoded_extra_data = []; } @@ -167,9 +162,6 @@ public function prepareEndUserAnswer(Question $question, mixed $answer): mixed try { return ReservationQuestionAnswer::fromArray($answer)->toArray(); } catch (InvalidArgumentException) { - // The question is optional (or was simply left untouched) and - // the widget's hidden inputs are empty: treat this the same as - // "not answered" instead of failing the whole form submission. return null; } } diff --git a/src/Model/QuestionType/ReservationQuestionAnswer.php b/src/Model/QuestionType/ReservationQuestionAnswer.php index 678f911..a853727 100644 --- a/src/Model/QuestionType/ReservationQuestionAnswer.php +++ b/src/Model/QuestionType/ReservationQuestionAnswer.php @@ -45,9 +45,7 @@ public function __construct( private string $end, ) {} - /** - * @param array $data - */ + /** @param array $data */ public static function fromArray(array $data): self { foreach (['reservationitems_id', 'begin', 'end'] as $key) { @@ -71,13 +69,7 @@ public static function fromArray(array $data): self ); } - /** - * @return array{ - * reservationitems_id: int, - * begin: string, - * end: string - * } - */ + /** @return array{reservationitems_id: int, begin: string, end: string} */ public function toArray(): array { return [ diff --git a/src/Model/QuestionType/ReservationQuestionConfig.php b/src/Model/QuestionType/ReservationQuestionConfig.php index 863e2a8..bfc38fe 100644 --- a/src/Model/QuestionType/ReservationQuestionConfig.php +++ b/src/Model/QuestionType/ReservationQuestionConfig.php @@ -41,18 +41,12 @@ // Unique reference to hardcoded name used for serialization public const ALLOWED_ITEMTYPES = 'allowed_itemtypes'; - /** - * @param array $allowed_itemtypes - */ + /** @param array $allowed_itemtypes */ public function __construct( private array $allowed_itemtypes = [], ) {} - /** - * @param array{ - * allowed_itemtypes?: array - * } $data - */ + /** @param array{allowed_itemtypes?: array} $data */ #[Override] public static function jsonDeserialize(array $data): self { @@ -61,11 +55,7 @@ public static function jsonDeserialize(array $data): self ); } - /** - * @return array{ - * allowed_itemtypes: array - * } $data - */ + /** @return array{allowed_itemtypes: array} */ #[Override] public function jsonSerialize(): array { @@ -74,17 +64,13 @@ public function jsonSerialize(): array ]; } - /** - * @return array - */ + /** @return array */ public function getAllowedItemtypes(): array { return $this->allowed_itemtypes; } - /** - * @return array - */ + /** @return array */ public function getEffectiveAllowedItemtypes(): array { global $CFG_GLPI; diff --git a/src/Model/TicketReservationRequest.php b/src/Model/TicketReservationRequest.php index 75d3f39..068f767 100644 --- a/src/Model/TicketReservationRequest.php +++ b/src/Model/TicketReservationRequest.php @@ -39,12 +39,7 @@ use Reservation; use Ticket; -/** - * A request, made from a ticket, to reserve a reservable item (asset) for a - * given timeframe. It is created by the "reservation" question type's - * destination field and must be approved or refused by a ticket actor before - * an actual `Reservation` is created. - */ +/** A ticket-driven request to reserve an equipment item for a timeframe. */ final class TicketReservationRequest extends CommonDBChild { public static $itemtype = 'Ticket'; @@ -64,10 +59,6 @@ final class TicketReservationRequest extends CommonDBChild #[Override] public static function getTable($classname = null): string { - // The default table name computation (based on the fully qualified - // class name) would produce `glpi_plugin_advancedforms_models_ticketreservationrequests` - // because of the `Model` sub-namespace. The table must be forced to - // its expected name instead. return 'glpi_plugin_advancedforms_ticketreservationrequests'; } @@ -88,22 +79,6 @@ public function post_addItem(): void { parent::post_addItem(); - // Only notify ticket actors that a new request needs an answer when - // the request is actually created in a waiting state. A request - // created already accepted (e.g. a future "direct reservation, no - // approval needed" path) has nothing to approve/refuse, so it must - // not raise this event. - // - // A caller can also explicitly opt out of this notification (via the - // standard `_disablenotif` input key, see `CommonDBTM`/`Ticket`/etc.) - // even while inserting the row in the WAITING state: this is used by - // `PreReservationField` for its "no approval required" (direct - // reservation) path, where the row is inserted as WAITING only as a - // transient step before immediately transitioning it to its real - // final state (ACCEPTED/CANCELED) via `approve()`/`markUnavailable()`, - // which each raise their own distinct event. Without this, the - // "please wait for approval" notification would fire for something - // that never actually waits for approval. if ( $this->getIntField('status') === self::STATUS_WAITING && !isset($this->input['_disablenotif']) @@ -112,13 +87,7 @@ public function post_addItem(): void } } - /** - * Whether the requested reservation item is still free for this - * request's timeframe, i.e. no existing `Reservation` overlaps it. - * - * Mirrors core's `Reservation::is_reserved()` conflict detection exactly - * (strict inequalities: `end > begin AND begin < end`). - */ + /** Mirrors core's Reservation::is_reserved() overlap check (strict inequalities). */ public function isSlotStillAvailable(): bool { global $DB; @@ -140,18 +109,7 @@ public function isSlotStillAvailable(): bool return $count === 0; } - /** - * Approve this request: create the actual `Reservation` (attributed to - * the original requester, not the validator) and mark this request as - * accepted. - * - * @param int $users_id_validate Id of the user validating the request - * @param string $comment Optional comment from the validator - * - * @return bool False if the `Reservation` could not be created (for - * instance because the slot is no longer available); in - * that case, this request is left untouched. - */ + /** Creates the Reservation for the original requester and accepts this request; returns false, untouched, if the slot is no longer available. */ public function approve(int $users_id_validate, string $comment = ''): bool { $reservation = new Reservation(); @@ -159,7 +117,7 @@ public function approve(int $users_id_validate, string $comment = ''): bool 'reservationitems_id' => $this->fields['reservationitems_id'], 'begin' => $this->fields['begin'], 'end' => $this->fields['end'], - 'users_id' => $this->fields['users_id'], // original requester, not the validator + 'users_id' => $this->fields['users_id'], 'comment' => $this->fields['comment_submission'] ?? '', ]); @@ -182,12 +140,6 @@ public function approve(int $users_id_validate, string $comment = ''): bool return $updated; } - /** - * Refuse this request. No `Reservation` is created. - * - * @param int $users_id_validate Id of the user refusing the request - * @param string $comment Optional comment from the validator - */ public function refuse(int $users_id_validate, string $comment = ''): bool { $updated = $this->update([ @@ -205,10 +157,7 @@ public function refuse(int $users_id_validate, string $comment = ''): bool return $updated; } - /** - * Mark this request as no longer available (e.g. the reservable item was - * removed or deactivated). - */ + /** Marks this request canceled (e.g. the reservable item was removed or deactivated). */ public function markUnavailable(): bool { $updated = $this->update([ @@ -224,8 +173,6 @@ public function markUnavailable(): bool } /** - * Data needed to render this request in a ticket's timeline. - * * @return array{ * id: int, * status: int, @@ -252,11 +199,7 @@ public function getTimelineInfo(): array ]; } - /** - * Whether the current session's user may approve/refuse this request: - * it must still be waiting, and the user must be able to update the - * parent ticket. - */ + /** Whether the current user may approve/refuse this still-waiting request. */ public function canAnswer(): bool { if ($this->getIntField('status') !== self::STATUS_WAITING) { diff --git a/src/Service/InitManager.php b/src/Service/InitManager.php index 7f257a9..1189215 100644 --- a/src/Service/InitManager.php +++ b/src/Service/InitManager.php @@ -106,13 +106,7 @@ private function registerDestinationFields(): void { $destination_manager = FormDestinationManager::getInstance(); - // `FormDestinationManager` is a plain singleton with no way to - // unregister a field: `init()` may legitimately be called several - // times during the same request/process (e.g. once automatically on - // plugin boot, then again whenever a configurable item is toggled). - // Guard against registering the same field more than once, which - // would otherwise make `applyConfiguratedValueAfterDestinationCreation()` - // run multiple times per created ticket. + // init() may run multiple times per process; avoid double-registering the field. $already_registered = array_filter( $destination_manager->getPluginCommonITILConfigFields(FormDestinationTicket::class), fn($field) => $field instanceof PreReservationField, diff --git a/src/Service/InstallManager.php b/src/Service/InstallManager.php index 3370146..2bd3796 100644 --- a/src/Service/InstallManager.php +++ b/src/Service/InstallManager.php @@ -52,12 +52,6 @@ public function install(): bool $this->installTicketReservationRequestsTable($DB); - // No addField()/addKey()/... calls are queued on `$migration` yet - // (the table above is created with a plain, guarded `CREATE TABLE`), - // so this call is currently a no-op. It is kept as a safety net so - // that any future schema change added here (e.g. via `addField()`) - // is not silently forgotten, matching the convention used by other - // GLPI plugins (see `plugins/fields/hook.php`). $migration->executeMigration(); return true; @@ -66,12 +60,7 @@ public function install(): bool private function installTicketReservationRequestsTable(DBmysql $DB): void { $table = TicketReservationRequest::getTable(); - // `tableExists()`'s cache only ever grows (a table once found never - // leaves it), it is never invalidated when a table is dropped. Since - // `uninstall()` can genuinely drop this table within the same PHP - // process (e.g. an uninstall immediately followed by a reinstall), - // relying on the cache here could make this guard wrongly believe - // the table still exists and skip recreating it. Bypass the cache. + if ($DB->tableExists($table, false)) { return; } @@ -113,8 +102,6 @@ public function uninstall(): bool $config->deleteByCriteria(['context' => 'advancedforms']); $table = TicketReservationRequest::getTable(); - // Bypass the cache here too, for the same reason as in - // installTicketReservationRequestsTable(). if ($DB->tableExists($table, false)) { $DB->dropTable($table); } diff --git a/src/Service/TimelineManager.php b/src/Service/TimelineManager.php index f44478c..e7956b1 100644 --- a/src/Service/TimelineManager.php +++ b/src/Service/TimelineManager.php @@ -40,26 +40,15 @@ use ReservationItem; use Ticket; -/** - * Injects `TicketReservationRequest` entries into a `Ticket`'s timeline. - * - * Registered against `Hooks::TIMELINE_ITEMS` by `InitManager::registerTimelineHooks()`. - */ +/** Injects `TicketReservationRequest` entries into a `Ticket`'s timeline (registered against `Hooks::TIMELINE_ITEMS`). */ final class TimelineManager { /** * @param array{item: object, timeline: array>} $params - * Called by core's `Plugin::doHook(Hooks::TIMELINE_ITEMS, ['item' => $this, 'timeline' => &$timeline])` - * (see `CommonITILObject::getTimelineItems()`). The 'timeline' entry - * of the array holds a genuine PHP reference to the caller's - * `$timeline` variable: mutating `$params['timeline'][...]` here - * is visible to the caller even though `$params` itself is passed - * by value (a copied array preserves references held by its - * elements). The parameter is therefore intentionally NOT declared - * `array &$params`: `Plugin::doHook()` invokes this callback via - * `call_user_func()`, which cannot pass its argument by reference - * to a callee declaring a by-reference parameter (it triggers a - * PHP warning). + * `$params['timeline']` holds a real PHP reference to the caller's array (see + * `CommonITILObject::getTimelineItems()`), which survives being passed here by + * value — do not declare this `array &$params`, `Plugin::doHook()` calls it via + * `call_user_func()`, which warns on by-ref params. */ public static function addTimelineItems(array $params): void { @@ -72,10 +61,7 @@ public static function addTimelineItems(array $params): void $request = new TicketReservationRequest(); - // Only the ids are read off `find()`'s raw rows: their static return - // type is a generic (unshaped) `array`, so anything read from a row - // is reported as `mixed` by phpstan. All actual field values are - // instead read back from `$request->fields` after `getFromDB()`. + // Only ids come from find(); actual fields are re-read via getFromDB() below. $ids = array_keys($request->find(['tickets_id' => $ticket->getID()])); foreach ($ids as $id) { @@ -111,11 +97,7 @@ public static function addTimelineItems(array $params): void } } - /** - * Resolve a human-readable name for the reservable asset behind a - * `ReservationItem`, e.g. "Laptop #42". Returns an empty string if the - * reservation item or its underlying asset can no longer be found. - */ + /** Resolves the reservable asset's display name; empty string if it no longer exists. */ private static function getEquipmentName(int $reservationitems_id): string { $reservation_item = new ReservationItem(); diff --git a/templates/destination/prereservation_config_field.html.twig b/templates/destination/prereservation_config_field.html.twig index 92174c0..cd03dfe 100644 --- a/templates/destination/prereservation_config_field.html.twig +++ b/templates/destination/prereservation_config_field.html.twig @@ -31,12 +31,6 @@ {% import 'components/form/fields_macros.html.twig' as fields %} -{# The strategy dropdown itself is already rendered by the generic - destination field wrapper (form_destination_commonitil_config.html.twig). - This template only renders the additional fields shown when the - FROM_SPECIFIC_QUESTION strategy is selected: it relies on the same - `data-glpi-itildestination-field-config-display-condition` convention used - by core fields (e.g. UrgencyField) to be shown/hidden automatically. #}
{{ fields.dropdownArrayField( question_extra_field.input_name, @@ -53,14 +47,19 @@ }) ) }} - {{ fields.checkboxField( - require_approval_extra_field.input_name, - require_approval_extra_field.value, - __('Require technician approval', 'advancedforms'), - options|merge({ - field_class: '', - mb: '', - no_label: true, - }) - ) }} +
diff --git a/templates/reservation_question.html.twig b/templates/reservation_question.html.twig index ea892ab..2ba0f41 100644 --- a/templates/reservation_question.html.twig +++ b/templates/reservation_question.html.twig @@ -29,6 +29,8 @@ # ------------------------------------------------------------------------- #} +{% import 'components/form/fields_macros.html.twig' as fields %} +
-
-
-
- -
-
- -
+
+
+ {{ fields.datetimeField( + input_name ~ '[begin]', + '', + __('Start date', 'advancedforms'), + { + is_horizontal: false, + full_width: true, + additional_attributes: {'data-reservation-question-begin-picker': ''}, + } + ) }} +
+
+ {{ fields.datetimeField( + input_name ~ '[end]', + '', + __('End date', 'advancedforms'), + { + is_horizontal: false, + full_width: true, + additional_attributes: {'data-reservation-question-end-picker': ''}, + } + ) }}
-
+
- -