Clean up FlashMessenger calls.#5058
Conversation
| // Set flash message if requested: | ||
| if (null !== $flashNamespace && !empty($flashMsg)) { | ||
| $this->flashMessenger()->addMessage($flashMsg, $flashNamespace); | ||
| match ($flashNamespace) { |
There was a problem hiding this comment.
This is perhaps a bit idealistic, but I didn't like the way the FlashMessenger namespaces were involved here. But I can revert this part if necessary.
There was a problem hiding this comment.
Should we at least cover the known valid namespaces (i.e. add 'Info')? Do we need a default case that throws an exception for unexpected non-null values?
There was a problem hiding this comment.
I'll add them. They're not used by default, and there's nothing preventing one from adding flash messages by other means, but I guess someone could be using this locally with other options.
There was a problem hiding this comment.
Added. Note that I left default out intentionally. I'll propose that we don't support it in the upcoming rewrite, because in the Laminas implementation it's kind of a confusing mix of actual namespace and something set as default.
|
Mink tests are still passing for me. |
demiankatz
left a comment
demiankatz
left a comment
There was a problem hiding this comment.
Looks good to me, apart from the comment above regarding the match statement.
module/VuFind/src/VuFind/Controller/Feature/BulkActionControllerTrait.php
Outdated
Show resolved
Hide resolved
module/VuFind/src/VuFind/Controller/Feature/BulkActionControllerTrait.php
Outdated
Show resolved
Hide resolved
demiankatz
left a comment
demiankatz
left a comment
There was a problem hiding this comment.
Looks good to me now; thanks, @EreMaijala!
2 participants
Unifies the way flash messages are added.