Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary failed save form data session handling #1198

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 0 additions & 58 deletions src/Controller/Component/ModulesComponent.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
}

/**
Expand Down
7 changes: 0 additions & 7 deletions src/Controller/ModulesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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']) {
Expand Down
59 changes: 0 additions & 59 deletions tests/TestCase/Controller/Component/ModulesComponentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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`
*
Expand Down
Loading