neos / flow-development-collection

The unified repository containing the Flow core packages, used for Flow development.
https://flow.neos.io/
MIT License
137 stars 189 forks source link

Disable ErrorFlashMessage by default #578

Open neos-bot opened 9 years ago

neos-bot commented 9 years ago

Jira issue originally created by user ollehar:

We have a model with a float value, validated with Flow annotations:

/****
 * @Flow\Validate(type="NotEmpty")
 * @Flow\Validate(type="Number")
 * @var float
 */
protected $percentage;

A controller action new and create. We show flash messages in the layout and validation errors in the template. When validation of float value fails, the error message from the title is displayed as flash message: An error occurred while trying to call %1$s->%2$s()

The validation errors are shown normally.

(Not sure what beta version of Flow we're using, composer.json says "typo3/flow": "3.0.*@beta")

Jira-URL: https://jira.neos.io/browse/FLOW-322

neos-bot commented 9 years ago

Comment created by @bwaidelich:

[~ollehar] You can circumvent this by overriding the getErrorFlashMessage() method in your controller making it return FALSE.

We already discussed that we want to disable that technical message by default but didn't manage to change it for 3.0 unfortunately.

sorenmalling commented 4 years ago

@bwaidelich Based on your work with the flashmessage store, is this still relevant to remove the default error message?

bwaidelich commented 4 years ago

This ticket is about making ActionController::getErrorFlashMessage() return false by default, so this is not related to the FlashMessage implementation

albe commented 4 years ago

Uh, whenever I was about to just do it (which happened more then once 🙈), I always got caught by the thought, what happens when no default error flash message is provided. As in: A new developer, that has not yet customized error messages, what will he see? Then I don't have an answer and immediately stop the attempt.

Should we maybe make it conditional, if the message container is still empty? https://github.com/neos/flow-development-collection/blob/7db4d589f3e67024f4863e264d65fc86a86b95e2/Neos.Flow/Classes/Mvc/Controller/ActionController.php#L709

bwaidelich commented 4 years ago

Should we maybe make it conditional, if the message container is still empty?

Not sure if I got that, could you rephrase the question?

albe commented 4 years ago
if (!$this->controllerContext->getFlashMessageContainer()->hasMessages()) {
   $this->controllerContext->getFlashMessageContainer()->addMessage($errorFlashMessage); 
}

I'm better with code than with words, so... 😅

bwaidelich commented 4 years ago

Thanks for clarifying, a line of code is worth a thousand words :)

We should probably properly rephrase the issue, but IMO the bug is that every developer has to override the getErrorFlashMessage() method in their controllers.

Only rendering that weird default message when there are other flash messages present won't really solve that issue IMO. I would just disable it by default. This is a "breaking" change, but I doubt that anyone actually relies on that default message to be generated.. Theoretically it should be possible to create a core migration to detect that case, but it won't be easy and is probably not worth the effort

bwaidelich commented 3 years ago

I removed it from the 7.0 release board since I didn't make it in time.. sorry