Skip to content

1863 improve flash #2510

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

Closed
Closed
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
81 changes: 62 additions & 19 deletions src/Symfony/Component/HttpFoundation/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ class Session implements \Serializable
public function __construct(SessionStorageInterface $storage)
{
$this->storage = $storage;
$this->flashes = array();
$this->oldFlashes = array();
$this->flashes = array('status' => array());
$this->oldFlashes = array('status' => array());
$this->attributes = array();
$this->started = false;
$this->closed = false;
Expand Down Expand Up @@ -174,7 +174,7 @@ public function clear()
}

$this->attributes = array();
$this->flashes = array();
$this->flashes = array('status' => array());
}

/**
Expand Down Expand Up @@ -216,28 +216,54 @@ public function getId()
}

/**
* Gets the flash messages.
* Gets the flash messages of a given type.
*
* @param string $type
* @return array
*/
public function getFlashes()
public function getFlashes($type = 'status')
{
return $this->flashes[$type];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should do a check and throw an exception when the key is invalid instead of the PHP Notice

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected in 543fb3e

}

/**
* Gets the flash messages.
*
* @return array
*/
public function getAllFlashes() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the curly brace should be on its own line

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've corrected this in 543fb3e

return $this->flashes;
}

/**
* Sets the flash messages of a specific type.
*
* @param array $values
* @param string $type
*/
public function setFlashes($values, $type = 'status')
{
if (false === $this->started) {
$this->start();
}

$this->flashes[$type] = $values;
$this->oldFlashes = array('status' => array());
}

/**
* Sets the flash messages.
*
* @param array $values
*/
public function setFlashes($values)
public function setAllFlashes($values)
{
if (false === $this->started) {
$this->start();
}

$this->flashes = $values;
$this->oldFlashes = array();
$this->oldFlashes = array('status' => array());
}

/**
Expand All @@ -248,55 +274,59 @@ public function setFlashes($values)
*
* @return string
*/
public function getFlash($name, $default = null)
public function getFlash($name, $default = null, $type = 'status')
{
return array_key_exists($name, $this->flashes) ? $this->flashes[$name] : $default;
if (array_key_exists($type, $this->flashes)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess here you could also use an isset check

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected in 543fb3e

return array_key_exists($name, $this->flashes[$type]) ? $this->flashes[$type][$name] : $default;
}
}

/**
* Sets a flash message.
*
* @param string $name
* @param string $value
* @param string $type
*/
public function setFlash($name, $value)
public function setFlash($name, $value, $type = 'status')
{
if (false === $this->started) {
$this->start();
}

$this->flashes[$name] = $value;
unset($this->oldFlashes[$name]);
$this->flashes[$type][$name] = $value;
unset($this->oldFlashes[$type][$name]);
}

/**
* Checks whether a flash message exists.
*
* @param string $name
* @param string $type
*
* @return Boolean
*/
public function hasFlash($name)
public function hasFlash($name, $type = 'status')
{
if (false === $this->started) {
$this->start();
}

return array_key_exists($name, $this->flashes);
return array_key_exists($name, $this->flashes[$type]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to check if $this->flashes[$type] exists first

}

/**
* Removes a flash message.
*
* @param string $name
*/
public function removeFlash($name)
public function removeFlash($name, $type = 'status')
{
if (false === $this->started) {
$this->start();
}

unset($this->flashes[$name]);
unset($this->flashes[$type][$name]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

}

/**
Expand All @@ -308,8 +338,19 @@ public function clearFlashes()
$this->start();
}

$this->flashes = array();
$this->oldFlashes = array();
$this->flashes = array('status' => array());
$this->oldFlashes = array('status' => array());
}

/**
* Adds a generic flash message to the session.
*
* @param string $type
*
* @return array
*/
public function addFlash($value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the opening bracket for methods need to be on a new line

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected in 543fb3e

$this->flashes['status'][] = $value;
}

public function save()
Expand All @@ -318,7 +359,9 @@ public function save()
$this->start();
}

$this->flashes = array_diff_key($this->flashes, $this->oldFlashes);
foreach ($this->flashes as $type => $flashes) {
$this->flashes[$type] = array_diff_key($flashes, $this->oldFlashes[$type]);
}

$this->storage->write('_symfony2', array(
'attributes' => $this->attributes,
Expand Down
11 changes: 8 additions & 3 deletions tests/Symfony/Tests/Component/HttpFoundation/SessionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ public function testFlash()
$this->session->setFlashes($flashes);

$this->assertSame($flashes, $this->session->getFlashes());

$this->session->clearFlashes();
$this->session->addFlash('foo');
$compare = $this->session->getFlashes();
$this->assertSame($compare, array(0 => 'foo'));
}

public function testFlashesAreFlushedWhenNeeded()
Expand Down Expand Up @@ -149,7 +154,7 @@ public function testSave()
$this->session->set('foo', 'bar');

$this->session->save();
$compare = array('_symfony2' => array('attributes' => array('foo' => 'bar'), 'flashes' => array()));
$compare = array('_symfony2' => array('attributes' => array('foo' => 'bar'), 'flashes' => array('status' => array())));

$r = new \ReflectionObject($this->storage);
$p = $r->getProperty('data');
Expand Down Expand Up @@ -179,7 +184,7 @@ public function testSavedOnDestruct()

$expected = array(
'attributes'=>array('foo'=>'bar'),
'flashes'=>array(),
'flashes'=>array('status' => array()),
);
$saved = $this->storage->read('_symfony2');
$this->assertSame($expected, $saved);
Expand All @@ -195,7 +200,7 @@ public function testSavedOnDestructAfterManualSave()

$expected = array(
'attributes'=>array('foo'=>'bar'),
'flashes'=>array(),
'flashes'=>array('status' => array()),
);
$saved = $this->storage->read('_symfony2');
$this->assertSame($expected, $saved);
Expand Down