Issues

ZF-1705: Add optional namespace argument to *Messages() methods in FlashMessenger

Description

Currently, the various *Messages() methods of the FlashMessenger helper assume you've manually set the namespace. This makes it difficult to use the FlashMessenger with multiple namespaces in the same controller, as well as adds extra steps anytime you wish to use a namespace other than the default.

To add flexibility and simplify the API, the following BC changes should be made:

  • addMessage() should take an optional, second parameter indicating the desired namespace
  • (has|get|clear|hasCurrent)Messages() should all take an optional namespace argument

In each case, the namespace parameter, if not null, would result in a call to setNamespace(), setting the namespace for further method calls (unless overridden).

Comments

Scheduling for 1.1.0

Assigning to Ralph to review

Will evaluate in 2 weeks.

I dont think that its useful to set the namespace, if it is set as optional argument. Its possible, that other components will write to the wrong namespace, if its changed without reason. So every part of your application has to make sure, that it is really the right namespace, even if its not changed explicitly. If one of this changes occur in an if-clause, it might not be executed the next request (for examle) and later calls will depend on a namespace, that is not set.

I would rather reset the namespace after adding the msg (or wathever method is called).

If you want to change the namespace persistently, you can use setNamespace()

Some code that maybe will resolve this issue.

if ($namespace !== null) {
  $oldNamespace = $this->getNamespace();
  $this->setNamespace($namespace);
}
// Do stuff
if ($namespace !== null) {
  $this->setNamespace($oldNamespace);
}

Currently, I am handling this issue by keeping things as they are and passing an array to the messenger.

// array with messages $messages = array(); $messages[] = array('type'=>'notice', 'heading'=>'You have successfully added a license type.'); $messages[] = array('type'=>'warning', 'heading'=>'You action encountered the following issues:', 'messages'=>'File(FileSizeZero.txt) must be greater than 0KB.'); $messages[] = array('type'=>'error', 'heading'=>'Unable to save record:', 'messages'=>'First Name is required');

// add messages to flash messenger foreach($messages as $msg){ $this->appFlashMessenger->addMessage($msg); }

Then in the view I loop through the array and show the flash messages if($this->flashMessenger->count()>0){ forech($this->flashMessenger->getMessages() as $msg){ ..... show message based on type } }

I hope this could help whoever encounter this issue.

Please test out the above patch made against trunk, this adds the namespace parameter to the FlashMessenger. This can be used for the use cased outlined in this issue (perhaps having warning, error, and so on messages.)

Let me know what you think, if everyone likes it, I will work on unit testing it (which is harder b/c of the reliance on Zend_Session).

-ralph

Ralph,

The patch looks fine -- however, I'm not sure that it's needed -- or solves the underlying use-case in the simplest way.

I'm passing either a string or an array to the FlashMessenger Action Helper. Where I'm passing an array the key sets the type of message "warning", "error" etc.

I then have a view helper that collects the messages and formats the output as appropriate.

This approach is pretty simple and effective. If we add namespaces then it would be necessary to try all of them (or have any view helper know which namespaces might be used - introducing unnecessary coupling) in order to safely collect messages.

I blogged about this at :

http://blog.noumenal.co.uk/2009/08/…

(If you'd help me through the process) I'd be happy to contribute my view helper to the Framework. With that (or something like it) and an added example or two in the documentation I'd reckon that the issue would be solved.

-carlton

Is this something which can be added in ZF 1.12? I can prepare the unit tests and documentation to go with Ralph's original patch, if necessary.

Assuming it's an optional argument, and defaults to current behavior, absolutely; go for it!

Attached patch which augments the original patch with unit tests and a manual page entry.

Fixed in trunk (1.12.0): r24784