Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Update FlashMessenger.php #5759

Closed
wants to merge 10 commits into from

9 participants

Diego Bittencourt de Oliveira Blanchon Vincent Tim Fountain Frank Brückner Abdul Malik Ikhsan Martin-P Kristofer Karlsson Maks3w Matthew Weier O'Phinney
Diego Bittencourt de Oliveira

Hello, the feature of rendering is fantastic, but you can not print the current message, this second method would make this work.

Diego Bittencourt de Oliveira diegobittencourt Update FlashMessenger.php
Hello, the feature of rendering is fantastic, but you can not print the current message, this second method would make this work.
178f487
Blanchon Vincent

@diegobittencourt please provide unit tests

Tim Fountain

Doesn't this duplicate functionality we have in the Flash Messenger view helper? http://framework.zend.com/manual/2.2/en/modules/zend.view.helpers.flash-messenger.html

Frank Brückner

@tfountain
getMessagesFromNamespace != getCurrentMessagesFromNamespace

Diego Bittencourt de Oliveira

I am providing tests for the method I created.

Diego Bittencourt de Oliveira diegobittencourt referenced this pull request from a commit in diegobittencourt/zf2
Diego Bittencourt de Oliveira diegobittencourt Tests for #5759 c27f7cd
Diego Bittencourt de Oliveira diegobittencourt referenced this pull request
Closed

Tests for #5759 #5798

Diego Bittencourt de Oliveira

Tests -> #5759

Please if something is missing or incorrect, show me for me to get.

Abdul Malik Ikhsan

you should just add a commit here, make git push to your branch, no need to create new PR, so #5798 can be closed :)

Diego Bittencourt de Oliveira

Hello, I joined the 2 commits into one, I should do something else?
Note: I'm sorry for the inconvenience this attempt to contribute.

Abdul Malik Ikhsan

travis build failure (https://travis-ci.org/zendframework/zf2/jobs/18593426), you need to fix it with php-cs-fixer :

php php-cs-fixer.phar fix /path/to/file.php --fixers=trailing_spaces

you can download php-cs-fixer from http://get.sensiolabs.org/php-cs-fixer.phar

Diego Bittencourt de Oliveira

/path/to/file.php >>> "file" would be the file which I want to fix?

Diego Bittencourt de Oliveira

Well, I adjusted all the problems and everything was accepted, but left me a question regarding the last commit:

It was so

  • $ This-> assertTrue ($ this-> plugin-> hasCurrentMessages ());
  • $ This-> assertTrue ($ this-> plugin-> hasCurrentInfoMessages ());
  • $ This-> assertTrue ($ this-> plugin-> hasCurrentSuccessMessages ());
  • $ This-> assertTrue ($ this-> plugin-> hasCurrentErrorMessages ());

And was

  • $ This-> assertFalse ($ this-> plugin-> hasCurrentMessages ());
  • $ This-> assertFalse ($ this-> plugin-> hasCurrentInfoMessages ());
  • $ This-> assertFalse ($ this-> plugin-> hasCurrentSuccessMessages ());
  • $ This-> assertFalse ($ this-> plugin-> hasCurrentErrorMessages ());

hasCurrentMessages is not available in plugin? Is that correct? My knowledge in zend framework is still small, I got doubts.

Martin-P

No need for this pull request, because this functionality already exists.

The method Zend\View\Helper\FlashMessenger::__call() acts as a proxy for Zend\Mvc\Controller\Plugin\FlashMessenger. I already use this functionality to display the current messages.

In my layout.phtml I use:

<?php echo $this->render('layout/partial/messages'); ?>

In layout/partial/messages.phtml I use this code to map and merge the messages:

<?php
$namespaces = array(
    'default',
    'error',
    'info',
    'success',
    'warning',
);

$allMessages = array();
foreach ($namespaces as $namespace) {
    $currentMessages = $this->flashMessenger()->getCurrentMessagesFromNamespace($namespace);
    $this->flashMessenger()->clearCurrentMessagesFromNamespace($namespace);
    $allMessages[$namespace] = array_merge($currentMessages, $this->flashMessenger($namespace));
}

var_dump($allMessages);
Diego Bittencourt de Oliveira

Forgive me, but yes there is the need of the Implemented functionality.

The method render of class FlashMessenger does not print the current messages.

Martin-P

The method render of class FlashMessenger does not print the current messages.

Instead of creating a complete new method with lots of duplicate code I think a much cleaner solution would be to attach a flag to the view helper and act upon that flag:

$this->flashMessenger()->setUseCurrentMessages(true)->render();
Diego Bittencourt de Oliveira

That is, in one way or another, does not agree with my implementation. :P

Diego Bittencourt de Oliveira

I agree with this:

public function render($namespace = PluginFlashMessenger::NAMESPACE_DEFAULT, array $classes = array())
{
          $flashMessenger = $this->getPluginFlashMessenger();
          $messages = $flashMessenger->getMessagesFromNamespace($namespace);
          return renderAssistant($messages, $classes);
}
public function renderCurrent($namespace = PluginFlashMessenger::NAMESPACE_DEFAULT, array $classes = array())
{
          $flashMessenger = $this->getPluginFlashMessenger();
          $messages = $flashMessenger->getCurrentMessagesFromNamespace($namespace);
          return renderAssistant($messages, $classes);
}
protect function renderAssistant($messages, $classes)
{
...
}

But disagree on creating an attribute that would have no utility unless this method, taking into account the whole structure that FlashMessenger own class.

Martin-P

Please use markdown code to make your code readable: https://help.github.com/articles/github-flavored-markdown

But disagree on creating an attribute that would have no utility unless this method, taking into account the whole structure that FlashMessenger own class.

Depends on what you want. I assumed you want to merge the messages, but I understand you want to be able to display the current messages separately?

I would suggest a bit more descriptive method name for renderAssistant like renderMessages. IMO the word Assistent assumes a separate class which assists the FlashMessenger view helper.

Diego Bittencourt de Oliveira

Look at my last commit, maintains the structure and become clean. ;)

