Merge pull request #1198 from stefanorosanelli/fix/remove-failedsave-session

Remove unnecessary failed save form data session handling

Author: didoda

src/Controller/Component/ModulesComponent.php

@@ -431,25 +431,6 @@ public function checkRequestForUpload(array $requestData): bool
         return true;
     }
 
-    /**
-     * Set session data for `failedSave.{type}.{id}` and `failedSave.{type}.{id}__timestamp`.
-     *
-     * @param string $type The object type.
-     * @param array $data The data to store into session.
-     * @return void
-     */
-    public function setDataFromFailedSave(string $type, array $data): void
-    {
-        if (empty($data) || empty($data['id']) || empty($type)) {
-            return;
-        }
-        $key = sprintf('failedSave.%s.%s', $type, $data['id']);
-        $session = $this->getController()->getRequest()->getSession();
-        unset($data['id']); // remove 'id', avoid future merged with attributes
-        $session->write($key, $data);
-        $session->write(sprintf('%s__timestamp', $key), time());
-    }
-
     /**
      * Set current attributes from loaded $object data in `currentAttributes`.
      * Load session failure data if available.
@@ -461,45 +442,6 @@ public function setupAttributes(array &$object): void
     {
         $currentAttributes = json_encode((array)Hash::get($object, 'attributes'));
         $this->getController()->set(compact('currentAttributes'));
-
-        $this->updateFromFailedSave($object);
-    }
-
-    /**
-     * Update object, when failed save occurred.
-     * Check session data by `failedSave.{type}.{id}` key and `failedSave.{type}.{id}__timestamp`.
-     * If data is set and timestamp is not older than 5 minutes.
-     *
-     * @param array $object The object.
-     * @return void
-     */
-    protected function updateFromFailedSave(array &$object): void
-    {
-        // check session data for object id => use `failedSave.{type}.{id}` as key
-        $session = $this->getController()->getRequest()->getSession();
-        $key = sprintf(
-            'failedSave.%s.%s',
-            Hash::get($object, 'type'),
-            Hash::get($object, 'id')
-        );
-        $data = $session->read($key);
-        if (empty($data)) {
-            return;
-        }
-
-        // read timestamp session key
-        $timestampKey = sprintf('%s__timestamp', $key);
-        $timestamp = $session->read($timestampKey);
-
-        // if data exist for {type} and {id} and `__timestamp` not too old (<= 5 minutes)
-        if ($timestamp > strtotime('-5 minutes')) {
-            //  => merge with $object['attributes']
-            $object['attributes'] = array_merge($object['attributes'], (array)$data);
-        }
-
-        // remove session data
-        $session->delete($key);
-        $session->delete($timestampKey);
     }
 
     /**

src/Controller/ModulesController.php

@@ -291,9 +291,6 @@ public function save(): void
                 $this->set(['error' => __('Invalid numeric uname. Change it to a valid string')]);
                 $this->setSerialize(['error']);
 
-                // set session data to recover form
-                $this->Modules->setDataFromFailedSave($this->objectType, $requestData);
-
                 return;
             }
             $id = Hash::get($requestData, 'id');
@@ -328,17 +325,13 @@ public function save(): void
             ];
             $event = new Event('Controller.afterSave', $this, $options);
             $this->getEventManager()->dispatch($event);
-            $this->getRequest()->getSession()->delete(sprintf('failedSave.%s.%s', $this->objectType, $id));
         } catch (BEditaClientException $error) {
             $message = new Message($error);
             $this->log($message->get(), LogLevel::ERROR);
             $this->Flash->error($message->get(), ['params' => $error]);
             $this->set(['error' => $message->get()]);
             $this->setSerialize(['error']);
 
-            // set session data to recover form
-            $this->Modules->setDataFromFailedSave($this->objectType, $requestData);
-
             return;
         }
         if ($response['data']) {

tests/TestCase/Controller/Component/ModulesComponentTest.php

@@ -1073,65 +1073,6 @@ private function setupApi(): void
         $this->client->setupTokens($response['meta']);
     }
 
-    /**
-     * Test `setDataFromFailedSave`.
-     *
-     * @covers ::setDataFromFailedSave()
-     * @return void
-     */
-    public function testSetDataFromFailedSave(): void
-    {
-        // empty case
-        $this->Modules->setDataFromFailedSave('', ['id' => 123]);
-        $actual = $this->Modules->getController()->getRequest()->getSession()->read('failedSave.123');
-        static::assertEmpty($actual);
-
-        // data and expected
-        $expected = [ 'id' => 999, 'name' => 'gustavo' ];
-        $type = 'documents';
-
-        $this->Modules->setDataFromFailedSave($type, $expected);
-
-        // verify data
-        $key = sprintf('failedSave.%s.%s', $type, $expected['id']);
-        $actual = $this->Modules->getController()->getRequest()->getSession()->read($key);
-        unset($expected['id']);
-        static::assertEquals($expected, $actual);
-    }
-
-    /**
-     * Test `updateFromFailedSave` method.
-     *
-     * @return void
-     * @covers ::setupAttributes()
-     * @covers ::updateFromFailedSave()
-     */
-    public function testUpdateFromFailedSave(): void
-    {
-        // empty case
-        $this->Modules->setDataFromFailedSave('', ['id' => 123]); // wrong data, missing type
-        $object = ['id' => 123, 'type' => 'documents'];
-        $this->Modules->setupAttributes($object);
-        static::assertArrayNotHasKey('attributes', $object);
-
-        // write to session data, to simulate recover from session.
-        $object = [
-            'id' => 999,
-            'type' => 'documents',
-            'attributes' => [
-                'name' => 'john doe',
-            ],
-        ];
-        $recover = [ 'name' => 'gustavo' ];
-        $this->Modules->setDataFromFailedSave('documents', $recover + ['id' => 999]);
-
-        // verify data
-        $this->Modules->setupAttributes($object);
-        $expected = $object;
-        $expected['attributes'] = array_merge($object['attributes'], $recover);
-        static::assertEquals($expected, $object);
-    }
-
     /**
      * Data provider for `testSetupRelationsMeta`
      *