[BUGFIX] Prevent exception "required argument mail is not set" (#100)

In "intialize"-actions is it not possible to return anything. Thus returning a forward response will have no effect, when called in an initialize action. Re-introducing the old `forward` functionality for v11 solves the issue Related: #969
in2code-de · May 8, 2024 · 7b955b2 · 7b955b2
1 parent 567b02b
commit 7b955b2
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 31 deletions.
diff --git a/.ddev/config.yaml b/.ddev/config.yaml
@@ -1,7 +1,7 @@
 name: powermail-v11
 type: typo3
 docroot: .Build/Web/
-php_version: "8.0"
+php_version: "7.4"
 webserver_type: nginx-fpm
 router_http_port: "80"
 router_https_port: "443"

diff --git a/Classes/Controller/FormController.php b/Classes/Controller/FormController.php
@@ -340,10 +340,7 @@ public function optinConfirmAction(int $mail, string $hash): ResponseInterface
     {
         $this->signalDispatch(__CLASS__, __FUNCTION__ . 'BeforeRenderView', [$mail, $hash, $this]);
         $mail = $this->mailRepository->findByUid($mail);
-        $response = $this->forwardIfFormParamsDoNotMatchForOptinConfirm($mail);
-        if ($response !== null) {
-            return $response;
-        }
+        $this->forwardIfFormParamsDoNotMatchForOptinConfirm($mail);
         $labelKey = 'failed';
 
         /** @noinspection PhpUnhandledExceptionInspection */
@@ -466,40 +463,36 @@ protected function forwardIfFormParamsDoNotMatch(): void
     /**
      * Forward to formAction if no mail param given
      *
-     * @return ForwardResponse|null
+     * @throws StopActionException
      */
-    protected function forwardIfMailParamEmpty(): ?ForwardResponse
+    protected function forwardIfMailParamEmpty(): void
     {
         $arguments = $this->request->getArguments();
         if (empty($arguments['mail'])) {
             $logger = ObjectUtility::getLogger(__CLASS__);
             $logger->warning('Redirect (mail empty)', $arguments);
 
-            return new ForwardResponse('form');
+            $this->forward('form');
         }
-        return null;
     }
 
     /**
      * Forward to formAction if wrong form in plugin variables given
      *        used in optinConfirmAction()
      *
      * @param Mail|null $mail
-     * @return ResponseInterface|null
+     * @throws StopActionException
      */
-    protected function forwardIfFormParamsDoNotMatchForOptinConfirm(Mail $mail = null): ?ResponseInterface
+    protected function forwardIfFormParamsDoNotMatchForOptinConfirm(Mail $mail = null): void
     {
         if ($mail !== null) {
             $formsToContent = GeneralUtility::intExplode(',', $this->settings['main']['form']);
             if (!in_array($mail->getForm()->getUid(), $formsToContent)) {
                 $logger = ObjectUtility::getLogger(__CLASS__);
                 $logger->warning('Redirect (optin)', [$formsToContent, (array)$mail]);
-
-                return new ForwardResponse('form');
+                $this->forward('form');
             }
         }
-
-        return null;
     }
 
     /**

diff --git a/Classes/Utility/ArrayUtility.php b/Classes/Utility/ArrayUtility.php
@@ -148,7 +148,7 @@ public static function arrayMergeRecursiveOverrule(
                             $firstArray[$key] = $value;
                         }
                     }
-                // @codeCoverageIgnoreEnd
+                    // @codeCoverageIgnoreEnd
                 } else {
                     if ($emptyValuesOverride || !empty($value)) {
                         $firstArray[$key] = $value;

diff --git a/Classes/Utility/StringUtility.php b/Classes/Utility/StringUtility.php
@@ -27,7 +27,7 @@ public static function isNotEmpty($value): bool
             if (isset($value) && strlen((string)$value)) {
                 return true;
             }
-        // array (checkboxes)
+            // array (checkboxes)
         } else {
             foreach ($value as $subValue) {
                 if (isset($value) && strlen((string)$subValue)) {

diff --git a/Tests/Unit/Controller/FormControllerTest.php b/Tests/Unit/Controller/FormControllerTest.php
@@ -8,9 +8,6 @@
 use In2code\Powermail\Tests\Helper\TestingHelper;
 use Nimut\TestingFramework\TestCase\UnitTestCase;
 use TYPO3\CMS\Core\Http\Response;
-use TYPO3\CMS\Core\Http\ResponseFactory;
-use TYPO3\CMS\Core\Http\StreamFactory;
-use TYPO3\CMS\Extbase\Http\ForwardResponse;
 use TYPO3\CMS\Extbase\Mvc\Exception\StopActionException;
 use TYPO3\CMS\Extbase\Mvc\Request;
 use TYPO3\CMS\Extbase\Mvc\Web\Routing\UriBuilder;
@@ -139,8 +136,8 @@ public function forwardIfFormParamsDoNotMatchReturnsVoid($arguments, $settings,
             $this->expectException(StopActionException::class);
         }
 
-        $response = $this->generalValidatorMock->_callRef('forwardIfFormParamsDoNotMatch');
-        self::assertNull($response);
+        $this->generalValidatorMock->_callRef('forwardIfFormParamsDoNotMatch');
+        self::assertTrue(true);
     }
 
     /**
@@ -179,11 +176,11 @@ public function forwardIfMailParamEmpty($arguments, $forward)
         TestingHelper::setDefaultConstants();
         $this->setDefaultControllerProperties($arguments);
 
-        $response = $this->generalValidatorMock->_call('forwardIfMailParamEmpty');
         if ($forward === true) {
-            self::assertInstanceOf(ForwardResponse::class, $response);
+            $this->expectException(StopActionException::class);
         }
-        self::assertTrue(true);
+        $response = $this->generalValidatorMock->_call('forwardIfMailParamEmpty');
+        self::assertNull($response);
     }
 
     /**
@@ -195,6 +192,9 @@ public function forwardIfFormParamsDoNotMatchForOptinConfirmDataProvider()
     {
         return [
             'redirect, wrong form uid' => [
+                [
+                    'mail' => null,
+                ],
                 [
                     'main' => [
                         'form' => '55,6,7',
@@ -204,6 +204,9 @@ public function forwardIfFormParamsDoNotMatchForOptinConfirmDataProvider()
                 true,
             ],
             'no redirect, correct form uid' => [
+                [
+                    'mail' => null,
+                ],
                 [
                     'main' => [
                         'form' => '55,6,7',
@@ -224,21 +227,19 @@ public function forwardIfFormParamsDoNotMatchForOptinConfirmDataProvider()
      * @test
      * @covers ::forwardIfFormParamsDoNotMatchForOptinConfirm
      */
-    public function forwardIfFormParamsDoNotMatchForOptinConfirm(array $settings, $formUid, $forward)
+    public function forwardIfFormParamsDoNotMatchForOptinConfirmReturnsVoid($arguments, $settings, $formUid, $forward)
     {
         TestingHelper::setDefaultConstants();
+        $this->setDefaultControllerProperties($arguments);
         $this->generalValidatorMock->_set('settings', $settings);
         $form = new Form();
         $form->_setProperty('uid', $formUid);
         $mail = new Mail();
         $mail->setForm($form);
-
-        $this->generalValidatorMock->injectResponseFactory(new ResponseFactory());
-        $this->generalValidatorMock->injectStreamFactory(new StreamFactory());
-        $response = $this->generalValidatorMock->_call('forwardIfFormParamsDoNotMatchForOptinConfirm', $mail);
         if ($forward === true) {
-            self::assertInstanceOf(ForwardResponse::class, $response);
+            $this->expectException(StopActionException::class);
         }
+        $this->generalValidatorMock->_call('forwardIfFormParamsDoNotMatchForOptinConfirm', $mail);
         self::assertTrue(true);
     }