Martin-P

Nice! Much cleaner now :smiley: Now it's a matter of waiting until this PR is accepted.

Diego Bittencourt de Oliveira

This is a question of mine, who performs this work?

Martin-P

You can look at the closed PR's for this: https://github.com/zendframework/zf2/pulls?direction=desc&page=1&sort=created&state=closed

Some PR's are merged in 2 hours, but if I look at some of my PR's there has not been any response in 24 days, so I honostly got no idea how long it takes. I think they are somewhat behind of schedule looking at the fact there are still 160 PR's open.

Kristofer Karlsson

The who idea with FlashMessenger is to render something on the NEXT request. If you need to render the current message, doesn't that just mean you're using it wrong?

Martin-P

If you need to render the current message, doesn't that just mean you're using it wrong?

The documented way of using it is indeed rendering messages on the next request. The FlashMessenger plugin however has a method getCurrentMessagesFromNamespace. What is the use of that method when messages should only be rendered at the next request? Personally I already used the FlashMessenger for displaying messages on the current request, because it has the fuctionality I need and I don't see why I have to reinvent the wheel.

What would be your suggestion to render messages on the current request?

Diego Bittencourt de Oliveira

My changes in my opinion obeys the principles of FlashMessenger. No reinvention of the wheel. I await an opinion of the management as my changes.

Maks3w
Collaborator

@diegobittencourt This PR conflicts with current master. Please rebase or merge master in the branch

Diego Bittencourt de Oliveira

Is there any practice of documentation should I execute to perform this merge in my repository?

Abdul Malik Ikhsan

@diegobittencourt you can do :

git checkout master
git pull git://github.com/zendframework/zf2.git 
git checkout patch-1
git rebase master

On this stage, maybe you will got conflict, so you need to fix conflict and git add the conflicted file :

git add /path/to/conflicted/file.php

and then git commit and then push --force to your branch

git commit -m "your commit message" -a
git push --force origin patch-1
Diego Bittencourt de Oliveira

