Added an optional flashmessage listener

[Miliooo]

This adds an optional flash eventlistener, which shows flashes to the user when they do certain actions.

What's included:

• the listener
• tests for the listener
• updates for the english translation file
• updates for the dutch translation file

The current supported actions are:

• user sends a message
• user deletes a thread
• user undeletes a thread

How to enable

fos_message:
    flash_messages:  //optional if user doesn't add this we just don't show flashes
        enabled: true  //boolean but if false user shouldn't even add this segment
        flash_key: my_custom_key //optional string  = the key name of the flash, defaults to success

Todos

• Add some documentation for this

Credits

The code is based on https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/EventListener/FlashListener.php

[merk]

Move this definition to a new file, flash.xml, and only include it from the Extension if flash messages are to be shown.

[merk]

Please change the service to add a .default to its id, and alias the default service like most of the other services in this bundle

[stof]

I don't think making the id of the listener configurable is really useful. If they want a different listener, they should disable the built-in one and register their own listener IMO. We cannot add a tag for external services in the DI extension, so it would require adding a compiler pass to handle this configuration

[Miliooo]

One thing to note, I don't need to set that tag anymore since now the file don't get included. when it's not enabled by the config

The old way.. check if service is wanted => get definition ->addtag(kernel.event_subscriber)
The new way: include the file which has the tag already

[stof]

@Miliooo yeah, but it means that passign a custom service id will not replace the listener (to register your own service as listener, you have to tag it, not to pass it as id here, it simply cannot work). So the broken config key should be removed

[merk]

It may be worth it for you to run php-cs-fixer over pull requests before sending them, alternatively if you use phpstorm, you can set it up to use phpcs to analyse your files for errors based on the PSR2 standard which lets you see errors inside the IDE as you develop.

[Miliooo]

I use NetBeans and I had setup that cs-fixer but a few days ago my system freezed when opening a project. So i had to disable that plugin.

Also I had enabled some reformatting options when saving but that caused whole files to be reformatted when I saved them. Not very good when you want to do pull requests...

I know phpstorm is like the standard now so maybe it's time to consider switching. Anyways I'll try to get this things sorted before doing more pull requests, it saves everyone problems. Not sure how easy it will be to fix this though.

I'll try to edit your remarks by hand for now...

[stof]

this should be enabled for consistency with Symfony nodes. As of 2.3, there is even 2 shortcut methods for array nodes: canBeEnabled() and canBeDisabled(), which handle the call to addDefaultsIfNotSet() and setting the enabled with a default value (but you cannot use them yet as the bundle still need to support 2.2)

[stof]

I don't think making the key configurable is really needed. As Symfony 2.1+ supports putting multiple flash messages per type, it is quite logical to use success, error and warning as type IMO instead of inventing weird ones

[Miliooo]

Or I'm doing something silly but my theme has multiple places where I want to show a flash message.

I'm not sure how I would handle that with only a flash bag that has success, error and warning. How could I know then where to show them? Personally I set the key to 'message_info'. And then in my layout i check for message_info flashes on the messenger pages. So then I can show them in the right place and I know they are 'message flashes' not other ones...

But I agree for most it would be overkill... or again maybe I do something wrong

[merk]

You can solve this by extending the listener in your own app and overriding the functions that actually add flashes. Maybe move it to a getKey method which can be overridden.

[merk]

missing space between if ()

[merk]

This occurs in a few places

[merk]

Regarding the configurable service id, I requested it primarily for consistency with the bundle as it stands. It does seem a bit strange for an event listener though.

I still want the user to be able to configure the class of the listener, rather than forcing them to override the entire service definition (or define their own). If you can change the class of the listener service definition to a parameter and remove the .default/alias that'd be good.

[Miliooo]

Thanks for all the feedback. I implemented that parameter solution, and refactored that key so now it's a requirement.
I hope to make my editor more code standard proof soon.

If there are more updates needed I'll fix them tomorrow. Also we should document this. But I think @merk knows which structure he wants there. My main language isn't english either but I'll give it a try if you want me to.

[stof]

this curly brace should be on the previous line

[merk]

@stof Regarding ->canBeEnabled(), obviously this is a 2.3 specific feature. How do you feel about bumping the requirement?

Maybe leave the enabled config as it is for 1.2 of this bundle, and I'll mark 2.0(or 3.0) as needing symfony >=2.3?

[stof]

@merk Yeah, as I said, ->canBeEnabled() is just a shortcut for something possible in previous versions too. I mentionned it to explain why I asked to use enabled instead of show

[Miliooo]

I updated the config reference and added a link to
http://symfony.com/doc/current/book/controller.html#flash-messages
See Miliooo@ee1d275

Not sure if you want me to implement this in the current layout...

[Miliooo]

I also added a new config test file. Since there was no test file included I made one based on
https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/Tests/DependencyInjection/FOSUserExtensionTest.php

Unless you want me to change some stuff I think this commit is done.

[stof]

please rename the property to containerBuilder. It will be more readable

[tgalopin]

This is a good idea. I will think about it for the next version. I close this as it's old, but please tell me if you want to work on this @Miliooo.