Correctly remove the mp_form_pageswitch value from the submitted data

terminal42 · Dec 19, 2023 · 4bad770 · 4bad770
1 parent bbff435
commit 4bad770
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 12 deletions.
diff --git a/src/EventListener/PrepareFomDataListener.php b/src/EventListener/PrepareFomDataListener.php
@@ -39,14 +39,6 @@ public function __invoke(array &$submitted, array &$labels, array $fields, Form
 
         $pageSwitchValue = $submittedBag->get('mp_form_pageswitch', '');
 
-        // Remove the page switch value field from the submitted data if that's the last step, so it's not passed on
-        // in e-mail notifications and the like. However, in intermediate steps we need it because otherwise it's not
-        // possible to have an empty step (e.g. only explanation fields) as the step data would be empty and
-        // getFirstInvalidStep() would return the one before the empty step.
-        if ($manager->isLastStep()) {
-            $submittedBag->remove('mp_form_pageswitch');
-        }
-
         // Store data in session
         $stepData = $manager->getDataOfCurrentStep();
         $stepData = $stepData->withSubmitted($submittedBag);
@@ -66,6 +58,12 @@ public function __invoke(array &$submitted, array &$labels, array $fields, Form
             $labels = $allData->getAllLabels();
             $files = $allData->getAllFiles();
 
+            // Remove the page switch value field from the submitted data, so it's not passed on
+            // in e-mail notifications and the like. However, in intermediate steps we need it because otherwise it's not
+            // possible to have an empty step (e.g. only explanation fields) as the step data would be empty and
+            // getFirstInvalidStep() would return the one before the empty step.
+            unset($submitted['mp_form_pageswitch']);
+
             // Add session data for Contao 4.13
             if (version_compare(ContaoCoreBundle::getVersion(), '5.0', '<')) {
                 // Override $_SESSION['FORM_DATA'] so it contains the data of

diff --git a/tests/EventListener/PrepareFormDataListenerTest.php b/tests/EventListener/PrepareFormDataListenerTest.php
@@ -54,17 +54,17 @@ public function testDataIsStoredProperlyAndDoesNotAdjustHookParametersIfNotOnLas
 
         $manager = $factory->forFormId(42);
 
-        $this->assertSame(['submitted1' => 'foobar', 'submitted2' => 'foobar'], $manager->getDataOfAllSteps()->getAllSubmitted());
+        $this->assertSame(['submitted1' => 'foobar', 'submitted2' => 'foobar', 'mp_form_pageswitch' => 'continue'], $manager->getDataOfAllSteps()->getAllSubmitted());
         $this->assertSame(['label1' => 'foobar'], $manager->getDataOfAllSteps()->getAllLabels());
     }
 
     public function testDataIsStoredProperlyAndDoesAdjustHookParametersOnLastStep(): void
     {
         $stepData = StepData::create(0);
-        $stepData = $stepData->withSubmitted(new ParameterBag(['submitted1' => 'foobar']));
+        $stepData = $stepData->withSubmitted(new ParameterBag(['submitted1' => 'foobar', 'mp_form_pageswitch' => 'continue']));
         $stepData = $stepData->withLabels(new ParameterBag(['label1' => 'foobar']));
         $stepData2 = StepData::create(1);
-        $stepData2 = $stepData2->withSubmitted(new ParameterBag(['submitted2' => 'foobar']));
+        $stepData2 = $stepData2->withSubmitted(new ParameterBag(['submitted2' => 'foobar', 'mp_form_pageswitch' => 'continue']));
         $initialData = (new StepDataCollection())->set($stepData)->set($stepData2);
         $storage = $this->createStorage($initialData);
 
@@ -86,7 +86,7 @@ public function testDataIsStoredProperlyAndDoesAdjustHookParametersOnLastStep():
 
         $listener($submitted, $labels, $fields, $form, $files); // Must not redirect, so no exception
 
-        // Submitted should now contain all values except for "mp_form_pageswitch"
+        // Submitted should now contain all values except for "mp_form_pageswitch" values
         $this->assertSame(['submitted1' => 'foobar', 'submitted2' => 'foobar', 'submitted3' => 'foobar'], $submitted);
         $this->assertSame(['label1' => 'foobar'], $labels); // Test we do not modify the hook parameters if not in last step
     }