Good at my base I already have the last change, so I think it is all right :D

Abdul Malik Ikhsan

when you did rebase and push --force, your commits must be showed under my last comment.... it seems you didn't do push --force ;)

Diego Bittencourt de Oliveira

Look again my friend.

It seems that now with your tips will produce more and less mess.

Matthew Weier O'Phinney weierophinney added this to the 2.3.0 milestone
Matthew Weier O'Phinney weierophinney self-assigned this
Matthew Weier O'Phinney

Merged to develop for release in 2.3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 29, 2014
  1. Diego Bittencourt de Oliveira

    Update FlashMessenger.php

    diegobittencourt authored
    Hello, the feature of rendering is fantastic, but you can not print the current message, this second method would make this work.
Commits on Feb 10, 2014
  1. Diego Bittencourt de Oliveira
  2. Diego Bittencourt de Oliveira

    Ajusting

    diegobittencourt authored
  3. Diego Bittencourt de Oliveira

    Ajusting

    diegobittencourt authored
  4. Diego Bittencourt de Oliveira

    Ajusting

    diegobittencourt authored
  5. Diego Bittencourt de Oliveira

    Ajusting

    diegobittencourt authored
Commits on Feb 13, 2014
  1. Diego Bittencourt de Oliveira
  2. Diego Bittencourt de Oliveira

    Ajusting

    diegobittencourt authored
  3. Diego Bittencourt de Oliveira

    Ajusting

    diegobittencourt authored
Commits on Mar 3, 2014
  1. Diego Bittencourt de Oliveira

    Ajusting Rebase

    diegobittencourt authored
