From f91041c380cce63b8680bf30761bf6d901208168 Mon Sep 17 00:00:00 2001 From: Daniel Gohlke Date: Wed, 12 Mar 2025 21:54:47 +0100 Subject: [PATCH 1/3] [BUGFIX] Use ContextualFeedbackServerity in addFlashMessage function In order to better understand the process in the controller, some code blocks have been moved to new private methods. Furthermore, the ENUM ContextualFeedbackSeverity was used directly instead of the serialised array. A method marked as internal is used, but it is not expected that this will change within a major version. However, this makes it much easier to read in the controller. Related: #646 --- Classes/Controller/Cart/ProductController.php | 163 ++++++++++-------- .../RetrieveProductsFromRequestEvent.php | 3 + 2 files changed, 91 insertions(+), 75 deletions(-) diff --git a/Classes/Controller/Cart/ProductController.php b/Classes/Controller/Cart/ProductController.php index b8828ddc..b93dc4b5 100644 --- a/Classes/Controller/Cart/ProductController.php +++ b/Classes/Controller/Cart/ProductController.php @@ -15,7 +15,7 @@ use Extcode\Cart\Event\CheckProductAvailabilityEvent; use Extcode\Cart\Event\RetrieveProductsFromRequestEvent; use Psr\Http\Message\ResponseInterface; -use TYPO3\CMS\Core\Messaging\AbstractMessage; +use TYPO3\CMS\Core\Messaging\FlashMessage; use TYPO3\CMS\Core\Type\ContextualFeedbackSeverity; use TYPO3\CMS\Extbase\Utility\LocalizationUtility; @@ -33,7 +33,6 @@ public function addAction(): ResponseInterface $this->restoreSession(); $event = new RetrieveProductsFromRequestEvent($this->request, $this->cart); - $this->eventDispatcher->dispatch($event); $errors = $event->getErrors(); @@ -49,46 +48,8 @@ public function addAction(): ResponseInterface } } - $messageBody = ''; - $messageTitle = ''; - $severity = ContextualFeedbackSeverity::OK->value; - - if (!empty($errors)) { - foreach ($errors as $error) { - if (!($error instanceof AbstractMessage)) { - continue; - } - $message = $error->jsonSerialize(); - if ($message['severity'] >= $severity) { - $severity = $message['severity']; - $messageBody = $message['message']; - $messageTitle = $message['title']; - } - } - - $pageType = $GLOBALS['TYPO3_REQUEST']->getAttribute('routing')->getPageType(); - if ($pageType === self::AJAX_CART_TYPE_NUM) { - $response = [ - 'status' => '412', - 'count' => $this->cart->getCount(), - 'net' => $this->cart->getNet(), - 'gross' => $this->cart->getGross(), - 'messageBody' => $messageBody, - 'messageTitle' => $messageTitle, - 'severity' => $severity, - ]; - - return $this->jsonResponse(json_encode($response)); - } - - $this->addFlashMessage( - $messageBody, - $messageTitle, - $severity, - true - ); - - return $this->redirect('show', 'Cart\Cart'); + if (empty($errors) === false) { + return $this->responseForAddActionWithErrors($errors); } $quantity = $this->addProductsToCart($cartProducts); @@ -97,39 +58,7 @@ public function addAction(): ResponseInterface $this->sessionHandler->writeCart($this->settings['cart']['pid'], $this->cart); - $messageBody = LocalizationUtility::translate( - 'tx_cart.success.stock_handling.add.' . ($quantity == 1 ? 'one' : 'more'), - 'Cart', - [$quantity] - ); - - $pageType = $GLOBALS['TYPO3_REQUEST']->getAttribute('routing')->getPageType(); - if ($pageType === self::AJAX_CART_TYPE_NUM) { - $productsChanged = $this->getChangedProducts($cartProducts); - - $response = [ - 'status' => '200', - 'added' => $quantity, - 'count' => $this->cart->getCount(), - 'net' => $this->cart->getNet(), - 'gross' => $this->cart->getGross(), - 'productsChanged' => $productsChanged, - 'messageBody' => $messageBody, - 'messageTitle' => $messageTitle, - 'severity' => $severity, - ]; - - return $this->jsonResponse(json_encode($response)); - } - - $this->addFlashMessage( - $messageBody, - $messageTitle, - $severity, - true - ); - - return $this->redirect('show', 'Cart\Cart'); + return $this->responseForAddAction($cartProducts, $quantity); } public function removeAction(): ResponseInterface @@ -167,6 +96,7 @@ protected function getChangedProducts(array $products): array $productsChanged[$product->getId()] = $productChanged->toArray(); } } + return $productsChanged; } @@ -180,6 +110,89 @@ protected function addProductsToCart(array $products): int $this->cart->addProduct($product); } } + return $quantity; } + + /** + * @param Product[] $cartProducts + */ + private function responseForAddAction(array $cartProducts, int $quantity): ResponseInterface + { + $messageBody = LocalizationUtility::translate( + 'tx_cart.success.stock_handling.add.' . ($quantity == 1 ? 'one' : 'more'), + 'Cart', + [$quantity] + ); + + $pageType = $GLOBALS['TYPO3_REQUEST']->getAttribute('routing')->getPageType(); + if ($pageType === self::AJAX_CART_TYPE_NUM) { + $response = [ + 'status' => '200', + 'added' => $quantity, + 'count' => $this->cart->getCount(), + 'net' => $this->cart->getNet(), + 'gross' => $this->cart->getGross(), + 'productsChanged' => $this->getChangedProducts($cartProducts), + 'messageBody' => $messageBody, + 'messageTitle' => '', + 'severity' => ContextualFeedbackSeverity::OK->value, + ]; + + return $this->jsonResponse(json_encode($response)); + } + + $this->addFlashMessage( + $messageBody + ); + + return $this->redirect('show', 'Cart\Cart'); + } + + /** + * @param FlashMessage[] $errors + */ + private function responseForAddActionWithErrors(array $errors): ResponseInterface + { + $errorWithHighestSeverity = $this->getErrorWithHighestSeverity($errors); + + $pageType = $GLOBALS['TYPO3_REQUEST']->getAttribute('routing')->getPageType(); + if ($pageType === self::AJAX_CART_TYPE_NUM) { + $response = [ + 'status' => '412', + 'count' => $this->cart->getCount(), + 'net' => $this->cart->getNet(), + 'gross' => $this->cart->getGross(), + 'messageBody' => $errorWithHighestSeverity->getMessage(), + 'messageTitle' => $errorWithHighestSeverity->getTitle(), + 'severity' => $errorWithHighestSeverity->getSeverity(), + ]; + + return $this->jsonResponse(json_encode($response)); + } + + $this->addFlashMessage( + $errorWithHighestSeverity->getMessage(), + $errorWithHighestSeverity->getTitle(), + $errorWithHighestSeverity->getSeverity(), + ); + + return $this->redirect('show', 'Cart\Cart'); + } + + /** + * @param FlashMessage[] $errors + */ + private function getErrorWithHighestSeverity(array $errors): FlashMessage + { + $errorToReturn = array_shift($errors); + + foreach ($errors as $error) { + if ($error->getSeverity()->value >= $errorToReturn->getSeverity()->value) { + $errorToReturn = $error; + } + } + + return $errorToReturn; + } } diff --git a/Classes/Event/RetrieveProductsFromRequestEvent.php b/Classes/Event/RetrieveProductsFromRequestEvent.php index 39f045a9..b296b4b6 100644 --- a/Classes/Event/RetrieveProductsFromRequestEvent.php +++ b/Classes/Event/RetrieveProductsFromRequestEvent.php @@ -60,6 +60,9 @@ public function addError(FlashMessage $error): void $this->errors[] = $error; } + /** + * @return FlashMessage[] + */ public function getErrors(): array { return $this->errors; From 423f13a745a0516a7b3ecf95701d81111df6b3f9 Mon Sep 17 00:00:00 2001 From: Daniel Gohlke Date: Wed, 12 Mar 2025 22:34:29 +0100 Subject: [PATCH 2/3] [TASK] Add test for getErrorWithHighestSeverity Relates: #646 --- .../Unit/Controller/ProductControllerTest.php | 203 ++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 Tests/Unit/Controller/ProductControllerTest.php diff --git a/Tests/Unit/Controller/ProductControllerTest.php b/Tests/Unit/Controller/ProductControllerTest.php new file mode 100644 index 00000000..b1b5f5b0 --- /dev/null +++ b/Tests/Unit/Controller/ProductControllerTest.php @@ -0,0 +1,203 @@ +getMethod('getErrorWithHighestSeverity'); + $error = $method->invoke($productController, $errors); + + self::assertSame( + $expectedSeverity, + $error->getSeverity() + ); + } + + public static function getHighestSeverityDataProvider(): Traversable + { + yield [ + 'errors' => [ + new FlashMessage( + 'OK', + 'OK', + ContextualFeedbackSeverity::OK + ), + new FlashMessage( + 'WARNING', + 'WARNING', + ContextualFeedbackSeverity::WARNING + ), + ], + 'expectedSeverity' => ContextualFeedbackSeverity::WARNING, + ]; + yield [ + 'errors' => [ + new FlashMessage( + 'OK', + 'OK', + ContextualFeedbackSeverity::OK + ), + new FlashMessage( + 'WARNING', + 'WARNING', + ContextualFeedbackSeverity::ERROR + ), + ], + 'expectedSeverity' => ContextualFeedbackSeverity::ERROR, + ]; + yield [ + 'errors' => [ + new FlashMessage( + 'OK', + 'OK', + ContextualFeedbackSeverity::WARNING + ), + new FlashMessage( + 'WARNING', + 'WARNING', + ContextualFeedbackSeverity::ERROR + ), + ], + 'expectedSeverity' => ContextualFeedbackSeverity::ERROR, + ]; + yield [ + 'errors' => [ + new FlashMessage( + 'OK', + 'OK', + ContextualFeedbackSeverity::OK + ), + new FlashMessage( + 'ERROR', + 'ERROR', + ContextualFeedbackSeverity::ERROR + ), + new FlashMessage( + 'WARNING', + 'WARNING', + ContextualFeedbackSeverity::WARNING + ), + ], + 'expectedSeverity' => ContextualFeedbackSeverity::ERROR, + ]; + } + + #[DataProvider('getLastHighestSeverityDataProvider')] + #[Test] + public function getLastHighestSeverity(array $errors, ContextualFeedbackSeverity $expectedSeverity, string $expectedMessage): void + { + $productController = GeneralUtility::makeInstance(ProductController::class); + + $reflection = new \ReflectionClass(ProductController::class); + $method = $reflection->getMethod('getErrorWithHighestSeverity'); + $error = $method->invoke($productController, $errors); + + self::assertSame( + $expectedSeverity, + $error->getSeverity() + ); + + self::assertSame( + $expectedMessage, + $error->getTitle() + ); + + self::assertSame( + $expectedMessage, + $error->getMessage() + ); + } + + public static function getLastHighestSeverityDataProvider(): Traversable + { + yield [ + 'errors' => [ + new FlashMessage( + 'WARNING 1', + 'WARNING 1', + ContextualFeedbackSeverity::WARNING + ), + new FlashMessage( + 'OK', + 'OK', + ContextualFeedbackSeverity::OK + ), + new FlashMessage( + 'WARNING 2', + 'WARNING 2', + ContextualFeedbackSeverity::WARNING + ), + ], + 'expectedSeverity' => ContextualFeedbackSeverity::WARNING, + 'expectedMessage' => 'WARNING 2', + ]; + yield [ + 'errors' => [ + new FlashMessage( + 'WARNING 2', + 'WARNING 2', + ContextualFeedbackSeverity::WARNING + ), + new FlashMessage( + 'WARNING 1', + 'WARNING 1', + ContextualFeedbackSeverity::WARNING + ), + ], + 'expectedSeverity' => ContextualFeedbackSeverity::WARNING, + 'expectedMessage' => 'WARNING 1', + ]; + yield [ + 'errors' => [ + new FlashMessage( + 'WARNING 1', + 'WARNING 1', + ContextualFeedbackSeverity::WARNING + ), + new FlashMessage( + 'ERROR 1', + 'ERROR 1', + ContextualFeedbackSeverity::ERROR + ), + new FlashMessage( + 'ERROR 2', + 'ERROR 2', + ContextualFeedbackSeverity::ERROR + ), + new FlashMessage( + 'WARNING 2', + 'WARNING 2', + ContextualFeedbackSeverity::WARNING + ), + ], + 'expectedSeverity' => ContextualFeedbackSeverity::ERROR, + 'expectedMessage' => 'ERROR 2', + ]; + } + +} From e8c93d6d6a7fa7430caa3c01b5a13ecd2d89fafe Mon Sep 17 00:00:00 2001 From: Daniel Gohlke Date: Fri, 14 Mar 2025 09:23:41 +0100 Subject: [PATCH 3/3] [TASK] Prepare bugfix release Resolves: #646 --- Documentation/guides.xml | 2 +- ext_emconf.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/guides.xml b/Documentation/guides.xml index b392ce7e..091aac0d 100644 --- a/Documentation/guides.xml +++ b/Documentation/guides.xml @@ -11,7 +11,7 @@ interlink-shortcode="extcode/cart" /> diff --git a/ext_emconf.php b/ext_emconf.php index 51c9bcdc..8282d989 100644 --- a/ext_emconf.php +++ b/ext_emconf.php @@ -4,7 +4,7 @@ 'title' => 'Cart', 'description' => 'Shopping Cart(s) for TYPO3', 'category' => 'plugin', - 'version' => '11.3.0', + 'version' => '11.3.1', 'state' => 'stable', 'author' => 'Daniel Gohlke', 'author_email' => 'ext@extco.de',