This page is out of date. Refresh to see the latest.
31 library/Zend/View/Helper/FlashMessenger.php
View
@@ -101,7 +101,32 @@ public function render($namespace = PluginFlashMessenger::NAMESPACE_DEFAULT, arr
{
$flashMessenger = $this->getPluginFlashMessenger();
$messages = $flashMessenger->getMessagesFromNamespace($namespace);
+ return $this->renderMessages($namespace, $messages, $classes);
+ }
+
+ /**
+ * Render Current Messages
+ *
+ * @param string $namespace
+ * @param array $classes
+ * @return string
+ */
+ public function renderCurrent($namespace = PluginFlashMessenger::NAMESPACE_DEFAULT, array $classes = array())
+ {
+ $flashMessenger = $this->getPluginFlashMessenger();
+ $messages = $flashMessenger->getCurrentMessagesFromNamespace($namespace);
+ return $this->renderMessages($namespace, $messages, $classes);
+ }
+ /**
+ * Render Messages
+ *
+ * @param array $messages
+ * @param array $classes
+ * @return string
+ */
+ protected function renderMessages($namespace = PluginFlashMessenger::NAMESPACE_DEFAULT, array $messages = array(), array $classes = array())
+ {
// Prepare classes for opening tag
if (empty($classes)) {
if (isset($this->classMessages[$namespace])) {
@@ -111,14 +136,11 @@ public function render($namespace = PluginFlashMessenger::NAMESPACE_DEFAULT, arr
}
$classes = array($classes);
}
-
// Flatten message array
$escapeHtml = $this->getEscapeHtmlHelper();
$messagesToPrint = array();
-
$translator = $this->getTranslator();
$translatorTextDomain = $this->getTranslatorTextDomain();
-
array_walk_recursive($messages, function ($item) use (&$messagesToPrint, $escapeHtml, $translator, $translatorTextDomain) {
if ($translator !== null) {
$item = $translator->translate(
@@ -128,16 +150,13 @@ public function render($namespace = PluginFlashMessenger::NAMESPACE_DEFAULT, arr
}
$messagesToPrint[] = $escapeHtml($item);
});
-
if (empty($messagesToPrint)) {
return '';
}
-
// Generate markup
$markup = sprintf($this->getMessageOpenFormat(), ' class="' . implode(' ', $classes) . '"');
$markup .= implode(sprintf($this->getMessageSeparatorString(), ' class="' . implode(' ', $classes) . '"'), $messagesToPrint);
$markup .= $this->getMessageCloseString();
-
return $markup;
}
193 tests/ZendTest/View/Helper/FlashMessengerTest.php
View
@@ -46,6 +46,17 @@ public function seedMessages()
unset($helper);
}
+ public function seedCurrentMessages()
+ {
+ $helper = new FlashMessenger();
+ $helper->setSessionManager($this->session);
+ $helper->addMessage('foo');
+ $helper->addMessage('bar');
+ $helper->addInfoMessage('bar-info');
+ $helper->addSuccessMessage('bar-success');
+ $helper->addErrorMessage('bar-error');
+ }
+
public function testCanAssertPluginClass()
{
$this->assertEquals(
@@ -84,6 +95,28 @@ public function testCanRetrieveMessages()
$this->assertTrue($this->plugin->hasErrorMessages());
}
+ public function testCanRetrieveCurrentMessages()
+ {
+ $helper = $this->helper;
+
+ $this->assertFalse($helper()->hasCurrentMessages());
+ $this->assertFalse($helper()->hasCurrentInfoMessages());
+ $this->assertFalse($helper()->hasCurrentSuccessMessages());
+ $this->assertFalse($helper()->hasCurrentErrorMessages());
+
+ $this->seedCurrentMessages();
+
+ $this->assertTrue(count($helper('default')) > 0);
+ $this->assertTrue(count($helper('info')) > 0);
+ $this->assertTrue(count($helper('success')) > 0);
+ $this->assertTrue(count($helper('error')) > 0);
+
+ $this->assertFalse($this->plugin->hasCurrentMessages());
+ $this->assertFalse($this->plugin->hasCurrentInfoMessages());
+ $this->assertFalse($this->plugin->hasCurrentSuccessMessages());
+ $this->assertFalse($this->plugin->hasCurrentErrorMessages());
+ }
+
public function testCanProxyAndRetrieveMessagesFromPluginController()
{
$this->assertFalse($this->helper->hasMessages());
@@ -99,6 +132,21 @@ public function testCanProxyAndRetrieveMessagesFromPluginController()
$this->assertTrue($this->helper->hasErrorMessages());
}
+ public function testCanProxyAndRetrieveCurrentMessagesFromPluginController()
+ {
+ $this->assertFalse($this->helper->hasCurrentMessages());
+ $this->assertFalse($this->helper->hasCurrentInfoMessages());
+ $this->assertFalse($this->helper->hasCurrentSuccessMessages());
+ $this->assertFalse($this->helper->hasCurrentErrorMessages());
+
+ $this->seedCurrentMessages();
+
+ $this->assertTrue($this->helper->hasCurrentMessages());
+ $this->assertTrue($this->helper->hasCurrentInfoMessages());
+ $this->assertTrue($this->helper->hasCurrentSuccessMessages());
+ $this->assertTrue($this->helper->hasCurrentErrorMessages());
+ }
+
public function testCanDisplayListOfMessages()
{
$displayInfoAssertion = '';
@@ -112,6 +160,19 @@ public function testCanDisplayListOfMessages()
$this->assertEquals($displayInfoAssertion, $displayInfo);
}
+ public function testCanDisplayListOfCurrentMessages()
+ {
+ $displayInfoAssertion = '';
+ $displayInfo = $this->helper->renderCurrent(PluginFlashMessenger::NAMESPACE_INFO);
+ $this->assertEquals($displayInfoAssertion, $displayInfo);
+
+ $this->seedCurrentMessages();
+
+ $displayInfoAssertion = '<ul class="info"><li>bar-info</li></ul>';
+ $displayInfo = $this->helper->renderCurrent(PluginFlashMessenger::NAMESPACE_INFO);
+ $this->assertEquals($displayInfoAssertion, $displayInfo);
+ }
+
public function testCanDisplayListOfMessagesByDefaultParameters()
{
$helper = $this->helper;
@@ -122,6 +183,16 @@ public function testCanDisplayListOfMessagesByDefaultParameters()
$this->assertEquals($displayInfoAssertion, $displayInfo);
}
+ public function testCanDisplayListOfMessagesByDefaultCurrentParameters()
+ {
+ $helper = $this->helper;
+ $this->seedCurrentMessages();
+
+ $displayInfoAssertion = '<ul class="default"><li>foo</li><li>bar</li></ul>';
+ $displayInfo = $helper()->renderCurrent();
+ $this->assertEquals($displayInfoAssertion, $displayInfo);
+ }
+
public function testCanDisplayListOfMessagesByInvoke()
{
$helper = $this->helper;
@@ -132,6 +203,16 @@ public function testCanDisplayListOfMessagesByInvoke()
$this->assertEquals($displayInfoAssertion, $displayInfo);
}
+ public function testCanDisplayListOfCurrentMessagesByInvoke()
+ {
+ $helper = $this->helper;
+ $this->seedCurrentMessages();
+
+ $displayInfoAssertion = '<ul class="info"><li>bar-info</li></ul>';
+ $displayInfo = $helper()->renderCurrent(PluginFlashMessenger::NAMESPACE_INFO);
+ $this->assertEquals($displayInfoAssertion, $displayInfo);
+ }
+
public function testCanDisplayListOfMessagesCustomised()
{
$this->seedMessages();
@@ -145,6 +226,19 @@ public function testCanDisplayListOfMessagesCustomised()
$this->assertEquals($displayInfoAssertion, $displayInfo);
}
+ public function testCanDisplayListOfCurrentMessagesCustomised()
+ {
+ $this->seedCurrentMessages();
+
+ $displayInfoAssertion = '<div class="foo-baz foo-bar"><p>bar-info</p></div>';
+ $displayInfo = $this->helper
+ ->setMessageOpenFormat('<div%s><p>')
+ ->setMessageSeparatorString('</p><p>')
+ ->setMessageCloseString('</p></div>')
+ ->renderCurrent(PluginFlashMessenger::NAMESPACE_INFO, array('foo-baz', 'foo-bar'));
+ $this->assertEquals($displayInfoAssertion, $displayInfo);
+ }
+
public function testCanDisplayListOfMessagesCustomisedSeparator()
{
$this->seedMessages();
@@ -158,6 +252,19 @@ public function testCanDisplayListOfMessagesCustomisedSeparator()
$this->assertEquals($displayInfoAssertion, $displayInfo);
}
+ public function testCanDisplayListOfCurrentMessagesCustomisedSeparator()
+ {
+ $this->seedCurrentMessages();
+
+ $displayInfoAssertion = '<div><p class="foo-baz foo-bar">foo</p><p class="foo-baz foo-bar">bar</p></div>';
+ $displayInfo = $this->helper
+ ->setMessageOpenFormat('<div><p%s>')
+ ->setMessageSeparatorString('</p><p%s>')
+ ->setMessageCloseString('</p></div>')
+ ->renderCurrent(PluginFlashMessenger::NAMESPACE_DEFAULT, array('foo-baz', 'foo-bar'));
+ $this->assertEquals($displayInfoAssertion, $displayInfo);
+ }
+
public function testCanDisplayListOfMessagesCustomisedByConfig()
{
$this->seedMessages();
@@ -193,6 +300,40 @@ public function testCanDisplayListOfMessagesCustomisedByConfig()
$this->assertEquals($displayInfoAssertion, $displayInfo);
}
+ public function testCanDisplayListOfCurrentMessagesCustomisedByConfig()
+ {
+ $this->seedCurrentMessages();
+ $config = array(
+ 'view_helper_config' => array(
+ 'flashmessenger' => array(
+ 'message_open_format' => '<div%s><ul><li>',
+ 'message_separator_string' => '</li><li>',
+ 'message_close_string' => '</li></ul></div>',
+ ),
+ ),
+ );
+ $sm = new ServiceManager();
+ $sm->setService('Config', $config);
+ $helperPluginManager = new HelperPluginManager(new Config(array(
+ 'factories' => array(
+ 'flashmessenger' => 'Zend\View\Helper\Service\FlashMessengerFactory',
+ ),
+ )));
+ $controllerPluginManager = new PluginManager(new Config(array(
+ 'invokables' => array(
+ 'flashmessenger' => 'Zend\Mvc\Controller\Plugin\FlashMessenger',
+ ),
+ )));
+ $helperPluginManager->setServiceLocator($sm);
+ $controllerPluginManager->setServiceLocator($sm);
+ $sm->setService('ControllerPluginManager', $controllerPluginManager);
+ $helper = $helperPluginManager->get('flashmessenger');
+
+ $displayInfoAssertion = '<div class="info"><ul><li>bar-info</li></ul></div>';
+ $displayInfo = $helper->renderCurrent(PluginFlashMessenger::NAMESPACE_INFO);
+ $this->assertEquals($displayInfoAssertion, $displayInfo);
+ }
+
public function testCanDisplayListOfMessagesCustomisedByConfigSeparator()
{
$this->seedMessages();
@@ -228,6 +369,41 @@ public function testCanDisplayListOfMessagesCustomisedByConfigSeparator()
$this->assertEquals($displayInfoAssertion, $displayInfo);
}
+ public function testCanDisplayListOfCurrentMessagesCustomisedByConfigSeparator()
+ {
+ $this->seedCurrentMessages();
+
+ $config = array(
+ 'view_helper_config' => array(
+ 'flashmessenger' => array(
+ 'message_open_format' => '<div><ul><li%s>',
+ 'message_separator_string' => '</li><li%s>',
+ 'message_close_string' => '</li></ul></div>',
+ ),
+ ),
+ );
+ $sm = new ServiceManager();
+ $sm->setService('Config', $config);
+ $helperPluginManager = new HelperPluginManager(new Config(array(
+ 'factories' => array(
+ 'flashmessenger' => 'Zend\View\Helper\Service\FlashMessengerFactory',
+ ),
+ )));
+ $controllerPluginManager = new PluginManager(new Config(array(
+ 'invokables' => array(
+ 'flashmessenger' => 'Zend\Mvc\Controller\Plugin\FlashMessenger',
+ ),
+ )));
+ $helperPluginManager->setServiceLocator($sm);
+ $controllerPluginManager->setServiceLocator($sm);
+ $sm->setService('ControllerPluginManager', $controllerPluginManager);
+ $helper = $helperPluginManager->get('flashmessenger');
+
+ $displayInfoAssertion = '<div><ul><li class="foo-baz foo-bar">foo</li><li class="foo-baz foo-bar">bar</li></ul></div>';
+ $displayInfo = $helper->renderCurrent(PluginFlashMessenger::NAMESPACE_DEFAULT, array('foo-baz', 'foo-bar'));
+ $this->assertEquals($displayInfoAssertion, $displayInfo);
+ }
+
public function testCanTranslateMessages()
{
$mockTranslator = $this->getMock('Zend\I18n\Translator\Translator');
@@ -244,4 +420,21 @@ public function testCanTranslateMessages()
$display = $this->helper->render(PluginFlashMessenger::NAMESPACE_INFO);
$this->assertEquals($displayAssertion, $display);
}
+
+ public function testCanTranslateCurrentMessages()
+ {
+ $mockTranslator = $this->getMock('Zend\I18n\Translator\Translator');
+ $mockTranslator->expects($this->exactly(1))
+ ->method('translate')
+ ->will($this->returnValue('translated message'));
+
+ $this->helper->setTranslator($mockTranslator);
+ $this->assertTrue($this->helper->hasTranslator());
+
+ $this->seedCurrentMessages();
+
+ $displayAssertion = '<ul class="info"><li>translated message</li></ul>';
+ $display = $this->helper->renderCurrent(PluginFlashMessenger::NAMESPACE_INFO);
+ $this->assertEquals($displayAssertion, $display);
+ }
}
Something went wrong with that request. Please try again.